All of lore.kernel.org
 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 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.