[libvirt] [PATCH v2 00/10] Reorganizing qemu_command.c continued
v1: http://www.redhat.com/archives/libvir-list/2016-February/msg00804.html v2 changes: Patch 1 makes changes as requested from review. Patches 2-7 are v1's 4-10 take a different approach by creating qemuBuildXXXXCommandLine helper routines. Patches 8-10 are new, but follow the others... John Ferlan (10): qemu: Make basic upfront checks before create command qemu: Rename qemuBuildMachineArgStr qemu: Rename qemuBuildCpuArgStr to qemuBuildCpuCommandLine qemu: Introduce qemuBuildMemCommandLine qemu: Rename qemuBuildSmpArgStr to qemuBuildSmpCommandLine qemu: Introduce qemuBuildIOThreadCommandLine qemu: Introduce qemuBuildNumaCommandLine qemu: Introduce qemuBuildSmbiosCommandLine qemu: Introduce qemuBuildSgaCommandLine qemu: Introduce qemuBuildMonitorCommandLine src/qemu/qemu_command.c | 740 ++++++++++++++++++++++++++++-------------------- 1 file changed, 434 insertions(+), 306 deletions(-) -- 2.5.0
Create qemuBuildCommandLineValidate to make some checks before trying to build the command. This will move some logic from much later to much earlier - we shouldn't be adjusting any data so that shouldn't matter. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 174 ++++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7c1a457..f68509d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6608,6 +6608,98 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, } +/* + * qemuBuildCommandLineValidate: + * + * Prior to taking the plunge and building a long command line only + * to find some configuration option isn't valid, let's do a couple + * of checks and fail early. + * + * Returns 0 on success, returns -1 and messages what the issue is. + */ +static int +qemuBuildCommandLineValidate(virQEMUDriverPtr driver, + const virDomainDef *def) +{ + size_t i; + int sdl = 0; + int vnc = 0; + int spice = 0; + + if (!virQEMUDriverIsPrivileged(driver)) { + /* If we have no cgroups then we can have no tunings that + * require them */ + + if (virMemoryLimitIsSet(def->mem.hard_limit) || + virMemoryLimitIsSet(def->mem.soft_limit) || + def->mem.min_guarantee || + virMemoryLimitIsSet(def->mem.swap_hard_limit)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Memory tuning is not available in session mode")); + return -1; + } + + if (def->blkio.weight) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Block I/O tuning is not available in session mode")); + return -1; + } + + if (def->cputune.sharesSpecified || def->cputune.period || + def->cputune.quota || def->cputune.emulator_period || + def->cputune.emulator_quota) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("CPU tuning is not available in session mode")); + return -1; + } + } + + for (i = 0; i < def->ngraphics; ++i) { + switch (def->graphics[i]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + ++sdl; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + ++vnc; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + ++spice; + break; + } + } + + if (sdl > 1 || vnc > 1 || spice > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 1 graphics device of each type " + "(sdl, vnc, spice) is supported")); + return -1; + } + + if (def->virtType == VIR_DOMAIN_VIRT_XEN || + def->os.type == VIR_DOMAIN_OSTYPE_XEN || + def->os.type == VIR_DOMAIN_OSTYPE_LINUX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu emulator '%s' does not support xen"), + def->emulator); + return -1; + } + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (disk->src->driverName != NULL && + STRNEQ(disk->src->driverName, "qemu")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported driver name '%s' for disk '%s'"), + disk->src->driverName, disk->src->path); + return -1; + } + } + + return 0; +} + + qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName, }; @@ -6641,14 +6733,12 @@ qemuBuildCommandLine(virConnectPtr conn, char uuid[VIR_UUID_STRING_BUFLEN]; char *cpu; char *smp; + bool havespice = false; int last_good_net = -1; bool hasHwVirt = false; virCommandPtr cmd = NULL; bool allowReboot = true; bool emitBootindex = false; - int sdl = 0; - int vnc = 0; - int spice = 0; int usbcontroller = 0; int actualSerials = 0; bool usblegacy = false; @@ -6692,47 +6782,8 @@ qemuBuildCommandLine(virConnectPtr conn, virUUIDFormat(def->uuid, uuid); - if (!virQEMUDriverIsPrivileged(driver)) { - /* If we have no cgroups then we can have no tunings that - * require them */ - - if (virMemoryLimitIsSet(def->mem.hard_limit) || - virMemoryLimitIsSet(def->mem.soft_limit) || - def->mem.min_guarantee || - virMemoryLimitIsSet(def->mem.swap_hard_limit)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Memory tuning is not available in session mode")); - goto error; - } - - if (def->blkio.weight) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Block I/O tuning is not available in session mode")); - goto error; - } - - if (def->cputune.sharesSpecified || def->cputune.period || - def->cputune.quota || def->cputune.emulator_period || - def->cputune.emulator_quota) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("CPU tuning is not available in session mode")); - goto error; - } - } - - for (i = 0; i < def->ngraphics; ++i) { - switch (def->graphics[i]->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - ++sdl; - break; - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - ++vnc; - break; - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - ++spice; - break; - } - } + if (qemuBuildCommandLineValidate(driver, def) < 0) + goto error; /* * do not use boot=on for drives when not using KVM since this @@ -6872,14 +6923,6 @@ qemuBuildCommandLine(virConnectPtr conn, } virCommandAddArgList(cmd, "-uuid", uuid, NULL); - if (def->virtType == VIR_DOMAIN_VIRT_XEN || - def->os.type == VIR_DOMAIN_OSTYPE_XEN || - def->os.type == VIR_DOMAIN_OSTYPE_LINUX) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("qemu emulator '%s' does not support xen"), - def->emulator); - goto error; - } if ((def->os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) && (def->os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) { @@ -7394,18 +7437,6 @@ qemuBuildCommandLine(virConnectPtr conn, } } - for (i = 0; i < def->ndisks; i++) { - virDomainDiskDefPtr disk = def->disks[i]; - - if (disk->src->driverName != NULL && - STRNEQ(disk->src->driverName, "qemu")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported driver name '%s' for disk '%s'"), - disk->src->driverName, disk->src->path); - goto error; - } - } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { for (i = 0; i < def->ncontrollers; i++) { @@ -7814,11 +7845,17 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgBuffer(cmd, &opt); } + if (def->nserials) { + for (i = 0; i < def->ngraphics && !havespice; i++) { + if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) + havespice = true; + } + } for (i = 0; i < def->nserials; i++) { virDomainChrDefPtr serial = def->serials[i]; char *devstr; - if (serial->source.type == VIR_DOMAIN_CHR_TYPE_SPICEPORT && !spice) + if (serial->source.type == VIR_DOMAIN_CHR_TYPE_SPICEPORT && !havespice) continue; /* Use -chardev with -device if they are available */ @@ -8052,13 +8089,6 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (sdl > 1 || vnc > 1 || spice > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only 1 graphics device of each type " - "(sdl, vnc, spice) is supported")); - goto error; - } - for (i = 0; i < def->ngraphics; ++i) { if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps, def->graphics[i]) < 0) -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:34 -0500, John Ferlan wrote:
Create qemuBuildCommandLineValidate to make some checks before trying to build the command. This will move some logic from much later to much earlier - we shouldn't be adjusting any data so that shouldn't matter.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 174 ++++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7c1a457..f68509d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6608,6 +6608,98 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, }
+/* + * qemuBuildCommandLineValidate:
These kinds of comments use two stars on the first line of the comment.
+ * + * Prior to taking the plunge and building a long command line only + * to find some configuration option isn't valid, let's do a couple + * of checks and fail early. + * + * Returns 0 on success, returns -1 and messages what the issue is. + */
[...]
@@ -7814,11 +7845,17 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgBuffer(cmd, &opt); }
+ if (def->nserials) { + for (i = 0; i < def->ngraphics && !havespice; i++) {
Fair enough, but we usually use break from the next condition.
+ if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) + havespice = true; + } + }
ACK
Renane to qemuBuildMachineCommandLine to fit current (and future) helper naming conventions. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f68509d..5f2a0c5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5031,9 +5031,9 @@ qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps, } static int -qemuBuildMachineArgStr(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) +qemuBuildMachineCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { bool obsoleteAccel = false; @@ -6812,7 +6812,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (enableFips) virCommandAddArg(cmd, "-enable-fips"); - if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0) + if (qemuBuildMachineCommandLine(cmd, def, qemuCaps) < 0) goto error; if (qemuBuildCpuArgStr(driver, def, qemuCaps, -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:35 -0500, John Ferlan wrote:
Renane to qemuBuildMachineCommandLine to fit current (and future) helper naming conventions.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
ACK
Rename function and move code from mainline qemuBuildCommandLine to keep alike code together. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5f2a0c5..bff5b09 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4773,22 +4773,21 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, } static int -qemuBuildCpuArgStr(virQEMUDriverPtr driver, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps, - virArch hostarch, - char **opt, - bool *hasHwVirt, - bool migrating) +qemuBuildCpuCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + bool migrating) { + virArch hostarch = virArchFromHost(); + char *cpu = NULL; + bool hasHwVirt = false; const char *default_model; bool have_cpu = false; int ret = -1; virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; - *hasHwVirt = false; - if (def->os.arch == VIR_ARCH_I686) default_model = "qemu32"; else @@ -4797,7 +4796,7 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, if (def->cpu && (def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) { if (qemuBuildCpuModelArgStr(driver, def, &buf, qemuCaps, - hasHwVirt, migrating) < 0) + &hasHwVirt, migrating) < 0) goto cleanup; have_cpu = true; } else { @@ -4857,7 +4856,8 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK]) { char sign; - if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == VIR_TRISTATE_SWITCH_ON) + if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == + VIR_TRISTATE_SWITCH_ON) sign = '+'; else sign = '-'; @@ -4941,14 +4941,23 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, if (virBufferCheckError(&buf) < 0) goto cleanup; - *opt = virBufferContentAndReset(&buf); + cpu = virBufferContentAndReset(&buf); + + if (cpu) { + virCommandAddArgList(cmd, "-cpu", cpu, NULL); + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NESTING) && hasHwVirt) + virCommandAddArg(cmd, "-enable-nesting"); + } ret = 0; cleanup: + VIR_FREE(cpu); return ret; } + static int qemuBuildObsoleteAccelArg(virCommandPtr cmd, const virDomainDef *def, @@ -6731,11 +6740,9 @@ qemuBuildCommandLine(virConnectPtr conn, virErrorPtr originalError = NULL; size_t i, j; char uuid[VIR_UUID_STRING_BUFLEN]; - char *cpu; char *smp; bool havespice = false; int last_good_net = -1; - bool hasHwVirt = false; virCommandPtr cmd = NULL; bool allowReboot = true; bool emitBootindex = false; @@ -6766,7 +6773,6 @@ qemuBuildCommandLine(virConnectPtr conn, VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_CONTROLLER_TYPE_CCID, }; - virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virBuffer boot_buf = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; @@ -6815,19 +6821,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildMachineCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildCpuArgStr(driver, def, qemuCaps, - hostarch, &cpu, &hasHwVirt, !!migrateURI) < 0) + if (qemuBuildCpuCommandLine(cmd, driver, def, qemuCaps, !!migrateURI) < 0) goto error; - if (cpu) { - virCommandAddArgList(cmd, "-cpu", cpu, NULL); - VIR_FREE(cpu); - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NESTING) && - hasHwVirt) - virCommandAddArg(cmd, "-enable-nesting"); - } - if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0) goto error; -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:36 -0500, John Ferlan wrote:
Rename function and move code from mainline qemuBuildCommandLine to keep alike code together.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-)
ACK
Add new function to manage adding the '-m' memory options to the command line removing that task from the mainline qemuBuildCommandLine Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 89 +++++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bff5b09..2c22a08 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5225,7 +5225,7 @@ qemuBuildSmpArgStr(const virDomainDef *def, static int qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, - virDomainDefPtr def, + const virDomainDef *def, virQEMUCapsPtr qemuCaps, virCommandPtr cmd) { @@ -5288,6 +5288,56 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, return 0; } + +static int +qemuBuildMemCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0) + goto error; + + virCommandAddArg(cmd, "-m"); + + if (virDomainDefHasMemoryHotplug(def)) { + /* Use the 'k' suffix to let qemu handle the units */ + virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk", + virDomainDefGetMemoryInitial(def), + def->mem.memory_slots, + def->mem.max_memory); + + } else { + virCommandAddArgFormat(cmd, "%llu", + virDomainDefGetMemoryInitial(def) / 1024); + } + + /* + * Add '-mem-path' (and '-mem-prealloc') parameter here only if + * there is no numa node specified. + */ + if (!virDomainNumaGetNodeCount(def->numa) && + qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0) + goto error; + + if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory locking not supported by QEMU binary")); + goto error; + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) { + virCommandAddArg(cmd, "-realtime"); + virCommandAddArgFormat(cmd, "mlock=%s", + def->mem.locked ? "on" : "off"); + } + + return 0; + + error: + return -1; +} + + static int qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virDomainDefPtr def, @@ -6827,45 +6877,12 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (!migrateURI && !snapshot && - qemuDomainAlignMemorySizes(def) < 0) + if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0) goto error; - if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0) + if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error; - virCommandAddArg(cmd, "-m"); - - if (virDomainDefHasMemoryHotplug(def)) { - /* Use the 'k' suffix to let qemu handle the units */ - virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk", - virDomainDefGetMemoryInitial(def), - def->mem.memory_slots, - def->mem.max_memory); - - } else { - virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024); - } - - /* - * Add '-mem-path' (and '-mem-prealloc') parameter here only if - * there is no numa node specified. - */ - if (!virDomainNumaGetNodeCount(def->numa) && - qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0) - goto error; - - if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("memory locking not supported by QEMU binary")); - goto error; - } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) { - virCommandAddArg(cmd, "-realtime"); - virCommandAddArgFormat(cmd, "mlock=%s", - def->mem.locked ? "on" : "off"); - } - virCommandAddArg(cmd, "-smp"); if (!(smp = qemuBuildSmpArgStr(def, qemuCaps))) goto error; -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:37 -0500, John Ferlan wrote:
Add new function to manage adding the '-m' memory options to the command line removing that task from the mainline qemuBuildCommandLine
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 89 +++++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bff5b09..2c22a08 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5225,7 +5225,7 @@ qemuBuildSmpArgStr(const virDomainDef *def,
static int qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, - virDomainDefPtr def, + const virDomainDef *def, virQEMUCapsPtr qemuCaps, virCommandPtr cmd) { @@ -5288,6 +5288,56 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, return 0; }
+ +static int +qemuBuildMemCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0) + goto error;
[1] (below .. )
+ + virCommandAddArg(cmd, "-m"); + + if (virDomainDefHasMemoryHotplug(def)) { + /* Use the 'k' suffix to let qemu handle the units */ + virCommandAddArgFormat(cmd, "size=%lluk,slots=%u,maxmem=%lluk", + virDomainDefGetMemoryInitial(def), + def->mem.memory_slots, + def->mem.max_memory); + + } else { + virCommandAddArgFormat(cmd, "%llu", + virDomainDefGetMemoryInitial(def) / 1024); + } + + /* + * Add '-mem-path' (and '-mem-prealloc') parameter here only if + * there is no numa node specified. + */ + if (!virDomainNumaGetNodeCount(def->numa) && + qemuBuildMemPathStr(cfg, def, qemuCaps, cmd) < 0) + goto error; + + if (def->mem.locked && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("memory locking not supported by QEMU binary")); + goto error; + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MLOCK)) { + virCommandAddArg(cmd, "-realtime"); + virCommandAddArgFormat(cmd, "mlock=%s", + def->mem.locked ? "on" : "off"); + } + + return 0; + + error:
This has nothing to clean up, so the error label can be omitted and -1 returned directly.
+ return -1; +} + + static int qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virDomainDefPtr def, @@ -6827,45 +6877,12 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0) goto error;
- if (!migrateURI && !snapshot && - qemuDomainAlignMemorySizes(def) < 0) + if (!migrateURI && !snapshot && qemuDomainAlignMemorySizes(def) < 0) goto error;
Later on we could add a function that will collate all the operations that tweak the domain config prior to start and extract it from the command line builder, which would be called prior to the validator, ...
- if (qemuDomainDefValidateMemoryHotplug(def, qemuCaps, NULL) < 0)
[1] ... and then this call could be moved to the validator from patch 1. But that's not for this patch.
+ if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error;
ACK if you remove the 'error' label from the newly added function. Peter
Rename function and move code in from qemuBuildCommandLine to keep smp related code together. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2c22a08..01d838e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5186,17 +5186,22 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, return 0; } -static char * -qemuBuildSmpArgStr(const virDomainDef *def, - virQEMUCapsPtr qemuCaps) +static int +qemuBuildSmpCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { + char *smp; virBuffer buf = VIR_BUFFER_INITIALIZER; + virCommandAddArg(cmd, "-smp"); + virBufferAsprintf(&buf, "%u", virDomainDefGetVcpus(def)); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMP_TOPOLOGY)) { if (virDomainDefHasVcpusOffline(def)) - virBufferAsprintf(&buf, ",maxcpus=%u", virDomainDefGetVcpusMax(def)); + virBufferAsprintf(&buf, ",maxcpus=%u", + virDomainDefGetVcpusMax(def)); /* sockets, cores, and threads are either all zero * or all non-zero, thus checking one of them is enough */ if (def->cpu && def->cpu->sockets) { @@ -5204,7 +5209,8 @@ qemuBuildSmpArgStr(const virDomainDef *def, virBufferAsprintf(&buf, ",cores=%u", def->cpu->cores); virBufferAsprintf(&buf, ",threads=%u", def->cpu->threads); } else { - virBufferAsprintf(&buf, ",sockets=%u", virDomainDefGetVcpusMax(def)); + virBufferAsprintf(&buf, ",sockets=%u", + virDomainDefGetVcpusMax(def)); virBufferAsprintf(&buf, ",cores=%u", 1); virBufferAsprintf(&buf, ",threads=%u", 1); } @@ -5214,15 +5220,20 @@ qemuBuildSmpArgStr(const virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("setting current vcpu count less than maximum is " "not supported with this QEMU binary")); - return NULL; + return -1; } if (virBufferCheckError(&buf) < 0) - return NULL; + return -1; - return virBufferContentAndReset(&buf); + smp = virBufferContentAndReset(&buf); + virCommandAddArg(cmd, smp); + VIR_FREE(smp); + + return 0; } + static int qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, const virDomainDef *def, @@ -6790,7 +6801,6 @@ qemuBuildCommandLine(virConnectPtr conn, virErrorPtr originalError = NULL; size_t i, j; char uuid[VIR_UUID_STRING_BUFLEN]; - char *smp; bool havespice = false; int last_good_net = -1; virCommandPtr cmd = NULL; @@ -6883,11 +6893,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error; - virCommandAddArg(cmd, "-smp"); - if (!(smp = qemuBuildSmpArgStr(def, qemuCaps))) + if (qemuBuildSmpCommandLine(cmd, def, qemuCaps) < 0) goto error; - virCommandAddArg(cmd, smp); - VIR_FREE(smp); if (def->niothreadids) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:38 -0500, John Ferlan wrote:
Rename function and move code in from qemuBuildCommandLine to keep smp related code together.
+ a few styllistic changes.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
ACK
Add new function to manage adding the IOThread '-object' to the command line removing that task from the mainline qemuBuildCommandLine Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 51 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 01d838e..3449dea 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5350,6 +5350,37 @@ qemuBuildMemCommandLine(virCommandPtr cmd, static int +qemuBuildIOThreadCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + if (def->niothreadids == 0) + return 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported for this QEMU")); + return -1; + } + + /* Create iothread objects using the defined iothreadids list + * and the defined id and name from the list. These may be used + * by a disk definition which will associate to an iothread by + * supplying a value of an id from the list + */ + for (i = 0; i < def->niothreadids; i++) { + virCommandAddArg(cmd, "-object"); + virCommandAddArgFormat(cmd, "iothread,id=iothread%u", + def->iothreadids[i]->iothread_id); + } + + return 0; +} + + +static int qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, virDomainDefPtr def, virCommandPtr cmd, @@ -6896,24 +6927,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSmpCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (def->niothreadids) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOThreads not supported for this QEMU")); - goto error; - } - - /* Create iothread objects using the defined iothreadids list - * and the defined id and name from the list. These may be used - * by a disk definition which will associate to an iothread by - * supplying a value of an id from the list - */ - for (i = 0; i < def->niothreadids; i++) { - virCommandAddArg(cmd, "-object"); - virCommandAddArgFormat(cmd, "iothread,id=iothread%u", - def->iothreadids[i]->iothread_id); - } - } + if (qemuBuildIOThreadCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (virDomainNumaGetNodeCount(def->numa) && qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:39 -0500, John Ferlan wrote:
Add new function to manage adding the IOThread '-object' to the command line removing that task from the mainline qemuBuildCommandLine
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 51 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 18 deletions(-)
ACK
Add new function to manage adding the '-numa' options to the command line removing that task from the mainline qemuBuildCommandLine Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 66 ++++++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3449dea..27952e5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5523,6 +5523,44 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, static int +qemuBuildNumaCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virBitmapPtr nodeset) +{ + size_t i; + + if (virDomainNumaGetNodeCount(def->numa) && + qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) + return -1; + + /* memory hotplug requires NUMA to be enabled - we already checked + * that memory devices are present only when NUMA is */ + for (i = 0; i < def->nmems; i++) { + char *backStr; + char *dimmStr; + + if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def, + qemuCaps, cfg))) + return -1; + + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) { + VIR_FREE(backStr); + return -1; + } + + virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, NULL); + + VIR_FREE(backStr); + VIR_FREE(dimmStr); + } + + return 0; +} + + +static int qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, virDomainDefPtr def, @@ -6930,32 +6968,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildIOThreadCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (virDomainNumaGetNodeCount(def->numa) && - qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) - goto error; - - /* memory hotplug requires NUMA to be enabled - we already checked - * that memory devices are present only when NUMA is */ - - - for (i = 0; i < def->nmems; i++) { - char *backStr; - char *dimmStr; - - if (!(backStr = qemuBuildMemoryDimmBackendStr(def->mems[i], def, - qemuCaps, cfg))) - goto error; - - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) { - VIR_FREE(backStr); - goto error; - } - - virCommandAddArgList(cmd, "-object", backStr, "-device", dimmStr, NULL); - - VIR_FREE(backStr); - VIR_FREE(dimmStr); - } + if (qemuBuildNumaCommandLine(cmd, cfg, def, qemuCaps, nodeset) < 0) + goto error; virCommandAddArgList(cmd, "-uuid", uuid, NULL); -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:40 -0500, John Ferlan wrote:
Add new function to manage adding the '-numa' options to the command line removing that task from the mainline qemuBuildCommandLine
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 66 ++++++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 26 deletions(-)
ACK
Add new function to manage adding the '-smbios' options to the command line removing that task from the mainline qemuBuildCommandLine Also while I was looking at it, move the uuid processing closer to usage. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 147 +++++++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 64 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 27952e5..0e3b516 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4457,6 +4457,86 @@ static char *qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def) return NULL; } + +static int +qemuBuildSmbiosCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + virSysinfoDefPtr source = NULL; + bool skip_uuid = false; + + if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_NONE || + def->os.smbios_mode == VIR_DOMAIN_SMBIOS_EMULATE) + return 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMBIOS_TYPE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support smbios settings"), + def->emulator); + goto error; + } + + /* should we really error out or just warn in those cases ? */ + if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_HOST) { + if (driver->hostsysinfo == NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Host SMBIOS information is not available")); + goto error; + } + source = driver->hostsysinfo; + /* Host and guest uuid must differ, by definition of UUID. */ + skip_uuid = true; + } else if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO) { + if (def->sysinfo == NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("Domain '%s' sysinfo are not available"), + def->name); + goto error; + } + source = def->sysinfo; + /* domain_conf guaranteed that system_uuid matches guest uuid. */ + } + if (source != NULL) { + char *smbioscmd; + + smbioscmd = qemuBuildSmbiosBiosStr(source->bios); + if (smbioscmd != NULL) { + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } + smbioscmd = qemuBuildSmbiosSystemStr(source->system, skip_uuid); + if (smbioscmd != NULL) { + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } + + if (source->nbaseBoard > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("qemu does not support more than " + "one entry to Type 2 in SMBIOS table")); + goto error; + } + + for (i = 0; i < source->nbaseBoard; i++) { + if (!(smbioscmd = + qemuBuildSmbiosBaseBoardStr(source->baseBoard + i))) + goto error; + + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } + } + + return 0; + + error: + return -1; +} + + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { @@ -6915,8 +6995,6 @@ qemuBuildCommandLine(virConnectPtr conn, conn, driver, def, monitor_chr, monitor_json, qemuCaps, migrateURI, snapshot, vmop); - virUUIDFormat(def->uuid, uuid); - if (qemuBuildCommandLineValidate(driver, def) < 0) goto error; @@ -6971,70 +7049,11 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildNumaCommandLine(cmd, cfg, def, qemuCaps, nodeset) < 0) goto error; + virUUIDFormat(def->uuid, uuid); virCommandAddArgList(cmd, "-uuid", uuid, NULL); - if ((def->os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) && - (def->os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) { - virSysinfoDefPtr source = NULL; - bool skip_uuid = false; - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMBIOS_TYPE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support smbios settings"), - def->emulator); - goto error; - } - - /* should we really error out or just warn in those cases ? */ - if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_HOST) { - if (driver->hostsysinfo == NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Host SMBIOS information is not available")); - goto error; - } - source = driver->hostsysinfo; - /* Host and guest uuid must differ, by definition of UUID. */ - skip_uuid = true; - } else if (def->os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO) { - if (def->sysinfo == NULL) { - virReportError(VIR_ERR_XML_ERROR, - _("Domain '%s' sysinfo are not available"), - def->name); - goto error; - } - source = def->sysinfo; - /* domain_conf guaranteed that system_uuid matches guest uuid. */ - } - if (source != NULL) { - char *smbioscmd; - - smbioscmd = qemuBuildSmbiosBiosStr(source->bios); - if (smbioscmd != NULL) { - virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); - VIR_FREE(smbioscmd); - } - smbioscmd = qemuBuildSmbiosSystemStr(source->system, skip_uuid); - if (smbioscmd != NULL) { - virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); - VIR_FREE(smbioscmd); - } - - if (source->nbaseBoard > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("qemu does not support more than " - "one entry to Type 2 in SMBIOS table")); - goto error; - } - - for (i = 0; i < source->nbaseBoard; i++) { - if (!(smbioscmd = qemuBuildSmbiosBaseBoardStr(source->baseBoard + i))) - goto error; - - virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); - VIR_FREE(smbioscmd); - } - } - } + if (qemuBuildSmbiosCommandLine(cmd, driver, def, qemuCaps) < 0) + goto error; /* * NB, -nographic *MUST* come before any serial, or monitor -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:41 -0500, John Ferlan wrote:
Add new function to manage adding the '-smbios' options to the command line removing that task from the mainline qemuBuildCommandLine
Also while I was looking at it, move the uuid processing closer to usage.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 147 +++++++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 64 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 27952e5..0e3b516 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4457,6 +4457,86 @@ static char *qemuBuildSmbiosBaseBoardStr(virSysinfoBaseBoardDefPtr def)
[...]
+ + virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL); + VIR_FREE(smbioscmd); + } + } + + return 0; + + error:
No need for the error label in this function.
+ return -1; +} + + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) {
ACK with the label removed
Add new function to manage adding the '-device sga' to the command line removing that task from the mainline qemuBuildCommandLine Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0e3b516..e554648 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4537,6 +4537,35 @@ qemuBuildSmbiosCommandLine(virCommandPtr cmd, } +static int +qemuBuildSgaCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + /* Serial graphics adapter */ + if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu does not support -device")); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu does not support SGA")); + return -1; + } + if (!def->nserials) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("need at least one serial port to use SGA")); + return -1; + } + virCommandAddArgList(cmd, "-device", "sga", NULL); + } + + return 0; +} + + static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { @@ -7080,25 +7109,8 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-nodefaults"); } - /* Serial graphics adapter */ - if (def->os.bios.useserial == VIR_TRISTATE_BOOL_YES) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu does not support -device")); - goto error; - } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SGA)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("qemu does not support SGA")); - goto error; - } - if (!def->nserials) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("need at least one serial port to use SGA")); - goto error; - } - virCommandAddArgList(cmd, "-device", "sga", NULL); - } + if (qemuBuildSgaCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (monitor_chr) { char *chrdev; -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:42 -0500, John Ferlan wrote:
Add new function to manage adding the '-device sga' to the command line removing that task from the mainline qemuBuildCommandLine
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 19 deletions(-)
ACK
Add new function to manage adding the '-mon' or '-monitor' options to the command line removing that task from the mainline qemuBuildCommandLine Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 78 ++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e554648..017d511 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3847,7 +3847,8 @@ qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static char * -qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias, +qemuBuildChrChardevStr(const virDomainChrSourceDef *dev, + const char *alias, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3975,7 +3976,8 @@ qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias, static char * -qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char *prefix) +qemuBuildChrArgStr(const virDomainChrSourceDef *dev, + const char *prefix) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4068,6 +4070,47 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char *prefix) } +static int +qemuBuildMonitorCommandLine(virCommandPtr cmd, + virQEMUCapsPtr qemuCaps, + const virDomainChrSourceDef *monitor_chr, + bool monitor_json) +{ + char *chrdev; + + if (!monitor_chr) + return 0; + + /* Use -chardev if it's available */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) { + + virCommandAddArg(cmd, "-chardev"); + if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", + qemuCaps))) + return -1; + virCommandAddArg(cmd, chrdev); + VIR_FREE(chrdev); + + virCommandAddArg(cmd, "-mon"); + virCommandAddArgFormat(cmd, + "chardev=charmonitor,id=monitor,mode=%s", + monitor_json ? "control" : "readline"); + } else { + const char *prefix = NULL; + if (monitor_json) + prefix = "control,"; + + virCommandAddArg(cmd, "-monitor"); + if (!(chrdev = qemuBuildChrArgStr(monitor_chr, prefix))) + return -1; + virCommandAddArg(cmd, chrdev); + VIR_FREE(chrdev); + } + + return 0; +} + + static char * qemuBuildVirtioSerialPortDevStr(virDomainDefPtr def, virDomainChrDefPtr dev, @@ -7112,34 +7155,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSgaCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (monitor_chr) { - char *chrdev; - /* Use -chardev if it's available */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV)) { - - virCommandAddArg(cmd, "-chardev"); - if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", - qemuCaps))) - goto error; - virCommandAddArg(cmd, chrdev); - VIR_FREE(chrdev); - - virCommandAddArg(cmd, "-mon"); - virCommandAddArgFormat(cmd, - "chardev=charmonitor,id=monitor,mode=%s", - monitor_json ? "control" : "readline"); - } else { - const char *prefix = NULL; - if (monitor_json) - prefix = "control,"; - - virCommandAddArg(cmd, "-monitor"); - if (!(chrdev = qemuBuildChrArgStr(monitor_chr, prefix))) - goto error; - virCommandAddArg(cmd, chrdev); - VIR_FREE(chrdev); - } - } + if (qemuBuildMonitorCommandLine(cmd, qemuCaps, monitor_chr, + monitor_json) < 0) + goto error; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC)) { char *rtcopt; -- 2.5.0
On Wed, Feb 17, 2016 at 21:25:43 -0500, John Ferlan wrote:
Add new function to manage adding the '-mon' or '-monitor' options to the command line removing that task from the mainline qemuBuildCommandLine
+ two stylistic/alignment changes
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 78 ++++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 30 deletions(-)
ACK
On 02/17/2016 09:25 PM, John Ferlan wrote:
v1: http://www.redhat.com/archives/libvir-list/2016-February/msg00804.html
v2 changes:
Patch 1 makes changes as requested from review.
Patches 2-7 are v1's 4-10 take a different approach by creating qemuBuildXXXXCommandLine helper routines.
Patches 8-10 are new, but follow the others...
John Ferlan (10): qemu: Make basic upfront checks before create command qemu: Rename qemuBuildMachineArgStr qemu: Rename qemuBuildCpuArgStr to qemuBuildCpuCommandLine qemu: Introduce qemuBuildMemCommandLine qemu: Rename qemuBuildSmpArgStr to qemuBuildSmpCommandLine qemu: Introduce qemuBuildIOThreadCommandLine qemu: Introduce qemuBuildNumaCommandLine qemu: Introduce qemuBuildSmbiosCommandLine qemu: Introduce qemuBuildSgaCommandLine qemu: Introduce qemuBuildMonitorCommandLine
src/qemu/qemu_command.c | 740 ++++++++++++++++++++++++++++-------------------- 1 file changed, 434 insertions(+), 306 deletions(-)
Adjustments made, patches pushed - Thanks for the quick review. John On to the next pile...
participants (3)
-
John Ferlan -
Pavel Hrdina -
Peter Krempa