On Tue, Jan 12, 2010 at 03:25:42PM +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.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_conf.c | 137 ++++++++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_conf.h | 3 +- 2 files changed, 123 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d3da776..bc4736a 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1,7 +1,7 @@ /* * qemu_conf.c: QEMU configuration management * - * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008, 2009, 2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1115,6 +1115,10 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_CHARDEV; if (strstr(help, "-balloon")) flags |= QEMUD_CMD_FLAG_BALLOON; + if (strstr(help, "cores=") && + strstr(help, "threads=") && + strstr(help, "sockets=")) + flags |= QEMUD_CMD_FLAG_SMP_TOPOLOGY;
if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -1900,7 +1904,6 @@ int qemudBuildCommandLine(virConnectPtr conn, const char *migrateFrom) { int i; char memory[50]; - char vcpus[50]; char boot[VIR_DOMAIN_BOOT_LAST]; struct utsname ut; int disableKQEMU = 0; @@ -2049,7 +2052,6 @@ int qemudBuildCommandLine(virConnectPtr conn, * is not supported, then they're out of luck anyway */ snprintf(memory, sizeof(memory), "%lu", def->maxmem/1024); - snprintf(vcpus, sizeof(vcpus), "%lu", def->vcpus); snprintf(domid, sizeof(domid), "%d", def->id);
ADD_ENV_LIT("LC_ALL=C"); @@ -2112,8 +2114,34 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-mem-path"); ADD_ARG_LIT(driver->hugepage_path); } + ADD_ARG_LIT("-smp"); - ADD_ARG_LIT(vcpus); + if ((qemuCmdFlags & QEMUD_CMD_FLAG_SMP_TOPOLOGY) && def->cpu) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferVSprintf(&buf, "%lu", def->vcpus); + + if (def->cpu->sockets > 0) + virBufferVSprintf(&buf, ",sockets=%u", def->cpu->sockets); + + if (def->cpu->cores > 0) + virBufferVSprintf(&buf, ",cores=%u", def->cpu->cores); + + if (def->cpu->threads > 0) + virBufferVSprintf(&buf, ",threads=%u", def->cpu->threads); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + } + + ADD_ARG(virBufferContentAndReset(&buf)); + } + else { + char vcpus[50]; + snprintf(vcpus, sizeof(vcpus), "%lu", def->vcpus); + ADD_ARG_LIT(vcpus); + }
Can you split this piece of code out into a separate method with a signature like static char *qemuBuildCPUStr(virDomainDefPtr def, int qemuCmdFlags); The main qemuBuildCommandLine method is getting out of control :-) Also, in the case where 'def->cpu' is NULL, but QEMUD_CMD_FLAG_SMP_TOPOLOGY is available, I think we should explicitly set an arg based on sockets=def->vcpus cores=1 threads=1 so that we avoid relying on any changable QEMU default values.
if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME) { ADD_ARG_LIT("-name"); @@ -3729,6 +3757,27 @@ error: }
+static virCPUDefPtr +qemuInitGuestCPU(virConnectPtr conn, + virDomainDefPtr dom) +{ + if (!dom->cpu) { + virCPUDefPtr cpu; + + if (VIR_ALLOC(cpu) < 0) { + virReportOOMError(conn); + return NULL; + } + + cpu->type = VIR_CPU_TYPE_GUEST; + cpu->match = VIR_CPU_MATCH_EXACT; + dom->cpu = cpu; + } + + return dom->cpu; +} + + static int qemuParseCommandLineCPU(virConnectPtr conn, virDomainDefPtr dom, @@ -3738,10 +3787,8 @@ qemuParseCommandLineCPU(virConnectPtr conn, const char *p = val; const char *next;
- if (VIR_ALLOC(cpu) < 0) - goto no_memory; - - cpu->type = VIR_CPU_TYPE_GUEST; + if (!(cpu = qemuInitGuestCPU(conn, dom))) + goto error;
do { if (*p == '\0' || *p == ',') @@ -3785,7 +3832,6 @@ qemuParseCommandLineCPU(virConnectPtr conn, } } while ((p = next));
- dom->cpu = cpu; return 0;
syntax: @@ -3796,7 +3842,71 @@ syntax: no_memory: virReportOOMError(conn); error: - virCPUDefFree(cpu); + return -1; +} + + +static int +qemuParseCommandLineSmp(virConnectPtr conn, + virDomainDefPtr dom, + const char *val) +{ + const char *p = val; + const char *next; + const char *const options[] = { "sockets=", "cores=", "threads=" }; + unsigned int topology[] = { 0, 0, 0 }; + char *end; + + do { + if (*p == '\0' || *p == ',') + goto syntax; + + if ((next = strchr(p, ','))) + next++; + + if (c_isdigit(*p)) { + int n; + if (virStrToLong_i(p, &end, 10, &n) < 0 || + !end || (*end != ',' && *end != '\0')) + goto syntax; + dom->vcpus = n; + } else { + int i; + int n = -1; + for (i = 0; i < ARRAY_CARDINALITY(options); i++) { + if (STRPREFIX(p, options[i])) { + p += strlen(options[i]); + if (virStrToLong_i(p, &end, 10, &n) < 0 || + !end || (*end != ',' && *end != '\0')) + goto syntax; + topology[i] = n; + break; + } + } + + if (n < 0) + goto syntax; + } + } while ((p = next));
If you strip off the initial CPU count value, then you can use qemuParseCommandLineKeywords to parse the reset of the name=value args.
+ + if (topology[0] && topology[1] && topology[2]) { + virCPUDefPtr cpu; + + if (!(cpu = qemuInitGuestCPU(conn, dom))) + goto error; + + cpu->sockets = topology[0]; + cpu->cores = topology[1]; + cpu->threads = topology[2]; + } else if (topology[0] || topology[1] || topology[2]) + goto syntax; + + return 0; + +syntax: + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot parse CPU topology '%s'"), val); +error: return -1; }
@@ -3948,14 +4058,9 @@ virDomainDefPtr qemuParseCommandLine(virConnectPtr conn, } def->memory = def->maxmem = mem * 1024; } else if (STREQ(arg, "-smp")) { - int vcpus; WANT_VALUE(); - if (virStrToLong_i(val, NULL, 10, &vcpus) < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, \ - _("cannot parse CPU count '%s'"), val); + if (qemuParseCommandLineSmp(conn, def, val) < 0) goto error; - } - def->vcpus = vcpus; } else if (STREQ(arg, "-uuid")) { WANT_VALUE(); if (virUUIDParse(val, def->uuid) < 0) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 82254ca..3f74cc9 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -1,7 +1,7 @@ /* * qemu_conf.h: QEMU configuration management * - * Copyright (C) 2006, 2007, 2009 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2009, 2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -78,6 +78,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_ENABLE_KVM = (1 << 23), /* Is the -enable-kvm flag available to "enable KVM full virtualization support" */ QEMUD_CMD_FLAG_MONITOR_JSON = (1 << 24), /* JSON mode for monitor */ QEMUD_CMD_FLAG_BALLOON = (1 << 25), /* -balloon available */ + QEMUD_CMD_FLAG_SMP_TOPOLOGY = (1 << 26), /* Is sockets=s,cores=c,threads=t available for -smp? */ };
/* Main driver state */ --
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 :|