Re: [Qemu-devel] [PATCH v7 04/11] hmp: disable monitor in preconfig state

[Markus Armbruster <armbru@redhat.com>]

Markus Armbruster <armbru@redhat.com> writes:

> Igor Mammedov <imammedo@redhat.com> writes:
>
>> Ban it for now, if someone would need it to work early,
>> one would have to implement checks if HMP command is valid
>> at preconfig state.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> v5:
>>   * add 'use QMP instead" to error message, suggesting user
>>     the right interface to use
>> v4:
>>   * v3 was only printing error but not preventing command execution,
>>     Fix it by returning after printing error message.
>>     ("Dr. David Alan Gilbert" <dgilbert@redhat.com>)
>> ---
>>  monitor.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 39f8ee1..0ffdf1d 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -3374,6 +3374,12 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>>  
>>      trace_handle_hmp_command(mon, cmdline);
>>  
>> +    if (runstate_check(RUN_STATE_PRECONFIG)) {
>> +        monitor_printf(mon, "HMP not available in preconfig state, "
>> +                            "use QMP instead\n");
>> +        return;
>> +    }
>> +
>>      cmd = monitor_parse_command(mon, cmdline, &cmdline, mon->cmd_table);
>>      if (!cmd) {
>>          return;
>
> So we offer the user an HMP monitor, but we summarily fail all commands.
> I'm sorry, but that's... searching for polite word... embarrassing.  We
> should accept HMP output only when we're ready to accept it.  Yes, that
> would involve a bit more surgery rather than this cheap hack.  The whole
> preconfig thing smells like a cheap hack to me, but let's not overdo it.

Clarification: I don't think we need to hold the series because of
this.  I do think you should investigate delaying HMP until it can work.