[libvirt] [PATCH REPOST 0/8] Reorganize qemu_command.c in smaller piles (round 2)

Rather than hope for a review of 25 patches or creating some remote place to place a branch - I'll repost the original: http://www.redhat.com/archives/libvir-list/2016-February/msg00975.html in smaller chunks. Differences in this set of patches vs. original series includes handling merge conflicts for logManager series plus a few minor edits/tweaks for better formatting/flow. This series was based off of git head commit 'ef2ab8fdc' which is the round 1 of the split: http://www.redhat.com/archives/libvir-list/2016-March/msg00288.html John Ferlan (8): qemu: Introduce qemuBuildSmartcardCommandLine qemu: Introduce qemuBuildSerialCommandLine qemu: Introduce qemuBuildParallelsCommandLine qemu: Introduce qemuBuildChannelsCommandLine qemu: Introduce qemuBuildConsoleCommandLine qemu: Modify qemuBuildTPMCommandLine qemu: Introduce qemuBuildInputCommandLine qemu: Introduce qemuBuildVideoCommandLine src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 1410 +++++++++++++++++++++++------------------- src/qemu/qemu_command.h | 6 +- 4 files changed, 760 insertions(+), 660 deletions(-) -- 2.5.0

Add new function to manage adding the smartcard device options to the command line removing that task from the mainline qemuBuildCommandLine. Alter the logic slightly to make !nsmartcards check first so that remainder of the code is less indented. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 227 ++++++++++++++++++++++++++---------------------- 1 file changed, 122 insertions(+), 105 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 834ad33..982541a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4247,7 +4247,7 @@ qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, static int qemuBuildChrChardevFileStr(virLogManagerPtr logManager, virCommandPtr cmd, - virDomainDefPtr def, + const virDomainDef *def, virBufferPtr buf, const char *filearg, const char *fileval, const char *appendarg, int appendval) @@ -4298,7 +4298,7 @@ qemuBuildChrChardevFileStr(virLogManagerPtr logManager, static char * qemuBuildChrChardevStr(virLogManagerPtr logManager, virCommandPtr cmd, - virDomainDefPtr def, + const virDomainDef *def, const virDomainChrSourceDef *dev, const char *alias, virQEMUCapsPtr qemuCaps) @@ -7506,6 +7506,124 @@ qemuBuildNetCommandLine(virCommandPtr cmd, } +static int +qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + virDomainSmartcardDefPtr smartcard; + char *devstr; + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *database; + + if (!def->nsmartcards) + return 0; + + smartcard = def->smartcards[0]; + + /* -device usb-ccid was already emitted along with other + * controllers. For now, qemu handles only one smartcard. */ + if (def->nsmartcards > 1 || + smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID || + smartcard->info.addr.ccid.controller != 0 || + smartcard->info.addr.ccid.slot != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks multiple smartcard " + "support")); + virBufferFreeAndReset(&opt); + return -1; + } + + switch (smartcard->type) { + case VIR_DOMAIN_SMARTCARD_TYPE_HOST: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + return -1; + } + + virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); + break; + + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + return -1; + } + + virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); + for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { + if (strchr(smartcard->data.cert.file[i], ',')) { + virBufferFreeAndReset(&opt); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid certificate name: %s"), + smartcard->data.cert.file[i]); + return -1; + } + virBufferAsprintf(&opt, ",cert%zu=%s", i + 1, + smartcard->data.cert.file[i]); + } + if (smartcard->data.cert.database) { + if (strchr(smartcard->data.cert.database, ',')) { + virBufferFreeAndReset(&opt); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid database name: %s"), + smartcard->data.cert.database); + return -1; + } + database = smartcard->data.cert.database; + } else { + database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; + } + virBufferAsprintf(&opt, ",db=%s", database); + break; + + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard " + "passthrough mode support")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &smartcard->data.passthru, + smartcard->info.alias, + qemuCaps))) { + virBufferFreeAndReset(&opt); + return -1; + } + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", + smartcard->info.alias); + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected smartcard type %d"), + smartcard->type); + virBufferFreeAndReset(&opt); + return -1; + } + virCommandAddArg(cmd, "-device"); + virBufferAsprintf(&opt, ",id=%s,bus=ccid0.0", smartcard->info.alias); + virCommandAddArgBuffer(cmd, &opt); + + return 0; +} + + char * qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, @@ -8134,109 +8252,8 @@ qemuBuildCommandLine(virConnectPtr conn, &bootHostdevNet) < 0) goto error; - if (def->nsmartcards) { - /* -device usb-ccid was already emitted along with other - * controllers. For now, qemu handles only one smartcard. */ - virDomainSmartcardDefPtr smartcard = def->smartcards[0]; - char *devstr; - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *database; - - if (def->nsmartcards > 1 || - smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID || - smartcard->info.addr.ccid.controller != 0 || - smartcard->info.addr.ccid.slot != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks multiple smartcard " - "support")); - virBufferFreeAndReset(&opt); - goto error; - } - - switch (smartcard->type) { - case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - goto error; - } - - virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); - break; - - case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - goto error; - } - - virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); - for (j = 0; j < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; j++) { - if (strchr(smartcard->data.cert.file[j], ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid certificate name: %s"), - smartcard->data.cert.file[j]); - goto error; - } - virBufferAsprintf(&opt, ",cert%zu=%s", j + 1, - smartcard->data.cert.file[j]); - } - if (smartcard->data.cert.database) { - if (strchr(smartcard->data.cert.database, ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid database name: %s"), - smartcard->data.cert.database); - goto error; - } - database = smartcard->data.cert.database; - } else { - database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; - } - virBufferAsprintf(&opt, ",db=%s", database); - break; - - case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard " - "passthrough mode support")); - goto error; - } - - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - &smartcard->data.passthru, - smartcard->info.alias, - qemuCaps))) { - virBufferFreeAndReset(&opt); - goto error; - } - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", - smartcard->info.alias); - break; - - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); - virBufferFreeAndReset(&opt); - goto error; - } - virCommandAddArg(cmd, "-device"); - virBufferAsprintf(&opt, ",id=%s,bus=ccid0.0", smartcard->info.alias); - virCommandAddArgBuffer(cmd, &opt); - } + if (qemuBuildSmartcardCommandLine(logManager, cmd, def, qemuCaps) < 0) + goto error; if (def->nserials) { for (i = 0; i < def->ngraphics; i++) { -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the smartcard device options to the command line removing that task from the mainline qemuBuildCommandLine.
Alter the logic slightly to make !nsmartcards check first so that remainder of the code is less indented.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 227 ++++++++++++++++++++++++++---------------------- 1 file changed, 122 insertions(+), 105 deletions(-)
ACK - Cole

Add new function to manage adding the serial device options to the command line removing that task from the mainline qemuBuildCommandLine. Using const virDomainDef causes collateral damage in other called APIs which need to make the similar adjustment Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++++++------------------- src/qemu/qemu_command.h | 2 +- 4 files changed, 67 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 403d872..b223837 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3839,7 +3839,7 @@ virQEMUCapsCacheFree(virQEMUCapsCachePtr cache) bool -virQEMUCapsSupportsChardev(virDomainDefPtr def, +virQEMUCapsSupportsChardev(const virDomainDef *def, virQEMUCapsPtr qemuCaps, virDomainChrDefPtr chr) { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 01ceb8e..caf3d1b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -450,7 +450,7 @@ int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); VIR_ENUM_DECL(virQEMUCaps); -bool virQEMUCapsSupportsChardev(virDomainDefPtr def, +bool virQEMUCapsSupportsChardev(const virDomainDef *def, virQEMUCapsPtr qemuCaps, virDomainChrDefPtr chr); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 982541a..1659e71 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4586,7 +4586,7 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager, static char * -qemuBuildVirtioSerialPortDevStr(virDomainDefPtr def, +qemuBuildVirtioSerialPortDevStr(const virDomainDef *def, virDomainChrDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -7743,7 +7743,7 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, static int qemuBuildChrDeviceCommandLine(virCommandPtr cmd, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -7757,6 +7757,62 @@ qemuBuildChrDeviceCommandLine(virCommandPtr cmd, return 0; } + +static int +qemuBuildSerialCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + int actualSerials = 0; + bool havespice = false; + + 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 && !havespice) + continue; + + /* Use -chardev with -device if they are available */ + if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &serial->source, + serial->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, serial, qemuCaps) < 0) + return -1; + } else { + virCommandAddArg(cmd, "-serial"); + if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + actualSerials++; + } + + /* If we have -device, then we set -nodefaults already */ + if (!actualSerials && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-serial", "none", NULL); + + return 0; +} + + static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, @@ -8122,10 +8178,8 @@ qemuBuildCommandLine(virConnectPtr conn, virErrorPtr originalError = NULL; size_t i, j; char uuid[VIR_UUID_STRING_BUFLEN]; - bool havespice = false; virCommandPtr cmd = NULL; bool emitBootindex = false; - int actualSerials = 0; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int bootHostdevNet = 0; @@ -8255,48 +8309,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSmartcardCommandLine(logManager, cmd, def, qemuCaps) < 0) goto error; - if (def->nserials) { - for (i = 0; i < def->ngraphics; i++) { - if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - havespice = true; - break; - } - } - } - - for (i = 0; i < def->nserials; i++) { - virDomainChrDefPtr serial = def->serials[i]; - char *devstr; - - if (serial->source.type == VIR_DOMAIN_CHR_TYPE_SPICEPORT && !havespice) - continue; - - /* Use -chardev with -device if they are available */ - if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - &serial->source, - serial->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, serial, qemuCaps) < 0) - goto error; - } else { - virCommandAddArg(cmd, "-serial"); - if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - actualSerials++; - } - - /* If we have -device, then we set -nodefault already */ - if (!actualSerials && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); + if (qemuBuildSerialCommandLine(logManager, cmd, def, qemuCaps) < 0) + goto error; if (!def->nparallels) { /* If we have -device, then we set -nodefault already */ @@ -9204,7 +9218,7 @@ qemuBuildCommandLine(virConnectPtr conn, */ static int qemuBuildSerialChrDeviceStr(char **deviceStr, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr serial, virQEMUCapsPtr qemuCaps, virArch arch, @@ -9299,7 +9313,7 @@ qemuBuildParallelChrDeviceStr(char **deviceStr, static int qemuBuildChannelChrDeviceStr(char **deviceStr, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -9339,7 +9353,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, static int qemuBuildConsoleChrDeviceStr(char **deviceStr, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -9379,7 +9393,7 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, int qemuBuildChrDeviceStr(char **deviceStr, - virDomainDefPtr vmdef, + const virDomainDef *vmdef, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 8167af8..0316f43 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -82,7 +82,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, /* Generate '-device' string for chardev device */ int qemuBuildChrDeviceStr(char **deviceStr, - virDomainDefPtr vmdef, + const virDomainDef *vmdef, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps); -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the serial device options to the command line removing that task from the mainline qemuBuildCommandLine.
Using const virDomainDef causes collateral damage in other called APIs which need to make the similar adjustment
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 114 ++++++++++++++++++++++++------------------- src/qemu/qemu_command.h | 2 +- 4 files changed, 67 insertions(+), 53 deletions(-)
ACK - Cole

Add new function to manage adding the parallels device options to the command line removing that task from the mainline qemuBuildCommandLine. Alter logic slight to reduce indention level. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 78 +++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1659e71..a86cbb3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7814,6 +7814,50 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager, static int +qemuBuildParallelsCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + /* If we have -device, then we set -nodefaults already */ + if (!def->nparallels && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-parallel", "none", NULL); + + for (i = 0; i < def->nparallels; i++) { + virDomainChrDefPtr parallel = def->parallels[i]; + char *devstr; + + /* Use -chardev with -device if they are available */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + ¶llel->source, + parallel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, parallel, + qemuCaps) < 0) + return -1; + } else { + virCommandAddArg(cmd, "-parallel"); + if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8312,38 +8356,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSerialCommandLine(logManager, cmd, def, qemuCaps) < 0) goto error; - if (!def->nparallels) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-parallel", "none", NULL); - } else { - for (i = 0; i < def->nparallels; i++) { - virDomainChrDefPtr parallel = def->parallels[i]; - char *devstr; - - /* Use -chardev with -device if they are available */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - ¶llel->source, - parallel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, parallel, qemuCaps) < 0) - goto error; - } else { - virCommandAddArg(cmd, "-parallel"); - if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - } - } + if (qemuBuildParallelsCommandLine(logManager, cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->nchannels; i++) { virDomainChrDefPtr channel = def->channels[i]; -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the parallels device options to the command line removing that task from the mainline qemuBuildCommandLine. Alter logic slight to reduce indention level.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 78 +++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 32 deletions(-)
ACK - Cole

Add new function to manage adding the channel device 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 | 163 ++++++++++++++++++++++++++---------------------- 1 file changed, 90 insertions(+), 73 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a86cbb3..0d0e598 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7858,6 +7858,93 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager, static int +qemuBuildChannelsCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + const char *domainChannelTargetDir) +{ + size_t i; + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDefPtr channel = def->channels[i]; + char *devstr; + + switch (channel->targetType) { + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("guestfwd requires QEMU to support -chardev & -device")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, + channel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceStr(&devstr, def, channel, qemuCaps) < 0) + return -1; + virCommandAddArgList(cmd, "-netdev", devstr, NULL); + VIR_FREE(devstr); + break; + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + return -1; + } + + /* + * TODO: Refactor so that we generate this (and onther + * things) somewhere else then where we are building the + * command line. + */ + if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && + !channel->source.data.nix.path) { + if (virAsprintf(&channel->source.data.nix.path, + "%s/%s", domainChannelTargetDir, + channel->target.name ? channel->target.name + : "unknown.sock") < 0) + return -1; + + channel->source.data.nix.listen = true; + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && + channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + /* spicevmc was originally introduced via a -device + * with a backend internal to qemu; although we prefer + * the newer -chardev interface. */ + ; + } else { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &channel->source, + channel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + + if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0) + return -1; + break; + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8359,79 +8446,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildParallelsCommandLine(logManager, cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->nchannels; i++) { - virDomainChrDefPtr channel = def->channels[i]; - char *devstr; - - switch (channel->targetType) { - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("guestfwd requires QEMU to support -chardev & -device")); - goto error; - } - - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - &channel->source, - channel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceStr(&devstr, def, channel, qemuCaps) < 0) - goto error; - virCommandAddArgList(cmd, "-netdev", devstr, NULL); - VIR_FREE(devstr); - break; - - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } - - /* - * TODO: Refactor so that we generate this (and onther - * things) somewhere else then where we are building the - * command line. - */ - if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && - !channel->source.data.nix.path) { - if (virAsprintf(&channel->source.data.nix.path, - "%s/%s", domainChannelTargetDir, - channel->target.name ? channel->target.name - : "unknown.sock") < 0) - goto error; - - channel->source.data.nix.listen = true; - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && - channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - /* spicevmc was originally introduced via a -device - * with a backend internal to qemu; although we prefer - * the newer -chardev interface. */ - ; - } else { - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - &channel->source, - channel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - - if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0) - goto error; - break; - } - } + if (qemuBuildChannelsCommandLine(logManager, cmd, def, qemuCaps, + domainChannelTargetDir) < 0) + goto error; /* Explicit console devices */ for (i = 0; i < def->nconsoles; i++) { -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the channel device 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 | 163 ++++++++++++++++++++++++++---------------------- 1 file changed, 90 insertions(+), 73 deletions(-)
ACK - Cole

Add new function to manage adding the console device 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 | 139 +++++++++++++++++++++++++++--------------------- 1 file changed, 77 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d0e598..dd62815 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7945,6 +7945,81 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager, static int +qemuBuildConsoleCommandLine(virLogManagerPtr logManager, + virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + /* Explicit console devices */ + for (i = 0; i < def->nconsoles; i++) { + virDomainChrDefPtr console = def->consoles[i]; + char *devstr; + + switch (console->targetType) { + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp console requires QEMU to support -device")); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCLP_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp console requires QEMU to support s390-sclp")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, + console->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, + &console->source, + console->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported console target type %s"), + NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); + return -1; + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8450,68 +8525,8 @@ qemuBuildCommandLine(virConnectPtr conn, domainChannelTargetDir) < 0) goto error; - /* Explicit console devices */ - for (i = 0; i < def->nconsoles; i++) { - virDomainChrDefPtr console = def->consoles[i]; - char *devstr; - - switch (console->targetType) { - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sclp console requires QEMU to support -device")); - goto error; - } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCLP_S390)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sclp console requires QEMU to support s390-sclp")); - goto error; - } - - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - &console->source, - console->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) - goto error; - break; - - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } - - if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, def, - &console->source, - console->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) - goto error; - break; - - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: - break; - - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported console target type %s"), - NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); - goto error; - } - } + if (qemuBuildConsoleCommandLine(logManager, cmd, def, qemuCaps) < 0) + goto error; if (def->tpm) { if (qemuBuildTPMCommandLine(def, cmd, qemuCaps, def->emulator) < 0) -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the console device 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 | 139 +++++++++++++++++++++++++++--------------------- 1 file changed, 77 insertions(+), 62 deletions(-)
ACK - Cole

Modify the argument order and types to match other similar helpers. Also modify called functions to use the def->emulator instead of passing def->emulator and def. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dd62815..258fac4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8091,8 +8091,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, static char * qemuBuildTPMDevStr(const virDomainDef *def, - virQEMUCapsPtr qemuCaps, - const char *emulator) + virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; const virDomainTPMDef *tpm = def->tpm; @@ -8102,7 +8101,7 @@ qemuBuildTPMDevStr(const virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "model %s"), - emulator, model); + def->emulator, model); goto error; } @@ -8124,7 +8123,6 @@ static char * qemuBuildTPMBackendStr(const virDomainDef *def, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - const char *emulator, int *tpmfd, int *cancelfd) { @@ -8200,7 +8198,7 @@ qemuBuildTPMBackendStr(const virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "backend type %s"), - emulator, type); + def->emulator, type); error: VIR_FREE(devset); @@ -8212,17 +8210,19 @@ qemuBuildTPMBackendStr(const virDomainDef *def, static int -qemuBuildTPMCommandLine(virDomainDefPtr def, - virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, - const char *emulator) +qemuBuildTPMCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { char *optstr; int tpmfd = -1; int cancelfd = -1; char *fdset; - if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps, emulator, + if (!def->tpm) + return 0; + + if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps, &tpmfd, &cancelfd))) return -1; @@ -8247,7 +8247,7 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, VIR_FREE(fdset); } - if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator))) + if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps))) return -1; virCommandAddArgList(cmd, "-device", optstr, NULL); @@ -8528,10 +8528,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildConsoleCommandLine(logManager, cmd, def, qemuCaps) < 0) goto error; - if (def->tpm) { - if (qemuBuildTPMCommandLine(def, cmd, qemuCaps, def->emulator) < 0) - goto error; - } + if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->ninputs; i++) { virDomainInputDefPtr input = def->inputs[i]; -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Modify the argument order and types to match other similar helpers.
Also modify called functions to use the def->emulator instead of passing def->emulator and def.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACK - Cole

Add new function to manage adding the input device options to the command line removing that task from the mainline qemuBuildCommandLine. Make qemuBuildUSBInputDevStr static since only this module calls it. Also the change to use const virDomainDef forces other changes. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 87 +++++++++++++++++++++++++++++-------------------- src/qemu/qemu_command.h | 4 --- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 258fac4..defc3e9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3433,7 +3433,7 @@ qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) } static char * -qemuBuildVirtioInputDevStr(virDomainDefPtr def, +qemuBuildVirtioInputDevStr(const virDomainDef *def, virDomainInputDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3502,8 +3502,8 @@ qemuBuildVirtioInputDevStr(virDomainDefPtr def, return NULL; } -char * -qemuBuildUSBInputDevStr(virDomainDefPtr def, +static char * +qemuBuildUSBInputDevStr(const virDomainDef *def, virDomainInputDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3537,6 +3537,52 @@ qemuBuildUSBInputDevStr(virDomainDefPtr def, } +static int +qemuBuildInputCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->ninputs; i++) { + virDomainInputDefPtr input = def->inputs[i]; + + if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + char *optstr; + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + switch (input->type) { + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + virCommandAddArgList(cmd, "-usbdevice", "mouse", NULL); + break; + case VIR_DOMAIN_INPUT_TYPE_TABLET: + virCommandAddArgList(cmd, "-usbdevice", "tablet", NULL); + break; + case VIR_DOMAIN_INPUT_TYPE_KBD: + virCommandAddArgList(cmd, "-usbdevice", "keyboard", + NULL); + break; + } + } + } else if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { + char *optstr; + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildVirtioInputDevStr(def, input, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + } + + return 0; +} + + char * qemuBuildSoundDevStr(virDomainDefPtr def, virDomainSoundDefPtr sound, @@ -8531,39 +8577,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->ninputs; i++) { - virDomainInputDefPtr input = def->inputs[i]; - - if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - char *optstr; - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } else { - switch (input->type) { - case VIR_DOMAIN_INPUT_TYPE_MOUSE: - virCommandAddArgList(cmd, "-usbdevice", "mouse", NULL); - break; - case VIR_DOMAIN_INPUT_TYPE_TABLET: - virCommandAddArgList(cmd, "-usbdevice", "tablet", NULL); - break; - case VIR_DOMAIN_INPUT_TYPE_KBD: - virCommandAddArgList(cmd, "-usbdevice", "keyboard", NULL); - break; - } - } - } else if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { - char *optstr; - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildVirtioInputDevStr(def, input, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } - } + if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->ngraphics; ++i) { if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0316f43..d3e676b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -138,10 +138,6 @@ char *qemuBuildMemballoonDevStr(virDomainDefPtr domainDef, virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps); -char *qemuBuildUSBInputDevStr(virDomainDefPtr domainDef, - virDomainInputDefPtr dev, - virQEMUCapsPtr qemuCaps); - char *qemuBuildSoundDevStr(virDomainDefPtr domainDef, virDomainSoundDefPtr sound, virQEMUCapsPtr qemuCaps); -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the input device options to the command line removing that task from the mainline qemuBuildCommandLine.
Make qemuBuildUSBInputDevStr static since only this module calls it.
Also the change to use const virDomainDef forces other changes.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 87 +++++++++++++++++++++++++++++-------------------- src/qemu/qemu_command.h | 4 --- 2 files changed, 51 insertions(+), 40 deletions(-)
ACK - Cole

Add new function to manage adding the video device 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 | 332 +++++++++++++++++++++++++----------------------- 1 file changed, 173 insertions(+), 159 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index defc3e9..ba8c216 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3690,7 +3690,7 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, } static char * -qemuBuildDeviceVideoStr(virDomainDefPtr def, +qemuBuildDeviceVideoStr(const virDomainDef *def, virDomainVideoDefPtr video, virQEMUCapsPtr qemuCaps) { @@ -3803,6 +3803,176 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, } +static int +qemuBuildVideoCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + int primaryVideoType = def->videos[0]->type; + + if (!def->nvideos) { + /* If we have -device, then we set -nodefaults already */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_NONE)) + virCommandAddArgList(cmd, "-vga", "none", NULL); + return 0; + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY) && + ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) + ) { + for (i = 0; i < def->nvideos; i++) { + char *str; + virCommandAddArg(cmd, "-device"); + if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], + qemuCaps))) + return -1; + + virCommandAddArg(cmd, str); + VIR_FREE(str); + } + } else { + if (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_XEN) { + /* nothing - vga has no effect on Xen pvfb */ + } else { + if ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_QXL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU does not support QXL graphics adapters")); + return -1; + } + + const char *vgastr = qemuVideoTypeToString(primaryVideoType); + if (!vgastr || STREQ(vgastr, "")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type %s is not supported with QEMU"), + virDomainVideoTypeToString(primaryVideoType)); + return -1; + } + + virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + /* If we cannot use --device option to specify the video device + * in QEMU we will fallback to the old --vga option. To get the + * correct device name for the --vga option the 'qemuVideo' is + * used, but to set some device attributes we need to use the + * --global option and for that we need to specify the device + * name the same as for --device option and for that we need to + * use 'qemuDeviceVideo'. + * + * See 'Graphics Devices' section in docs/qdev-device-use.txt in + * QEMU repository. + */ + const char *dev = qemuDeviceVideoTypeToString(primaryVideoType); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + (def->videos[0]->vram || def->videos[0]->ram) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + unsigned int ram = def->videos[0]->ram; + unsigned int vram = def->videos[0]->vram; + unsigned int vram64 = def->videos[0]->vram64; + unsigned int vgamem = def->videos[0]->vgamem; + + if (vram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + if (ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + + if (ram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.ram_size=%u", + dev, ram * 1024); + } + if (vram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram_size=%u", + dev, vram * 1024); + } + if (vram64 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram64_size_mb=%u", + dev, vram64 / 1024); + } + if (vgamem && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", + dev, vgamem / 1024); + } + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && + def->videos[0]->vram && + ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) { + unsigned int vram = def->videos[0]->vram; + + if (vram < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("value for 'vgamem' must be at " + "least 1 MiB (1024 KiB)")); + return -1; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", + dev, vram / 1024); + } + } + + if (def->nvideos > 1) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + for (i = 1; i < def->nvideos; i++) { + char *str; + if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type %s is only valid as primary video card"), + virDomainVideoTypeToString(def->videos[0]->type)); + return -1; + } + + virCommandAddArg(cmd, "-device"); + + if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], + qemuCaps))) + return -1; + + virCommandAddArg(cmd, str); + VIR_FREE(str); + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only one video card is currently supported")); + return -1; + } + } + } + + return 0; +} + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev) { @@ -8586,164 +8756,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (def->nvideos > 0) { - int primaryVideoType = def->videos[0]->type; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY) && - ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) - ) { - for (i = 0; i < def->nvideos; i++) { - char *str; - virCommandAddArg(cmd, "-device"); - if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], - qemuCaps))) - goto error; - - virCommandAddArg(cmd, str); - VIR_FREE(str); - } - } else { - if (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_XEN) { - /* nothing - vga has no effect on Xen pvfb */ - } else { - if ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_QXL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU does not support QXL graphics adapters")); - goto error; - } - - const char *vgastr = qemuVideoTypeToString(primaryVideoType); - if (!vgastr || STREQ(vgastr, "")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type %s is not supported with QEMU"), - virDomainVideoTypeToString(primaryVideoType)); - goto error; - } - - virCommandAddArgList(cmd, "-vga", vgastr, NULL); - - /* If we cannot use --device option to specify the video device - * in QEMU we will fallback to the old --vga option. To get the - * correct device name for the --vga option the 'qemuVideo' is - * used, but to set some device attributes we need to use the - * --global option and for that we need to specify the device - * name the same as for --device option and for that we need to - * use 'qemuDeviceVideo'. - * - * See 'Graphics Devices' section in docs/qdev-device-use.txt in - * QEMU repository. - */ - const char *dev = qemuDeviceVideoTypeToString(primaryVideoType); - - if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - (def->videos[0]->vram || def->videos[0]->ram) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - unsigned int ram = def->videos[0]->ram; - unsigned int vram = def->videos[0]->vram; - unsigned int vram64 = def->videos[0]->vram64; - unsigned int vgamem = def->videos[0]->vgamem; - - if (vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'vram' must be less than '%u'"), - UINT_MAX / 1024); - goto error; - } - if (ram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'ram' must be less than '%u'"), - UINT_MAX / 1024); - goto error; - } - - if (ram) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.ram_size=%u", - dev, ram * 1024); - } - if (vram) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vram_size=%u", - dev, vram * 1024); - } - if (vram64 && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vram64_size_mb=%u", - dev, vram64 / 1024); - } - if (vgamem && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", - dev, vgamem / 1024); - } - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && - def->videos[0]->vram && - ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) { - unsigned int vram = def->videos[0]->vram; - - if (vram < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("value for 'vgamem' must be at " - "least 1 MiB (1024 KiB)")); - goto error; - } - - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", - dev, vram / 1024); - } - } - - if (def->nvideos > 1) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - for (i = 1; i < def->nvideos; i++) { - char *str; - if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type %s is only valid as primary video card"), - virDomainVideoTypeToString(def->videos[0]->type)); - goto error; - } - - virCommandAddArg(cmd, "-device"); - - if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], - qemuCaps))) - goto error; - - virCommandAddArg(cmd, str); - VIR_FREE(str); - } - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("only one video card is currently supported")); - goto error; - } - } - } - - } else { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_NONE)) - virCommandAddArgList(cmd, "-vga", "none", NULL); - } + if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) + goto error; /* Add sound hardware */ if (def->nsounds) { -- 2.5.0

On 03/11/2016 07:32 AM, John Ferlan wrote:
Add new function to manage adding the video device 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 | 332 +++++++++++++++++++++++++----------------------- 1 file changed, 173 insertions(+), 159 deletions(-)
ACK - Cole
participants (2)
-
Cole Robinson
-
John Ferlan