[PATCH 00/14] qemu: Type-aware generation of properties for -device (part 2)
This series finishes the refactor to generate '-device' via JSON. This will be needed later on when qemu will consider only the JSON format to be long term stable. Next part will deal with the testing and finishing touches. Peter Krempa (14): qemuBuildStorageSourceAttachPrepareDrive: Fix function comment qemuBuildDeviceCommandlineFromJSON: Remove unused keyword qemuValidateDomainSmartcardDef: Unbreak error messages qemuValidateDomainSmartcardDef: Move chardev validation under VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH virDomainSmartcardDef: Declare 'type' as virDomainSmartcardType qemuBuildSmartcardCommandLine: Replace qemuBuildSmartcardFindCCIDController qemuValidateDomainSmartcardDef: Move validation of smartcard count qemuBuildVMGenIDCommandLine: Generate via JSON qemuBuildTPMProxyCommandLine: Generate via JSON qemuBuildVMCoreInfoCommandLine: Generate via JSON qemuBuildIOMMUCommandLine: Generate via JSON qemuBuildSmartcardCommandLine: Generate via JSON qemuBuildFloppyCommandLineControllerOptions: Extract formatting of implicit/explicit fdc qemuBuildFloppyCommandLineControllerOptionsExplicit: Generate via JSON src/conf/domain_conf.c | 23 +- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 366 +++++++++++++++++--------------- src/qemu/qemu_validate.c | 35 +-- src/security/security_selinux.c | 10 +- 5 files changed, 232 insertions(+), 204 deletions(-) -- 2.31.1
Remove mention of argument which no longer exists. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cc8fd68680..ade82eef22 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11063,7 +11063,6 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) * qemuBuildStorageSourceAttachPrepareDrive: * @disk: disk object to prepare * @qemuCaps: qemu capabilities object - * @driveBoot: bootable flag for disks which don't have -device part * * Prepare qemuBlockStorageSourceAttachData *for use with the old approach * using -drive/drive_add. See qemuBlockStorageSourceAttachPrepareBlockdev. -- 2.31.1
Now that the code was converted to use this helper we can remove it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ade82eef22..fae2d1569c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -221,7 +221,7 @@ qemuBuildNetdevCommandlineFromJSON(virCommand *cmd, } -static G_GNUC_UNUSED int +static int qemuBuildDeviceCommandlineFromJSON(virCommand *cmd, virJSONValue *props, virQEMUCaps *qemuCaps) -- 2.31.1
https://www.libvirt.org/coding-style.html#error-message-format Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 0e7684973e..5973f019d2 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2093,8 +2093,7 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, case VIR_DOMAIN_SMARTCARD_TYPE_HOST: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); + _("this QEMU binary lacks smartcard host mode support")); return -1; } break; @@ -2102,8 +2101,7 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); + _("this QEMU binary lacks smartcard host mode support")); return -1; } break; @@ -2111,8 +2109,7 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard " - "passthrough mode support")); + _("this QEMU binary lacks smartcard passthrough mode support")); return -1; } break; -- 2.31.1
Don't check the type twice, move the chardev validation into the switch. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 5973f019d2..9865e29637 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2112,6 +2112,9 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, _("this QEMU binary lacks smartcard passthrough mode support")); return -1; } + + if (qemuValidateDomainChrSourceDef(def->data.passthru, qemuCaps) < 0) + return -1; break; default: @@ -2119,10 +2122,6 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, return -1; } - if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH && - qemuValidateDomainChrSourceDef(def->data.passthru, qemuCaps) < 0) - return -1; - return 0; } -- 2.31.1
Use 'virXMLPropEnum' to parse it and fix all switch statements which didn't include the VIR_DOMAIN_SMARTCARD_TYPE_LAST case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 23 +++++++---------------- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 1 + src/qemu/qemu_validate.c | 1 + src/security/security_selinux.c | 10 ++++------ 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4799070bc4..aeb64c9c83 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2884,6 +2884,7 @@ void virDomainSmartcardDefFree(virDomainSmartcardDef *def) virObjectUnref(def->data.passthru); break; + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: break; } @@ -11602,7 +11603,6 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { g_autoptr(virDomainSmartcardDef) def = NULL; - g_autofree char *mode = NULL; g_autofree char *type = NULL; g_autofree xmlNodePtr *certificates = NULL; int n = 0; @@ -11611,18 +11611,9 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, ctxt->node = node; def = g_new0(virDomainSmartcardDef, 1); - mode = virXMLPropString(node, "mode"); - if (mode == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing smartcard device mode")); - return NULL; - } - if ((def->type = virDomainSmartcardTypeFromString(mode)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown smartcard device mode: %s"), - mode); + if (virXMLPropEnum(node, "mode", virDomainSmartcardTypeFromString, + VIR_XML_PROP_REQUIRED, &def->type) < 0) return NULL; - } switch (def->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: @@ -11687,9 +11678,9 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt, break; + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unknown smartcard mode")); + virReportEnumRangeError(virDomainSmartcardType, def->type); return NULL; } @@ -25406,9 +25397,9 @@ virDomainSmartcardDefFormat(virBuffer *buf, virDomainChrSourceDefFormat(&childBuf, def->data.passthru, flags); break; + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), def->type); + virReportEnumRangeError(virDomainSmartcardType, def->type); return -1; } virDomainDeviceInfoFormat(&childBuf, &def->info, flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c1871b1757..55be01bbcf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1320,7 +1320,7 @@ typedef enum { #define VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE "/etc/pki/nssdb" struct _virDomainSmartcardDef { - int type; /* virDomainSmartcardType */ + virDomainSmartcardType type; union { /* no extra data for 'host' */ struct { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fae2d1569c..2d7a6ebde7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9146,6 +9146,7 @@ qemuBuildSmartcardCommandLine(virLogManager *logManager, smartcard->info.alias); break; + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: virReportEnumRangeError(virDomainSmartcardType, smartcard->type); return -1; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 9865e29637..48d5c172c5 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2117,6 +2117,7 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, return -1; break; + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: virReportEnumRangeError(virDomainSmartcardType, def->type); return -1; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index cc72453329..622a8f4c02 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2696,10 +2696,9 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDef *def, return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev->data.passthru, false); + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown smartcard type %d"), - dev->type); + virReportEnumRangeError(virDomainSmartcardType, dev->type); return -1; } @@ -3103,10 +3102,9 @@ virSecuritySELinuxSetSecuritySmartcardCallback(virDomainDef *def, return virSecuritySELinuxSetChardevLabel(mgr, def, dev->data.passthru, false); + case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown smartcard type %d"), - dev->type); + virReportEnumRangeError(virDomainSmartcardType, dev->type); return -1; } -- 2.31.1
We have a commonly used helper virDomainControllerAliasFind, which does the same thing and also reports errors internally. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 35 +++-------------------------------- 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2d7a6ebde7..e41c7dfaaa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9048,32 +9048,6 @@ qemuBuildNetCommandLine(virQEMUDriver *driver, } -static const char * -qemuBuildSmartcardFindCCIDController(const virDomainDef *def, - const virDomainSmartcardDef *smartcard) -{ - size_t i; - - /* Should never happen. But doesn't hurt to check. */ - if (smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID) - return NULL; - - for (i = 0; i < def->ncontrollers; i++) { - const virDomainControllerDef *tmp = def->controllers[i]; - - if (tmp->type != VIR_DOMAIN_CONTROLLER_TYPE_CCID) - continue; - - if (tmp->idx != smartcard->info.addr.ccid.controller) - continue; - - return tmp->info.alias; - } - - return NULL; -} - - static int qemuBuildSmartcardCommandLine(virLogManager *logManager, virSecurityManager *secManager, @@ -9152,13 +9126,10 @@ qemuBuildSmartcardCommandLine(virLogManager *logManager, return -1; } - if (!(contAlias = qemuBuildSmartcardFindCCIDController(def, - smartcard))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to find controller for %s"), - smartcard->info.alias); + if (!(contAlias = virDomainControllerAliasFind(def, + VIR_DOMAIN_CONTROLLER_TYPE_CCID, + smartcard->info.addr.ccid.controller))) return -1; - } virCommandAddArg(cmd, "-device"); virBufferAsprintf(&opt, ",id=%s,bus=%s.0", smartcard->info.alias, contAlias); -- 2.31.1
Move it into the validator. Note that the placement into the device validation part is intentional so that it also covers hotplug code paths. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 12 ------------ src/qemu/qemu_validate.c | 20 +++++++++++++++----- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e41c7dfaaa..6fa07e3e17 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9073,18 +9073,6 @@ qemuBuildSmartcardCommandLine(virLogManager *logManager, 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")); - return -1; - } - switch (smartcard->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 48d5c172c5..75dc9edb7d 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2086,10 +2086,20 @@ qemuValidateDomainChrDef(const virDomainChrDef *dev, static int -qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, +qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *smartcard, + const virDomainDef *def, virQEMUCaps *qemuCaps) { - switch (def->type) { + 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")); + return -1; + } + + switch (smartcard->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2113,13 +2123,13 @@ qemuValidateDomainSmartcardDef(const virDomainSmartcardDef *def, return -1; } - if (qemuValidateDomainChrSourceDef(def->data.passthru, qemuCaps) < 0) + if (qemuValidateDomainChrSourceDef(smartcard->data.passthru, qemuCaps) < 0) return -1; break; case VIR_DOMAIN_SMARTCARD_TYPE_LAST: default: - virReportEnumRangeError(virDomainSmartcardType, def->type); + virReportEnumRangeError(virDomainSmartcardType, smartcard->type); return -1; } @@ -5105,7 +5115,7 @@ qemuValidateDomainDeviceDef(const virDomainDeviceDef *dev, return qemuValidateDomainChrDef(dev->data.chr, def, qemuCaps); case VIR_DOMAIN_DEVICE_SMARTCARD: - return qemuValidateDomainSmartcardDef(dev->data.smartcard, qemuCaps); + return qemuValidateDomainSmartcardDef(dev->data.smartcard, def, qemuCaps); case VIR_DOMAIN_DEVICE_RNG: return qemuValidateDomainRNGDef(dev->data.rng, qemuCaps); -- 2.31.1
QEMU declares the 'guid' property as: guid=<str> - UUID (aka GUID) or "auto" for random value (default) (default: "auto") Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6fa07e3e17..bf15a4801a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6144,19 +6144,26 @@ qemuBuildSysinfoCommandLine(virCommand *cmd, static int qemuBuildVMGenIDCommandLine(virCommand *cmd, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCaps *qemuCaps) { - g_auto(virBuffer) opts = VIR_BUFFER_INITIALIZER; + g_autoptr(virJSONValue) props = NULL; char guid[VIR_UUID_STRING_BUFLEN]; if (!def->genidRequested) return 0; virUUIDFormat(def->genid, guid); - virBufferAsprintf(&opts, "vmgenid,guid=%s,id=vmgenid0", guid); - virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &opts); + if (virJSONValueObjectCreate(&props, + "s:driver", "vmgenid", + "s:guid", guid, + "s:id", "vmgenid0", + NULL) < 0) + return -1; + + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; return 0; } @@ -10658,7 +10665,7 @@ qemuBuildCommandLine(virQEMUDriver *driver, if (qemuBuildSysinfoCommandLine(cmd, def) < 0) return NULL; - if (qemuBuildVMGenIDCommandLine(cmd, def) < 0) + if (qemuBuildVMGenIDCommandLine(cmd, def, qemuCaps) < 0) return NULL; /* -- 2.31.1
All properties are strings according to QEMU. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bf15a4801a..86affcdf6f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9980,19 +9980,20 @@ qemuBuildTPMCommandLine(virCommand *cmd, static int qemuBuildTPMProxyCommandLine(virCommand *cmd, - virDomainTPMDef *tpm) + virDomainTPMDef *tpm, + virQEMUCaps *qemuCaps) { - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - const char *filePath = NULL; + g_autoptr(virJSONValue) props = NULL; - filePath = tpm->data.passthrough.source.data.file.path; + if (virJSONValueObjectCreate(&props, + "s:driver", virDomainTPMModelTypeToString(tpm->model), + "s:id", tpm->info.alias, + "s:host-path", tpm->data.passthrough.source.data.file.path, + NULL) < 0) + return -1; - virCommandAddArg(cmd, "-device"); - virBufferAsprintf(&buf, "%s,id=%s,host-path=", - virDomainTPMModelTypeToString(tpm->model), - tpm->info.alias); - virQEMUBuildBufferEscapeComma(&buf, filePath); - virCommandAddArgBuffer(cmd, &buf); + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; return 0; } @@ -10007,7 +10008,7 @@ qemuBuildTPMsCommandLine(virCommand *cmd, for (i = 0; i < def->ntpms; i++) { if (def->tpms[i]->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) { - if (qemuBuildTPMProxyCommandLine(cmd, def->tpms[i]) < 0) + if (qemuBuildTPMProxyCommandLine(cmd, def->tpms[i], qemuCaps) < 0) return -1; } else if (qemuBuildTPMCommandLine(cmd, def, def->tpms[i], qemuCaps) < 0) { -- 2.31.1
While this device doesn't have any properties it must be converted to use qemuBuildDeviceCommandlineFromJSON so that we can validate it in the future. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 86affcdf6f..56bf17b6b4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10097,14 +10097,22 @@ qemuBuildSecCommandLine(virDomainObj *vm, virCommand *cmd, static int qemuBuildVMCoreInfoCommandLine(virCommand *cmd, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCaps *qemuCaps) { - virTristateSwitch vmci = def->features[VIR_DOMAIN_FEATURE_VMCOREINFO]; + g_autoptr(virJSONValue) props = NULL; - if (vmci != VIR_TRISTATE_SWITCH_ON) + if (def->features[VIR_DOMAIN_FEATURE_VMCOREINFO] != VIR_TRISTATE_SWITCH_ON) return 0; - virCommandAddArgList(cmd, "-device", "vmcoreinfo", NULL); + if (virJSONValueObjectCreate(&props, + "s:driver", "vmcoreinfo", + NULL) < 0) + return -1; + + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; + return 0; } @@ -10785,7 +10793,7 @@ qemuBuildCommandLine(virQEMUDriver *driver, if (qemuBuildNVRAMCommandLine(cmd, def) < 0) return NULL; - if (qemuBuildVMCoreInfoCommandLine(cmd, def) < 0) + if (qemuBuildVMCoreInfoCommandLine(cmd, def, qemuCaps) < 0) return NULL; if (qemuBuildSecCommandLine(vm, cmd, def->sec) < 0) -- 2.31.1
QEMU declares the following types for fields we control: intremap=<OnOffAuto> - on/off/auto (default: "auto") caching-mode=<bool> - (default: false) eim=<OnOffAuto> - on/off/auto (default: "auto") device-iotlb=<bool> - (default: false) aw-bits=<uint8> - (default: 39) Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 42 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 56bf17b6b4..8913110b00 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6486,7 +6486,8 @@ qemuBuildBootCommandLine(virCommand *cmd, static int qemuBuildIOMMUCommandLine(virCommand *cmd, - const virDomainDef *def) + const virDomainDef *def, + virQEMUCaps *qemuCaps) { const virDomainIOMMUDef *iommu = def->iommu; @@ -6495,31 +6496,22 @@ qemuBuildIOMMUCommandLine(virCommand *cmd, switch (iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_INTEL: { - g_auto(virBuffer) opts = VIR_BUFFER_INITIALIZER; + g_autoptr(virJSONValue) props = NULL; - virBufferAddLit(&opts, "intel-iommu"); - if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&opts, ",intremap=%s", - virTristateSwitchTypeToString(iommu->intremap)); - } - if (iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&opts, ",caching-mode=%s", - virTristateSwitchTypeToString(iommu->caching_mode)); - } - if (iommu->eim != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&opts, ",eim=%s", - virTristateSwitchTypeToString(iommu->eim)); - } - if (iommu->iotlb != VIR_TRISTATE_SWITCH_ABSENT) { - virBufferAsprintf(&opts, ",device-iotlb=%s", - virTristateSwitchTypeToString(iommu->iotlb)); - } - if (iommu->aw_bits > 0) - virBufferAsprintf(&opts, ",aw-bits=%d", iommu->aw_bits); + if (virJSONValueObjectCreate(&props, + "s:driver", "intel-iommu", + "S:intremap", qemuOnOffAuto(iommu->intremap), + "T:caching-mode", iommu->caching_mode, + "S:eim", qemuOnOffAuto(iommu->eim), + "T:device-iotlb", iommu->iotlb, + "z:aw-bits", iommu->aw_bits, + NULL) < 0) + return -1; - virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &opts); - break; + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; + + return 0; } case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: @@ -10705,7 +10697,7 @@ qemuBuildCommandLine(virQEMUDriver *driver, if (qemuBuildBootCommandLine(cmd, def) < 0) return NULL; - if (qemuBuildIOMMUCommandLine(cmd, def) < 0) + if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0) return NULL; if (qemuBuildGlobalControllerCommandLine(cmd, def) < 0) -- 2.31.1
QEMU declares the props we control as: 'ccid-card-emulated' backend=<str> cert1=<str> cert2=<str> cert3=<str> db=<str> 'ccid-card-passthru' chardev=<str> - ID of a chardev to use as a backend Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 88 +++++++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8913110b00..1d5986b85a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9056,16 +9056,10 @@ qemuBuildSmartcardCommandLine(virLogManager *logManager, virQEMUCaps *qemuCaps, bool chardevStdioLogd) { - size_t i; + g_autoptr(virJSONValue) props = NULL; virDomainSmartcardDef *smartcard; - g_autofree char *devstr = NULL; - g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER; - const char *database; const char *contAlias = NULL; - unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT | - QEMU_BUILD_CHARDEV_UNIX_FD_PASS; - if (chardevStdioLogd) - cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD; + g_autofree char *bus = NULL; if (!def->nsmartcards) return 0; @@ -9074,37 +9068,56 @@ qemuBuildSmartcardCommandLine(virLogManager *logManager, switch (smartcard->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); + if (virJSONValueObjectCreate(&props, + "s:driver", "ccid-card-emulated", + "s:backend", "nss-emulated", + NULL) < 0) + return -1; + break; - case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: - virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); - for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { - virBufferAsprintf(&opt, ",cert%zu=", i + 1); - virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.file[i]); - } - if (smartcard->data.cert.database) { + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: { + const char *database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; + + if (smartcard->data.cert.database) database = smartcard->data.cert.database; - } else { - database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; - } - virBufferAddLit(&opt, ",db="); - virQEMUBuildBufferEscapeComma(&opt, database); + + if (virJSONValueObjectCreate(&props, + "s:driver", "ccid-card-emulated", + "s:backend", "certificates", + "s:cert1", smartcard->data.cert.file[0], + "s:cert2", smartcard->data.cert.file[1], + "s:cert3", smartcard->data.cert.file[2], + "s:db", database, + NULL) < 0) + return -1; + } break; - case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (!(devstr = qemuBuildChrChardevStr(logManager, secManager, - cmd, cfg, def, - smartcard->data.passthru, - smartcard->info.alias, - qemuCaps, cdevflags))) { + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: { + unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT | QEMU_BUILD_CHARDEV_UNIX_FD_PASS; + g_autofree char *chardevstr = NULL; + g_autofree char *chardevalias = g_strdup_printf("char%s", smartcard->info.alias); + + if (chardevStdioLogd) + cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD; + + if (!(chardevstr = qemuBuildChrChardevStr(logManager, secManager, + cmd, cfg, def, + smartcard->data.passthru, + smartcard->info.alias, + qemuCaps, cdevflags))) { return -1; } - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); - virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", - smartcard->info.alias); + virCommandAddArgList(cmd, "-chardev", chardevstr, NULL); + + if (virJSONValueObjectCreate(&props, + "s:driver", "ccid-card-passthru", + "s:chardev", chardevalias, + NULL) < 0) + return -1; + } break; case VIR_DOMAIN_SMARTCARD_TYPE_LAST: @@ -9118,9 +9131,16 @@ qemuBuildSmartcardCommandLine(virLogManager *logManager, smartcard->info.addr.ccid.controller))) return -1; - virCommandAddArg(cmd, "-device"); - virBufferAsprintf(&opt, ",id=%s,bus=%s.0", smartcard->info.alias, contAlias); - virCommandAddArgBuffer(cmd, &opt); + bus = g_strdup_printf("%s.0", contAlias); + + if (virJSONValueObjectAdd(props, + "s:id", smartcard->info.alias, + "s:bus", bus, + NULL) < 0) + return -1; + + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; return 0; } -- 2.31.1
qemuBuildFloppyCommandLineControllerOptions was generating config for both the implicit and explicit fdc. The explicit FDC is using '-device' and thus will need to be converted to JSON. Split up the lookup of the floppy drive configs from the actuall command generation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 126 +++++++++++++++++++++++++++------------- 1 file changed, 86 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1d5986b85a..665cd739ec 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2153,23 +2153,79 @@ qemuCommandAddExtDevice(virCommand *cmd, return 0; } + +static void +qemuBuildFloppyCommandLineControllerOptionsImplicit(virCommand *cmd, + unsigned int bootindexA, + unsigned int bootindexB, + const char *backendA, + const char *backendB) +{ + if (backendA) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.driveA=%s", backendA); + } + + if (bootindexA > 0) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.bootindexA=%u", bootindexA); + } + + if (backendB) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.driveB=%s", backendB); + } + + if (bootindexB > 0) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.bootindexB=%u", bootindexB); + } +} + + +static void +qemuBuildFloppyCommandLineControllerOptionsExplicit(virCommand *cmd, + unsigned int bootindexA, + unsigned int bootindexB, + const char *backendA, + const char *backendB) +{ + g_auto(virBuffer) fdc_opts = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&fdc_opts, "isa-fdc,"); + + if (backendA) + virBufferAsprintf(&fdc_opts, "driveA=%s,", backendA); + + if (bootindexA > 0) + virBufferAsprintf(&fdc_opts, "bootindexA=%u,", bootindexA); + + if (backendB) + virBufferAsprintf(&fdc_opts, "driveB=%s,", backendB); + + if (bootindexB > 0) + virBufferAsprintf(&fdc_opts, "bootindexB=%u,", bootindexB); + + virBufferTrim(&fdc_opts, ","); + virCommandAddArg(cmd, "-device"); + virCommandAddArgBuffer(cmd, &fdc_opts); +} + + static int qemuBuildFloppyCommandLineControllerOptions(virCommand *cmd, const virDomainDef *def, virQEMUCaps *qemuCaps) { - g_auto(virBuffer) fdc_opts = VIR_BUFFER_INITIALIZER; - bool explicitfdc = qemuDomainNeedsFDC(def); + unsigned int bootindexA = 0; + unsigned int bootindexB = 0; + g_autofree char *backendA = NULL; + g_autofree char *backendB = NULL; bool hasfloppy = false; - char driveLetter; size_t i; - virBufferAddLit(&fdc_opts, "isa-fdc,"); - for (i = 0; i < def->ndisks; i++) { g_autofree char *backendAlias = NULL; - g_autofree char *backendStr = NULL; - g_autofree char *bootindexStr = NULL; virDomainDiskDef *disk = def->disks[i]; if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC) @@ -2177,45 +2233,35 @@ qemuBuildFloppyCommandLineControllerOptions(virCommand *cmd, hasfloppy = true; - if (disk->info.addr.drive.unit) - driveLetter = 'B'; - else - driveLetter = 'A'; - - if (disk->info.effectiveBootIndex > 0) - bootindexStr = g_strdup_printf("bootindex%c=%u", driveLetter, - disk->info.effectiveBootIndex); - /* with -blockdev we setup the floppy device and it's backend with -device */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { - if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0) - return -1; - - if (backendAlias) - backendStr = g_strdup_printf("drive%c=%s", driveLetter, backendAlias); - } - - if (!explicitfdc) { - if (backendStr) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.%s", backendStr); - } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && + qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0) + return -1; - if (bootindexStr) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.%s", bootindexStr); - } + if (disk->info.addr.drive.unit) { + bootindexB = disk->info.effectiveBootIndex; + backendB = g_steal_pointer(&backendAlias); } else { - virBufferStrcat(&fdc_opts, backendStr, ",", NULL); - virBufferStrcat(&fdc_opts, bootindexStr, ",", NULL); + bootindexA = disk->info.effectiveBootIndex; + backendA = g_steal_pointer(&backendAlias); } } - if (explicitfdc && hasfloppy) { - /* Newer Q35 machine types require an explicit FDC controller */ - virBufferTrim(&fdc_opts, ","); - virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &fdc_opts); + if (!hasfloppy) + return 0; + + if (qemuDomainNeedsFDC(def)) { + qemuBuildFloppyCommandLineControllerOptionsExplicit(cmd, + bootindexA, + bootindexB, + backendA, + backendB); + } else { + qemuBuildFloppyCommandLineControllerOptionsImplicit(cmd, + bootindexA, + bootindexB, + backendA, + backendB); } return 0; -- 2.31.1
On a Tuesday in 2021, Peter Krempa wrote:
qemuBuildFloppyCommandLineControllerOptions was generating config for both the implicit and explicit fdc. The explicit FDC is using '-device' and thus will need to be converted to JSON.
Split up the lookup of the floppy drive configs from the actuall command
*actual Jano
generation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 126 +++++++++++++++++++++++++++------------- 1 file changed, 86 insertions(+), 40 deletions(-)
QEMU declares the bootindex types as: bootindexA=<int32> bootindexB=<int32> The driveA/driveB parameters were deprecated and removed in qemu-6.0. We'll keep them for compatibility, but they are not used with -blockdev. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 45 ++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 665cd739ec..3b2f88bcb9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2183,32 +2183,29 @@ qemuBuildFloppyCommandLineControllerOptionsImplicit(virCommand *cmd, } -static void +static int qemuBuildFloppyCommandLineControllerOptionsExplicit(virCommand *cmd, unsigned int bootindexA, unsigned int bootindexB, const char *backendA, - const char *backendB) + const char *backendB, + virQEMUCaps *qemuCaps) { - g_auto(virBuffer) fdc_opts = VIR_BUFFER_INITIALIZER; - - virBufferAddLit(&fdc_opts, "isa-fdc,"); - - if (backendA) - virBufferAsprintf(&fdc_opts, "driveA=%s,", backendA); - - if (bootindexA > 0) - virBufferAsprintf(&fdc_opts, "bootindexA=%u,", bootindexA); + g_autoptr(virJSONValue) props = NULL; - if (backendB) - virBufferAsprintf(&fdc_opts, "driveB=%s,", backendB); + if (virJSONValueObjectCreate(&props, + "s:driver", "isa-fdc", + "S:driveA", backendA, + "p:bootindexA", bootindexA, + "S:driveB", backendB, + "p:bootindexB", bootindexB, + NULL) < 0) + return -1; - if (bootindexB > 0) - virBufferAsprintf(&fdc_opts, "bootindexB=%u,", bootindexB); + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + return -1; - virBufferTrim(&fdc_opts, ","); - virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &fdc_opts); + return 0; } @@ -2251,11 +2248,13 @@ qemuBuildFloppyCommandLineControllerOptions(virCommand *cmd, return 0; if (qemuDomainNeedsFDC(def)) { - qemuBuildFloppyCommandLineControllerOptionsExplicit(cmd, - bootindexA, - bootindexB, - backendA, - backendB); + if (qemuBuildFloppyCommandLineControllerOptionsExplicit(cmd, + bootindexA, + bootindexB, + backendA, + backendB, + qemuCaps) < 0) + return -1; } else { qemuBuildFloppyCommandLineControllerOptionsImplicit(cmd, bootindexA, -- 2.31.1
On a Tuesday in 2021, Peter Krempa wrote:
This series finishes the refactor to generate '-device' via JSON. This will be needed later on when qemu will consider only the JSON format to be long term stable.
Next part will deal with the testing and finishing touches.
Peter Krempa (14): qemuBuildStorageSourceAttachPrepareDrive: Fix function comment qemuBuildDeviceCommandlineFromJSON: Remove unused keyword qemuValidateDomainSmartcardDef: Unbreak error messages qemuValidateDomainSmartcardDef: Move chardev validation under VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH virDomainSmartcardDef: Declare 'type' as virDomainSmartcardType qemuBuildSmartcardCommandLine: Replace qemuBuildSmartcardFindCCIDController qemuValidateDomainSmartcardDef: Move validation of smartcard count qemuBuildVMGenIDCommandLine: Generate via JSON qemuBuildTPMProxyCommandLine: Generate via JSON qemuBuildVMCoreInfoCommandLine: Generate via JSON qemuBuildIOMMUCommandLine: Generate via JSON qemuBuildSmartcardCommandLine: Generate via JSON qemuBuildFloppyCommandLineControllerOptions: Extract formatting of implicit/explicit fdc qemuBuildFloppyCommandLineControllerOptionsExplicit: Generate via JSON
src/conf/domain_conf.c | 23 +- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 366 +++++++++++++++++--------------- src/qemu/qemu_validate.c | 35 +-- src/security/security_selinux.c | 10 +- 5 files changed, 232 insertions(+), 204 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa