diff mbox series

[v4,2/3] qga: add implementation of guest-get-disks for Linux

Message ID 5630bd7edf4985024cdd9b0ac521f2d7e0825246.1602491752.git.tgolembi@redhat.com
State Superseded
Headers show
Series qga: add command guest-get-disks | expand

Commit Message

Tomáš Golembiovský Oct. 12, 2020, 8:36 a.m. UTC
The command lists all disks (real and virtual) as well as disk
partitions. For each disk the list of dependent disks is also listed and
/dev path is used as a handle so it can be matched with "name" field of
other returned disk entries. For disk partitions the "dependents" list
is populated with the the parent device for easier tracking of
hierarchy.

Example output:
{
  "return": [
    ...
    {
      "name": "/dev/dm-0",
      "partition": false,
      "dependents": [
        "/dev/sda2"
      ],
      "alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"
    },
    {
      "name": "/dev/sda2",
      "partition": true,
      "dependents": [
        "/dev/sda"
      ]
    },
    {
      "name": "/dev/sda",
      "partition": false,
      "address": {
        "serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
        "bus-type": "sata",
        ...
        "dev": "/dev/sda",
        "target": 0
      },
      "dependents": []
    },
    ...
  ]
}

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 qga/commands-posix.c | 296 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 285 insertions(+), 11 deletions(-)

Comments

Marc-André Lureau Oct. 13, 2020, 7:26 a.m. UTC | #1
Hi

On Mon, Oct 12, 2020 at 12:36 PM Tomáš Golembiovský <tgolembi@redhat.com>
wrote:

> The command lists all disks (real and virtual) as well as disk

> partitions. For each disk the list of dependent disks is also listed and

> /dev path is used as a handle so it can be matched with "name" field of

> other returned disk entries. For disk partitions the "dependents" list

> is populated with the the parent device for easier tracking of

> hierarchy.

>

> Example output:

> {

>   "return": [

>     ...

>     {

>       "name": "/dev/dm-0",

>       "partition": false,

>       "dependents": [

>         "/dev/sda2"

>       ],

>       "alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"

>     },

>     {

>       "name": "/dev/sda2",

>       "partition": true,

>       "dependents": [

>         "/dev/sda"

>       ]

>     },

>     {

>       "name": "/dev/sda",

>       "partition": false,

>       "address": {

>         "serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",

>         "bus-type": "sata",

>         ...

>         "dev": "/dev/sda",

>         "target": 0

>       },

>       "dependents": []

>     },

>     ...

>   ]

> }

>

> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>

> ---

>  qga/commands-posix.c | 296 +++++++++++++++++++++++++++++++++++++++++--

>  1 file changed, 285 insertions(+), 11 deletions(-)

>

> diff --git a/qga/commands-posix.c b/qga/commands-posix.c

> index 422144bcff..14683dfbd5 100644

> --- a/qga/commands-posix.c

> +++ b/qga/commands-posix.c

> @@ -1150,13 +1150,27 @@ static void

> build_guest_fsinfo_for_virtual_device(char const *syspath,

>      closedir(dir);

>  }

>

> +static bool is_disk_virtual(const char *devpath, Error **errp)

> +{

> +    g_autofree char *syspath = realpath(devpath, NULL);

> +

> +    if (!syspath) {

> +        error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);

> +        return false;

> +    }

> +    return strstr(syspath, "/devices/virtual/block/") != NULL;

> +}

> +

>  /* Dispatch to functions for virtual/real device */

>  static void build_guest_fsinfo_for_device(char const *devpath,

>                                            GuestFilesystemInfo *fs,

>                                            Error **errp)

>  {

> -    char *syspath = realpath(devpath, NULL);

> +    ERRP_GUARD();

> +    g_autofree char *syspath = NULL;

> +    bool is_virtual = false;

>

> +    syspath = realpath(devpath, NULL);

>      if (!syspath) {

>          error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);

>          return;

> @@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char

> const *devpath,

>      }

>

>      g_debug("  parse sysfs path '%s'", syspath);

> -

> -    if (strstr(syspath, "/devices/virtual/block/")) {

> +    is_virtual = is_disk_virtual(syspath, errp);

> +    if (*errp != NULL) {

> +        return;

> +    }

> +    if (is_virtual) {

>          build_guest_fsinfo_for_virtual_device(syspath, fs, errp);

>      } else {

>          build_guest_fsinfo_for_real_device(syspath, fs, errp);

>      }

> +}

> +

> +#ifdef CONFIG_LIBUDEV

> +

> +/*

> + * Wrapper around build_guest_fsinfo_for_device() for getting just

> + * the disk address.

> + */

> +static GuestDiskAddress *get_disk_address(const char *syspath, Error

> **errp)

> +{

> +    g_autoptr(GuestFilesystemInfo) fs = NULL;

> +

> +    fs = g_new0(GuestFilesystemInfo, 1);

> +    build_guest_fsinfo_for_device(syspath, fs, errp);

> +    if (fs->disk != NULL) {

> +        return g_steal_pointer(&fs->disk->value);

> +    }

> +    return NULL;

> +}

> +

> +static char *get_alias_for_syspath(const char *syspath)

> +{

> +    struct udev *udev = NULL;

> +    struct udev_device *udevice = NULL;

> +    char *ret = NULL;

> +

> +    udev = udev_new();

> +    if (udev == NULL) {

> +        g_debug("failed to query udev");

> +        goto out;

> +    }

> +    udevice = udev_device_new_from_syspath(udev, syspath);

> +    if (udevice == NULL) {

> +        g_debug("failed to query udev for path: %s", syspath);

> +        goto out;

> +    } else {

> +        const char *alias = udev_device_get_property_value(

> +            udevice, "DM_NAME");

> +        /*

> +         * NULL means there was an error and empty string means there is

> no

> +         * alias. In case of no alias we return NULL instead of empty

> string.

> +         */

> +        if (alias == NULL) {

> +            g_debug("failed to query udev for device alias for: %s",

> +                syspath);

> +        } else if (*alias != 0) {

> +            ret = g_strdup(alias);

> +        }

> +    }

> +

> +out:

> +    udev_unref(udev);

> +    udev_device_unref(udevice);

> +    return ret;

> +}

> +

> +static char *get_device_for_syspath(const char *syspath)

> +{

> +    struct udev *udev = NULL;

> +    struct udev_device *udevice = NULL;

> +    char *ret = NULL;

> +

> +    udev = udev_new();

> +    if (udev == NULL) {

> +        g_debug("failed to query udev");

> +        goto out;

> +    }

> +    udevice = udev_device_new_from_syspath(udev, syspath);

> +    if (udevice == NULL) {

> +        g_debug("failed to query udev for path: %s", syspath);

> +        goto out;

> +    } else {

> +        ret = g_strdup(udev_device_get_devnode(udevice));

> +    }

> +

> +out:

> +    udev_unref(udev);

> +    udev_device_unref(udevice);

> +    return ret;

> +}

> +

> +static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)

> +{

> +    g_autofree char *deps_dir = NULL;

> +    const gchar *dep;

> +    GDir *dp_deps = NULL;

> +

> +    /* List dependent disks */

> +    deps_dir = g_strdup_printf("%s/slaves", disk_dir);

> +    g_debug("  listing entries in: %s", deps_dir);

> +    dp_deps = g_dir_open(deps_dir, 0, NULL);

> +    if (dp_deps == NULL) {

> +        g_debug("failed to list entries in %s", deps_dir);

> +        return;

> +    }

> +    while ((dep = g_dir_read_name(dp_deps)) != NULL) {

> +        g_autofree char *dep_dir = NULL;

> +        strList *dep_item = NULL;

> +        char *dev_name;

>

> -    free(syspath);

> +        /* Add dependent disks */

> +        dep_dir = g_strdup_printf("%s/%s", deps_dir, dep);

> +        dev_name = get_device_for_syspath(dep_dir);

> +        if (dev_name != NULL) {

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

> +        }

> +    }

> +    g_dir_close(dp_deps);

>  }

>

> +/*

> + * Detect partitions subdirectory, name is "<disk_name><number>" or

> + * "<disk_name>p<number>"

> + *

> + * @disk_name -- last component of /sys path (e.g. sda)

> + * @disk_dir -- sys path of the disk (e.g. /sys/block/sda)

> + * @disk_dev -- device node of the disk (e.g. /dev/sda)

> + */

> +static GuestDiskInfoList *get_disk_partitions(

> +    GuestDiskInfoList *list,

> +    const char *disk_name, const char *disk_dir,

> +    const char *disk_dev)

> +{

> +    GuestDiskInfoList *item, *ret = list;

> +    struct dirent *de_disk;

> +    DIR *dp_disk = NULL;

> +    size_t len = strlen(disk_name);

> +

> +    dp_disk = opendir(disk_dir);

>


Any reason to use both readdir and g_dir APIs in the same patch? Maybe use
g_dir alone?

+    while ((de_disk = readdir(dp_disk)) != NULL) {
> +        g_autofree char *partition_dir = NULL;

> +        char *dev_name;

> +        GuestDiskInfo *partition;

> +

> +        if (!(de_disk->d_type & DT_DIR)) {

> +            continue;

> +        }

> +

> +        if (!(strncmp(disk_name, de_disk->d_name, len) == 0 &&

> +            ((*(de_disk->d_name + len) == 'p' &&

> +            isdigit(*(de_disk->d_name + len + 1))) ||

> +                isdigit(*(de_disk->d_name + len))))) {

> +            continue;

> +        }

> +

> +        partition_dir = g_strdup_printf("%s/%s",

> +            disk_dir, de_disk->d_name);

> +        dev_name = get_device_for_syspath(partition_dir);

> +        if (dev_name == NULL) {

> +            g_debug("Failed to get device name for syspath: %s",

> +                disk_dir);

> +            continue;

> +        }

> +        partition = g_new0(GuestDiskInfo, 1);

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

> +

> +        item = g_new0(GuestDiskInfoList, 1);

> +        item->value = partition;

> +        item->next = ret;

> +        ret = item;

> +

> +    }

> +    closedir(dp_disk);

> +

> +    return ret;

> +}

> +

> +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)

> +{

> +    GuestDiskInfoList *item, *ret = NULL;

> +    GuestDiskInfo *disk;

> +    DIR *dp = NULL;

> +    struct dirent *de = NULL;

> +

> +    g_debug("listing /sys/block directory");

> +    dp = opendir("/sys/block");

> +    if (dp == NULL) {

> +        error_setg_errno(errp, errno, "Can't open directory

> \"/sys/block\"");

> +        return NULL;

> +    }

> +    while ((de = readdir(dp)) != NULL) {

> +        g_autofree char *disk_dir = NULL, *line = NULL,

> +            *size_path = NULL, *deps_dir = NULL;

> +        char *dev_name;

> +        Error *local_err = NULL;

> +        if (de->d_type != DT_LNK) {

> +            g_debug("  skipping entry: %s", de->d_name);

> +            continue;

> +        }

> +

> +        /* Check size and skip zero-sized disks */

> +        g_debug("  checking disk size");

> +        size_path = g_strdup_printf("/sys/block/%s/size", de->d_name);

> +        if (!g_file_get_contents(size_path, &line, NULL, NULL)) {

> +            g_debug("  failed to read disk size");

> +            continue;

> +        }

> +        if (g_strcmp0(line, "0\n") == 0) {

> +            g_debug("  skipping zero-sized disk");

> +            continue;

> +        }

> +

> +        g_debug("  adding %s", de->d_name);

> +        disk_dir = g_strdup_printf("/sys/block/%s", de->d_name);

> +        dev_name = get_device_for_syspath(disk_dir);

> +        if (dev_name == NULL) {

> +            g_debug("Failed to get device name for syspath: %s",

> +                disk_dir);

> +            continue;

> +        }

> +        disk = g_new0(GuestDiskInfo, 1);

> +        disk->name = dev_name;

> +        disk->partition = false;

> +        disk->alias = get_alias_for_syspath(disk_dir);

> +        disk->has_alias = (disk->alias != NULL);

> +        item = g_new0(GuestDiskInfoList, 1);

> +        item->value = disk;

> +        item->next = ret;

> +        ret = item;

> +

> +        /* Get address for non-virtual devices */

> +        bool is_virtual = is_disk_virtual(disk_dir, &local_err);

> +        if (local_err != NULL) {

> +            g_debug("  failed to check disk path, ignoring error: %s",

> +                error_get_pretty(local_err));

> +            error_free(local_err);

> +            local_err = NULL;

> +            /* Don't try to get the address */

> +            is_virtual = true;

> +        }

> +        if (!is_virtual) {

> +            disk->address = get_disk_address(disk_dir, &local_err);

> +            if (local_err != NULL) {

> +                g_debug("  failed to get device info, ignoring error: %s",

> +                    error_get_pretty(local_err));

> +                error_free(local_err);

> +                local_err = NULL;

> +            } else if (disk->address != NULL) {

> +                disk->has_address = true;

> +            }

> +        }

> +

> +        get_disk_deps(disk_dir, disk);

> +        ret = get_disk_partitions(ret, de->d_name, disk_dir, dev_name);

> +    }

> +    return ret;

> +}

> +

> +#else

> +

> +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)

> +{

> +    error_setg(errp, QERR_UNSUPPORTED);

> +    return NULL;

> +}

> +

> +#endif

> +

>  /* Return a list of the disk device(s)' info which @mount lies on */

>  static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,

>                                                 Error **errp)

> @@ -2809,7 +3088,8 @@ GList *ga_command_blacklist_init(GList *blacklist)

>          const char *list[] = {

>              "guest-get-fsinfo", "guest-fsfreeze-status",

>              "guest-fsfreeze-freeze", "guest-fsfreeze-freeze-list",

> -            "guest-fsfreeze-thaw", "guest-get-fsinfo", NULL};

> +            "guest-fsfreeze-thaw", "guest-get-fsinfo",

> +            "guest-get-disks", NULL};

>          char **p = (char **)list;

>

>          while (*p) {

> @@ -3051,9 +3331,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error

> **errp)

>

>      return NULL;

>  }

> -

> -GuestDiskInfoList *qmp_guest_get_disks(Error **errp)

> -{

> -    error_setg(errp, QERR_UNSUPPORTED);

> -    return NULL;

> -}

> --

> 2.28.0

>

>

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


-- 
Marc-André Lureau
<div dir="ltr"><div dir="ltr">Hi<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 12, 2020 at 12:36 PM Tomáš Golembiovský &lt;<a href="mailto:tgolembi@redhat.com">tgolembi@redhat.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The command lists all disks (real and virtual) as well as disk<br>
partitions. For each disk the list of dependent disks is also listed and<br>
/dev path is used as a handle so it can be matched with &quot;name&quot; field of<br>
other returned disk entries. For disk partitions the &quot;dependents&quot; list<br>
is populated with the the parent device for easier tracking of<br>
hierarchy.<br>
<br>
Example output:<br>
{<br>
  &quot;return&quot;: [<br>
    ...<br>
    {<br>
      &quot;name&quot;: &quot;/dev/dm-0&quot;,<br>
      &quot;partition&quot;: false,<br>
      &quot;dependents&quot;: [<br>
        &quot;/dev/sda2&quot;<br>
      ],<br>
      &quot;alias&quot;: &quot;luks-7062202e-5b9b-433e-81e8-6628c40da9f7&quot;<br>
    },<br>
    {<br>
      &quot;name&quot;: &quot;/dev/sda2&quot;,<br>
      &quot;partition&quot;: true,<br>
      &quot;dependents&quot;: [<br>
        &quot;/dev/sda&quot;<br>
      ]<br>
    },<br>
    {<br>
      &quot;name&quot;: &quot;/dev/sda&quot;,<br>
      &quot;partition&quot;: false,<br>
      &quot;address&quot;: {<br>
        &quot;serial&quot;: &quot;SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493&quot;,<br>
        &quot;bus-type&quot;: &quot;sata&quot;,<br>
        ...<br>
        &quot;dev&quot;: &quot;/dev/sda&quot;,<br>
        &quot;target&quot;: 0<br>
      },<br>
      &quot;dependents&quot;: []<br>
    },<br>
    ...<br>
  ]<br>
}<br>
<br>
Signed-off-by: Tomáš Golembiovský &lt;<a href="mailto:tgolembi@redhat.com" target="_blank">tgolembi@redhat.com</a>&gt;<br>

---<br>
 qga/commands-posix.c | 296 +++++++++++++++++++++++++++++++++++++++++--<br>
 1 file changed, 285 insertions(+), 11 deletions(-)<br>
<br>
diff --git a/qga/commands-posix.c b/qga/commands-posix.c<br>
index 422144bcff..14683dfbd5 100644<br>
--- a/qga/commands-posix.c<br>
+++ b/qga/commands-posix.c<br>
@@ -1150,13 +1150,27 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,<br>
     closedir(dir);<br>
 }<br>
<br>
+static bool is_disk_virtual(const char *devpath, Error **errp)<br>
+{<br>
+    g_autofree char *syspath = realpath(devpath, NULL);<br>
+<br>
+    if (!syspath) {<br>
+        error_setg_errno(errp, errno, &quot;realpath(\&quot;%s\&quot;)&quot;, devpath);<br>
+        return false;<br>
+    }<br>
+    return strstr(syspath, &quot;/devices/virtual/block/&quot;) != NULL;<br>
+}<br>
+<br>
 /* Dispatch to functions for virtual/real device */<br>
 static void build_guest_fsinfo_for_device(char const *devpath,<br>
                                           GuestFilesystemInfo *fs,<br>
                                           Error **errp)<br>
 {<br>
-    char *syspath = realpath(devpath, NULL);<br>
+    ERRP_GUARD();<br>
+    g_autofree char *syspath = NULL;<br>
+    bool is_virtual = false;<br>
<br>
+    syspath = realpath(devpath, NULL);<br>
     if (!syspath) {<br>
         error_setg_errno(errp, errno, &quot;realpath(\&quot;%s\&quot;)&quot;, devpath);<br>
         return;<br>
@@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char const *devpath,<br>
     }<br>
<br>
     g_debug(&quot;  parse sysfs path &#39;%s&#39;&quot;, syspath);<br>
-<br>
-    if (strstr(syspath, &quot;/devices/virtual/block/&quot;)) {<br>
+    is_virtual = is_disk_virtual(syspath, errp);<br>
+    if (*errp != NULL) {<br>
+        return;<br>
+    }<br>
+    if (is_virtual) {<br>
         build_guest_fsinfo_for_virtual_device(syspath, fs, errp);<br>
     } else {<br>
         build_guest_fsinfo_for_real_device(syspath, fs, errp);<br>
     }<br>
+}<br>
+<br>
+#ifdef CONFIG_LIBUDEV<br>
+<br>
+/*<br>
+ * Wrapper around build_guest_fsinfo_for_device() for getting just<br>
+ * the disk address.<br>
+ */<br>
+static GuestDiskAddress *get_disk_address(const char *syspath, Error **errp)<br>
+{<br>
+    g_autoptr(GuestFilesystemInfo) fs = NULL;<br>
+<br>
+    fs = g_new0(GuestFilesystemInfo, 1);<br>
+    build_guest_fsinfo_for_device(syspath, fs, errp);<br>
+    if (fs-&gt;disk != NULL) {<br>
+        return g_steal_pointer(&amp;fs-&gt;disk-&gt;value);<br>
+    }<br>
+    return NULL;<br>
+}<br>
+<br>
+static char *get_alias_for_syspath(const char *syspath)<br>
+{<br>
+    struct udev *udev = NULL;<br>
+    struct udev_device *udevice = NULL;<br>
+    char *ret = NULL;<br>
+<br>
+    udev = udev_new();<br>
+    if (udev == NULL) {<br>
+        g_debug(&quot;failed to query udev&quot;);<br>
+        goto out;<br>
+    }<br>
+    udevice = udev_device_new_from_syspath(udev, syspath);<br>
+    if (udevice == NULL) {<br>
+        g_debug(&quot;failed to query udev for path: %s&quot;, syspath);<br>
+        goto out;<br>
+    } else {<br>
+        const char *alias = udev_device_get_property_value(<br>
+            udevice, &quot;DM_NAME&quot;);<br>
+        /*<br>
+         * NULL means there was an error and empty string means there is no<br>
+         * alias. In case of no alias we return NULL instead of empty string.<br>
+         */<br>
+        if (alias == NULL) {<br>
+            g_debug(&quot;failed to query udev for device alias for: %s&quot;,<br>
+                syspath);<br>
+        } else if (*alias != 0) {<br>
+            ret = g_strdup(alias);<br>
+        }<br>
+    }<br>
+<br>
+out:<br>
+    udev_unref(udev);<br>
+    udev_device_unref(udevice);<br>
+    return ret;<br>
+}<br>
+<br>
+static char *get_device_for_syspath(const char *syspath)<br>
+{<br>
+    struct udev *udev = NULL;<br>
+    struct udev_device *udevice = NULL;<br>
+    char *ret = NULL;<br>
+<br>
+    udev = udev_new();<br>
+    if (udev == NULL) {<br>
+        g_debug(&quot;failed to query udev&quot;);<br>
+        goto out;<br>
+    }<br>
+    udevice = udev_device_new_from_syspath(udev, syspath);<br>
+    if (udevice == NULL) {<br>
+        g_debug(&quot;failed to query udev for path: %s&quot;, syspath);<br>
+        goto out;<br>
+    } else {<br>
+        ret = g_strdup(udev_device_get_devnode(udevice));<br>
+    }<br>
+<br>
+out:<br>
+    udev_unref(udev);<br>
+    udev_device_unref(udevice);<br>
+    return ret;<br>
+}<br>
+<br>
+static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)<br>
+{<br>
+    g_autofree char *deps_dir = NULL;<br>
+    const gchar *dep;<br>
+    GDir *dp_deps = NULL;<br>
+<br>
+    /* List dependent disks */<br>
+    deps_dir = g_strdup_printf(&quot;%s/slaves&quot;, disk_dir);<br>
+    g_debug(&quot;  listing entries in: %s&quot;, deps_dir);<br>
+    dp_deps = g_dir_open(deps_dir, 0, NULL);<br>
+    if (dp_deps == NULL) {<br>
+        g_debug(&quot;failed to list entries in %s&quot;, deps_dir);<br>
+        return;<br>
+    }<br>
+    while ((dep = g_dir_read_name(dp_deps)) != NULL) {<br>
+        g_autofree char *dep_dir = NULL;<br>
+        strList *dep_item = NULL;<br>
+        char *dev_name;<br>
<br>
-    free(syspath);<br>
+        /* Add dependent disks */<br>
+        dep_dir = g_strdup_printf(&quot;%s/%s&quot;, deps_dir, dep);<br>
+        dev_name = get_device_for_syspath(dep_dir);<br>
+        if (dev_name != NULL) {<br>
+            g_debug(&quot;  adding dependent device: %s&quot;, dev_name);<br>
+            dep_item = g_new0(strList, 1);<br>
+            dep_item-&gt;value = dev_name;<br>
+            dep_item-&gt;next = disk-&gt;dependents;<br>
+            disk-&gt;dependents = dep_item;<br>
+        }<br>
+    }<br>
+    g_dir_close(dp_deps);<br>
 }<br>
<br>
+/*<br>
+ * Detect partitions subdirectory, name is &quot;&lt;disk_name&gt;&lt;number&gt;&quot; or<br>
+ * &quot;&lt;disk_name&gt;p&lt;number&gt;&quot;<br>
+ *<br>
+ * @disk_name -- last component of /sys path (e.g. sda)<br>
+ * @disk_dir -- sys path of the disk (e.g. /sys/block/sda)<br>
+ * @disk_dev -- device node of the disk (e.g. /dev/sda)<br>
+ */<br>
+static GuestDiskInfoList *get_disk_partitions(<br>
+    GuestDiskInfoList *list,<br>
+    const char *disk_name, const char *disk_dir,<br>
+    const char *disk_dev)<br>
+{<br>
+    GuestDiskInfoList *item, *ret = list;<br>
+    struct dirent *de_disk;<br>
+    DIR *dp_disk = NULL;<br>
+    size_t len = strlen(disk_name);<br>
+<br>
+    dp_disk = opendir(disk_dir);<br></blockquote><div><br></div><div>Any reason to use both readdir and g_dir APIs in the same patch? Maybe use g_dir alone?<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    while ((de_disk = readdir(dp_disk)) != NULL) {<br>
+        g_autofree char *partition_dir = NULL;<br>
+        char *dev_name;<br>
+        GuestDiskInfo *partition;<br>
+<br>
+        if (!(de_disk-&gt;d_type &amp; DT_DIR)) {<br>
+            continue;<br>
+        }<br>
+<br>
+        if (!(strncmp(disk_name, de_disk-&gt;d_name, len) == 0 &amp;&amp;<br>
+            ((*(de_disk-&gt;d_name + len) == &#39;p&#39; &amp;&amp;<br>
+            isdigit(*(de_disk-&gt;d_name + len + 1))) ||<br>
+                isdigit(*(de_disk-&gt;d_name + len))))) {<br>
+            continue;<br>
+        }<br>
+<br>
+        partition_dir = g_strdup_printf(&quot;%s/%s&quot;,<br>
+            disk_dir, de_disk-&gt;d_name);<br>
+        dev_name = get_device_for_syspath(partition_dir);<br>
+        if (dev_name == NULL) {<br>
+            g_debug(&quot;Failed to get device name for syspath: %s&quot;,<br>
+                disk_dir);<br>
+            continue;<br>
+        }<br>
+        partition = g_new0(GuestDiskInfo, 1);<br>
+        partition-&gt;name = dev_name;<br>
+        partition-&gt;partition = true;<br>
+        /* Add parent disk as dependent for easier tracking of hierarchy */<br>
+        partition-&gt;dependents = g_new0(strList, 1);<br>
+        partition-&gt;dependents-&gt;value = g_strdup(disk_dev);<br>
+<br>
+        item = g_new0(GuestDiskInfoList, 1);<br>
+        item-&gt;value = partition;<br>
+        item-&gt;next = ret;<br>
+        ret = item;<br>
+<br>
+    }<br>
+    closedir(dp_disk);<br>
+<br>
+    return ret;<br>
+}<br>
+<br>
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)<br>
+{<br>
+    GuestDiskInfoList *item, *ret = NULL;<br>
+    GuestDiskInfo *disk;<br>
+    DIR *dp = NULL;<br>
+    struct dirent *de = NULL;<br>
+<br>
+    g_debug(&quot;listing /sys/block directory&quot;);<br>
+    dp = opendir(&quot;/sys/block&quot;);<br>
+    if (dp == NULL) {<br>
+        error_setg_errno(errp, errno, &quot;Can&#39;t open directory \&quot;/sys/block\&quot;&quot;);<br>
+        return NULL;<br>
+    }<br>
+    while ((de = readdir(dp)) != NULL) {<br>
+        g_autofree char *disk_dir = NULL, *line = NULL,<br>
+            *size_path = NULL, *deps_dir = NULL;<br>
+        char *dev_name;<br>
+        Error *local_err = NULL;<br>
+        if (de-&gt;d_type != DT_LNK) {<br>
+            g_debug(&quot;  skipping entry: %s&quot;, de-&gt;d_name);<br>
+            continue;<br>
+        }<br>
+<br>
+        /* Check size and skip zero-sized disks */<br>
+        g_debug(&quot;  checking disk size&quot;);<br>
+        size_path = g_strdup_printf(&quot;/sys/block/%s/size&quot;, de-&gt;d_name);<br>
+        if (!g_file_get_contents(size_path, &amp;line, NULL, NULL)) {<br>
+            g_debug(&quot;  failed to read disk size&quot;);<br>
+            continue;<br>
+        }<br>
+        if (g_strcmp0(line, &quot;0\n&quot;) == 0) {<br>
+            g_debug(&quot;  skipping zero-sized disk&quot;);<br>
+            continue;<br>
+        }<br>
+<br>
+        g_debug(&quot;  adding %s&quot;, de-&gt;d_name);<br>
+        disk_dir = g_strdup_printf(&quot;/sys/block/%s&quot;, de-&gt;d_name);<br>
+        dev_name = get_device_for_syspath(disk_dir);<br>
+        if (dev_name == NULL) {<br>
+            g_debug(&quot;Failed to get device name for syspath: %s&quot;,<br>
+                disk_dir);<br>
+            continue;<br>
+        }<br>
+        disk = g_new0(GuestDiskInfo, 1);<br>
+        disk-&gt;name = dev_name;<br>
+        disk-&gt;partition = false;<br>
+        disk-&gt;alias = get_alias_for_syspath(disk_dir);<br>
+        disk-&gt;has_alias = (disk-&gt;alias != NULL);<br>
+        item = g_new0(GuestDiskInfoList, 1);<br>
+        item-&gt;value = disk;<br>
+        item-&gt;next = ret;<br>
+        ret = item;<br>
+<br>
+        /* Get address for non-virtual devices */<br>
+        bool is_virtual = is_disk_virtual(disk_dir, &amp;local_err);<br>
+        if (local_err != NULL) {<br>
+            g_debug(&quot;  failed to check disk path, ignoring error: %s&quot;,<br>
+                error_get_pretty(local_err));<br>
+            error_free(local_err);<br>
+            local_err = NULL;<br>
+            /* Don&#39;t try to get the address */<br>
+            is_virtual = true;<br>
+        }<br>
+        if (!is_virtual) {<br>
+            disk-&gt;address = get_disk_address(disk_dir, &amp;local_err);<br>
+            if (local_err != NULL) {<br>
+                g_debug(&quot;  failed to get device info, ignoring error: %s&quot;,<br>
+                    error_get_pretty(local_err));<br>
+                error_free(local_err);<br>
+                local_err = NULL;<br>
+            } else if (disk-&gt;address != NULL) {<br>
+                disk-&gt;has_address = true;<br>
+            }<br>
+        }<br>
+<br>
+        get_disk_deps(disk_dir, disk);<br>
+        ret = get_disk_partitions(ret, de-&gt;d_name, disk_dir, dev_name);<br>
+    }<br>
+    return ret;<br>
+}<br>
+<br>
+#else<br>
+<br>
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)<br>
+{<br>
+    error_setg(errp, QERR_UNSUPPORTED);<br>
+    return NULL;<br>
+}<br>
+<br>
+#endif<br>
+<br>
 /* Return a list of the disk device(s)&#39; info which @mount lies on */<br>
 static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,<br>
                                                Error **errp)<br>
@@ -2809,7 +3088,8 @@ GList *ga_command_blacklist_init(GList *blacklist)<br>
         const char *list[] = {<br>
             &quot;guest-get-fsinfo&quot;, &quot;guest-fsfreeze-status&quot;,<br>
             &quot;guest-fsfreeze-freeze&quot;, &quot;guest-fsfreeze-freeze-list&quot;,<br>
-            &quot;guest-fsfreeze-thaw&quot;, &quot;guest-get-fsinfo&quot;, NULL};<br>
+            &quot;guest-fsfreeze-thaw&quot;, &quot;guest-get-fsinfo&quot;,<br>
+            &quot;guest-get-disks&quot;, NULL};<br>
         char **p = (char **)list;<br>
<br>
         while (*p) {<br>
@@ -3051,9 +3331,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)<br>
<br>
     return NULL;<br>
 }<br>
-<br>
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)<br>
-{<br>
-    error_setg(errp, QERR_UNSUPPORTED);<br>
-    return NULL;<br>
-}<br>
-- <br>
2.28.0<br>
<br>
</blockquote></div><div><br></div><div>lgtm otherwise</div><div>Reviewed-by: Marc-André Lureau &lt;<a href="mailto:marcandre.lureau@redhat.com">marcandre.lureau@redhat.com</a>&gt;<br></div><br>-- <br><div dir="ltr" class="gmail_signature">Marc-André Lureau<br></div></div>
Tomáš Golembiovský Oct. 14, 2020, 9:34 a.m. UTC | #2
On Tue, Oct 13, 2020 at 11:26:45AM +0400, Marc-André Lureau wrote:
> Hi

> 

> On Mon, Oct 12, 2020 at 12:36 PM Tomáš Golembiovský <tgolembi@redhat.com>

> wrote:

> 

> > The command lists all disks (real and virtual) as well as disk

> > partitions. For each disk the list of dependent disks is also listed and

> > /dev path is used as a handle so it can be matched with "name" field of

> > other returned disk entries. For disk partitions the "dependents" list

> > is populated with the the parent device for easier tracking of

> > hierarchy.

> >

> > Example output:

> > {

> >   "return": [

> >     ...

> >     {

> >       "name": "/dev/dm-0",

> >       "partition": false,

> >       "dependents": [

> >         "/dev/sda2"

> >       ],

> >       "alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"

> >     },

> >     {

> >       "name": "/dev/sda2",

> >       "partition": true,

> >       "dependents": [

> >         "/dev/sda"

> >       ]

> >     },

> >     {

> >       "name": "/dev/sda",

> >       "partition": false,

> >       "address": {

> >         "serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",

> >         "bus-type": "sata",

> >         ...

> >         "dev": "/dev/sda",

> >         "target": 0

> >       },

> >       "dependents": []

> >     },

> >     ...

> >   ]

> > }

> >

> > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>

> > ---

> >  qga/commands-posix.c | 296 +++++++++++++++++++++++++++++++++++++++++--

> >  1 file changed, 285 insertions(+), 11 deletions(-)

> >

> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c

> > index 422144bcff..14683dfbd5 100644

> > --- a/qga/commands-posix.c

> > +++ b/qga/commands-posix.c

> > @@ -1150,13 +1150,27 @@ static void

> > build_guest_fsinfo_for_virtual_device(char const *syspath,

> >      closedir(dir);

> >  }

> >

> > +static bool is_disk_virtual(const char *devpath, Error **errp)

> > +{

> > +    g_autofree char *syspath = realpath(devpath, NULL);

> > +

> > +    if (!syspath) {

> > +        error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);

> > +        return false;

> > +    }

> > +    return strstr(syspath, "/devices/virtual/block/") != NULL;

> > +}

> > +

> >  /* Dispatch to functions for virtual/real device */

> >  static void build_guest_fsinfo_for_device(char const *devpath,

> >                                            GuestFilesystemInfo *fs,

> >                                            Error **errp)

> >  {

> > -    char *syspath = realpath(devpath, NULL);

> > +    ERRP_GUARD();

> > +    g_autofree char *syspath = NULL;

> > +    bool is_virtual = false;

> >

> > +    syspath = realpath(devpath, NULL);

> >      if (!syspath) {

> >          error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);

> >          return;

> > @@ -1167,16 +1181,281 @@ static void build_guest_fsinfo_for_device(char

> > const *devpath,

> >      }

> >

> >      g_debug("  parse sysfs path '%s'", syspath);

> > -

> > -    if (strstr(syspath, "/devices/virtual/block/")) {

> > +    is_virtual = is_disk_virtual(syspath, errp);

> > +    if (*errp != NULL) {

> > +        return;

> > +    }

> > +    if (is_virtual) {

> >          build_guest_fsinfo_for_virtual_device(syspath, fs, errp);

> >      } else {

> >          build_guest_fsinfo_for_real_device(syspath, fs, errp);

> >      }

> > +}

> > +

> > +#ifdef CONFIG_LIBUDEV

> > +

> > +/*

> > + * Wrapper around build_guest_fsinfo_for_device() for getting just

> > + * the disk address.

> > + */

> > +static GuestDiskAddress *get_disk_address(const char *syspath, Error

> > **errp)

> > +{

> > +    g_autoptr(GuestFilesystemInfo) fs = NULL;

> > +

> > +    fs = g_new0(GuestFilesystemInfo, 1);

> > +    build_guest_fsinfo_for_device(syspath, fs, errp);

> > +    if (fs->disk != NULL) {

> > +        return g_steal_pointer(&fs->disk->value);

> > +    }

> > +    return NULL;

> > +}

> > +

> > +static char *get_alias_for_syspath(const char *syspath)

> > +{

> > +    struct udev *udev = NULL;

> > +    struct udev_device *udevice = NULL;

> > +    char *ret = NULL;

> > +

> > +    udev = udev_new();

> > +    if (udev == NULL) {

> > +        g_debug("failed to query udev");

> > +        goto out;

> > +    }

> > +    udevice = udev_device_new_from_syspath(udev, syspath);

> > +    if (udevice == NULL) {

> > +        g_debug("failed to query udev for path: %s", syspath);

> > +        goto out;

> > +    } else {

> > +        const char *alias = udev_device_get_property_value(

> > +            udevice, "DM_NAME");

> > +        /*

> > +         * NULL means there was an error and empty string means there is

> > no

> > +         * alias. In case of no alias we return NULL instead of empty

> > string.

> > +         */

> > +        if (alias == NULL) {

> > +            g_debug("failed to query udev for device alias for: %s",

> > +                syspath);

> > +        } else if (*alias != 0) {

> > +            ret = g_strdup(alias);

> > +        }

> > +    }

> > +

> > +out:

> > +    udev_unref(udev);

> > +    udev_device_unref(udevice);

> > +    return ret;

> > +}

> > +

> > +static char *get_device_for_syspath(const char *syspath)

> > +{

> > +    struct udev *udev = NULL;

> > +    struct udev_device *udevice = NULL;

> > +    char *ret = NULL;

> > +

> > +    udev = udev_new();

> > +    if (udev == NULL) {

> > +        g_debug("failed to query udev");

> > +        goto out;

> > +    }

> > +    udevice = udev_device_new_from_syspath(udev, syspath);

> > +    if (udevice == NULL) {

> > +        g_debug("failed to query udev for path: %s", syspath);

> > +        goto out;

> > +    } else {

> > +        ret = g_strdup(udev_device_get_devnode(udevice));

> > +    }

> > +

> > +out:

> > +    udev_unref(udev);

> > +    udev_device_unref(udevice);

> > +    return ret;

> > +}

> > +

> > +static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)

> > +{

> > +    g_autofree char *deps_dir = NULL;

> > +    const gchar *dep;

> > +    GDir *dp_deps = NULL;

> > +

> > +    /* List dependent disks */

> > +    deps_dir = g_strdup_printf("%s/slaves", disk_dir);

> > +    g_debug("  listing entries in: %s", deps_dir);

> > +    dp_deps = g_dir_open(deps_dir, 0, NULL);

> > +    if (dp_deps == NULL) {

> > +        g_debug("failed to list entries in %s", deps_dir);

> > +        return;

> > +    }

> > +    while ((dep = g_dir_read_name(dp_deps)) != NULL) {

> > +        g_autofree char *dep_dir = NULL;

> > +        strList *dep_item = NULL;

> > +        char *dev_name;

> >

> > -    free(syspath);

> > +        /* Add dependent disks */

> > +        dep_dir = g_strdup_printf("%s/%s", deps_dir, dep);

> > +        dev_name = get_device_for_syspath(dep_dir);

> > +        if (dev_name != NULL) {

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

> > +        }

> > +    }

> > +    g_dir_close(dp_deps);

> >  }

> >

> > +/*

> > + * Detect partitions subdirectory, name is "<disk_name><number>" or

> > + * "<disk_name>p<number>"

> > + *

> > + * @disk_name -- last component of /sys path (e.g. sda)

> > + * @disk_dir -- sys path of the disk (e.g. /sys/block/sda)

> > + * @disk_dev -- device node of the disk (e.g. /dev/sda)

> > + */

> > +static GuestDiskInfoList *get_disk_partitions(

> > +    GuestDiskInfoList *list,

> > +    const char *disk_name, const char *disk_dir,

> > +    const char *disk_dev)

> > +{

> > +    GuestDiskInfoList *item, *ret = list;

> > +    struct dirent *de_disk;

> > +    DIR *dp_disk = NULL;

> > +    size_t len = strlen(disk_name);

> > +

> > +    dp_disk = opendir(disk_dir);

> >

> 

> Any reason to use both readdir and g_dir APIs in the same patch? Maybe use

> g_dir alone?


g_dir_* provides only names of the entries in the other places where
opendir() is used we check also type of the entry (d_type).

Tomas

> 

> +    while ((de_disk = readdir(dp_disk)) != NULL) {

> > +        g_autofree char *partition_dir = NULL;

> > +        char *dev_name;

> > +        GuestDiskInfo *partition;

> > +

> > +        if (!(de_disk->d_type & DT_DIR)) {

> > +            continue;

> > +        }

> > +

> > +        if (!(strncmp(disk_name, de_disk->d_name, len) == 0 &&

> > +            ((*(de_disk->d_name + len) == 'p' &&

> > +            isdigit(*(de_disk->d_name + len + 1))) ||

> > +                isdigit(*(de_disk->d_name + len))))) {

> > +            continue;

> > +        }

> > +

> > +        partition_dir = g_strdup_printf("%s/%s",

> > +            disk_dir, de_disk->d_name);

> > +        dev_name = get_device_for_syspath(partition_dir);

> > +        if (dev_name == NULL) {

> > +            g_debug("Failed to get device name for syspath: %s",

> > +                disk_dir);

> > +            continue;

> > +        }

> > +        partition = g_new0(GuestDiskInfo, 1);

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

> > +

> > +        item = g_new0(GuestDiskInfoList, 1);

> > +        item->value = partition;

> > +        item->next = ret;

> > +        ret = item;

> > +

> > +    }

> > +    closedir(dp_disk);

> > +

> > +    return ret;

> > +}

> > +

> > +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)

> > +{

> > +    GuestDiskInfoList *item, *ret = NULL;

> > +    GuestDiskInfo *disk;

> > +    DIR *dp = NULL;

> > +    struct dirent *de = NULL;

> > +

> > +    g_debug("listing /sys/block directory");

> > +    dp = opendir("/sys/block");

> > +    if (dp == NULL) {

> > +        error_setg_errno(errp, errno, "Can't open directory

> > \"/sys/block\"");

> > +        return NULL;

> > +    }

> > +    while ((de = readdir(dp)) != NULL) {

> > +        g_autofree char *disk_dir = NULL, *line = NULL,

> > +            *size_path = NULL, *deps_dir = NULL;

> > +        char *dev_name;

> > +        Error *local_err = NULL;

> > +        if (de->d_type != DT_LNK) {

> > +            g_debug("  skipping entry: %s", de->d_name);

> > +            continue;

> > +        }

> > +

> > +        /* Check size and skip zero-sized disks */

> > +        g_debug("  checking disk size");

> > +        size_path = g_strdup_printf("/sys/block/%s/size", de->d_name);

> > +        if (!g_file_get_contents(size_path, &line, NULL, NULL)) {

> > +            g_debug("  failed to read disk size");

> > +            continue;

> > +        }

> > +        if (g_strcmp0(line, "0\n") == 0) {

> > +            g_debug("  skipping zero-sized disk");

> > +            continue;

> > +        }

> > +

> > +        g_debug("  adding %s", de->d_name);

> > +        disk_dir = g_strdup_printf("/sys/block/%s", de->d_name);

> > +        dev_name = get_device_for_syspath(disk_dir);

> > +        if (dev_name == NULL) {

> > +            g_debug("Failed to get device name for syspath: %s",

> > +                disk_dir);

> > +            continue;

> > +        }

> > +        disk = g_new0(GuestDiskInfo, 1);

> > +        disk->name = dev_name;

> > +        disk->partition = false;

> > +        disk->alias = get_alias_for_syspath(disk_dir);

> > +        disk->has_alias = (disk->alias != NULL);

> > +        item = g_new0(GuestDiskInfoList, 1);

> > +        item->value = disk;

> > +        item->next = ret;

> > +        ret = item;

> > +

> > +        /* Get address for non-virtual devices */

> > +        bool is_virtual = is_disk_virtual(disk_dir, &local_err);

> > +        if (local_err != NULL) {

> > +            g_debug("  failed to check disk path, ignoring error: %s",

> > +                error_get_pretty(local_err));

> > +            error_free(local_err);

> > +            local_err = NULL;

> > +            /* Don't try to get the address */

> > +            is_virtual = true;

> > +        }

> > +        if (!is_virtual) {

> > +            disk->address = get_disk_address(disk_dir, &local_err);

> > +            if (local_err != NULL) {

> > +                g_debug("  failed to get device info, ignoring error: %s",

> > +                    error_get_pretty(local_err));

> > +                error_free(local_err);

> > +                local_err = NULL;

> > +            } else if (disk->address != NULL) {

> > +                disk->has_address = true;

> > +            }

> > +        }

> > +

> > +        get_disk_deps(disk_dir, disk);

> > +        ret = get_disk_partitions(ret, de->d_name, disk_dir, dev_name);

> > +    }

> > +    return ret;

> > +}

> > +

> > +#else

> > +

> > +GuestDiskInfoList *qmp_guest_get_disks(Error **errp)

> > +{

> > +    error_setg(errp, QERR_UNSUPPORTED);

> > +    return NULL;

> > +}

> > +

> > +#endif

> > +

> >  /* Return a list of the disk device(s)' info which @mount lies on */

> >  static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,

> >                                                 Error **errp)

> > @@ -2809,7 +3088,8 @@ GList *ga_command_blacklist_init(GList *blacklist)

> >          const char *list[] = {

> >              "guest-get-fsinfo", "guest-fsfreeze-status",

> >              "guest-fsfreeze-freeze", "guest-fsfreeze-freeze-list",

> > -            "guest-fsfreeze-thaw", "guest-get-fsinfo", NULL};

> > +            "guest-fsfreeze-thaw", "guest-get-fsinfo",

> > +            "guest-get-disks", NULL};

> >          char **p = (char **)list;

> >

> >          while (*p) {

> > @@ -3051,9 +3331,3 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error

> > **errp)

> >

> >      return NULL;

> >  }

> > -

> > -GuestDiskInfoList *qmp_guest_get_disks(Error **errp)

> > -{

> > -    error_setg(errp, QERR_UNSUPPORTED);

> > -    return NULL;

> > -}

> > --

> > 2.28.0

> >

> >

> lgtm otherwise

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

> 

> -- 

> Marc-André Lureau


-- 
Tomáš Golembiovský <tgolembi@redhat.com>
diff mbox series

Patch

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 422144bcff..14683dfbd5 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1150,13 +1150,27 @@  static void build_guest_fsinfo_for_virtual_device(char const *syspath,
     closedir(dir);
 }
 
+static bool is_disk_virtual(const char *devpath, Error **errp)
+{
+    g_autofree char *syspath = realpath(devpath, NULL);
+
+    if (!syspath) {
+        error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
+        return false;
+    }
+    return strstr(syspath, "/devices/virtual/block/") != NULL;
+}
+
 /* Dispatch to functions for virtual/real device */
 static void build_guest_fsinfo_for_device(char const *devpath,
                                           GuestFilesystemInfo *fs,
                                           Error **errp)
 {
-    char *syspath = realpath(devpath, NULL);
+    ERRP_GUARD();
+    g_autofree char *syspath = NULL;
+    bool is_virtual = false;
 
+    syspath = realpath(devpath, NULL);
     if (!syspath) {
         error_setg_errno(errp, errno, "realpath(\"%s\")", devpath);
         return;
@@ -1167,16 +1181,281 @@  static void build_guest_fsinfo_for_device(char const *devpath,
     }
 
     g_debug("  parse sysfs path '%s'", syspath);
-
-    if (strstr(syspath, "/devices/virtual/block/")) {
+    is_virtual = is_disk_virtual(syspath, errp);
+    if (*errp != NULL) {
+        return;
+    }
+    if (is_virtual) {
         build_guest_fsinfo_for_virtual_device(syspath, fs, errp);
     } else {
         build_guest_fsinfo_for_real_device(syspath, fs, errp);
     }
+}
+
+#ifdef CONFIG_LIBUDEV
+
+/*
+ * Wrapper around build_guest_fsinfo_for_device() for getting just
+ * the disk address.
+ */
+static GuestDiskAddress *get_disk_address(const char *syspath, Error **errp)
+{
+    g_autoptr(GuestFilesystemInfo) fs = NULL;
+
+    fs = g_new0(GuestFilesystemInfo, 1);
+    build_guest_fsinfo_for_device(syspath, fs, errp);
+    if (fs->disk != NULL) {
+        return g_steal_pointer(&fs->disk->value);
+    }
+    return NULL;
+}
+
+static char *get_alias_for_syspath(const char *syspath)
+{
+    struct udev *udev = NULL;
+    struct udev_device *udevice = NULL;
+    char *ret = NULL;
+
+    udev = udev_new();
+    if (udev == NULL) {
+        g_debug("failed to query udev");
+        goto out;
+    }
+    udevice = udev_device_new_from_syspath(udev, syspath);
+    if (udevice == NULL) {
+        g_debug("failed to query udev for path: %s", syspath);
+        goto out;
+    } else {
+        const char *alias = udev_device_get_property_value(
+            udevice, "DM_NAME");
+        /*
+         * NULL means there was an error and empty string means there is no
+         * alias. In case of no alias we return NULL instead of empty string.
+         */
+        if (alias == NULL) {
+            g_debug("failed to query udev for device alias for: %s",
+                syspath);
+        } else if (*alias != 0) {
+            ret = g_strdup(alias);
+        }
+    }
+
+out:
+    udev_unref(udev);
+    udev_device_unref(udevice);
+    return ret;
+}
+
+static char *get_device_for_syspath(const char *syspath)
+{
+    struct udev *udev = NULL;
+    struct udev_device *udevice = NULL;
+    char *ret = NULL;
+
+    udev = udev_new();
+    if (udev == NULL) {
+        g_debug("failed to query udev");
+        goto out;
+    }
+    udevice = udev_device_new_from_syspath(udev, syspath);
+    if (udevice == NULL) {
+        g_debug("failed to query udev for path: %s", syspath);
+        goto out;
+    } else {
+        ret = g_strdup(udev_device_get_devnode(udevice));
+    }
+
+out:
+    udev_unref(udev);
+    udev_device_unref(udevice);
+    return ret;
+}
+
+static void get_disk_deps(const char *disk_dir, GuestDiskInfo *disk)
+{
+    g_autofree char *deps_dir = NULL;
+    const gchar *dep;
+    GDir *dp_deps = NULL;
+
+    /* List dependent disks */
+    deps_dir = g_strdup_printf("%s/slaves", disk_dir);
+    g_debug("  listing entries in: %s", deps_dir);
+    dp_deps = g_dir_open(deps_dir, 0, NULL);
+    if (dp_deps == NULL) {
+        g_debug("failed to list entries in %s", deps_dir);
+        return;
+    }
+    while ((dep = g_dir_read_name(dp_deps)) != NULL) {
+        g_autofree char *dep_dir = NULL;
+        strList *dep_item = NULL;
+        char *dev_name;
 
-    free(syspath);
+        /* Add dependent disks */
+        dep_dir = g_strdup_printf("%s/%s", deps_dir, dep);
+        dev_name = get_device_for_syspath(dep_dir);
+        if (dev_name != NULL) {
+            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;
+        }
+    }
+    g_dir_close(dp_deps);
 }
 
+/*
+ * Detect partitions subdirectory, name is "<disk_name><number>" or
+ * "<disk_name>p<number>"
+ *
+ * @disk_name -- last component of /sys path (e.g. sda)
+ * @disk_dir -- sys path of the disk (e.g. /sys/block/sda)
+ * @disk_dev -- device node of the disk (e.g. /dev/sda)
+ */
+static GuestDiskInfoList *get_disk_partitions(
+    GuestDiskInfoList *list,
+    const char *disk_name, const char *disk_dir,
+    const char *disk_dev)
+{
+    GuestDiskInfoList *item, *ret = list;
+    struct dirent *de_disk;
+    DIR *dp_disk = NULL;
+    size_t len = strlen(disk_name);
+
+    dp_disk = opendir(disk_dir);
+    while ((de_disk = readdir(dp_disk)) != NULL) {
+        g_autofree char *partition_dir = NULL;
+        char *dev_name;
+        GuestDiskInfo *partition;
+
+        if (!(de_disk->d_type & DT_DIR)) {
+            continue;
+        }
+
+        if (!(strncmp(disk_name, de_disk->d_name, len) == 0 &&
+            ((*(de_disk->d_name + len) == 'p' &&
+            isdigit(*(de_disk->d_name + len + 1))) ||
+                isdigit(*(de_disk->d_name + len))))) {
+            continue;
+        }
+
+        partition_dir = g_strdup_printf("%s/%s",
+            disk_dir, de_disk->d_name);
+        dev_name = get_device_for_syspath(partition_dir);
+        if (dev_name == NULL) {
+            g_debug("Failed to get device name for syspath: %s",
+                disk_dir);
+            continue;
+        }
+        partition = g_new0(GuestDiskInfo, 1);
+        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);
+
+        item = g_new0(GuestDiskInfoList, 1);
+        item->value = partition;
+        item->next = ret;
+        ret = item;
+
+    }
+    closedir(dp_disk);
+
+    return ret;
+}
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+    GuestDiskInfoList *item, *ret = NULL;
+    GuestDiskInfo *disk;
+    DIR *dp = NULL;
+    struct dirent *de = NULL;
+
+    g_debug("listing /sys/block directory");
+    dp = opendir("/sys/block");
+    if (dp == NULL) {
+        error_setg_errno(errp, errno, "Can't open directory \"/sys/block\"");
+        return NULL;
+    }
+    while ((de = readdir(dp)) != NULL) {
+        g_autofree char *disk_dir = NULL, *line = NULL,
+            *size_path = NULL, *deps_dir = NULL;
+        char *dev_name;
+        Error *local_err = NULL;
+        if (de->d_type != DT_LNK) {
+            g_debug("  skipping entry: %s", de->d_name);
+            continue;
+        }
+
+        /* Check size and skip zero-sized disks */
+        g_debug("  checking disk size");
+        size_path = g_strdup_printf("/sys/block/%s/size", de->d_name);
+        if (!g_file_get_contents(size_path, &line, NULL, NULL)) {
+            g_debug("  failed to read disk size");
+            continue;
+        }
+        if (g_strcmp0(line, "0\n") == 0) {
+            g_debug("  skipping zero-sized disk");
+            continue;
+        }
+
+        g_debug("  adding %s", de->d_name);
+        disk_dir = g_strdup_printf("/sys/block/%s", de->d_name);
+        dev_name = get_device_for_syspath(disk_dir);
+        if (dev_name == NULL) {
+            g_debug("Failed to get device name for syspath: %s",
+                disk_dir);
+            continue;
+        }
+        disk = g_new0(GuestDiskInfo, 1);
+        disk->name = dev_name;
+        disk->partition = false;
+        disk->alias = get_alias_for_syspath(disk_dir);
+        disk->has_alias = (disk->alias != NULL);
+        item = g_new0(GuestDiskInfoList, 1);
+        item->value = disk;
+        item->next = ret;
+        ret = item;
+
+        /* Get address for non-virtual devices */
+        bool is_virtual = is_disk_virtual(disk_dir, &local_err);
+        if (local_err != NULL) {
+            g_debug("  failed to check disk path, ignoring error: %s",
+                error_get_pretty(local_err));
+            error_free(local_err);
+            local_err = NULL;
+            /* Don't try to get the address */
+            is_virtual = true;
+        }
+        if (!is_virtual) {
+            disk->address = get_disk_address(disk_dir, &local_err);
+            if (local_err != NULL) {
+                g_debug("  failed to get device info, ignoring error: %s",
+                    error_get_pretty(local_err));
+                error_free(local_err);
+                local_err = NULL;
+            } else if (disk->address != NULL) {
+                disk->has_address = true;
+            }
+        }
+
+        get_disk_deps(disk_dir, disk);
+        ret = get_disk_partitions(ret, de->d_name, disk_dir, dev_name);
+    }
+    return ret;
+}
+
+#else
+
+GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
+{
+    error_setg(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+#endif
+
 /* Return a list of the disk device(s)' info which @mount lies on */
 static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount,
                                                Error **errp)
@@ -2809,7 +3088,8 @@  GList *ga_command_blacklist_init(GList *blacklist)
         const char *list[] = {
             "guest-get-fsinfo", "guest-fsfreeze-status",
             "guest-fsfreeze-freeze", "guest-fsfreeze-freeze-list",
-            "guest-fsfreeze-thaw", "guest-get-fsinfo", NULL};
+            "guest-fsfreeze-thaw", "guest-get-fsinfo",
+            "guest-get-disks", NULL};
         char **p = (char **)list;
 
         while (*p) {
@@ -3051,9 +3331,3 @@  GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
 
     return NULL;
 }
-
-GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
-{
-    error_setg(errp, QERR_UNSUPPORTED);
-    return NULL;
-}