[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 0/4] qapi/migration: Dedup migration parameter objects and
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 0/4] qapi/migration: Dedup migration parameter objects and fix tls-authz crash |
Date: |
Mon, 16 Oct 2023 09:08:40 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Let me try to summarize our findings so far.
PATCH 1 has been merged. PATCH 2 has been queued, but not merged (not
sure why, accident?).
The remaining two are the actual de-triplication:
PATCH 3: Fuse MigrationParameters and MigrateSetParameters
PATCH 4: De-document MigrationParameter
The latter is a much simpler problem, so let's discuss it first.
Enum MigrationParameter is used only internally. It's in the QAPI
schema just because we want code generated for it. It shouldn't be
documented in the QEMU QMP Reference Manual, but is, because the
generator is too stupid to elide internal-only stuff.
PATCH 4 moves it out of the schema. It has to add back the lost
generated code in hand-written form, which is a bit unfortunate. I
proposed to instead drop most of the useless doc comment by exploiting a
QAPI generator loophole.
Aside: the QAPI generator should elide internal-only stuff from the QEMU
QMP Reference manual, and it should not require doc comments then.
Future work, let's not worry about it now.
The fusing of MigrationParameters and MigrateSetParameters is kind of
stuck. Several options, all with drawbacks or problems:
1. Pick StrOrNull for the tls_FOO members
This is what PATCH 3 does. Blocked on the pre-existing class of
crash bugs discussed in
Subject: QAPI string visitors crashes
Message-ID: <875y3epv3y.fsf@pond.sub.org>
875y3epv3y.fsf@pond.sub.org/">https://lore.kernel.org/qemu-devel/875y3epv3y.fsf@pond.sub.org/
Needs fixing, but when a fix will be available is unclear.
2. Pick str for the tls_FOO members
This is what v1 did. Incompatible change: JSON null no longer works.
Libvirt doesn't use it (it uses deprecated "" instead), but we cannot
know for sure nothing else out there uses it.
I don't think reducing development friction (that's what
de-duplication accomplishes) justifies breaking our compatibility
promise.
To keep the promise, we'd have to deprecate null, un-deprecate "",
let the grace period pass, and only then de-duplicate.
3. Do nothing, live with the duplication
Giving up like this would be sad. Unless we commit to a more
complete overhaul of migration's QAPI/QMP configuration interface,
but I doubt we're ready for that.
Thoughts?
- Re: [PATCH v3 0/4] qapi/migration: Dedup migration parameter objects and fix tls-authz crash,
Markus Armbruster <=