On Fri, Nov 13, 2020 at 10:49 PM Michael Roth <michael.r...@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 <ebl...@redhat.com>
> Cc: Tomáš Golembiovský <tgole...@redhat.com>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Signed-off-by: Michael Roth <michael.r...@amd.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@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
