[PULL 0/2] NBD pull request for 2023-05-03

[PULL 0/2] NBD pull request for 2023-05-03

[Eric Blake]

The following changes since commit 044f8cf70a2fdf3b9e4c4d849c66e7855d2c446a:

  Merge tag 'migration-20230428-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-05-03 10:29:30 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2023-05-03

for you to fetch changes up to de79b52604e43fdeba6cee4f5af600b62169f2d2:

  block/export: call blk_set_dev_ops(blk, NULL, NULL) (2023-05-03 14:00:08 -0500)

----------------------------------------------------------------
nbd patches for 2023-05-03

- Eric Blake: clear LISTEN_FDNAMES when consuming systemd sockets
- Stefan Hajnoczi: clear export BlockDeviceOps in central location

----------------------------------------------------------------
Eric Blake (1):
      systemd: Also clear LISTEN_FDNAMES during systemd socket activation

Stefan Hajnoczi (1):
      block/export: call blk_set_dev_ops(blk, NULL, NULL)

 block/export/export.c    | 2 ++
 block/export/vduse-blk.c | 1 -
 util/systemd.c           | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

base-commit: 044f8cf70a2fdf3b9e4c4d849c66e7855d2c446a
-- 
2.40.1

[PULL 1/2] systemd: Also clear LISTEN_FDNAMES during systemd socket activation

[Eric Blake]

Some time after systemd documented LISTEN_PID and LISTEN_FDS for
socket activation, they later added LISTEN_FDNAMES; now documented at:
https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html

In particular, look at the implementation of sd_listen_fds_with_names():
https://github.com/systemd/systemd/blob/main/src/libsystemd/sd-daemon/sd-daemon.c

If we ever pass LISTEN_PID=xxx and LISTEN_FDS=n to a child process,
but leave LISTEN_FDNAMES=... unchanged as inherited from our parent
process, then our child process using sd_listen_fds_with_names() might
see a mismatch in the number of names (unexpected -EINVAL failure), or
even if the number of names matches the values of those names may be
unexpected (with even less predictable results).

Usually, this is not an issue - the point of LISTEN_PID is to tell
systemd socket activation to ignore all other LISTEN_* if they were
not directed to this particular pid.  But if we end up consuming a
socket directed to this qemu process, and later decide to spawn a
child process that also needs systemd socket activation, we must
ensure we are not leaking any stale systemd variables through to that
child.  The easiest way to do this is to wipe ALL LISTEN_* variables
at the time we consume a socket, even if we do not yet care about a
LISTEN_FDNAMES passed in from the parent process.

See also https://lists.freedesktop.org/archives/systemd-devel/2023-March/048920.html

Thanks: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230324153349.1123774-1-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/systemd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/systemd.c b/util/systemd.c
index 5bcac9b4016..ced518f771b 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -51,6 +51,7 @@ unsigned int check_socket_activation(void)
     /* So these are not passed to any child processes we might start. */
     unsetenv("LISTEN_FDS");
     unsetenv("LISTEN_PID");
+    unsetenv("LISTEN_FDNAMES");

     /* So the file descriptors don't leak into child processes. */
     for (i = 0; i < nr_fds; ++i) {
-- 
2.40.1

[PULL 2/2] block/export: call blk_set_dev_ops(blk, NULL, NULL)

[Eric Blake]

From: Stefan Hajnoczi <stefanha@redhat.com>

Most export types install BlockDeviceOps pointers. It is easy to forget
to remove them because that happens automatically via the "drive" qdev
property in hw/ but not block/export/.

Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so
the export types don't need to remember.

This fixes the nbd and vhost-user-blk export types.

Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server")
Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230502211119.720647-1-stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/export/export.c    | 2 ++
 block/export/vduse-blk.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/export/export.c b/block/export/export.c
index e3fee606116..62c7c22d45d 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -192,6 +192,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
     return exp;

 fail:
+    blk_set_dev_ops(exp->blk, NULL, NULL);
     blk_unref(blk);
     aio_context_release(ctx);
     if (exp) {
@@ -219,6 +220,7 @@ static void blk_exp_delete_bh(void *opaque)
     assert(exp->refcount == 0);
     QLIST_REMOVE(exp, next);
     exp->drv->delete(exp);
+    blk_set_dev_ops(exp->blk, NULL, NULL);
     blk_unref(exp->blk);
     qapi_event_send_block_export_deleted(exp->id);
     g_free(exp->id);
diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
index f7ae44e3cea..b53ef39da02 100644
--- a/block/export/vduse-blk.c
+++ b/block/export/vduse-blk.c
@@ -346,7 +346,6 @@ static void vduse_blk_exp_delete(BlockExport *exp)

     blk_remove_aio_context_notifier(exp->blk, blk_aio_attached, blk_aio_detach,
                                     vblk_exp);
-    blk_set_dev_ops(exp->blk, NULL, NULL);
     ret = vduse_dev_destroy(vblk_exp->dev);
     if (ret != -EBUSY) {
         unlink(vblk_exp->recon_file);
-- 
2.40.1

Re: [PULL 0/2] NBD pull request for 2023-05-03

[Richard Henderson]

On 5/3/23 20:02, Eric Blake wrote:
> The following changes since commit 044f8cf70a2fdf3b9e4c4d849c66e7855d2c446a:
> 
>    Merge tag 'migration-20230428-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-05-03 10:29:30 +0100)
> 
> are available in the Git repository at:
> 
>    https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2023-05-03
> 
> for you to fetch changes up to de79b52604e43fdeba6cee4f5af600b62169f2d2:
> 
>    block/export: call blk_set_dev_ops(blk, NULL, NULL) (2023-05-03 14:00:08 -0500)
> 
> ----------------------------------------------------------------
> nbd patches for 2023-05-03
> 
> - Eric Blake: clear LISTEN_FDNAMES when consuming systemd sockets
> - Stefan Hajnoczi: clear export BlockDeviceOps in central location
> 
> ----------------------------------------------------------------
> Eric Blake (1):
>        systemd: Also clear LISTEN_FDNAMES during systemd socket activation
> 
> Stefan Hajnoczi (1):
>        block/export: call blk_set_dev_ops(blk, NULL, NULL)
> 
>   block/export/export.c    | 2 ++
>   block/export/vduse-blk.c | 1 -
>   util/systemd.c           | 1 +
>   3 files changed, 3 insertions(+), 1 deletion(-)
> 
> base-commit: 044f8cf70a2fdf3b9e4c4d849c66e7855d2c446a

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


r~