On Tue, May 10, 2022 at 17:20:24 +0200, Jiri Denemark wrote:
> The function can now optionally return a bitmap describing the current
> state of each migration capability.
> 
> Signed-off-by: Jiri Denemark <jdene...@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c     |  2 +-
>  src/qemu/qemu_migration_params.c |  2 +-
>  src/qemu/qemu_monitor.c          |  5 +++--
>  src/qemu/qemu_monitor.h          |  3 ++-
>  src/qemu/qemu_monitor_json.c     | 18 +++++++++++++++++-
>  src/qemu/qemu_monitor_json.h     |  3 ++-
>  tests/qemumonitorjsontest.c      | 12 +++++++++++-
>  7 files changed, 37 insertions(+), 8 deletions(-)

[...]

> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 9e611e93e8..532aad348e 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6210,12 +6210,14 @@ qemuMonitorJSONGetTargetArch(qemuMonitor *mon)

I'm not exactly a fan of such loose correlation of data. Asking you to
return a list of stucts would probably cause too much conflicts though,
so as an alternative please document the relation between the
'capabilities' array and the positions in the 'state' bitmap in the
comment for this function (that you'll need to add).

>  int
>  qemuMonitorJSONGetMigrationCapabilities(qemuMonitor *mon,
> -                                        char ***capabilities)
> +                                        char ***capabilities,
> +                                        virBitmap **state)
>  {
>      g_autoptr(virJSONValue) cmd = NULL;
>      g_autoptr(virJSONValue) reply = NULL;
>      virJSONValue *caps;
>      g_auto(GStrv) list = NULL;
> +    g_autoptr(virBitmap) bitmap = NULL;
>      size_t i;
>      size_t n;
>  
> @@ -6235,10 +6237,12 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitor 
> *mon,
>      n = virJSONValueArraySize(caps);
>  
>      list = g_new0(char *, n + 1);
> +    bitmap = virBitmapNew(n);
>  
>      for (i = 0; i < n; i++) {
>          virJSONValue *cap = virJSONValueArrayGet(caps, i);
>          const char *name;
> +        bool enabled = false;
>  
>          if (!cap || virJSONValueGetType(cap) != VIR_JSON_TYPE_OBJECT) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -6252,10 +6256,22 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitor 
> *mon,
>              return -1;
>          }
>  
> +        if (virJSONValueObjectGetBoolean(cap, "state", &enabled) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("missing migration capability state"));
> +            return -1;
> +        }
> +
>          list[i] = g_strdup(name);
> +
> +        if (enabled)
> +            ignore_value(virBitmapSetBit(bitmap, i));
>      }
>  
>      *capabilities = g_steal_pointer(&list);
> +    if (state)
> +        *state = g_steal_pointer(&bitmap);
> +
>      return n;
>  }


Reviewed-by: Peter Krempa <pkre...@redhat.com>

Reply via email to