[libvirt PATCH v2 2/5] qemu_monitor_json: Add qemuMonitorJSONGetMigrationBlockers

[Eugenio Perez Martin]

On Wed, Jul 20, 2022 at 1:13 PM Jiri Denemark <jdenemar at redhat.com> wrote:
>
> On Wed, Jul 20, 2022 at 12:43:44 +0200, Eugenio Perez Martin wrote:
> > On Wed, Jul 20, 2022 at 12:14 PM Jiri Denemark <jdenemar at redhat.com> wrote:
> > >
> > > On Wed, Jul 20, 2022 at 11:11:51 +0200, Eugenio Pérez wrote:
> > > > This will allow qemuMigrationSrcIsAllowed to dynamically ask for
> > > > migration blockers, reducing duplication.
> > > >
> > > > Signed-off-by: Eugenio Pérez <eperezma at redhat.com>
> > > > ---
> > > >  src/qemu/qemu_monitor_json.c | 35 +++++++++++++++++++++++++++++++++++
> > > >  src/qemu/qemu_monitor_json.h |  3 +++
> > > >  2 files changed, 38 insertions(+)
> > > >
> > > > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> > > > index 5e4a86e5ad..a53d721720 100644
> > > > --- a/src/qemu/qemu_monitor_json.c
> > > > +++ b/src/qemu/qemu_monitor_json.c
> > > > @@ -3338,6 +3338,41 @@ int qemuMonitorJSONMigrate(qemuMonitor *mon,
> > > >      return 0;
> > > >  }
> > > >
> > > > +int qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon,
> > > > +                                        char ***blockers)
> > > > +{
> > > > +    g_autoptr(virJSONValue) cmd = NULL;
> > > > +    g_autoptr(virJSONValue) reply = NULL;
> > > > +    virJSONValue *data;
> > > > +    virJSONValue *jblockers;
> > > > +    size_t i;
> > > > +
> > > > +    if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL)))
> > > > +        return -1;
> > >
> > > We already have a function which calls query-migrate in JSON monitor,
> > > but I actually agree with adding this separate function as it serves a
> > > completely different purpose.
> > >
> >
> > I'm totally ok if anybody prefers to merge both too.
>
> I don't think anyone would prefer that as the existing function is a big
> beast which would get even more complicated if generalized for this use
> case.
>
> > > > +
> > > > +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> > > > +        return -1;
> > > > +
> > > > +    if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
> > > > +        return -1;
> > > > +
> > > > +    data = virJSONValueObjectGetObject(reply, "return");
> > > > +
> > > > +    if (!(jblockers = virJSONValueObjectGetArray(data, "blocked-reasons")))
> > > > +        return -1;
> > >
> > > This is not an error (not to mention that you would return -1 without a
> > > proper error message) as missing blocked-reasons means there's no
> > > migration blocker and the domain can be migrated (as long as the
> > > QEMU_CAPS_MIGRATION_BLOCKED_REASONS capability is set).
> > >
> >
> > Right, I'll fix it.
> >
> > Regarding the message on failure, a lot of the functions in this file
> > returns -1 without a message. How can I print it properly here or
> > propagate to the caller?
>
> Well, returning -1 is fine if the function called
> (qemuMonitorJSONCommand, qemuMonitorJSONCheckReply) already reports an
> error. But this is not the case of virJSONValueObjectGetArray. Reporting
> an error would be done just by calling virReportError() here instead of
> having it in the caller. Anyway, nothing to worry about here as you
> will be returning 0.
>

Got it.

Thanks!

> Jirka
>