All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Roman Penyaev <r.peniaev@gmail.com>
Cc: "Marc-André Lureau" <marcandre.lureau@gmail.com>,
	qemu-devel@nongnu.org, "Kevin Wolf" <kwolf@redhat.com>
Subject: Re: [PATCH v4 6/8] chardev/char-mux: implement backend chardev multiplexing
Date: Thu, 19 Dec 2024 14:45:36 +0100	[thread overview]
Message-ID: <87ed23iy67.fsf@pond.sub.org> (raw)
In-Reply-To: <CACZ9PQUk7ZjwfYWVNq3z2Wp_pnkKO8ObhLc6uy5ABHq2yCL9Ag@mail.gmail.com> (Roman Penyaev's message of "Tue, 17 Dec 2024 11:32:46 +0100")

Roman Penyaev <r.peniaev@gmail.com> writes:

> 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()?

Three, to be honest:

* QemuOpts, commonly with qemu_opts_parse_noisily()

* QAPI, commonly with qobject_input_visitor_new_str()

* Ad hoc parsers (you don't wan't to know more)

>                                  (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.

It's not easy.

The main difficulty is assessing compatibility breaks, and whether they
matter.

>                                                          I can,
> of course, be wrong, but if I understand correctly, util/keyval.c
> does not work with QemuOpts,

Correct.

>                              and the entire char/* is very much
> tied to this classic way of getting arguments.

In the beginning, there was the command line (CLI), and then the human
monitor (HMP).

As CLI options and HMP commands were implemented with QemuOpts, the
underlying internal interfaces tended to be adjusted to take QemuOpts
arguments.

Then there was QMP, and then there was QAPI.  As QMP commands were
implemented with QAPI, the underlying internal interfaces tended to be
adjusted to take generated QAPI type arguments.

This got us two internal interfaces doing the same thing.  To not have
two implementations, one interface needs to wrap around the other.
Wrapping the QemuOpts one around the QAPI one is more sane.

Have a look at qmp_chardev_add() and hmp_chardev_add().

qmp_chardev_add() wraps around chardev_new(), which takes QAPI type
ChardevBackend.

hmp_chardev_add() wraps around qemu_chr_new_from_opts(), which wraps
around do_qemu_chr_new_from_opts(), which wraps around
qemu_chardev_new(), which wraps around chardev_new().  CLI -chardev
works the same way.

So...  Yes, at some point the entire chardev/ was very much tied to
QemuOpts.  By now, its core *should* be untied from it.  There *may* be
remnants of the old way that still need to be untied.

>                                                Is there a
> transitional way to parse the arguments? Use the modern way, but
> still represent the arguments as QemuOpts?

You could convert manually from QAPI to QemuOpts, but that would be a
mistake.  We know, because we made the mistake with device_add and
netdev_add.  Fixing the mistake for netdev_add was painful (see commit
db2a380c84574d8c76d7193b8af8535234fe5156 (net: Complete qapi-fication of
netdev_add)).  device_add remains unfixed, which has been a source of
trouble.



  reply	other threads:[~2024-12-19 16:41 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 [this message]
2025-01-16 11:27         ` Kevin Wolf
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=87ed23iy67.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=kwolf@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 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.