On Wed, Jul 20, 2022 at 12:14 PM Jiri Denemark <jdenemar@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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.
> > +
> > + 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?
> I think we should return 0 and set *blockers = NULL in this case.
>
Sure, I'll change that.
> > +
> > + /* NULL terminated array */
>
> This comment would be better as part of a proper documentation above the
> function.
>
I'll move to the function doc.
Thanks!
> > + *blockers = g_new0(char *, virJSONValueArraySize(jblockers) + 1);
> > + for (i = 0; i < virJSONValueArraySize(jblockers); i++) {
> > + virJSONValue *jblocker = virJSONValueArrayGet(jblockers, i);
> > + const char *blocker = virJSONValueGetString(jblocker);
> > +
> > + (*blockers)[i] = g_strdup(blocker);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int qemuMonitorJSONMigrateCancel(qemuMonitor *mon)
> > {
> > g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("migrate_cancel", NULL);
>
> Jirka
>
