All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v1 2/2] char: allow passing pre-opened socket file descriptor at startup
Date: Thu, 21 Dec 2017 14:20:11 +0000	[thread overview]
Message-ID: <20171221142011.GH30327@redhat.com> (raw)
In-Reply-To: <20171221132717.30284-3-berrange@redhat.com>

On Thu, Dec 21, 2017 at 01:27:17PM +0000, Daniel P. Berrange wrote:
> 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
> 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

Having written this, it occurs to me that using fdset for this is really
just adding uncessary complication.

The 'fd' parameter in SocketAddress is required to be a string that refers
to a named FD  passed over the monitor. I now see, however, that these
strings are forbidden to start with a digit. This means we could easily
re-use this facility to just directly reference a passed-in file descriptor
number, without clashing with named FD strings. eg we could do

   -chardev socket,fd=3,id=mon
   -mon chardev=mon,mode=control

This would simplify this patch significantly, and give close alignement
between the HMP & cli usage.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

  parent reply	other threads:[~2017-12-21 14:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-21 13:27 [Qemu-devel] [PATCH v1 0/2] Enable passing pre-opened chardev socket FDs Daniel P. Berrange
2017-12-21 13:27 ` [Qemu-devel] [PATCH v1 1/2] io: move fd_is_socket() into common sockets code Daniel P. Berrange
2017-12-21 13:49   ` Marc-André Lureau
2017-12-21 13:27 ` [Qemu-devel] [PATCH v1 2/2] char: allow passing pre-opened socket file descriptor at startup Daniel P. Berrange
2017-12-21 13:48   ` Marc-André Lureau
2017-12-21 14:18     ` Eric Blake
2017-12-21 14:20   ` Daniel P. Berrange [this message]
2017-12-21 16:49   ` Markus Armbruster
2017-12-21 16:53     ` Daniel P. Berrange
2017-12-21 19:05     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171221142011.GH30327@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.