All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>,
	"Daniel P. Berrangé" <berrange@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:53:19 +0200	[thread overview]
Message-ID: <YV7tv9t7FznwRbdw@redhat.com> (raw)
In-Reply-To: <3abc4e8e-5657-14bb-ba89-5b7669c01201@redhat.com>

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.

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.

Kevin



  reply	other threads:[~2021-10-07 12:58 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 [this message]
2021-10-07 13:02             ` Daniel P. Berrangé
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=YV7tv9t7FznwRbdw@redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=jsnow@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.