[Qemu-devel] [PATCH for-2.9] qga: don't fail if mount doesn't have slave devices

Michael Roth posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1490892852-6771-1-git-send-email-mdroth@linux.vnet.ibm.com
Test checkpatch passed
Test docker failed
Test s390x passed
qga/commands-posix.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH for-2.9] qga: don't fail if mount doesn't have slave devices
Posted by Michael Roth 7 years ago
In some cases the slave devices of a virtual block device are tracked
by the parent in the corresponding sysfs node. For instance, if we
have a loop-back mount of the form:

  /dev/loop3p1 on /home/mdroth/mnt type ext4 (rw,relatime,data=ordered)

this will be reflected in sysfs as:

  /sys/devices/virtual/block/loop3/
  ...
  /sys/devices/virtual/block/loop3/slaves
  /sys/devices/virtual/block/loop3/loop3p1

The current code however assumes the mounted virtual block device,
loop3p1 in this case, contains the slaves directory, and reports an
error otherwise. This breaks 'make check' in certain environments.

Fix this by simply skipping attempts to generate disk topology
information in these cases. Since this information is documented
in QAPI as optionally-reported, this should be ok from an API
perspective.

In the future, this can possibly be improved upon by collecting
topology information from the parent in these cases.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands-posix.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 73d93eb..915df9e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -999,7 +999,9 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
     dirpath = g_strdup_printf("%s/slaves", syspath);
     dir = opendir(dirpath);
     if (!dir) {
-        error_setg_errno(errp, errno, "opendir(\"%s\")", dirpath);
+        if (errno != ENOENT) {
+            error_setg_errno(errp, errno, "opendir(\"%s\")", dirpath);
+        }
         g_free(dirpath);
         return;
     }
-- 
2.7.4


Re: [Qemu-devel] [PATCH for-2.9] qga: don't fail if mount doesn't have slave devices
Posted by Peter Maydell 7 years ago
On 30 March 2017 at 17:54, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> In some cases the slave devices of a virtual block device are tracked
> by the parent in the corresponding sysfs node. For instance, if we
> have a loop-back mount of the form:
>
>   /dev/loop3p1 on /home/mdroth/mnt type ext4 (rw,relatime,data=ordered)
>
> this will be reflected in sysfs as:
>
>   /sys/devices/virtual/block/loop3/
>   ...
>   /sys/devices/virtual/block/loop3/slaves
>   /sys/devices/virtual/block/loop3/loop3p1
>
> The current code however assumes the mounted virtual block device,
> loop3p1 in this case, contains the slaves directory, and reports an
> error otherwise. This breaks 'make check' in certain environments.
>
> Fix this by simply skipping attempts to generate disk topology
> information in these cases. Since this information is documented
> in QAPI as optionally-reported, this should be ok from an API
> perspective.
>
> In the future, this can possibly be improved upon by collecting
> topology information from the parent in these cases.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qga/commands-posix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 73d93eb..915df9e 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -999,7 +999,9 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
>      dirpath = g_strdup_printf("%s/slaves", syspath);
>      dir = opendir(dirpath);
>      if (!dir) {
> -        error_setg_errno(errp, errno, "opendir(\"%s\")", dirpath);
> +        if (errno != ENOENT) {
> +            error_setg_errno(errp, errno, "opendir(\"%s\")", dirpath);
> +        }
>          g_free(dirpath);
>          return;
>      }
> --
> 2.7.4

Tested-by: Peter Maydell <peter.maydell@linaro.org>

though only confirming that the test now passes on this sparc box.

thanks
-- PMM