Re: [Qemu-devel] [PATCH v2 11/12] qmp: update qmp_query_spice fallback

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <mlureau@redhat.com> writes:

> ----- Original Message -----
>> marcandre.lureau@redhat.com writes:
>> 
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > There are a few commands that are undef #ifdef conditions in
>> 
>> under #ifdef
>> 
>> > qmp-commands.hx. Move all the qmp_query_spice fallback in the same
>> > location, return an error and update the comment.
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  monitor.c | 14 ++++++++++++++
>> >  qmp.c     | 16 ----------------
>> >  2 files changed, 14 insertions(+), 16 deletions(-)
>> >
>> > diff --git a/monitor.c b/monitor.c
>> > index c87089f..46966d5 100644
>> > --- a/monitor.c
>> > +++ b/monitor.c
>> > @@ -4048,6 +4048,20 @@ QemuOptsList qemu_mon_opts = {
>> >      },
>> >  };
>> >  
>> > +/*
>> > + * the QAPI schema is blissfully unaware #ifdef FOO commands, and the
>> > + * QAPI code generator happily generates a dead qmp_marshal_foo_cmd()
>> > + * that calls qmp_foo_cmd().  Provide it one, or else linking fails.
>> > + * FIXME Educate the QAPI schema on #ifdef commands.
>> > + */
>> > +#ifndef CONFIG_SPICE
>> > +SpiceInfo *qmp_query_spice(Error **errp)
>> > +{
>> > +    error_setg(errp, QERR_FEATURE_DISABLED, "spice");
>> > +    return NULL;
>> 
>> Why do you change from abort() to error_setg()?
>
> The rest of the commands do not abort. It sounds to easy to trigger, and it's quite harmless to return an error instead.

If you can trigger it, the comment is wrong to claim "dead".

Can you trigger it before your series?  If not, which commit makes it
triggerable?

[...]