Re: [PATCH for-5.2] qga: update schema for guest-get-disks 'dependents' field

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4241 bytes --]

On Fri, Nov 13, 2020 at 10:49 PM Michael Roth <michael.roth@amd.com> wrote:

> The recently-added 'guest-get-disk' command returns a list of
> GuestDiskInfo entries, which in turn have a 'dependents' field which
> lists devices these entries are dependent upon. Thus, 'dependencies'
> is a better name for this field. Address this by renaming the field
> accordingly.
>
> Additionally, 'dependents' is specified as non-optional, even though
> it's not implemented for w32. This is misleading, since it gives users
> the impression that a particular disk might not have dependencies,
> when in reality that information is simply not known to the guest
> agent. Address this by making 'dependents' an optional field, and only
> marking it as in-use when the facilities to obtain this information are
> available to the guest agent.
>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Tomáš Golembiovský <tgolembi@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  qga/commands-posix.c | 10 ++++++----
>  qga/qapi-schema.json |  8 ++++----
>  2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 12c1ba5ef7..c089e38120 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1285,6 +1285,7 @@ static void get_disk_deps(const char *disk_dir,
> GuestDiskInfo *disk)
>          g_debug("failed to list entries in %s", deps_dir);
>          return;
>      }
> +    disk->has_dependencies = true;
>      while ((dep = g_dir_read_name(dp_deps)) != NULL) {
>          g_autofree char *dep_dir = NULL;
>          strList *dep_item = NULL;
> @@ -1297,8 +1298,8 @@ static void get_disk_deps(const char *disk_dir,
> GuestDiskInfo *disk)
>              g_debug("  adding dependent device: %s", dev_name);
>              dep_item = g_new0(strList, 1);
>              dep_item->value = dev_name;
> -            dep_item->next = disk->dependents;
> -            disk->dependents = dep_item;
> +            dep_item->next = disk->dependencies;
> +            disk->dependencies = dep_item;
>          }
>      }
>      g_dir_close(dp_deps);
> @@ -1351,8 +1352,9 @@ static GuestDiskInfoList *get_disk_partitions(
>          partition->name = dev_name;
>          partition->partition = true;
>          /* Add parent disk as dependent for easier tracking of hierarchy
> */
> -        partition->dependents = g_new0(strList, 1);
> -        partition->dependents->value = g_strdup(disk_dev);
> +        partition->dependencies = g_new0(strList, 1);
> +        partition->dependencies->value = g_strdup(disk_dev);
> +        partition->has_dependencies = true;
>
>          item = g_new0(GuestDiskInfoList, 1);
>          item->value = partition;
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 6ca85f995f..3b3d1d0bd9 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -870,9 +870,9 @@
>  #
>  # @name: device node (Linux) or device UNC (Windows)
>  # @partition: whether this is a partition or disk
> -# @dependents: list of dependent devices; e.g. for LVs of the LVM this
> will
> -#              hold the list of PVs, for LUKS encrypted volume this will
> -#              contain the disk where the volume is placed.     (Linux)
> +# @dependencies: list of device dependencies; e.g. for LVs of the LVM
> this will
> +#                hold the list of PVs, for LUKS encrypted volume this will
> +#                contain the disk where the volume is placed.     (Linux)
>  # @address: disk address information (only for non-virtual devices)
>  # @alias: optional alias assigned to the disk, on Linux this is a name
> assigned
>  #         by device mapper
> @@ -880,7 +880,7 @@
>  # Since 5.2
>  ##
>  { 'struct': 'GuestDiskInfo',
> -  'data': {'name': 'str', 'partition': 'bool', 'dependents': ['str'],
> +  'data': {'name': 'str', 'partition': 'bool', '*dependencies': ['str'],
>             '*address': 'GuestDiskAddress', '*alias': 'str'} }
>
>  ##
> --
> 2.25.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 5737 bytes --]