qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PULL 32/32] qapi: add conditions to REPLICATION type/c


From: Marc-André Lureau
Subject: Re: [Qemu-devel] [PULL 32/32] qapi: add conditions to REPLICATION type/commands on the schema
Date: Mon, 17 Dec 2018 22:38:14 +0400

Hi

On Mon, Dec 17, 2018 at 8:01 PM Thomas Huth <address@hidden> wrote:
>
> On 2018-12-13 19:43, Markus Armbruster wrote:
> > From: Marc-André Lureau <address@hidden>
> >
> > Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the
> > code accordingly.
> >
> > Made conditional:
> >
> > * xen-set-replication, query-xen-replication-status,
> >   xen-colo-do-checkpoint
> >
> >   Before the patch, we first register the commands unconditionally in
> >   generated code (requires a stub), then conditionally unregister in
> >   qmp_unregister_commands_hack().
> >
> >   Afterwards, we register only when CONFIG_REPLICATION.  The command
> >   fails exactly the same, with CommandNotFound.
> >
> >   Improvement, because now query-qmp-schema is accurate, and we're one
> >   step closer to killing qmp_unregister_commands_hack().
> >
> > * enum BlockdevDriver value "replication" in command blockdev-add
> >
> > * BlockdevOptions variant @replication
> >
> > and related structures.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > Reviewed-by: Markus Armbruster <address@hidden>
> > Message-Id: <address@hidden>
> > Signed-off-by: Markus Armbruster <address@hidden>
> > ---
> >  migration/colo.c     | 16 ++++------------
> >  monitor.c            |  5 -----
> >  qapi/block-core.json | 13 +++++++++----
> >  qapi/migration.json  | 12 ++++++++----
> >  4 files changed, 21 insertions(+), 25 deletions(-)
>
> I think this might have broken compilation with --disable-replication:
>
>  https://gitlab.com/huth/qemu/-/jobs/135648240
>
> Any ideas how to fix it?

We introduced a regression by dropping osdep.h include from headers.

To me, it's best to include osdep.h in headers, since the ifdef exist
there. But I have been told we include it in .c instead. I'll upstream
the qapi/scripts to include it.

thanks for the report, sorry for the regression


-- 
Marc-André Lureau



reply via email to

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