Bug 2055229

Summary: [qemu-kvm] Support systemd socket activation in qemu-storage-daemon
Product: Red Hat Enterprise Linux 9 Reporter: Nir Soffer <nsoffer>
Component: qemu-kvmAssignee: Eric Blake <eblake>
qemu-kvm sub component: NBD QA Contact: Tingting Mao <timao>
Status: CLOSED MIGRATED Docs Contact:
Severity: medium    
Priority: medium CC: aliang, berrange, bstinson, coli, eblake, jinzhao, juzhang, jwboyer, kwolf, qinwang, rjones, vgoyal, virt-maint, ymankad
Version: CentOS StreamKeywords: FutureFeature, MigratedToJIRA, RFE, Triaged
Target Milestone: rcFlags: pm-rhel: mirror+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-09-22 16:26:43 UTC Type: Story
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Nir Soffer 2022-02-16 13:47:56 UTC
Description of problem:

If I understand correctly new qemu-storage-daemon replace qemu-nbd. New features
added to qemu-storage-daemon may not be available in qemu-nbd. Project using
qemu-nbd (e.g. oVirt, virt-v2v, libnbd) should move to use qemu-storage-daemon.

A very useful feature of qemu-nbd is systemd socket activation is missing in
qemu-storage-daemon. We would like to have it.

Using systemd socket activation eliminate the need to wait until the NBD server
is ready. This can be done today with waiting for the pid file, but it requires
polling when starting the server, and removing the pid file after the server
is terminate. This has to be implemented in all clients using the server.

Supporting systemd socket activation on the server side is relatively simple.

However it seems that qemu-storage-daemon has a better solution - passing listen
socket to the daemon:

       Launch  the daemon with QMP monitor socket qmp.sock so clients can exe‐
       cute QMP commands:

          $ qemu-storage-daemon \
              --chardev socket,path=qmp.sock,server=on,wait=off,id=char1 \
              --monitor chardev=char1

       Launch the daemon from Python with a QMP monitor socket using file  de‐
       scriptor  passing  so there is no need to busy wait for the QMP monitor
       to become available:

          #!/usr/bin/env python3
          import subprocess
          import socket

          sock_path = '/var/run/qmp.sock'

          with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as listen_sock:
              listen_sock.bind(sock_path)
              listen_sock.listen()

              fd = listen_sock.fileno()

              subprocess.Popen(
                  ['qemu-storage-daemon',
                   '--chardev', f'socket,fd={fd},server=on,id=char1',
                   '--monitor', 'chardev=char1'],
                  pass_fds=[fd],
              )

          # listen_sock was automatically closed when leaving the 'with' statement
          # body. If the daemon process terminated early then the following connect()
          # will fail with "Connection refused" because no process has the listen
          # socket open anymore. Launch errors can be detected this way.

          qmp_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
          qmp_sock.connect(sock_path)
          ...QMP interaction...

       The same socket spawning approach  also  works  with  the  --nbd-server
       addr.type=fd,addr.str=<fd>                 and                 --export
       type=vhost-user-blk,addr.type=fd,addr.str=<fd> options.


So I'm not sure systemd socket activation is really needed. Opening
this bug for discussion.

Comment 1 Nir Soffer 2022-02-16 13:49:53 UTC
Richard, what do you think?

Comment 2 Daniel Berrangé 2022-02-16 14:44:48 UTC
This discussion should probably go to upstream qemu-devel instead. I believe systemd activation would be a benefit though, as it would make it possible to directly spawn qsd from a system .service unit file with sockets, which is not possible with QEMU's home grown FD passing.

With SystemD activation you can give names to passed in FDs, which are visible in the "LISTEN_FDNAMES" env variable. I'd like to see QEMU support an FD name of 'qmp' to indicate that the passed in FD is to be used to run a QMP monitor.

This would allow for a normal systemd config to spawn QSD with no args and get a QMP socket available to do all runtime config:

 $ cat myapp-qsd.service
 [Unit]
 Description=something QSD something

 [Service]
 ExecStart=/usr/bin/qemu-storage-daemon

 $ cat myapp-qsd-qmp.socket
 [Unit]
 Description=something QSD something socket

 [Socket]
 ListenStream=/var/run/myapp/qsd.qmp
 FileDescriptorName=qmp
 Service=myapp-qsd.service

If we do this it shouldn't be tied to QSD - it should be universal across any QEMU binary that has QMP support

Using 'LISTEN_FDNAMES' we can extend this to other sockets in QEMU too if needed by defining more well known names.

Comment 3 Richard W.M. Jones 2022-02-16 17:08:20 UTC
Yup, systemd socket activation is great and generally easy to implement, so why not.
It's a requirement (as well as full and simple command line configuration) if
qsd is ever to replace uses like:

$ nbdcopy -- [ qemu-nbd -r -f qcow2 https://example.com/disk.qcow2 ] -
$ nbdinfo -- [ qemu-nbd -r -f qcow2 file.qcow2 ]
etc

Comment 4 Klaus Heinrich Kiwi 2022-02-21 18:01:08 UTC
Adding a few more folks for awareness, but indeed should be an upstream discussion - feel free to refer back to this BZ though.

Once we establish a (better?) severity/prioritization/target, we can use this BZ to track.

Comment 6 Daniel Berrangé 2022-09-21 14:53:03 UTC
I thought about this again after my previous comment, and I think we ought to explore how to integrate socket activation with our 'fd' passing support in the SocketAddress struct.

Currently SocketAddressType has:

# @fd: decimal is for file descriptor number, otherwise a file descriptor name.
#      Named file descriptors are permitted in monitor commands, in combination
#      with the 'getfd' command. Decimal file descriptors are permitted at
#      startup or other contexts where no monitor context is active.

The distinction between FDs being named when using QMP and raw FD numbers during startup has always irritated me. With systemd activation, the LISTEN_FDNAMES env variable gives names to each passed in FD at startup. We could make use of this, to allow SocketAddressType to support named FDs during startup, on a par with what's possible with QMP already.

If we did this, then any QEMU program with command line args that have a mapping to the SocketAddress  struct will get transparent support for socket activation usage. This will be way more general purpose than the limited socket activation support we have today.

Comment 7 Richard W.M. Jones 2022-09-21 14:57:22 UTC
However you do it, please make it compatible with
https://libguestfs.org/nbd_connect_systemd_socket_activation.3.html

Comment 8 Kevin Wolf 2022-09-23 16:13:47 UTC
I'm not sure if I would shoehorn socket activation into the existing 'fd' SocketAddress type or add a separate new type, but I agree that any SocketAddress(Legacy) user should automatically support this and am pretty sure I argued for it already in past upstream discussions about this. But the final design needs to be discussed upstream again anyway.

Comment 9 Eric Blake 2023-01-27 21:36:57 UTC
Revisiting this bug; I've tried to start an upstream discussion:
https://lists.nongnu.org/archive/html/qemu-block/2023-01/msg00710.html

However, as it does need to play out in upstream first, it's hard to state what timeline we can expect for downstream usage of whatever solution the list settles on.

Comment 11 qing.wang 2023-07-17 07:46:04 UTC
What is the development plan? 
Does it need to close this or extend the stale date?

Comment 12 Eric Blake 2023-07-17 19:11:17 UTC
(In reply to qing.wang from comment #11)
> What is the development plan? 
> Does it need to close this or extend the stale date?

At present, this has not received any upstream traction. I intend to revive the topic upstream once 64-bit NBD extensions have landed, but that does mean that it is likely that it may take more than 30 days before this bug is addressed.

Comment 15 RHEL Program Management 2023-09-22 16:18:39 UTC
Issue migration from Bugzilla to Jira is in process at this time. This will be the last message in Jira copied from the Bugzilla bug.