Re: Who / what uses QMP command add_client?

[Daniel P . Berrangé]

On Wed, Nov 30, 2022 at 01:30:43PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, Nov 29, 2022 at 03:54:56PM +0100, Markus Armbruster wrote:
> >> QMP command add_client's schema:
> >> 
> >>     ##
> >>     # @add_client:
> >>     #
> >>     # Allow client connections for VNC, Spice and socket based
> >>     # character devices to be passed in to QEMU via SCM_RIGHTS.
> >>     #
> >>     # @protocol: protocol name. Valid names are "vnc", "spice", 
> >> "@dbus-display" or
> >>     #            the name of a character device (eg. from -chardev id=XXXX)
> >>     #
> >>     # @fdname: file descriptor name previously passed via 'getfd' command
> >>     #
> >>     # @skipauth: whether to skip authentication. Only applies
> >>     #            to "vnc" and "spice" protocols
> >>     #
> >>     # @tls: whether to perform TLS. Only applies to the "spice"
> >>     #       protocol
> >>     #
> >>     # Returns: nothing on success.
> >>     #
> >>     # Since: 0.14
> >>     #
> >>     # Example:
> >>     #
> >>     # -> { "execute": "add_client", "arguments": { "protocol": "vnc",
> >>     #                                              "fdname": "myclient" } }
> >>     # <- { "return": {} }
> >>     #
> >>     ##
> >>     { 'command': 'add_client',
> >>       'data': { 'protocol': 'str', 'fdname': 'str', '*skipauth': 'bool',
> >>                 '*tls': 'bool' } }
> >> 
> >> Spot the design flaw!
> >> 
> >> It's overloaded @protocol.  Two issues.
> >
> > My bad. Can't imagine why I called its impl add_graphics_client
> > but then made it work for graphics clients and chardevs all
> > the way back in day 1.
> 
> We had a lot less experience with QMP interface design back then.
> 
> The obvious fix (if we want to) is to add protocol "chardev" with
> additional member for the chardev's ID, and deprecate use of chardev IDs
> as protocol.
> 
> Compatibility break: a chardev with ID "chardev" no longer works.
> 
> Could also use "socket" instead of "chardev"if we're confident no other
> chardev type will ever be needed here.

Or introduce a new 'id' field that are refer to a qdev ID, since
we can assign IDs to VNC/SPICE server instances too, when there
are multiple instances, and they'll be non-overlapping with
chardev IDs ?

IOW we make 'protocol' and 'id' both optional in QAPI schema, and
declare them mutually exclusive. Deprecate 'protocol' in favour
of 'id'. Then eventually delete 'protocol' and make 'id'
mandatory.


> >> Are there any known uses with character devices?
> 
> See [*] below.
> 
> >> If not, any ideas why one would want to use the command with character
> >> devices?
> >
> > Ordinarily a client will directly connect() to QEMU to setup the
> > socket connection. Depending on the protocol this may involve both
> > TLS negotiation and authentication. This is a good thing when exposed
> > over a public IP address. It is tedious when connecting from a local
> > client though.
> >
> > The idea behind the 'add_client' method was to enable short circuiting
> > of encryption and authentication, for local only clients. For example,
> > virt-viewer/virt-manager can do socketpair() and pass one of the FDs
> > across to QEMU, and bypass any VNC authentication. This is still secure,
> > as FD passing is mediated by libvirt which the app has already
> > authenticated against.
> >
> > This is conceptually useful for any backend exposed as a network
> > socket, accepting ad-hoc client connections. So it is in scope for
> > chardevs, nbd, vnc, spice.
> 
> Does libvirt implement this with socket character devices?

Opps, I meant to say that libvirt only uses add_client for
graphics devices. We've never used it for chardevs AFAICT.

With 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 :|