[libvirt] [PATCH 0/5] qemu: parse: Fix parsing of weird cpu topology specifications
Peter Krempa (5): qemu: parse: Validate that the VM has at least one cpu qemu: parse: Allow the 'cpus=' prefix for current cpu number qemu: parse: Assign topology info earlier qemu: parse: Assign maximum cpu count from topology if not provided qemu: process: Set current vcpu count to maximum if it was not specified src/qemu/qemu_parse_command.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) -- 2.10.2
Libvirt's code relies on this fact so don't allow parsing a command line which would have none. Libvirtd would crash in the post parse callback on such config. --- src/qemu/qemu_parse_command.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index c3b27aa..b19c523 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1702,6 +1702,9 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, if (maxcpus == 0) maxcpus = vcpus; + if (maxcpus == 0) + goto syntax; + if (virDomainDefSetVcpusMax(dom, maxcpus, xmlopt) < 0) goto error; -- 2.10.2
qemu allows following syntax: -smp [cpus=]n[,cores=cores][,threads=threads][,sockets=sockets][,maxcpus=maxcpus] Allow the "cpus" prefix. --- src/qemu/qemu_parse_command.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index b19c523..98051be 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1694,6 +1694,8 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, threads = n; else if (STREQ(kws[i], "maxcpus")) maxcpus = n; + else if (STREQ(kws[i], "cpus")) + vcpus = n; else goto syntax; } -- 2.10.2
Qemu can also use the topology to calculate the total vcpu count. To allow parsing this move the assignment earlier. --- src/qemu/qemu_parse_command.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 98051be..7ab3dcf 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1701,18 +1701,6 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, } } - if (maxcpus == 0) - maxcpus = vcpus; - - if (maxcpus == 0) - goto syntax; - - if (virDomainDefSetVcpusMax(dom, maxcpus, xmlopt) < 0) - goto error; - - if (virDomainDefSetVcpus(dom, vcpus) < 0) - goto error; - if (sockets && cores && threads) { virCPUDefPtr cpu; @@ -1725,6 +1713,18 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, goto syntax; } + if (maxcpus == 0) + maxcpus = vcpus; + + if (maxcpus == 0) + goto syntax; + + if (virDomainDefSetVcpusMax(dom, maxcpus, xmlopt) < 0) + goto error; + + if (virDomainDefSetVcpus(dom, vcpus) < 0) + goto error; + ret = 0; cleanup: -- 2.10.2
qemu uses this if 'maxcpus' is not present. Do the same in the parsing code. --- src/qemu/qemu_parse_command.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 7ab3dcf..ed92caa 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1713,8 +1713,14 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, goto syntax; } - if (maxcpus == 0) - maxcpus = vcpus; + if (maxcpus == 0) { + if (cores) { + if (virDomainDefGetVcpusTopology(dom, &maxcpus) < 0) + goto error; + } else { + maxcpus = vcpus; + } + } if (maxcpus == 0) goto syntax; -- 2.10.2
Mimic qemu's behavior on the given command line. --- src/qemu/qemu_parse_command.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index ed92caa..cfebcc7 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1725,6 +1725,9 @@ qemuParseCommandLineSmp(virDomainDefPtr dom, if (maxcpus == 0) goto syntax; + if (vcpus == 0) + vcpus = maxcpus; + if (virDomainDefSetVcpusMax(dom, maxcpus, xmlopt) < 0) goto error; -- 2.10.2
On Mon, Nov 14, 2016 at 10:23 PM, Peter Krempa <pkrempa@redhat.com> wrote:
Peter Krempa (5): qemu: parse: Validate that the VM has at least one cpu qemu: parse: Allow the 'cpus=' prefix for current cpu number qemu: parse: Assign topology info earlier qemu: parse: Assign maximum cpu count from topology if not provided qemu: process: Set current vcpu count to maximum if it was not specified
src/qemu/qemu_parse_command.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
-- 2.10.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
ACK Series. Fixes everything mentioned in the thread: https://www.redhat.com/archives/libvir-list/2016-November/msg00627.html -- Nehal J Wani
On Mon, Nov 14, 2016 at 05:53:46PM +0100, Peter Krempa wrote:
Peter Krempa (5): qemu: parse: Validate that the VM has at least one cpu qemu: parse: Allow the 'cpus=' prefix for current cpu number qemu: parse: Assign topology info earlier qemu: parse: Assign maximum cpu count from topology if not provided qemu: process: Set current vcpu count to maximum if it was not specified
src/qemu/qemu_parse_command.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
ACK series. Jan
participants (3)
-
Ján Tomko -
Nehal J Wani -
Peter Krempa