Re: [libvirt] [PATCH v2] Implement CPU topology support for QEMU driver
On Fri, Jan 15, 2010 at 01:31:49PM +0100, Jiri Denemark wrote:
QEMU's command line equivalent for the following domain XML
fragment
<vcpus>2</vcpus>
<cpu ...>
...
<topology sockets='1' cores='2', threads='1'/>
</cpu>
is
-smp 2,sockets=1,cores=2,threads=1
This syntax was introduced in QEMU-0.12.
Version 2 changes:
- -smp argument build split into a separate function
- always add ",sockets=S,cores=C,threads=T" to -smp if qemu supports it
- use qemuParseCommandLineKeywords for command line parsing
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
@@ -2112,8 +2148,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
ADD_ARG_LIT("-mem-path");
ADD_ARG_LIT(driver->hugepage_path);
}
+
+ if (!(smp = qemudBuildCommandLineSmp(conn, def, qemuCmdFlags)))
+ goto error;
+
ADD_ARG_LIT("-smp");
- ADD_ARG_LIT(vcpus);
+ ADD_ARG_LIT(smp);
+ VIR_FREE(smp);
If you've got an allocated string, then just use 'ADD_ARG(smp)' and which
avoids the strdup() that ADD_ARG_LIT does and avoids need for VIR_FREE
too. Also you should move the qemudBuildCommandLineSmp() call to *after*
the ADD_ARG_LIT("-smp") line, otherwise you can leak 'smp' on OOM
handling
in the ADD_ARG_LIT("-smp") call.
ACK, if you make that minor memory handling fix before committing
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|