Re: [PATCH 15/22] qemu: capabilities: Add alternative detection of QEMU_CAPS_NUMA

On Mon, Aug 16, 2021 at 15:09:39 +0200, Martin Kletzander wrote: > On Thu, Aug 12, 2021 at 04:49:08PM +0200, Peter Krempa wrote: > > 'set-numa-node' is the command which can set the equivalent parameters > > to '-numa' in preconfig mode, so we can use it as witness to see that > > -numa is supported. > > > > To ensure that the old detection method is removed once we'll be bumping > > qemu support add a comment with the appropriate version check. > > > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > > --- > > src/qemu/qemu_capabilities.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > > index abd5f0a0d0..cc92ab098b 100644 > > --- a/src/qemu/qemu_capabilities.c > > +++ b/src/qemu/qemu_capabilities.c > > @@ -1183,6 +1183,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { > > { "block-export-add", QEMU_CAPS_BLOCK_EXPORT_ADD }, > > { "query-display-options", QEMU_CAPS_QUERY_DISPLAY_OPTIONS }, > > { "blockdev-reopen", QEMU_CAPS_BLOCKDEV_REOPEN }, > > + { "set-numa-node", QEMU_CAPS_NUMA }, > > }; > > > > struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { > > @@ -3247,7 +3248,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { > > { "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP }, > > { "name", "debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS }, > > { "name", "guest", QEMU_CAPS_NAME_GUEST }, > > - { "numa", NULL, QEMU_CAPS_NUMA }, > > + { "numa", NULL, QEMU_CAPS_NUMA }, /* (qemuCaps->version < 3000000) */ > > Very minor detail, but it is not clear to me that the comment means we > can remove it once we bump the oldest qemu version supported to 3.0.0 or > higher. One or two words would do the trick. If this is universally > understood, however, then disregard this message. This was originally /* if (qemuCaps->version < 3000000) */ to basically add something that has the same format as explicit version checks and thus is easily greppable. Unfortunately our not-very-clever syntax check moaned that the formatting of 'if' isn't compliant thus I've removed the if.

The idea is to have something which references 'qemuCaps->version' and thus is the same as other version checks.