[libvirt] [PATCHv5 05/13] Add compatibility attribute to memballoon
Peter Krempa
pkrempa at redhat.com
Wed Sep 7 13:26:19 UTC 2016
On Wed, Aug 24, 2016 at 00:20:47 +0200, Ján Tomko wrote:
> A new attribute to alter the virtio revision:
> <memballoon model='virtio'>
> <driver compatibility='transitional'/>
> </memballoon>
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1227354
> ---
> docs/formatdomain.html.in | 8 ++++
> docs/schemas/domaincommon.rng | 15 ++++++
> src/conf/domain_conf.c | 40 ++++++++++++++++
> src/conf/domain_conf.h | 10 ++++
> .../qemuxml2argv-virtio-revision.xml | 53 ++++++++++++++++++++++
> .../qemuxml2xmlout-virtio-revision.xml | 53 ++++++++++++++++++++++
> tests/qemuxml2xmltest.c | 2 +
> 7 files changed, 181 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index bfbb0f2..56dddbd 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6373,6 +6373,7 @@ qemu-kvm -net nic,model=? /dev/null
> <memballoon model='virtio'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> <stats period='10'/>
> + <driver compatibility='transitional'/>
Hmm, this is a though one. Since virtio is not entirely meant to be qemu
specific but rather a standard the setting here makes sense.
> </memballoon>
> </devices>
> </domain></pre>
> @@ -6417,6 +6418,13 @@ qemu-kvm -net nic,model=? /dev/null
> <span class='since'>Since 1.1.1, requires QEMU 1.5</span>
> </p>
> </dd>
> + <dt><code>driver</code></dt>
> + <dd>
> + The <code>compatibility</code> attribute can be used to specify the
> + compatibility of virtio devices. Allowed values are <code>legacy</code>,
> + <code>transitional</code> and <code>modern</code>.
> + <span class="since">Since 2.2.0</span>.
I'd like to see a more specific description of the individual values.
Since virtio is meant to be a standard, we should describe the vales a
bit more IMO.
> + </dd>
> </dl>
> <h4><a name="elementsRng">Random number generator device</a></h4>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 052f28c..8ed4b9d 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3612,6 +3612,11 @@
> </attribute>
> </element>
> </optional>
> + <optional>
> + <element name="driver">
> + <ref name="compatibility"/>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> @@ -4830,6 +4835,16 @@
> </element>
> </define>
>
> + <define name="compatibility">
I'd add a <optional> block here since the attribute should not be
mandatory.
> + <attribute name="compatibility">
> + <choice>
> + <value>legacy</value>
> + <value>transitional</value>
> + <value>modern</value>
> + </choice>
> + </attribute>
> + </define>
> +
> <define name="usbmaster">
> <element name="master">
> <attribute name="startport">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1e694fd..53c3453 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -834,6 +834,13 @@ VIR_ENUM_IMPL(virDomainLoader,
> "rom",
> "pflash")
>
> +VIR_ENUM_IMPL(virDomainDriverCompatibility,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_LAST,
> + "default",
> + "legacy",
> + "transitional",
> + "modern")
> +
> /* Internal mapping: subset of block job types that can be present in
> * <mirror> XML (remaining types are not two-phase). */
> VIR_ENUM_DECL(virDomainBlockJob)
> @@ -1059,6 +1066,31 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt)
> return &xmlopt->ns;
> }
>
> +static int
> +virDomainDriverCompatibilityParseXML(xmlXPathContextPtr ctxt,
> + virDomainDriverCompatibility *res)
> +{
> + char *str = NULL;
> + int ret = -1;
> + int val;
> +
> + if (!(str = virXPathString("string(./driver/@compatibility)", ctxt)))
> + return 0;
> +
> + if ((val = virDomainDriverCompatibilityTypeFromString(str)) < 0) {
This still allows to use the "default" setting which is not documented
though.
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("unable to parse the compatibility attribute"));
> + goto cleanup;
> + }
> +
> + *res = val;
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(str);
> + return ret;
> +}
> +
>
> void
> virBlkioDeviceArrayClear(virBlkioDevicePtr devices,
> @@ -12065,6 +12097,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
> else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
> goto error;
>
> + if (virDomainDriverCompatibilityParseXML(ctxt, &def->compatibility) < 0)
> + goto error;
> +
> cleanup:
> VIR_FREE(model);
> VIR_FREE(deflate);
> @@ -21506,6 +21541,11 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
> return -1;
> }
>
> + if (def->compatibility) {
"default" is not formatted though.
> + virBufferAsprintf(&childrenBuf, "<driver compatibility='%s'/>\n",
> + virDomainDriverCompatibilityTypeToString(def->compatibility));
> + }
> +
> if (!virBufferUse(&childrenBuf)) {
> virBufferAddLit(buf, "/>\n");
> } else {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 8b26724..ac9f552 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -154,6 +154,14 @@ typedef virDomainTPMDef *virDomainTPMDefPtr;
> typedef struct _virDomainIOMMUDef virDomainIOMMUDef;
> typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
>
> +typedef enum {
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_DEFAULT,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_LEGACY,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_TRANSITIONAL,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_MODERN,
> + VIR_DOMAIN_DRIVER_COMPATIBILITY_LAST,
> +} virDomainDriverCompatibility;
> +
> /* Flags for the 'type' field in virDomainDeviceDef */
> typedef enum {
> VIR_DOMAIN_DEVICE_NONE = 0,
> @@ -1546,6 +1554,7 @@ struct _virDomainMemballoonDef {
> virDomainDeviceInfo info;
> int period; /* seconds between collections */
> int autodeflate; /* enum virTristateSwitch */
> + virDomainDriverCompatibility compatibility;
> };
>
> struct _virDomainNVRAMDef {
> @@ -3022,6 +3031,7 @@ VIR_ENUM_DECL(virDomainTPMBackend)
> VIR_ENUM_DECL(virDomainMemoryModel)
> VIR_ENUM_DECL(virDomainMemoryBackingModel)
> VIR_ENUM_DECL(virDomainIOMMUModel)
> +VIR_ENUM_DECL(virDomainDriverCompatibility)
> /* from libvirt.h */
> VIR_ENUM_DECL(virDomainState)
> VIR_ENUM_DECL(virDomainNostateReason)
ACK if you clarify the docs from me. I think this is the least worst
approach we can use and still hide the very weird tuple of qemu
arguments used to configure this.
Peter
More information about the libvir-list
mailing list