[PATCH 0/7] qemu: Add qemu-namespace XML to override device properties (-set replacement)
The use of '-set' via the qemu argument passthrough broke when we switched to using JSON with -device. Provide a replacement using the qemu namespace XML elements. Peter Krempa (7): qemuDomainDefNamespaceFormatXML*: Convert to virXMLFormatElement docs: drvqemu: Document overriding of device properties qemu: domain: Add XML namespace code for overriding device config conf: Introduce VIR_DOMAIN_TAINT_CUSTOM_DEVICE and use it in qemu qemuBuildDeviceCommandlineFromJSON: Pass 'virDomainDef' into the function qemu: command: Override device definition according to the namespace config NEWS: Mention the qemu device property override feature NEWS.rst | 7 + docs/drvqemu.rst | 52 ++++ docs/schemas/domaincommon.rng | 32 +++ src/conf/domain_conf.c | 2 + src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 133 ++++++---- src/qemu/qemu_domain.c | 241 ++++++++++++++++-- src/qemu/qemu_domain.h | 33 +++ .../qemu-ns.x86_64-4.0.0.args | 2 +- .../qemu-ns.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/qemu-ns.xml | 12 + .../qemu-ns.x86_64-latest.xml | 12 + 12 files changed, 457 insertions(+), 72 deletions(-) -- 2.35.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7180ae616b..3bf864bc5d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3353,25 +3353,19 @@ static void qemuDomainDefNamespaceFormatXMLCommandline(virBuffer *buf, qemuDomainXmlNsDef *cmd) { + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); GStrv n; size_t i; - if (!cmd->args && !cmd->num_env) - return; - - virBufferAddLit(buf, "<qemu:commandline>\n"); - virBufferAdjustIndent(buf, 2); - for (n = cmd->args; n && *n; n++) - virBufferEscapeString(buf, "<qemu:arg value='%s'/>\n", *n); + virBufferEscapeString(&childBuf, "<qemu:arg value='%s'/>\n", *n); for (i = 0; i < cmd->num_env; i++) { - virBufferAsprintf(buf, "<qemu:env name='%s'", cmd->env[i].name); - virBufferEscapeString(buf, " value='%s'", cmd->env[i].value); - virBufferAddLit(buf, "/>\n"); + virBufferAsprintf(&childBuf, "<qemu:env name='%s'", cmd->env[i].name); + virBufferEscapeString(&childBuf, " value='%s'", cmd->env[i].value); + virBufferAddLit(&childBuf, "/>\n"); } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</qemu:commandline>\n"); + virXMLFormatElement(buf, "qemu:commandline", NULL, &childBuf); } @@ -3379,22 +3373,16 @@ static void qemuDomainDefNamespaceFormatXMLCaps(virBuffer *buf, qemuDomainXmlNsDef *xmlns) { + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); GStrv n; - if (!xmlns->capsadd && !xmlns->capsdel) - return; - - virBufferAddLit(buf, "<qemu:capabilities>\n"); - virBufferAdjustIndent(buf, 2); - for (n = xmlns->capsadd; n && *n; n++) - virBufferEscapeString(buf, "<qemu:add capability='%s'/>\n", *n); + virBufferEscapeString(&childBuf, "<qemu:add capability='%s'/>\n", *n); for (n = xmlns->capsdel; n && *n; n++) - virBufferEscapeString(buf, "<qemu:del capability='%s'/>\n", *n); + virBufferEscapeString(&childBuf, "<qemu:del capability='%s'/>\n", *n); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</qemu:capabilities>\n"); + virXMLFormatElement(buf, "qemu:capabilities", NULL, &childBuf); } -- 2.35.1
On a Monday in 2022, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Upcoming patches will add possibility to override configuration of a device with custom properties as a more versatile replacement to using QEMU's '-set' parameter, which doesn't work when we use JSON to instantiate devices. Describe the XML used for the override as well as expectations of upstream support in case something breaks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/drvqemu.rst | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/docs/drvqemu.rst b/docs/drvqemu.rst index 9d7dd2656b..b13415936c 100644 --- a/docs/drvqemu.rst +++ b/docs/drvqemu.rst @@ -580,6 +580,58 @@ reconfigure libvirtd. *DO NOT* use in production. +Overriding properties of QEMU devices +------------------------------------- + +For development or testing the ``<qemu:deviceOverride>`` tag allows to override +specific properties of devices instantiated by libvirt in QEMU via the +``-device`` argument. + +The ``<qemu:device>`` sub-element groups overrides for a device identified via +the ``alias`` attribute. The alias corresponds to the ``<alias name=''>`` +property of a device. It's strongly recommended to use user-specified aliases +for devices with overriden properties. + +The individual properties are overriden by a ``<qemu:property>`` element. The +``name`` specifies the name of the property to override. In case when libvirt +doesn't configure the property a property with the name is added to the +commandline. The ``type`` attribute specifies a type of the argument used. The +type must correspond with the type that is expected by QEMU. Supported values +for the type attribute are: ``string``, ``number``, ``bool`` (allowed values for +``bool`` are ``true`` and ``false``) and ``remove``. The ``remove`` type is +special and instructs libvirt to remove the property without replacement. + +The overrides are applied only to initial device configuration passed to QEMU +via the commandline. Later hotplug operations will not apply any modifications. + +Configuring override for a device alias which is not used or attempting to +remove a device property which is not formatted by libvirt will cause failure +to startup the VM. + +*Note:* The libvirt project doesn't guarantee any form of compatibility and +stability of devices with overriden properties. The domain is tainted when +such configuration is used. + +Example: + +:: + + <domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> + <name>testvm</name> + + [...] + + <qemu:deviceOverride> + <qemu:device alias='ua-devalias'> + <qemu:property name='propname1' type='string' value='test'/> + <qemu:property name='propname2' type='unsigned' value='123'/> + <qemu:property name='propname2' type='signed' value='-123'/> + <qemu:property name='propname3' type='bool' value='false'/> + <qemu:property name='propname4' type='remove'/> + </qemu:device> + </qemu:deviceOverride> + </domain> + Example domain XML config ------------------------- -- 2.35.1
Implement the XML parser and formatter for overriding of device properties such as: <qemu:deviceOverride> <qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> <qemu:property name='prop3' type='unsigned' value='123'/> <qemu:property name='prop4' type='bool' value='true'/> <qemu:property name='prop5' type='bool' value='false'/> <qemu:property name='prop6' type='bool' value='false'/> <qemu:property name='prop6' type='remove'/> </qemu:device> </qemu:deviceOverride> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/schemas/domaincommon.rng | 32 +++ src/qemu/qemu_domain.c | 206 ++++++++++++++++++ src/qemu/qemu_domain.h | 33 +++ .../qemu-ns.x86_64-4.0.0.args | 2 +- .../qemu-ns.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/qemu-ns.xml | 12 + .../qemu-ns.x86_64-latest.xml | 12 + 7 files changed, 297 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9c1b64a644..804ade3f37 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -80,6 +80,9 @@ <optional> <ref name="qemudeprecation"/> </optional> + <optional> + <ref name="qemudeviceoverride"/> + </optional> <optional> <ref name="lxcsharens"/> </optional> @@ -7553,6 +7556,35 @@ </element> </define> + <define name="qemudeviceoverride"> + <element name="deviceOverride" ns="http://libvirt.org/schemas/domain/qemu/1.0"> + <interleave> + <zeroOrMore> + <element name="device"> + <attribute name="alias"/> + <zeroOrMore> + <element name="property"> + <attribute name="name"/> + <attribute name="type"> + <choice> + <value>string</value> + <value>signed</value> + <value>unsigned</value> + <value>bool</value> + <value>remove</value> + </choice> + </attribute> + <optional> + <attribute name="value"/> + </optional> + </element> + </zeroOrMore> + </element> + </zeroOrMore> + </interleave> + </element> + </define> + <!-- Optional hypervisor extensions in their own namespace: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3bf864bc5d..595440173c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3181,6 +3181,23 @@ qemuDomainXmlNsDefFree(qemuDomainXmlNsDef *def) g_free(def->deprecationBehavior); + for (i = 0; i < def->ndeviceOverride; i++) { + size_t j; + g_free(def->deviceOverride[i].alias); + + for (j = 0; j < def->deviceOverride[i].nprops; j++) { + qemuDomainXmlNsDeviceOverrideProperty *prop = def->deviceOverride[i].props + j; + + g_free(prop->name); + g_free(prop->value); + virJSONValueFree(prop->json); + } + + g_free(def->deviceOverride[i].props); + } + + g_free(def->deviceOverride); + g_free(def); } @@ -3319,6 +3336,159 @@ qemuDomainDefNamespaceParseCaps(qemuDomainXmlNsDef *nsdef, return 0; } +VIR_ENUM_IMPL(qemuDomainXmlNsDeviceOverride, + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_LAST, + "", + "string", + "signed", + "unsigned", + "bool", + "remove", + ); + + +static int +qemuDomainDefNamespaceParseDeviceOverrideProperties(qemuDomainXmlNsDeviceOverride *device, + xmlNodePtr *propnodes, + size_t npropnodes) +{ + size_t i; + + device->props = g_new0(qemuDomainXmlNsDeviceOverrideProperty, npropnodes); + device->nprops = npropnodes; + + for (i = 0; i < npropnodes; i++) { + qemuDomainXmlNsDeviceOverrideProperty *prop = device->props + i; + unsigned long long ull; + long long ll; + bool b; + + if (!(prop->name = virXMLPropString(propnodes[i], "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'name' attribute for qemu:property")); + return -1; + } + + if (STREQ(prop->name, "id")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("property with name 'id' can't be overriden")); + return -1; + } + + if (virXMLPropEnum(propnodes[i], "type", + qemuDomainXmlNsDeviceOverrideTypeFromString, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &prop->type) < 0) + return -1; + + if (!(prop->value = virXMLPropString(propnodes[i], "value"))) { + if (prop->type != QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_REMOVE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'value' attribute for 'qemu:property'")); + return -1; + } + } + + switch (prop->type) { + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_STRING: + prop->json = virJSONValueNewString(g_strdup(prop->value)); + break; + + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_SIGNED: + if (virStrToLong_ll(prop->value, NULL, 10, &ll) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid value '%s' of 'value' attribute of 'qemu:property'"), + prop->value); + return -1; + } + prop->json = virJSONValueNewNumberLong(ll); + break; + + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_UNSIGNED: + if (virStrToLong_ullp(prop->value, NULL, 10, &ull) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid value '%s' of 'value' attribute of 'qemu:property'"), + prop->value); + return -1; + } + prop->json = virJSONValueNewNumberUlong(ull); + break; + + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_BOOL: + if (STRNEQ(prop->value, "true") && STRNEQ(prop->value, "false")) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid value '%s' of 'value' attribute of 'qemu:property'"), + prop->value); + return -1; + } + + b = STREQ(prop->value, "true"); + prop->json = virJSONValueNewBoolean(b); + break; + + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_REMOVE: + if (prop->value) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("setting 'value' attribute of 'qemu:property' doesn't make sense with 'remove' type")); + return -1; + } + break; + + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_NONE: + case QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_LAST: + virReportEnumRangeError(qemuDomainXmlNsDeviceOverrideType, prop->type); + return -1; + } + } + + return 0; +} + + +static int +qemuDomainDefNamespaceParseDeviceOverride(qemuDomainXmlNsDef *nsdef, + xmlXPathContextPtr ctxt) +{ + g_autofree xmlNodePtr *devicenodes = NULL; + ssize_t ndevicenodes; + size_t i; + + if ((ndevicenodes = virXPathNodeSet("./qemu:deviceOverride/qemu:device", ctxt, &devicenodes)) < 0) + return -1; + + if (ndevicenodes == 0) + return 0; + + nsdef->deviceOverride = g_new0(qemuDomainXmlNsDeviceOverride, ndevicenodes); + nsdef->ndeviceOverride = ndevicenodes; + + for (i = 0; i < ndevicenodes; i++) { + qemuDomainXmlNsDeviceOverride *n = nsdef->deviceOverride + i; + g_autofree xmlNodePtr *propnodes = NULL; + ssize_t npropnodes; + VIR_XPATH_NODE_AUTORESTORE(ctxt); + + ctxt->node = devicenodes[i]; + + if (!(n->alias = virXMLPropString(devicenodes[i], "alias"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'alias' attribute for qemu:device")); + return -1; + } + + if ((npropnodes = virXPathNodeSet("./qemu:property", ctxt, &propnodes)) < 0) + return -1; + + if (npropnodes == 0) + continue; + + if (qemuDomainDefNamespaceParseDeviceOverrideProperties(n, propnodes, npropnodes) < 0) + return -1; + } + + return 0; +} + static int qemuDomainDefNamespaceParse(xmlXPathContextPtr ctxt, @@ -3331,6 +3501,7 @@ qemuDomainDefNamespaceParse(xmlXPathContextPtr ctxt, if (qemuDomainDefNamespaceParseCommandlineArgs(nsdata, ctxt) < 0 || qemuDomainDefNamespaceParseCommandlineEnv(nsdata, ctxt) < 0 || + qemuDomainDefNamespaceParseDeviceOverride(nsdata, ctxt) < 0 || qemuDomainDefNamespaceParseCaps(nsdata, ctxt) < 0) goto cleanup; @@ -3386,6 +3557,40 @@ qemuDomainDefNamespaceFormatXMLCaps(virBuffer *buf, } +static void +qemuDomainDefNamespaceFormatXMLDeviceOverride(virBuffer *buf, + qemuDomainXmlNsDef *xmlns) +{ + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + size_t i; + + for (i = 0; i < xmlns->ndeviceOverride; i++) { + qemuDomainXmlNsDeviceOverride *device = xmlns->deviceOverride + i; + g_auto(virBuffer) deviceAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) deviceChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); + size_t j; + + virBufferEscapeString(&deviceAttrBuf, " alias='%s'", device->alias); + + for (j = 0; j < device->nprops; j++) { + g_auto(virBuffer) propAttrBuf = VIR_BUFFER_INITIALIZER; + qemuDomainXmlNsDeviceOverrideProperty *prop = device->props + j; + + virBufferEscapeString(&propAttrBuf, " name='%s'", prop->name); + virBufferAsprintf(&propAttrBuf, " type='%s'", + qemuDomainXmlNsDeviceOverrideTypeToString(prop->type)); + virBufferEscapeString(&propAttrBuf, " value='%s'", prop->value); + + virXMLFormatElement(&deviceChildBuf, "qemu:property", &propAttrBuf, NULL); + } + + virXMLFormatElement(&childBuf, "qemu:device", &deviceAttrBuf, &deviceChildBuf); + } + + virXMLFormatElement(buf, "qemu:deviceOverride", NULL, &childBuf); +} + + static int qemuDomainDefNamespaceFormatXML(virBuffer *buf, void *nsdata) @@ -3394,6 +3599,7 @@ qemuDomainDefNamespaceFormatXML(virBuffer *buf, qemuDomainDefNamespaceFormatXMLCommandline(buf, cmd); qemuDomainDefNamespaceFormatXMLCaps(buf, cmd); + qemuDomainDefNamespaceFormatXMLDeviceOverride(buf, cmd); virBufferEscapeString(buf, "<qemu:deprecation behavior='%s'/>\n", cmd->deprecationBehavior); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index edafb585b3..fb7375026e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -443,6 +443,36 @@ struct _qemuDomainXmlNsEnvTuple { char *value; }; + +typedef enum { + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_NONE, + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_STRING, + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_SIGNED, + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_UNSIGNED, + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_BOOL, + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_REMOVE, + + QEMU_DOMAIN_XML_NS_DEVICE_OVERRIDE_LAST +} qemuDomainXmlNsDeviceOverrideType; +VIR_ENUM_DECL(qemuDomainXmlNsDeviceOverride); + +typedef struct _qemuDomainXmlNsDeviceOverrideProperty qemuDomainXmlNsDeviceOverrideProperty; +struct _qemuDomainXmlNsDeviceOverrideProperty { + char *name; + qemuDomainXmlNsDeviceOverrideType type; + char *value; + virJSONValue *json; +}; + +typedef struct _qemuDomainXmlNsDeviceOverride qemuDomainXmlNsDeviceOverride; +struct _qemuDomainXmlNsDeviceOverride { + char *alias; + + size_t nprops; + qemuDomainXmlNsDeviceOverrideProperty *props; +}; + + typedef struct _qemuDomainXmlNsDef qemuDomainXmlNsDef; struct _qemuDomainXmlNsDef { char **args; @@ -458,6 +488,9 @@ struct _qemuDomainXmlNsDef { * starting the VM to avoid any form of errors in the parser or when * changing qemu versions. The knob is mainly for development/CI purposes */ char *deprecationBehavior; + + size_t ndeviceOverride; + qemuDomainXmlNsDeviceOverride *deviceOverride; }; diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args index 5105f410ce..236f984a90 100644 --- a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args @@ -31,7 +31,7 @@ BAR='' \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ --device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ua-disk,bootindex=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ -unknown parameter \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args index 05ccade7e4..c0bf45000f 100644 --- a/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args @@ -33,7 +33,7 @@ BAR='' \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ --device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ua-disk","bootindex":1}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ -unknown parameter \ diff --git a/tests/qemuxml2argvdata/qemu-ns.xml b/tests/qemuxml2argvdata/qemu-ns.xml index 9d2f5502e9..69c93b1f9b 100644 --- a/tests/qemuxml2argvdata/qemu-ns.xml +++ b/tests/qemuxml2argvdata/qemu-ns.xml @@ -17,6 +17,7 @@ <disk type='block' device='disk'> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> + <alias name='ua-disk'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> @@ -32,5 +33,16 @@ <qemu:add capability="blockdev"/> <qemu:del capability="name"/> </qemu:capabilities> + <qemu:deviceOverride> + <qemu:device alias='ua-disk'> + <qemu:property name='prop1' type='string' value='propval1'/> + <qemu:property name='prop2' type='signed' value='-321'/> + <qemu:property name='prop3' type='unsigned' value='123'/> + <qemu:property name='prop4' type='bool' value='true'/> + <qemu:property name='prop5' type='bool' value='false'/> + <qemu:property name='prop6' type='bool' value='false'/> + <qemu:property name='prop6' type='remove'/> + </qemu:device> + </qemu:deviceOverride> <qemu:deprecation behavior='crash'/> </domain> diff --git a/tests/qemuxml2xmloutdata/qemu-ns.x86_64-latest.xml b/tests/qemuxml2xmloutdata/qemu-ns.x86_64-latest.xml index 674525d033..7638d7199c 100644 --- a/tests/qemuxml2xmloutdata/qemu-ns.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/qemu-ns.x86_64-latest.xml @@ -21,6 +21,7 @@ <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> + <alias name='ua-disk'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'> @@ -48,5 +49,16 @@ <qemu:add capability='blockdev'/> <qemu:del capability='name'/> </qemu:capabilities> + <qemu:deviceOverride> + <qemu:device alias='ua-disk'> + <qemu:property name='prop1' type='string' value='propval1'/> + <qemu:property name='prop2' type='signed' value='-321'/> + <qemu:property name='prop3' type='unsigned' value='123'/> + <qemu:property name='prop4' type='bool' value='true'/> + <qemu:property name='prop5' type='bool' value='false'/> + <qemu:property name='prop6' type='bool' value='false'/> + <qemu:property name='prop6' type='remove'/> + </qemu:device> + </qemu:deviceOverride> <qemu:deprecation behavior='crash'/> </domain> -- 2.35.1
On Mon, Mar 21, 2022 at 04:24:36PM +0100, Peter Krempa wrote:
Implement the XML parser and formatter for overriding of device properties such as:
<qemu:deviceOverride>
s/deviceOverride/devices/
<qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> <qemu:property name='prop3' type='unsigned' value='123'/> <qemu:property name='prop4' type='bool' value='true'/> <qemu:property name='prop5' type='bool' value='false'/> <qemu:property name='prop6' type='bool' value='false'/> <qemu:property name='prop6' type='remove'/> </qemu:device> </qemu:deviceOverride>
This all applies to the frontend. Could we make this work with the backend too ? Obviously there are many different types of backend, so it is less convenient to implement that, but would at least be nice to have a thought about how we could represent it in the XML config. Some ideas: 1. Type specific backend at the top level <qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:device> <qemu:blockdev alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:blockdev> 2. Type specific backend at the inner level <qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> <qemu:blockdev alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:blockdev> </qemu:device> 3. Inner generic backends/frontends: <qemu:device alias='ua-disk'> <qemu:frontend> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:frontend> <qemu:backend> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:backend> </qemu:device> 4. Inner generic backends: <qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> <qemu:backend> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:backend> </qemu:device> I'd probably lean slightly towards options (3)/(4), which happens to be possible to retrofit with this series as. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Mar 21, 2022 at 15:34:46 +0000, Daniel P. Berrangé wrote:
On Mon, Mar 21, 2022 at 04:24:36PM +0100, Peter Krempa wrote:
Implement the XML parser and formatter for overriding of device properties such as:
<qemu:deviceOverride>
s/deviceOverride/devices/
<qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> <qemu:property name='prop3' type='unsigned' value='123'/> <qemu:property name='prop4' type='bool' value='true'/> <qemu:property name='prop5' type='bool' value='false'/> <qemu:property name='prop6' type='bool' value='false'/> <qemu:property name='prop6' type='remove'/> </qemu:device> </qemu:deviceOverride>
This all applies to the frontend. Could we make this work with the backend too ? Obviously there are many different types of backend, so it is less convenient to implement that, but would at least be nice to have a thought about how we could represent it in the XML config.
Yes it can be easily ammended to work with certain backend types ... namely netdev, object, and maybe at some point even chardev, as they are/will be using JSON too. For others such as -fsdev we still have an ad-hoc formatter so that won't work. I'm not sure how easy it will be to wire up for -blockdev though, while that uses JSON, it has nested types and also nested backends, so the identification of which one to modify might be non-trivial. Nested types can be theoretically handled by nesting the equivalent of the 'property' tag such as: <qemu:property name='prop1' type='child'> <qemu:property name='overrideme' type='string' value='blurb'/> </qemu:property>
Some ideas:
1. Type specific backend at the top level
<qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:device> <qemu:blockdev alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:blockdev>
2. Type specific backend at the inner level
<qemu:device alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/>
<qemu:blockdev alias='ua-disk'> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:blockdev> </qemu:device>
3. Inner generic backends/frontends:
<qemu:device alias='ua-disk'> <qemu:frontend> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:frontend> <qemu:backend> <qemu:property name='prop1' type='string' value='propval1'/> <qemu:property name='prop2' type='signed' value='-321'/> </qemu:backend> </qemu:device>
I think that for symmetry we should go with this. It will also be really easy to adapt the current code to to it. It's a bit verbose for the user though. One other thing to consider is that on the top level we will probably want to leave some room for things which are not under <devices> but users may want to override such as CPUs, thus I'd leave the top level for containers and not deal with backends at that level.
Taint the domain object when the user requests custom device properties. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 ++ src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 3 +++ 3 files changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e0dfc9e45f..902f101996 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -88,6 +88,7 @@ VIR_ENUM_IMPL(virDomainTaint, "custom-ga-command", "custom-hypervisor-feature", "deprecated-config", + "custom-device", ); VIR_ENUM_IMPL(virDomainTaintMessage, @@ -105,6 +106,7 @@ VIR_ENUM_IMPL(virDomainTaintMessage, N_("custom guest agent control commands issued"), N_("hypervisor feature autodetection override"), N_("use of deprecated configuration settings"), + N_("custom device configuration"), ); VIR_ENUM_IMPL(virDomainVirt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a4de46773c..e06188caad 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3009,6 +3009,7 @@ typedef enum { VIR_DOMAIN_TAINT_CUSTOM_GA_COMMAND, /* Custom guest agent command */ VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, /* custom hypervisor feature control */ VIR_DOMAIN_TAINT_DEPRECATED_CONFIG, /* Configuration that is marked deprecated */ + VIR_DOMAIN_TAINT_CUSTOM_DEVICE, /* hypervisor device config customized */ VIR_DOMAIN_TAINT_LAST } virDomainTaintFlags; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 595440173c..a4eef97c95 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6700,6 +6700,9 @@ void qemuDomainObjCheckTaint(virQEMUDriver *driver, qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CUSTOM_ARGV, logCtxt); if (qemuxmlns->capsadd || qemuxmlns->capsdel) custom_hypervisor_feat = true; + + if (qemuxmlns->ndeviceOverride > 0) + qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CUSTOM_DEVICE, logCtxt); } if (custom_hypervisor_feat || -- 2.35.1
The definition object will be later used to access the qemu namespace definition used to override device properties. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 101 +++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c836799888..8af6179b5c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -235,6 +235,7 @@ qemuBuildNetdevCommandlineFromJSON(virCommand *cmd, static int qemuBuildDeviceCommandlineFromJSON(virCommand *cmd, virJSONValue *props, + const virDomainDef *def G_GNUC_UNUSED, virQEMUCaps *qemuCaps) { g_autofree char *arg = NULL; @@ -2327,6 +2328,7 @@ qemuBuildZPCIDevProps(virDomainDeviceInfo *dev) static int qemuCommandAddExtDevice(virCommand *cmd, virDomainDeviceInfo *dev, + const virDomainDef *def, virQEMUCaps *qemuCaps) { if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || @@ -2340,7 +2342,7 @@ qemuCommandAddExtDevice(virCommand *cmd, if (!(devprops = qemuBuildZPCIDevProps(dev))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; } @@ -2383,6 +2385,7 @@ qemuBuildFloppyCommandLineControllerOptionsExplicit(virCommand *cmd, unsigned int bootindexB, const char *backendA, const char *backendB, + const virDomainDef *def, virQEMUCaps *qemuCaps) { g_autoptr(virJSONValue) props = NULL; @@ -2396,7 +2399,7 @@ qemuBuildFloppyCommandLineControllerOptionsExplicit(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -2447,6 +2450,7 @@ qemuBuildFloppyCommandLineControllerOptions(virCommand *cmd, bootindexB, backendA, backendB, + def, qemuCaps) < 0) return -1; } else { @@ -2596,13 +2600,13 @@ qemuBuildDiskCommandLine(virCommand *cmd, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) return 0; - if (qemuCommandAddExtDevice(cmd, &disk->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &disk->info, def, qemuCaps) < 0) return -1; if (!(devprops = qemuBuildDiskDeviceProps(def, disk, qemuCaps))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; return 0; @@ -2690,13 +2694,13 @@ qemuBuildVHostUserFsCommandLine(virCommand *cmd, if (qemuBuildChardevCommand(cmd, chrsrc, chardev_alias, priv->qemuCaps) < 0) return -1; - if (qemuCommandAddExtDevice(cmd, &fs->info, priv->qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &fs->info, def, priv->qemuCaps) < 0) return -1; if (!(devprops = qemuBuildVHostUserFsDevProps(fs, def, chardev_alias, priv))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, priv->qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, priv->qemuCaps) < 0) return -1; return 0; @@ -2773,7 +2777,7 @@ qemuBuildFSDevCmd(virCommand *cmd, if (qemuBuildDeviceAddressProps(devprops, def, &fs->info) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; return 0; @@ -2793,7 +2797,7 @@ qemuBuildFSDevCommandLine(virCommand *cmd, return -1; virCommandAddArg(cmd, fsdevstr); - if (qemuCommandAddExtDevice(cmd, &fs->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &fs->info, def, qemuCaps) < 0) return -1; if (qemuBuildFSDevCmd(cmd, def, fs, qemuCaps) < 0) @@ -3421,10 +3425,10 @@ qemuBuildControllersByTypeCommandLine(virCommand *cmd, if (!props) continue; - if (qemuCommandAddExtDevice(cmd, &cont->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &cont->info, def, qemuCaps) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; } @@ -4382,13 +4386,13 @@ qemuBuildWatchdogCommandLine(virCommand *cmd, if (!def->watchdog) return 0; - if (qemuCommandAddExtDevice(cmd, &def->watchdog->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &def->watchdog->info, def, qemuCaps) < 0) return -1; if (!(props = qemuBuildWatchdogDevProps(def, watchdog))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps)) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps)) return -1; /* qemu doesn't have a 'dump' action; we tell qemu to 'pause', then @@ -4434,10 +4438,10 @@ qemuBuildMemballoonCommandLine(virCommand *cmd, if (qemuBuildDeviceAddressProps(props, def, &def->memballoon->info) < 0) return -1; - if (qemuCommandAddExtDevice(cmd, &def->memballoon->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &def->memballoon->info, def, qemuCaps) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -4583,7 +4587,7 @@ qemuBuildInputCommandLine(virCommand *cmd, for (i = 0; i < def->ninputs; i++) { virDomainInputDef *input = def->inputs[i]; - if (qemuCommandAddExtDevice(cmd, &input->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &input->info, def, qemuCaps) < 0) return -1; if (input->type == VIR_DOMAIN_INPUT_TYPE_EVDEV) { @@ -4616,7 +4620,7 @@ qemuBuildInputCommandLine(virCommand *cmd, } if (props && - qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; } } @@ -4687,7 +4691,7 @@ qemuBuildSoundDevCmd(virCommand *cmd, if (qemuBuildDeviceAddressProps(props, def, &sound->info) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -4720,7 +4724,7 @@ qemuBuildSoundCodecCmd(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -4743,7 +4747,7 @@ qemuBuildSoundCommandLine(virCommand *cmd, if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) { virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL); } else { - if (qemuCommandAddExtDevice(cmd, &sound->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &sound->info, def, qemuCaps) < 0) return -1; if (qemuBuildSoundDevCmd(cmd, def, sound, qemuCaps) < 0) @@ -4867,7 +4871,7 @@ qemuBuildDeviceVideoCmd(virCommand *cmd, if (qemuBuildDeviceAddressProps(props, def, &video->info) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -4905,7 +4909,7 @@ qemuBuildVideoCommandLine(virCommand *cmd, return -1; } - if (qemuCommandAddExtDevice(cmd, &def->videos[i]->info, priv->qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &def->videos[i]->info, def, priv->qemuCaps) < 0) return -1; if (qemuBuildDeviceVideoCmd(cmd, def, video, priv->qemuCaps) < 0) @@ -5035,7 +5039,7 @@ qemuBuildHubDevCmd(virCommand *cmd, if (qemuBuildDeviceAddressProps(props, def, &dev->info) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -5360,7 +5364,7 @@ qemuBuildHostdevSCSICommandLine(virCommand *cmd, if (!(devprops = qemuBuildSCSIHostdevDevProps(def, hostdev, backendAlias))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; return 0; @@ -5391,7 +5395,7 @@ qemuBuildHostdevCommandLine(virCommand *cmd, if (!(devprops = qemuBuildUSBHostdevDevProps(def, hostdev, qemuCaps))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; break; @@ -5401,13 +5405,13 @@ qemuBuildHostdevCommandLine(virCommand *cmd, if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED) continue; - if (qemuCommandAddExtDevice(cmd, hostdev->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, hostdev->info, def, qemuCaps) < 0) return -1; if (!(devprops = qemuBuildPCIHostdevDevProps(def, hostdev))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; break; @@ -5436,7 +5440,7 @@ qemuBuildHostdevCommandLine(virCommand *cmd, vhostfdName))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; } @@ -5459,7 +5463,7 @@ qemuBuildHostdevCommandLine(virCommand *cmd, if (!(devprops = qemuBuildHostdevMediatedDevProps(def, hostdev))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; break; @@ -5732,13 +5736,13 @@ qemuBuildRNGCommandLine(virCommand *cmd, return -1; /* add the device */ - if (qemuCommandAddExtDevice(cmd, &rng->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &rng->info, def, qemuCaps) < 0) return -1; if (!(devprops = qemuBuildRNGDevProps(def, rng, qemuCaps))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; } @@ -6078,7 +6082,7 @@ qemuBuildVMGenIDCommandLine(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -6404,7 +6408,7 @@ qemuBuildIOMMUCommandLine(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -7730,7 +7734,7 @@ qemuBuildMemoryDeviceCommandLine(virCommand *cmd, if (!(props = qemuBuildMemoryDeviceProps(cfg, priv, def, def->mems[i]))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, priv->qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, priv->qemuCaps) < 0) return -1; } @@ -8884,15 +8888,15 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, * New way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1 */ if (qemuDomainSupportsNicdev(def, net)) { - if (qemuCommandAddExtDevice(cmd, &net->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &net->info, def, qemuCaps) < 0) goto cleanup; if (!(nicprops = qemuBuildNicDevProps(def, net, net->driver.virtio.queues, qemuCaps))) goto cleanup; - if (qemuBuildDeviceCommandlineFromJSON(cmd, nicprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, nicprops, def, qemuCaps) < 0) goto cleanup; } else if (!requireNicdev) { - if (qemuCommandAddExtDevice(cmd, &net->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &net->info, def, qemuCaps) < 0) goto cleanup; if (!(nic = qemuBuildLegacyNicStr(net))) @@ -9055,7 +9059,7 @@ qemuBuildSmartcardCommandLine(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -9225,10 +9229,10 @@ qemuBuildShmemCommandLine(virCommand *cmd, if (!devProps) return -1; - if (qemuCommandAddExtDevice(cmd, &shmem->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &shmem->info, def, qemuCaps) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devProps, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devProps, def, qemuCaps) < 0) return -1; if (shmem->server.enabled) { @@ -9285,7 +9289,7 @@ qemuBuildChrDeviceCommandLine(virCommand *cmd, if (!(props = qemuBuildChrDeviceProps(def, chr, qemuCaps))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -9569,7 +9573,7 @@ qemuBuildRedirdevCommandLine(virCommand *cmd, if (!(devprops = qemuBuildRedirdevDevProps(def, redirdev))) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; } @@ -9671,7 +9675,7 @@ qemuBuildTPMDevCmd(virCommand *cmd, if (qemuBuildDeviceAddressProps(props, def, &tpm->info) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -9792,6 +9796,7 @@ qemuBuildTPMCommandLine(virCommand *cmd, static int qemuBuildTPMProxyCommandLine(virCommand *cmd, virDomainTPMDef *tpm, + const virDomainDef *def, virQEMUCaps *qemuCaps) { g_autoptr(virJSONValue) props = NULL; @@ -9803,7 +9808,7 @@ qemuBuildTPMProxyCommandLine(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -9819,7 +9824,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], priv->qemuCaps) < 0) + if (qemuBuildTPMProxyCommandLine(cmd, def->tpms[i], def, priv->qemuCaps) < 0) return -1; } else if (qemuBuildTPMCommandLine(cmd, def, def->tpms[i], priv) < 0) { return -1; @@ -9921,7 +9926,7 @@ qemuBuildVMCoreInfoCommandLine(virCommand *cmd, NULL) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; return 0; @@ -9954,7 +9959,7 @@ qemuBuildPanicCommandLine(virCommand *cmd, return -1; } - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) return -1; break; @@ -10283,10 +10288,10 @@ qemuBuildVsockCommandLine(virCommand *cmd, virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); priv->vhostfd = -1; - if (qemuCommandAddExtDevice(cmd, &vsock->info, qemuCaps) < 0) + if (qemuCommandAddExtDevice(cmd, &vsock->info, def, qemuCaps) < 0) return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, qemuCaps) < 0) + if (qemuBuildDeviceCommandlineFromJSON(cmd, devprops, def, qemuCaps) < 0) return -1; return 0; -- 2.35.1
Apply the user-requested changes to the device definition as requested by the <qemu:deviceOverride> element from the custom qemu XML namespace. Closes: https://gitlab.com/libvirt/libvirt/-/issues/287 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 34 ++++++++++++++++++- .../qemu-ns.x86_64-4.0.0.args | 2 +- .../qemu-ns.x86_64-latest.args | 2 +- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8af6179b5c..46a263cdae 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -232,14 +232,46 @@ qemuBuildNetdevCommandlineFromJSON(virCommand *cmd, } +static void +qemuBuildDeviceCommandlineHandleOverrides(virJSONValue *props, + qemuDomainXmlNsDef *nsdef) +{ + const char *alias = virJSONValueObjectGetString(props, "id"); + size_t i; + + for (i = 0; i < nsdef->ndeviceOverride; i++) { + qemuDomainXmlNsDeviceOverride *dev = nsdef->deviceOverride + i; + size_t j; + + if (STRNEQ(alias, dev->alias)) + continue; + + for (j = 0; j < dev->nprops; j++) { + qemuDomainXmlNsDeviceOverrideProperty *prop = dev->props + j; + + virJSONValueObjectRemoveKey(props, prop->name, NULL); + if (prop->json) { + g_autoptr(virJSONValue) copy = virJSONValueCopy(prop->json); + + virJSONValueObjectAppend(props, prop->name, ©); + } + } + } +} + + static int qemuBuildDeviceCommandlineFromJSON(virCommand *cmd, virJSONValue *props, - const virDomainDef *def G_GNUC_UNUSED, + const virDomainDef *def, virQEMUCaps *qemuCaps) { + qemuDomainXmlNsDef *nsdef = def->namespaceData; g_autofree char *arg = NULL; + if (nsdef && nsdef->ndeviceOverride > 0) + qemuBuildDeviceCommandlineHandleOverrides(props, nsdef); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_JSON)) { if (!(arg = virJSONValueToString(props, false))) return -1; diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args index 236f984a90..284f32d6a1 100644 --- a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args @@ -31,7 +31,7 @@ BAR='' \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ --device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ua-disk,bootindex=1 \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ua-disk,bootindex=1,prop1=propval1,prop2=-321,prop3=123,prop4=on,prop5=off \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ -unknown parameter \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args index c0bf45000f..c1949264f8 100644 --- a/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args @@ -33,7 +33,7 @@ BAR='' \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \ --device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ua-disk","bootindex":1}' \ +-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ua-disk","bootindex":1,"prop1":"propval1","prop2":-321,"prop3":123,"prop4":true,"prop5":false}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ -unknown parameter \ -- 2.35.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index b9b9807625..754687ac4d 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -25,6 +25,13 @@ v8.2.0 (unreleased) migration allowing migration to proceed even when the user expects certificate name not to match. + * qemu: Allow overrides of device properties via the qemu namespace + + Users wishing to override or modify properties of devices configured by + libvirt can use the ``<qemu:deviceOverride>`` QEMU namespace element to + specify the overrides instead of relying on the argv passthrough of the + ``-set`` qemu commandline option which no longer works with new qemu. + * **Bug fixes** * Both build and tests should now pass on Alpine Linux or any other -- 2.35.1
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa