1. Patchew
  2. Libvirt
  3. [PATCH 00/22] qemu: Capability probing from 'query-command-line-options' (part 1?)
  4. View patch

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

Peter Krempa posted 22 patches 4 years, 6 months ago
[PATCH 15/22] qemu: capabilities: Add alternative detection of QEMU_CAPS_NUMA
Posted by Peter Krempa 4 years, 6 months ago 
'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@redhat.com>
---
 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) */
     { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
     { "sandbox", "enable", QEMU_CAPS_SECCOMP_SANDBOX },
     { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST },
-- 
2.31.1
Re: [PATCH 15/22] qemu: capabilities: Add alternative detection of QEMU_CAPS_NUMA
Posted by Martin Kletzander 4 years, 5 months ago 
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@redhat.com>
>---
> 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.

>     { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
>     { "sandbox", "enable", QEMU_CAPS_SECCOMP_SANDBOX },
>     { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST },
>-- 
>2.31.1
>
Re: [PATCH 15/22] qemu: capabilities: Add alternative detection of QEMU_CAPS_NUMA
Posted by Peter Krempa 4 years, 5 months ago 
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@redhat.com>
> > ---
> > 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.
Re: [PATCH 15/22] qemu: capabilities: Add alternative detection of QEMU_CAPS_NUMA
Posted by Martin Kletzander 4 years, 5 months ago 
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@redhat.com>
>> > ---
>> > 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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.