On Mon, Aug 16, 2021 at 03:15:20PM +0200, Peter Krempa wrote:
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@xxxxxxxxxx> > --- > 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.
=D sorry, can't stop grinning =)
The idea is to have something which references 'qemuCaps->version' and thus is the same as other version checks.
I get it, I meant something like "not needed after qemuCaps->..." or "remove for (qemuCaps->...", but it's fine, it's just me overthinking comments. The idea will be clearly visible when someone git-blames the line, so it's fine.
Attachment:
signature.asc
Description: PGP signature