qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH V3 2/4] HMP: add infrastructure for sub command
Date: Thu, 10 Jan 2013 10:23:02 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)

wenchao xia <address@hidden> writes:

> 于 2013-1-10 11:30, Wenchao Xia 写道:
>>>> -static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
>>>> -{
>>>> -    return search_dispatch_table(qmp_cmds, cmdname);
>>>> -}
>>>
>>> Touching qmp_find_cmd() is unrelated to this series.
>>>
>>    OK.
>>
>>>> -
>>>> -static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>>>> -                                              const char *cmdline,
>>>> -                                              QDict *qdict)
>>>> +/*
>>>> + * Try find the target mon_cmd_t instance, if it have sub_table and
>>>> have string
>>>> + * for it, this function will try search it with remain string, and
>>>> if not
>>>> + * found it return NULL.
>>>> + */
>>>> +static const mon_cmd_t *hmp_parse_command(Monitor *mon,
>>>> +                                          const char *cmdline,
>>>> +                                          mon_cmd_t *table,
>>>> +                                          QDict *qdict)
>>>
>>> Renaming is also not necessary, but if you do it please do in a different
>>> patch.
>>>
>>    OK.
>>
>>>>   {
>>>> +        }
>>>> +        return hmp_parse_command(mon, p, cmd->sub_table, qdict);
>>>
>>> This works, and seems less hack then the previous attempt.
>>>
>>> Except for a small detail. This:
>>>
>>>    (qemu) info bla
>>>
>>> Currently returns a list of info commands, but with this series it
>>> returns
>>> an error.
>>>
>>    Yep, I think command completion is another topic, but I am pretty
>> sure it can be solved in this framework, for that it is not hack and
>> what need to do is, let command completion function knows there may be
>> another sub layer too.
>>
>   Sorry I missed your point. Returning an error instead of info
> commands, is made on purpose. IMHO always print the commands
> make user confused, message tipping what is wrong is better.

I agree with the change, but it needs to be mentioned in the commit
message.  Separate patch would be even better.

>>> I'd like to get a reviewed-by from Markus before applying this as,
>>> besides
>>> this being his idea, I honestly have a very hard time knowing how to
>>> move old
>>> hmp hackery forward.

I'll review v4.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]