8:25 p.m.
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
8:25 p.m.
New subject: [libvirt] [PATCH v2 01/10] qemu: Make basic upfront checks before create command
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(a)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
1:06 a.m.
New subject: [libvirt] [PATCH v2 01/10] qemu: Make basic upfront checks before create command
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(a)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
8:25 p.m.
New subject: [libvirt] [PATCH v2 02/10] qemu: Rename qemuBuildMachineArgStr
Renane to qemuBuildMachineCommandLine to fit current (and future)
helper naming conventions.
Signed-off-by: John Ferlan <jferlan(a)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
1:06 a.m.
New subject: [libvirt] [PATCH v2 02/10] qemu: Rename qemuBuildMachineArgStr
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(a)redhat.com>
---
src/qemu/qemu_command.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
ACK
8:25 p.m.
New subject: [libvirt] [PATCH v2 03/10] qemu: Rename qemuBuildCpuArgStr to qemuBuildCpuCommandLine
Rename function and move code from mainline qemuBuildCommandLine to
keep alike code together.
Signed-off-by: John Ferlan <jferlan(a)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
1:12 a.m.
New subject: [libvirt] [PATCH v2 03/10] qemu: Rename qemuBuildCpuArgStr to qemuBuildCpuCommandLine
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(a)redhat.com>
---
src/qemu/qemu_command.c | 48 ++++++++++++++++++++++--------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
ACK
8:25 p.m.
New subject: [libvirt] [PATCH v2 04/10] qemu: Introduce qemuBuildMemCommandLine
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(a)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
1:18 a.m.
New subject: [libvirt] [PATCH v2 04/10] qemu: Introduce qemuBuildMemCommandLine
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(a)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
8:25 p.m.
New subject: [libvirt] [PATCH v2 05/10] qemu: Rename qemuBuildSmpArgStr to qemuBuildSmpCommandLine
Rename function and move code in from qemuBuildCommandLine to
keep smp related code together.
Signed-off-by: John Ferlan <jferlan(a)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
1:22 a.m.
New subject: [libvirt] [PATCH v2 05/10] qemu: Rename qemuBuildSmpArgStr to qemuBuildSmpCommandLine
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(a)redhat.com>
---
src/qemu/qemu_command.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
ACK
8:25 p.m.
New subject: [libvirt] [PATCH v2 06/10] qemu: Introduce qemuBuildIOThreadCommandLine
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(a)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
1:23 a.m.
New subject: [libvirt] [PATCH v2 06/10] qemu: Introduce qemuBuildIOThreadCommandLine
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(a)redhat.com>
---
src/qemu/qemu_command.c | 51 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 18 deletions(-)
ACK
8:25 p.m.
New subject: [libvirt] [PATCH v2 07/10] qemu: Introduce qemuBuildNumaCommandLine
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(a)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
1:25 a.m.
New subject: [libvirt] [PATCH v2 07/10] qemu: Introduce qemuBuildNumaCommandLine
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(a)redhat.com>
---
src/qemu/qemu_command.c | 66 ++++++++++++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 26 deletions(-)
ACK
8:25 p.m.
New subject: [libvirt] [PATCH v2 08/10] qemu: Introduce qemuBuildSmbiosCommandLine
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(a)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
2:47 a.m.
New subject: [libvirt] [PATCH v2 08/10] qemu: Introduce qemuBuildSmbiosCommandLine
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(a)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
8:25 p.m.
New subject: [libvirt] [PATCH v2 09/10] qemu: Introduce qemuBuildSgaCommandLine
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(a)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
2:38 a.m.
New subject: [libvirt] [PATCH v2 09/10] qemu: Introduce qemuBuildSgaCommandLine
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(a)redhat.com>
---
src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 19 deletions(-)
ACK
8:25 p.m.
New subject: [libvirt] [PATCH v2 10/10] qemu: Introduce qemuBuildMonitorCommandLine
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(a)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
2:41 a.m.
New subject: [libvirt] [PATCH v2 10/10] qemu: Introduce qemuBuildMonitorCommandLine
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(a)redhat.com>
---
src/qemu/qemu_command.c | 78 ++++++++++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 30 deletions(-)
ACK
6:08 a.m.
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...
3203
days inactive
3203
days old
22 comments
3 participants
participants (3)
-
John Ferlan -
Pavel Hrdina -
Peter Krempa