qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"John Snow" <jsnow@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: QAPI sync meeting
Date: Thu, 7 Oct 2021 14:02:32 +0100	[thread overview]
Message-ID: <YV7v6K45m9RcZyBx@redhat.com> (raw)
In-Reply-To: <YV7tv9t7FznwRbdw@redhat.com>

On Thu, Oct 07, 2021 at 02:53:19PM +0200, Kevin Wolf wrote:
> Am 07.10.2021 um 12:23 hat Paolo Bonzini geschrieben:
> > On 07/10/21 12:01, Kevin Wolf wrote:
> > > 
> > >    * -chardev: I have patches that QAPIfy the option based on aliases,
> > >      getting rid of the old handwritten parser that is inconsistent with
> > >      QMP in non-obvious ways and replacing it with translation to QMP
> > >      (both using aliases and a little C code) that makes the differences
> > >      obvious.
> > > 
> > >      First posted in November 2020, more details in the cover letter:
> > >      https://patchew.org/QEMU/20201112175905.404472-1-kwolf@redhat.com/
> > > 
> > >      Later versions (not yet posted as a series because I'm waiting for
> > >      aliases) also make -chardev accept JSON syntax, which is what
> > >      libvirt really wants to use.
> > 
> > I'm still not sure about this...  It's an awful lot of code if the aliases
> > are only used by -chardev, and I'd rather use -object/object-add for
> > chardevs if that's at all possible.
> 
> The important part for me there is getting rid of the second parser that
> is inconsistent with QAPI - and people add to it without fully realising
> that it's a separate implementation, so they test -chardev and leave
> chardev-add behind broken.
> 
> My approach keeps the existing command line syntax and still makes sure
> that inputs from both the CLI and QMP go through a single code path,
> making sure that they are consistent.
> 
> Aliases are a helpful tool to achieve this, but the series can be
> rewritten a bit if people are fundamentally against having aliases.
> Aliases do nothing that C code can't do.
> 
> I don't think that aliases are a lot of code, or even complicated code.
> Current v4 looks like a lot of lines of code because Markus made me add
> big comments everywhere and tons of tests. The actual code additions are
> rather small. But I also notice that there is resistance against having
> multiple ways to specify the same thing (which is the essence of
> aliases), so if people hate them, let's throw them away. The only part I
> really dislike with this scenario is that I could have been told almost
> a year ago...
> 
> Anyway, your approach provides a different solution to the goal of
> getting rid of the second parser if you extend it: Add -object support
> to all chardev backends, then deprecate -chardev wholesale and drop it
> two releases later. This feels contentious, but I'm not opposed.

If we were thinking about QEMU from new ignoring existing design,
I could even imagine that all of -chardev, -netdev, -device, etc
would actually be -object. So from my POV I don't think it is
unreasonable to take this direction.

> Timeline: My series could be done for 6.2. Yours could have the
> replacement in 6.2 the earliest if we start working on it right now,
> then libvirt starts using it, deprecation in 7.0 or 7.1, then drop the
> old interface two releases later, i.e.  December next year or March
> 2023.

Are the two approaches mutually exclusive rather than complementary ?
eg is Kevin's work a worthwhile incremental step forward, even if we
eventually get to replacing -chardev with -object ?

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



  reply	other threads:[~2021-10-07 13:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-27 16:55 QAPI sync meeting John Snow
2021-09-28 11:38 ` Marc-André Lureau
2021-09-28 13:14 ` Kevin Wolf
2021-09-28 13:53 ` Daniel P. Berrangé
2021-09-28 17:43   ` John Snow
2021-09-29 12:18     ` Markus Armbruster
2021-09-30  0:49       ` John Snow
2021-10-07 10:01       ` Kevin Wolf
2021-10-07 10:23         ` Paolo Bonzini
2021-10-07 12:53           ` Kevin Wolf
2021-10-07 13:02             ` Daniel P. Berrangé [this message]
2021-10-08 10:06           ` Markus Armbruster
2021-10-08 17:07             ` Paolo Bonzini
2021-10-07 12:43         ` John Snow
2021-09-28 15:19 ` Paolo Bonzini
2021-09-29 13:42 ` Damien Hedde
2021-09-30  0:31   ` John Snow

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=YV7v6K45m9RcZyBx@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@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 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).