QAPI schema review only.
"Daniel P. Berrange" <berra...@redhat.com> writes:
> When starting QEMU management apps will usually setup a monitor socket, and
> then open it immediately after startup. If not using QEMU's own -daemonize
> arg, this process can be troublesome to handle correctly. The mgmt app will
> need to repeatedly call connect() until it succeeds, because it does not
> know when QEMU has created the listener socket. If can't retry connect()
> forever though, because an error might have caused QEMU to exit before it
> even creates the monitor.
>
> The obvious way to fix this kind of problem is to just pass in a pre-opened
> socket file descriptor for the QEMU monitor to listen on. The management app
> can now immediately call connect() just once. If connect() fails it knows
> that QEMU has exited with an error.
>
> The SocketAddress(Legacy) structs allow for FD passing via the monitor, using
> the 'getfd' command, but only when using QMP JSON syntax. The HMP syntax has
> no way to initialize the SocketAddress(Legacy) 'fd' variant. So this patch
> first wires up the 'fd' parameter to refer to a monitor file descriptor,
> allowing HMP to use
>
> getfd myfd
> chardev-add socket,fd=myfd
>
> The SocketAddress 'fd' variant is currently tied to the use of the monitor
> 'getfd' command, so we have a chicken & egg problem with reusing that at
> startup wher no monitor connection is available. We could define that the
s/wher/where/
> special fd name prefix '/dev/fdset' refers to a FD passed via the CLI, but
> magic strings feel unpleasant.
>
> Instead we define a SocketAddress 'fdset' variant that takes an fd set number
> that works in combination with the 'add-fd' command line argument. e.g.
>
> -add-fd fd=3,set=1
> -chardev socket,fdset=1,id=mon
> -mon chardev=mon,mode=control
>
> Note that we do not wire this up in the legacy chardev syntax, so you cannot
> use FD passing with '-qmp', you must use the modern '-mon' + '-chardev' pair
>
> An illustrative example of usage is:
>
> #!/usr/bin/perl
>
> use IO::Socket::UNIX;
> use Fcntl;
>
> unlink "/tmp/qmp";
> my $srv = IO::Socket::UNIX->new(
> Type => SOCK_STREAM(),
> Local => "/tmp/qmp",
> Listen => 1,
> );
>
> my $flags = fcntl $srv, F_GETFD, 0;
> fcntl $srv, F_SETFD, $flags & ~FD_CLOEXEC;
>
> my $fd = $srv->fileno();
>
> exec "qemu-system-x86_64", \
> "-add-fd", "fd=$fd,set=1", \
> "-chardev", "socket,fdset=1,server,nowait,id=mon", \
> "-mon", "chardev=mon,mode=control";
>
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
[...]
> diff --git a/qapi/common.json b/qapi/common.json
> index 6eb01821ef..a15cdc36e9 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -74,6 +74,17 @@
> { 'enum': 'OnOffSplit',
> 'data': [ 'on', 'off', 'split' ] }
>
> +##
> +# @Int:
> +#
> +# A fat type wrapping 'int', to be embedded in lists.
I figure you got the "to be embedded in lists" part from @String. That
one's occasionally used as list element type, but there are other uses.
@Int has only such other uses so far. Let's drop this line from both types.
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'Int',
> + 'data': {
> + 'i': 'int' } }
> +
> ##
> # @String:
> #
> diff --git a/qapi/sockets.json b/qapi/sockets.json
> index ac022c6ad0..f3cac02166 100644
> --- a/qapi/sockets.json
> +++ b/qapi/sockets.json
> @@ -112,7 +112,8 @@
> 'inet': 'InetSocketAddress',
> 'unix': 'UnixSocketAddress',
> 'vsock': 'VsockSocketAddress',
> - 'fd': 'String' } }
> + 'fd': 'String',
> + 'fdset': 'Int' } }
>
> ##
> # @SocketAddressType:
> @@ -123,10 +124,16 @@
> #
> # @unix: Unix domain socket
> #
> +# @vsock: VSOCK socket
> +#
> +# @fd: socket file descriptor passed over monitor
> +#
Indepedent doc fix. I'd put it in a separate patch.
One inaccuracy: @fd is *not* a file descriptor, it's the *name* of a
file descriptor. Please fix.
> +# @fdset: socket file descriptor passed via CLI (since 2.12)
> +#
I gather we have to ways to pass file descriptors. One way identifies
them by name (member @fd), the other by numeric ID (member @fdset).
0. This is disgusting. Is there any way to unify the two, and deprecate
the loser (hopefully the numeric one)?
1. What makes the second one a *set*?
2. What ties the second one to the CLI? Accidents of implementation or
something deeper?
> # Since: 2.9
> ##
> { 'enum': 'SocketAddressType',
> - 'data': [ 'inet', 'unix', 'vsock', 'fd' ] }
> + 'data': [ 'inet', 'unix', 'vsock', 'fd', 'fdset' ] }
>
> ##
> # @SocketAddress:
> @@ -144,4 +151,5 @@
> 'data': { 'inet': 'InetSocketAddress',
> 'unix': 'UnixSocketAddress',
> 'vsock': 'VsockSocketAddress',
> - 'fd': 'String' } }
> + 'fd': 'String',
> + 'fdset': 'Int' } }
[...]
