On 11/13/20 12:33 PM, Michael Roth 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>
> ---
> 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;
You could use QAPI_LIST_PREPEND() here (which was just recently added);
but if not, then my work to use that macro in more places in 6.0 will
revisit this code.
> }
> }
> 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);
Same here.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
