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

[Jiri Denemark]

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.

> +
> +    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).

I think we should return 0 and set *blockers = NULL in this case.

> +
> +    /* NULL terminated array */

This comment would be better as part of a proper documentation above the
function.

> +    *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