qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Eric Blake <eblake@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Ralph Schmieder <ralph.schmieder@gmail.com>,
	Stefano Brivio <sbrivio@redhat.com>
Subject: Re: [RFC PATCH v3 00/11] qapi: net: add unix socket type support to netdev backend
Date: Tue, 21 Jun 2022 13:16:18 +0100	[thread overview]
Message-ID: <YrG2ktpkLXWJL6R5@work-vm> (raw)
In-Reply-To: <YrGjbf17VdVF/Zj3@redhat.com>

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Tue, Jun 21, 2022 at 11:31:36AM +0100, Dr. David Alan Gilbert wrote:
> > * Laurent Vivier (lvivier@redhat.com) wrote:
> > > On 20/06/2022 20:24, Dr. David Alan Gilbert wrote:
> > > > * Laurent Vivier (lvivier@redhat.com) wrote:
> > > > > "-netdev socket" only supports inet sockets.
> > > > > 
> > > > > It's not a complex task to add support for unix sockets, but
> > > > > the socket netdev parameters are not defined to manage well unix
> > > > > socket parameters.
> > > > > 
> > > > > As discussed in:
> > > > > 
> > > > >    "socket.c added support for unix domain socket datagram transport"
> > > > >    https://lore.kernel.org/qemu-devel/1C0E1BC5-904F-46B0-8044-68E43E67BE60@gmail.com/
> > > > > 
> > > > > This series adds support of unix socket type using SocketAddress QAPI structure.
> > > > > 
> > > > > Two new netdev backends, "stream" and "dgram" are added, that are barely a copy of "socket"
> > > > > backend but they use the SocketAddress QAPI to provide socket parameters.
> > > > > And then they also implement unix sockets (TCP and UDP).
> > > > 
> > > > Had you considered a -netdev chardev?
> > > > 
> > > 
> > > I think by definition a "chardev" doesn't behave like a "netdev". Moreover
> > > "chardev" is already a frontend for several backends (socket, udp, ...),
> > > this would mean we use the frontend "chardev" as a backend of a "netdev".
> > > More and more layers...
> > 
> > Yeh definitely more layers; but perhaps avoiding some duplication.
> > 
> > > And in the case of "-netdev dgram", we can use unix socket and
> > > sendto()/recv() while something like "-chardev udp,id=char0 -netdev
> > > chardev,chardev=char0,id=net0" doesn't support unix (see
> > > qio_channel_socket_dgram_sync()/socket_dgram()) and uses a
> > > "connect()/sendmsg()/recv()" (that really changes the behaviour of the
> > > backend)
> > 
> > It was -chardev socket, path=/unix/socket/path    that I was thinking
> > of; -chardev socket supports both tcp and unix already.
> 
> IMHO we've over-used & abused chardevs in contexts where we really
> should not have done. The chardev API is passable when all you need
> is a persistent bidirectional channel, but is a really bad fit for
> backends wanting to be aware of the dynamic connection oriented
> semantics that sockets offer. The hoops we've had to jump through
> in places to deal with having chardevs open asynchronously or deal
> with automatic chardev re-connection is quite gross.
> 
> Chardev in the past was convenient to use, because we were not so
> great at doing CLI syntax modelling & implementation, so it was
> useful to re-use the chardev code for socket address handling on
> the CLI.  We also didn't historically have nice APIs for dealing
> with sockets - if you didn't use chardevs, you were stuck with
> the raw sockets APIs. With our aim for CLI to be modelled &
> implemented with QAPI these days, that benefit of re-using chardevs
> for CLI is largely eliminated.  With our QIOChannel APIs, the
> benefits of re-using chardevs from an impl POV is also largely
> eliminated.

OK, fair enough.

Dave

> 
> 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 :|
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



      reply	other threads:[~2022-06-21 12:19 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-20 10:18 [RFC PATCH v3 00/11] qapi: net: add unix socket type support to netdev backend Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 01/11] net: introduce convert_host_port() Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 02/11] net: remove the @errp argument of net_client_inits() Laurent Vivier
2022-06-20 11:39   ` Markus Armbruster
2022-06-20 10:18 ` [RFC PATCH v3 03/11] qapi: net: introduce a way to bypass qemu_opts_parse_noisily() Laurent Vivier
2022-06-20 12:43   ` Markus Armbruster
2022-06-20 13:46     ` Laurent Vivier
2022-06-22  8:00   ` Markus Armbruster
2022-06-20 10:18 ` [RFC PATCH v3 04/11] qapi: net: add stream and dgram netdevs Laurent Vivier
2022-06-20 15:21   ` Markus Armbruster
2022-06-20 17:52     ` Laurent Vivier
2022-06-21  8:49       ` Markus Armbruster
2022-06-21 19:27         ` Laurent Vivier
2022-06-22 11:47           ` Markus Armbruster
2022-06-22 16:18             ` Laurent Vivier
2022-06-23 12:49               ` Markus Armbruster
2022-06-20 10:18 ` [RFC PATCH v3 05/11] net: stream: Don't ignore EINVAL on netdev socket connection Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 06/11] net: stream: add unix socket Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 07/11] net: dgram: make dgram_dst generic Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 08/11] net: dgram: move mcast specific code from net_socket_fd_init_dgram() Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 09/11] net: dgram: add unix socket Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 10/11] qemu-sockets: introduce socket_uri() Laurent Vivier
2022-06-20 10:18 ` [RFC PATCH v3 11/11] net: stream: move to QIO Laurent Vivier
2022-06-20 18:24 ` [RFC PATCH v3 00/11] qapi: net: add unix socket type support to netdev backend Dr. David Alan Gilbert
2022-06-21  9:51   ` Laurent Vivier
2022-06-21 10:31     ` Dr. David Alan Gilbert
2022-06-21 10:54       ` Daniel P. Berrangé
2022-06-21 12:16         ` Dr. David Alan Gilbert [this message]

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=YrG2ktpkLXWJL6R5@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=ralph.schmieder@gmail.com \
    --cc=sbrivio@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).