qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@gmail.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Date: Thu, 16 Jan 2025 12:27:32 +0100	[thread overview]
Message-ID: <Z4jtJMNeXexEEpVZ@redhat.com> (raw)
In-Reply-To: <CACZ9PQUk7ZjwfYWVNq3z2Wp_pnkKO8ObhLc6uy5ABHq2yCL9Ag@mail.gmail.com>

Am 17.12.2024 um 11:32 hat Roman Penyaev geschrieben:
> Hi Markus,
> 
> Thanks for the explicit info. But I have a lot to ask :)
> Do I understand correctly that there are two ways to parse
> arguments: classic, via qemu_opts_parse_noisily() and modern, via
> qobject_input_visitor_new_str()? (for example, I look in
> net/net.c, netdev_parse_modern()). My goal is not to create a
> completely new option, but to add (extend) parameters for
> chardev, namely to add a new type of backend device. This
> complicates everything, since chardev uses
> qemu_opts_parse_noisily() for parsing and bypasses the modern
> way (I hope I'm not mistaken, maybe Marc can comment). And I'm
> not sure that it's easy to replace the classic way of parsing
> arguments with the modern way without breaking anything.

A few years ago, I tried to unify the QMP and CLI code paths for
creating chardevs and this involved using QAPI for everything. As far as
I can remember, chardevs don't use any of the QemuOpts features that
don't exist in they keyval parser, so it's easy from that angle.

What makes it more complicated is that CLI and QMP options have diverged
quite a bit, and while generally the same functionality is available, it
sometimes uses different names or is negated in one compared to the
other etc.

So I ended up writing compatibility code that translated legacy CLI
options into QAPI-compatible ones, and then I could use the exising QAPI
types. Part of it made use of aliases, which would have been a new
feature in QAPI, but Markus didn't like them in the end. They could have
been replaced by manual conversion code, but I didn't have time (nor, to
be honest, motivation) to work it any more when Markus had finally made
that decision. It shouldn't actually be very hard.

Anyway, if it's of any use for you, feel free to resurrect parts of it:

https://repo.or.cz/qemu/kevin.git/shortlog/refs/tags/qapi-alias-chardev-v4

(Or maybe I will eventually...)

Whatever you choose to do, my one request for you would be that you
really make sure that CLI and QMP are structured and behave exactly the
same with your new option, to avoid making the problem worse than it
already is.

Kevin



  parent reply	other threads:[~2025-01-16 11:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-16 10:25 [PATCH v4 0/8] chardev: implement backend chardev multiplexing Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 1/8] chardev/char: rename `MuxChardev` struct to `MuxFeChardev` Roman Penyaev
2024-10-16 10:25 ` [PATCH v4 2/8] chardev/char: rename `char-mux.c` to `char-mux-fe.c` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 3/8] chardev/char: move away mux suspend/resume calls Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 4/8] chardev/char: rename frontend mux calls Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 5/8] chardev/char: introduce `mux-be-id=ID` option Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing Roman Penyaev
2024-10-16 11:13   ` Marc-André Lureau
2024-10-16 11:18     ` Marc-André Lureau
2024-10-16 14:19     ` Roman Penyaev
2024-11-20  8:00     ` Roman Penyaev
2024-12-11  9:42     ` Markus Armbruster
2024-12-17 10:32       ` Roman Penyaev
2024-12-19 13:45         ` Markus Armbruster
2025-01-16 11:27         ` Kevin Wolf [this message]
2025-01-17  8:03           ` Roman Penyaev
2025-01-17 13:20             ` Kevin Wolf
2024-10-16 10:26 ` [PATCH v4 7/8] tests/unit/test-char: add unit test for the `mux-be` multiplexer Roman Penyaev
2024-10-16 11:36   ` Marc-André Lureau
2024-10-17 13:48     ` Roman Penyaev
2024-10-16 10:26 ` [PATCH v4 8/8] qemu-options.hx: describe multiplexing of several backend devices Roman Penyaev
2024-10-16 11:27   ` Marc-André Lureau

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=Z4jtJMNeXexEEpVZ@redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=r.peniaev@gmail.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).