[libvirt] [PATCH] Implement CPU topology support for QEMU driver
Daniel P. Berrange
berrange at redhat.com
Tue Jan 12 20:26:29 UTC 2010
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 at 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 :|
More information about the libvir-list
mailing list