On 13.02.25 11:26, Markus Armbruster wrote:
Eric Blake <ebl...@redhat.com> writes:
On Wed, Feb 12, 2025 at 05:33:51PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Instead of comment
"Keep this type consistent with the nbd-server-start arguments", we
can simply merge these things.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
No problem for me to rebase on top of
[PATCH 0/2] nbd: Allow debugging tuning of handshake limit
if it goes earlier.
I just sent the pull request for that, so this will indeed need
rebasing. But it's still worth reviewing as written.
Also, not that order of nbd-server-start is changed. I think the order
s/not/note/, I presume.
could not be a contract of JSON interface.
Correct - QMP does not mandate wire ordering, so although the change
causes C paramter rearrangement, it is not a breaking change.
Correct.
We could instead of making common base structure go another way
and define two structures with same data=NbdServerOptionsCommon, and
different bases (will have to define additional base strucutres with
SocketAddress and SocketAddressLegacy fields). But it would look a bit
unusual in QAPI.
blockdev-nbd.c | 4 +--
qapi/block-export.json | 57 ++++++++++++++++++++----------------------
2 files changed, 29 insertions(+), 32 deletions(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 9e61fbaf2b..b0b8993a1b 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -215,10 +215,10 @@ void nbd_server_start_options(NbdServerOptions *arg,
Error **errp)
arg->max_connections, errp);
}
-void qmp_nbd_server_start(SocketAddressLegacy *addr,
- const char *tls_creds,
+void qmp_nbd_server_start(const char *tls_creds,
const char *tls_authz,
bool has_max_connections, uint32_t max_connections,
+ SocketAddressLegacy *addr,
Error **errp)
{
SocketAddress *addr_flat = socket_address_flatten(addr);
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 117b05d13c..5eb94213db 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -9,13 +9,7 @@
{ 'include': 'block-core.json' }
##
-# @NbdServerOptions:
-#
-# Keep this type consistent with the nbd-server-start arguments. The
-# only intended difference is using SocketAddress instead of
-# SocketAddressLegacy.
-#
-# @addr: Address on which to listen.
+# @NbdServerOptionsBase:
#
# @tls-creds: ID of the TLS credentials object (since 2.6).
#
@@ -30,14 +24,35 @@
# server from advertising multiple client support (since 5.2;
# default: 100)
#
-# Since: 4.2
+# Since: 10.0
Markus, when refactoring types like this, should Since stay at the
point in time where the fields were first introduced (4.2) or at the
time where the refactoring introduced the new type (10.0)? Or does
type inlining make it all moot if this tag never shows up in the docs?
To answer this, we need to consider what doc comments are for:
documenting our external QMP interface for its users.
Types are not part of this QMP interface, only their members can be,
namely as arguments of commands and events, possibly nested into other
arguments. Types can change in ways that do not affect the interface.
Like in this patch.
Since only members are part of the interface, only members need since
information.
Having to write down since information member by member all the time
would be onerous, so we provide a shorthand: write it down for the type,
and any members without explicit since use that.
Happily in our case all shared members has explicit "since" equal in both copies
pre-patch. So it doesn't really matter which "Since:" we use for the whole shared
structure (I propose 10.0, to stress when it actually introduced).
"addr" field doesn't have explicit "since", so we'd better keep 1.3 for command and 4.2
for structure. I can denote it in "Since: " of corresponding small structures.
This leads us to the answer to your question. When you refactor types,
take care to preserve the interface documentation's since information,
just like you preserve the actual interface.
Much of the time, this is as trivial as "don't mess with since".
Sometimes, it involves things like adding a new type with an old since.
Occasionally, there's a conflict. Say you unify two similar types into
one, a perfectly sane refactoring. But the two types can have
conflicting since information. Perhaps one type's member @foo goes back
to 4.2, and the other's goes back only to 9.1. We don't have good
solutions for that. We do our best in prose.
Maintaining accurate since information is bothersome and error prone.
It should be done by the machine, not us. John and I have pondered ways
to do this, but we have other projects to finish first.
Questions?
[...]
--
Best regards,
Vladimir
