[PULL 0/4] 9p queue 2023-05-16

[PULL 0/4] 9p queue 2023-05-16

[Christian Schoenebeck]

The following changes since commit ab4c44d657aeca7e1da6d6dcb1741c8e7d357b8b:

  Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-05-15 13:54:33 -0700)

are available in the Git repository at:

  https://github.com/cschoenebeck/qemu.git tags/pull-9p-20230516

for you to fetch changes up to 3887702e5f8995638c98f9d9326b4913fb107be7:

  configure: make clear that VirtFS is 9p (2023-05-16 16:21:54 +0200)

----------------------------------------------------------------
9pfs: fixes

* Fixes for Xen, configure and a theoretical leak.

----------------------------------------------------------------
Christian Schoenebeck (2):
      tests/9p: fix potential leak in v9fs_rreaddir()
      configure: make clear that VirtFS is 9p

Jason Andryuk (1):
      9pfs/xen: Fix segfault on shutdown

Peter Foley (1):
      Don't require libcap-ng for virtfs support

 hw/9pfs/trace-events                  |  6 ++++++
 hw/9pfs/xen-9p-backend.c              | 35 ++++++++++++++++++++++-------------
 meson.build                           | 14 ++++++++++----
 meson_options.txt                     |  2 ++
 scripts/meson-buildoptions.sh         |  4 ++++
 tests/qtest/libqos/virtio-9p-client.c |  5 +++++
 6 files changed, 49 insertions(+), 17 deletions(-)

[PULL 2/4] tests/9p: fix potential leak in v9fs_rreaddir()

[Christian Schoenebeck]

Free allocated directory entries in v9fs_rreaddir() if argument
`entries` was passed as NULL, to avoid a memory leak. It is
explicitly allowed by design for `entries` to be NULL. [1]

[1] https://lore.kernel.org/all/1690923.g4PEXVpXuU@silver

Reported-by: Coverity (CID 1487558)
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <E1psh5T-0002XN-1C@lizzy.crudebyte.com>
---
 tests/qtest/libqos/virtio-9p-client.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/libqos/virtio-9p-client.c b/tests/qtest/libqos/virtio-9p-client.c
index e4a368e036..b8adc8d4b9 100644
--- a/tests/qtest/libqos/virtio-9p-client.c
+++ b/tests/qtest/libqos/virtio-9p-client.c
@@ -594,6 +594,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
 {
     uint32_t local_count;
     struct V9fsDirent *e = NULL;
+    /* only used to avoid a leak if entries was NULL */
+    struct V9fsDirent *unused_entries = NULL;
     uint16_t slen;
     uint32_t n = 0;
 
@@ -612,6 +614,8 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
             e = g_new(struct V9fsDirent, 1);
             if (entries) {
                 *entries = e;
+            } else {
+                unused_entries = e;
             }
         } else {
             e = e->next = g_new(struct V9fsDirent, 1);
@@ -628,6 +632,7 @@ void v9fs_rreaddir(P9Req *req, uint32_t *count, uint32_t *nentries,
         *nentries = n;
     }
 
+    v9fs_free_dirents(unused_entries);
     v9fs_req_free(req);
 }
 
-- 
2.30.2

[PULL 3/4] 9pfs/xen: Fix segfault on shutdown

[Christian Schoenebeck]

From: Jason Andryuk <jandryuk@gmail.com>

xen_9pfs_free can't use gnttabdev since it is already closed and NULL-ed
out when free is called.  Do the teardown in _disconnect().  This
matches the setup done in _connect().

trace-events are also added for the XenDevOps functions.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Message-Id: <20230502143722.15613-1-jandryuk@gmail.com>
[C.S.: - Remove redundant return in xen_9pfs_free().
       - Add comment to trace-events. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/trace-events     |  6 ++++++
 hw/9pfs/xen-9p-backend.c | 35 ++++++++++++++++++++++-------------
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
index 6c77966c0b..a12e55c165 100644
--- a/hw/9pfs/trace-events
+++ b/hw/9pfs/trace-events
@@ -48,3 +48,9 @@ v9fs_readlink(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
 v9fs_readlink_return(uint16_t tag, uint8_t id, char* target) "tag %d id %d name %s"
 v9fs_setattr(uint16_t tag, uint8_t id, int32_t fid, int32_t valid, int32_t mode, int32_t uid, int32_t gid, int64_t size, int64_t atime_sec, int64_t mtime_sec) "tag %u id %u fid %d iattr={valid %d mode %d uid %d gid %d size %"PRId64" atime=%"PRId64" mtime=%"PRId64" }"
 v9fs_setattr_return(uint16_t tag, uint8_t id) "tag %u id %u"
+
+# xen-9p-backend.c
+xen_9pfs_alloc(char *name) "name %s"
+xen_9pfs_connect(char *name) "name %s"
+xen_9pfs_disconnect(char *name) "name %s"
+xen_9pfs_free(char *name) "name %s"
diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 0e266c552b..4aa9c8c736 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -25,6 +25,8 @@
 #include "qemu/iov.h"
 #include "fsdev/qemu-fsdev.h"
 
+#include "trace.h"
+
 #define VERSIONS "1"
 #define MAX_RINGS 8
 #define MAX_RING_ORDER 9
@@ -337,6 +339,8 @@ static void xen_9pfs_disconnect(struct XenLegacyDevice *xendev)
     Xen9pfsDev *xen_9pdev = container_of(xendev, Xen9pfsDev, xendev);
     int i;
 
+    trace_xen_9pfs_disconnect(xendev->name);
+
     for (i = 0; i < xen_9pdev->num_rings; i++) {
         if (xen_9pdev->rings[i].evtchndev != NULL) {
             qemu_set_fd_handler(qemu_xen_evtchn_fd(xen_9pdev->rings[i].evtchndev),
@@ -345,40 +349,41 @@ static void xen_9pfs_disconnect(struct XenLegacyDevice *xendev)
                                    xen_9pdev->rings[i].local_port);
             xen_9pdev->rings[i].evtchndev = NULL;
         }
-    }
-}
-
-static int xen_9pfs_free(struct XenLegacyDevice *xendev)
-{
-    Xen9pfsDev *xen_9pdev = container_of(xendev, Xen9pfsDev, xendev);
-    int i;
-
-    if (xen_9pdev->rings[0].evtchndev != NULL) {
-        xen_9pfs_disconnect(xendev);
-    }
-
-    for (i = 0; i < xen_9pdev->num_rings; i++) {
         if (xen_9pdev->rings[i].data != NULL) {
             xen_be_unmap_grant_refs(&xen_9pdev->xendev,
                                     xen_9pdev->rings[i].data,
                                     xen_9pdev->rings[i].intf->ref,
                                     (1 << xen_9pdev->rings[i].ring_order));
+            xen_9pdev->rings[i].data = NULL;
         }
         if (xen_9pdev->rings[i].intf != NULL) {
             xen_be_unmap_grant_ref(&xen_9pdev->xendev,
                                    xen_9pdev->rings[i].intf,
                                    xen_9pdev->rings[i].ref);
+            xen_9pdev->rings[i].intf = NULL;
         }
         if (xen_9pdev->rings[i].bh != NULL) {
             qemu_bh_delete(xen_9pdev->rings[i].bh);
+            xen_9pdev->rings[i].bh = NULL;
         }
     }
 
     g_free(xen_9pdev->id);
+    xen_9pdev->id = NULL;
     g_free(xen_9pdev->tag);
+    xen_9pdev->tag = NULL;
     g_free(xen_9pdev->path);
+    xen_9pdev->path = NULL;
     g_free(xen_9pdev->security_model);
+    xen_9pdev->security_model = NULL;
     g_free(xen_9pdev->rings);
+    xen_9pdev->rings = NULL;
+}
+
+static int xen_9pfs_free(struct XenLegacyDevice *xendev)
+{
+    trace_xen_9pfs_free(xendev->name);
+
     return 0;
 }
 
@@ -390,6 +395,8 @@ static int xen_9pfs_connect(struct XenLegacyDevice *xendev)
     V9fsState *s = &xen_9pdev->state;
     QemuOpts *fsdev;
 
+    trace_xen_9pfs_connect(xendev->name);
+
     if (xenstore_read_fe_int(&xen_9pdev->xendev, "num-rings",
                              &xen_9pdev->num_rings) == -1 ||
         xen_9pdev->num_rings > MAX_RINGS || xen_9pdev->num_rings < 1) {
@@ -499,6 +506,8 @@ out:
 
 static void xen_9pfs_alloc(struct XenLegacyDevice *xendev)
 {
+    trace_xen_9pfs_alloc(xendev->name);
+
     xenstore_write_be_str(xendev, "versions", VERSIONS);
     xenstore_write_be_int(xendev, "max-rings", MAX_RINGS);
     xenstore_write_be_int(xendev, "max-ring-page-order", MAX_RING_ORDER);
-- 
2.30.2

Re: [PULL 3/4] 9pfs/xen: Fix segfault on shutdown

[Michael Tokarev]

16.05.2023 18:21, Christian Schoenebeck wrote:
> From: Jason Andryuk <jandryuk@gmail.com>
> 
> xen_9pfs_free can't use gnttabdev since it is already closed and NULL-ed
> out when free is called.  Do the teardown in _disconnect().  This
> matches the setup done in _connect().
> 
> trace-events are also added for the XenDevOps functions.

This, while somewhat big, still smells like a stable-8.0 (and stable-7.2) material.

/mjt

[PULL 1/4] Don't require libcap-ng for virtfs support

[Christian Schoenebeck]

From: Peter Foley <pefoley@google.com>

It's only required for the proxy helper.

Add a new option for the proxy helper rather than enabling it
implicitly.

Change-Id: I95b73fca625529e99d16b0a64e01c65c0c1d43f2
Signed-off-by: Peter Foley <pefoley@google.com>
Message-Id: <20230503130757.863824-1-pefoley@google.com>
[C.S.: - Resolve merge conflict. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 meson.build                   | 12 +++++++++---
 meson_options.txt             |  2 ++
 scripts/meson-buildoptions.sh |  4 ++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 25a4b9f2c1..c6b05149e9 100644
--- a/meson.build
+++ b/meson.build
@@ -1766,12 +1766,17 @@ have_virtfs = get_option('virtfs') \
              error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
     .require(targetos == 'linux' or cc.has_function('pthread_fchdir_np'),
              error_message: 'virtio-9p (virtfs) on macOS requires the presence of pthread_fchdir_np') \
-    .require(targetos == 'darwin' or (libattr.found() and libcap_ng.found()),
-             error_message: 'virtio-9p (virtfs) on Linux requires libcap-ng-devel and libattr-devel') \
+    .require(targetos == 'darwin' or libattr.found(),
+             error_message: 'virtio-9p (virtfs) on Linux requires libattr-devel') \
     .disable_auto_if(not have_tools and not have_system) \
     .allowed()
 
-have_virtfs_proxy_helper = targetos != 'darwin' and have_virtfs and have_tools
+have_virtfs_proxy_helper = get_option('virtfs_proxy_helper') \
+    .require(targetos != 'darwin', error_message: 'the virtfs proxy helper is incompatible with macOS') \
+    .require(have_virtfs, error_message: 'the virtfs proxy helper requires that virtfs is enabled') \
+    .disable_auto_if(not have_tools) \
+    .require(libcap_ng.found(), error_message: 'the virtfs proxy helper requires libcap-ng') \
+    .allowed()
 
 if get_option('block_drv_ro_whitelist') == ''
   config_host_data.set('CONFIG_BDRV_RO_WHITELIST', '')
@@ -3926,6 +3931,7 @@ if have_block
   summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')}
   summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')}
   summary_info += {'VirtFS support':    have_virtfs}
+  summary_info += {'VirtFS Proxy Helper support': have_virtfs_proxy_helper}
   summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')}
   summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')}
   summary_info += {'bochs support':     get_option('bochs').allowed()}
diff --git a/meson_options.txt b/meson_options.txt
index d8330a1f71..11aec2a441 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -274,6 +274,8 @@ option('vhost_user_blk_server', type: 'feature', value: 'auto',
        description: 'build vhost-user-blk server')
 option('virtfs', type: 'feature', value: 'auto',
        description: 'virtio-9p support')
+option('virtfs_proxy_helper', type: 'feature', value: 'auto',
+       description: 'virtio-9p proxy helper support')
 option('libvduse', type: 'feature', value: 'auto',
        description: 'build VDUSE Library')
 option('vduse_blk_export', type: 'feature', value: 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 2805d1c145..52fb079a60 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -181,6 +181,8 @@ meson_options_help() {
   printf "%s\n" '  vhost-vdpa      vhost-vdpa kernel backend support'
   printf "%s\n" '  virglrenderer   virgl rendering support'
   printf "%s\n" '  virtfs          virtio-9p support'
+  printf "%s\n" '  virtfs-proxy-helper'
+  printf "%s\n" '                  virtio-9p proxy helper support'
   printf "%s\n" '  vmdk            vmdk image format support'
   printf "%s\n" '  vmnet           vmnet.framework network backend support'
   printf "%s\n" '  vnc             VNC server'
@@ -474,6 +476,8 @@ _meson_option_parse() {
     --disable-virglrenderer) printf "%s" -Dvirglrenderer=disabled ;;
     --enable-virtfs) printf "%s" -Dvirtfs=enabled ;;
     --disable-virtfs) printf "%s" -Dvirtfs=disabled ;;
+    --enable-virtfs-proxy-helper) printf "%s" -Dvirtfs_proxy_helper=enabled ;;
+    --disable-virtfs-proxy-helper) printf "%s" -Dvirtfs_proxy_helper=disabled ;;
     --enable-vmdk) printf "%s" -Dvmdk=enabled ;;
     --disable-vmdk) printf "%s" -Dvmdk=disabled ;;
     --enable-vmnet) printf "%s" -Dvmnet=enabled ;;
-- 
2.30.2

[PULL 4/4] configure: make clear that VirtFS is 9p

[Christian Schoenebeck]

Add '9P' to the summary output section of 'VirtFS' to avoid being
confused with virtiofs.

Based-on: <20230503130757.863824-1-pefoley@google.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <E1px7Id-0000NE-OQ@lizzy.crudebyte.com>
---
 meson.build | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index c6b05149e9..5e2807ea7c 100644
--- a/meson.build
+++ b/meson.build
@@ -3930,8 +3930,8 @@ if have_block
   summary_info += {'Block whitelist (rw)': get_option('block_drv_rw_whitelist')}
   summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')}
   summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')}
-  summary_info += {'VirtFS support':    have_virtfs}
-  summary_info += {'VirtFS Proxy Helper support': have_virtfs_proxy_helper}
+  summary_info += {'VirtFS (9P) support':    have_virtfs}
+  summary_info += {'VirtFS (9P) Proxy Helper support': have_virtfs_proxy_helper}
   summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')}
   summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')}
   summary_info += {'bochs support':     get_option('bochs').allowed()}
-- 
2.30.2

Re: [PULL 0/4] 9p queue 2023-05-16

[Richard Henderson]

On 5/16/23 08:21, Christian Schoenebeck wrote:
> The following changes since commit ab4c44d657aeca7e1da6d6dcb1741c8e7d357b8b:
> 
>    Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-05-15 13:54:33 -0700)
> 
> are available in the Git repository at:
> 
>    https://github.com/cschoenebeck/qemu.git tags/pull-9p-20230516
> 
> for you to fetch changes up to 3887702e5f8995638c98f9d9326b4913fb107be7:
> 
>    configure: make clear that VirtFS is 9p (2023-05-16 16:21:54 +0200)
> 
> ----------------------------------------------------------------
> 9pfs: fixes
> 
> * Fixes for Xen, configure and a theoretical leak.

The public key I have for you is expired.
Have you pushed a refresh of it somewhere?


r~

> 
> ----------------------------------------------------------------
> Christian Schoenebeck (2):
>        tests/9p: fix potential leak in v9fs_rreaddir()
>        configure: make clear that VirtFS is 9p
> 
> Jason Andryuk (1):
>        9pfs/xen: Fix segfault on shutdown
> 
> Peter Foley (1):
>        Don't require libcap-ng for virtfs support
> 
>   hw/9pfs/trace-events                  |  6 ++++++
>   hw/9pfs/xen-9p-backend.c              | 35 ++++++++++++++++++++++-------------
>   meson.build                           | 14 ++++++++++----
>   meson_options.txt                     |  2 ++
>   scripts/meson-buildoptions.sh         |  4 ++++
>   tests/qtest/libqos/virtio-9p-client.c |  5 +++++
>   6 files changed, 49 insertions(+), 17 deletions(-)
> 

Re: [PULL 0/4] 9p queue 2023-05-16

[Richard Henderson]

On 5/16/23 10:17, Richard Henderson wrote:
> On 5/16/23 08:21, Christian Schoenebeck wrote:
>> The following changes since commit ab4c44d657aeca7e1da6d6dcb1741c8e7d357b8b:
>>
>>    Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging 
>> (2023-05-15 13:54:33 -0700)
>>
>> are available in the Git repository at:
>>
>>    https://github.com/cschoenebeck/qemu.git tags/pull-9p-20230516
>>
>> for you to fetch changes up to 3887702e5f8995638c98f9d9326b4913fb107be7:
>>
>>    configure: make clear that VirtFS is 9p (2023-05-16 16:21:54 +0200)
>>
>> ----------------------------------------------------------------
>> 9pfs: fixes
>>
>> * Fixes for Xen, configure and a theoretical leak.
> 
> The public key I have for you is expired.
> Have you pushed a refresh of it somewhere?

Nevermind, I've found it.


r~

Re: [PULL 0/4] 9p queue 2023-05-16

[Richard Henderson]

On 5/16/23 08:21, Christian Schoenebeck wrote:
> The following changes since commit ab4c44d657aeca7e1da6d6dcb1741c8e7d357b8b:
> 
>    Merge tag 'block-pull-request' ofhttps://gitlab.com/stefanha/qemu  into staging (2023-05-15 13:54:33 -0700)
> 
> are available in the Git repository at:
> 
>    https://github.com/cschoenebeck/qemu.git  tags/pull-9p-20230516
> 
> for you to fetch changes up to 3887702e5f8995638c98f9d9326b4913fb107be7:
> 
>    configure: make clear that VirtFS is 9p (2023-05-16 16:21:54 +0200)
> 
> ----------------------------------------------------------------
> 9pfs: fixes
> 
> * Fixes for Xen, configure and a theoretical leak.

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~