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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux