[libvirt] [PATCHv2 00/11] qemu: allow disabling certain virtio revisions
https://bugzilla.redhat.com/show_bug.cgi?id=1227354 v1: https://www.redhat.com/archives/libvir-list/2016-July/msg01235.html v2: * probe for the qemu capability * add the attribute to virtio1-only devices such as virtio-gpu and virtio-input devices * allow multiple revisions to be specified Ján Tomko (11): Use separate buffer for <input> subelements Add virtio revision attribute to memballoon Add virtio revision attribute to disks Add virtio revision attribute to controllers Add virtio revision attribute to filesystems Add virtio revision attribute to interfaces Add virtio revision to rng devices Add virtio revision attribute to video Add virtio revision attribute to input devices Introduce QEMU_CAPS_DEVICE_VIRTIO_REVISION qemu: format options for enforcing virtio revisions docs/formatdomain.html.in | 69 ++++++++++ docs/schemas/domaincommon.rng | 37 ++++++ src/conf/domain_conf.c | 148 +++++++++++++++++++-- src/conf/domain_conf.h | 16 +++ src/qemu/qemu_capabilities.c | 6 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 56 ++++++++ tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0-gicv2.aarch64.xml | 1 + .../caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + .../qemuxml2argv-virtio-revision.args | 62 +++++++++ .../qemuxml2argv-virtio-revision.xml | 113 ++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++ .../qemuxml2xmlout-virtio-revision.xml | 113 ++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 19 files changed, 630 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml -- 2.7.3
Instead of figuring out upfront whether <input> will be a single or a pair element, format the subelements into a separate buffer and close <input/> early if this buffer is empty. --- src/conf/domain_conf.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 82876f3..5dbeb1a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21844,6 +21844,7 @@ virDomainInputDefFormat(virBufferPtr buf, { const char *type = virDomainInputTypeToString(def->type); const char *bus = virDomainInputBusTypeToString(def->bus); + virBuffer childbuf = VIR_BUFFER_INITIALIZER; /* don't format keyboard into migratable XML for backward compatibility */ if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && @@ -21866,17 +21867,17 @@ virDomainInputDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<input type='%s' bus='%s'", type, bus); - if (virDomainDeviceInfoNeedsFormat(&def->info, flags) || - def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH) { + virBufferAdjustIndent(&childbuf, virBufferGetIndent(buf, false) + 2); + virBufferEscapeString(&childbuf, "<source evdev='%s'/>\n", def->source.evdev); + if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0) + return -1; + + if (!virBufferUse(&childbuf)) { + virBufferAddLit(buf, "/>\n"); + } else { virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferEscapeString(buf, "<source evdev='%s'/>\n", def->source.evdev); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; - virBufferAdjustIndent(buf, -2); + virBufferAddBuffer(buf, &childbuf); virBufferAddLit(buf, "</input>\n"); - } else { - virBufferAddLit(buf, "/>\n"); } return 0; -- 2.7.3
<memballoon model='virtio'> <virtio revision='0.9'/> </memballoon> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 9 +++ docs/schemas/domaincommon.rng | 16 ++++ src/conf/domain_conf.c | 86 ++++++++++++++++++++++ src/conf/domain_conf.h | 9 +++ .../qemuxml2argv-virtio-revision.xml | 54 ++++++++++++++ .../qemuxml2xmlout-virtio-revision.xml | 54 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 230 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 5acb3b9..d01f92d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6415,6 +6415,15 @@ qemu-kvm -net nic,model=? /dev/null <span class='since'>Since 1.1.1, requires QEMU 1.5</span> </p> </dd> + <dt><code>virtio</code></dt> + <dd> + <p> + An optional <code>virtio</code> can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + </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..64088ba 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3612,6 +3612,9 @@ </attribute> </element> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </interleave> </element> </define> @@ -4830,6 +4833,19 @@ </element> </define> + <define name="virtioRevision"> + <zeroOrMore> + <element name="virtio"> + <attribute name="revision"> + <choice> + <value>0.9</value> + <value>1.0</value> + </choice> + </attribute> + </element> + </zeroOrMore> + </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 5dbeb1a..b006bc9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -834,6 +834,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash") +VIR_ENUM_IMPL(virDomainVirtioRevision, + VIR_DOMAIN_VIRTIO_REVISION_LAST, + "default", + "0.9", + "1.0") + /* 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 +1065,81 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) return &xmlopt->ns; } +static int +virDomainVirtioRevisionParseXML(xmlXPathContextPtr ctxt, + virBitmapPtr *res) +{ + xmlNodePtr save = ctxt->node; + xmlNodePtr *nodes = NULL; + char *str = NULL; + int ret = -1; + size_t i; + int n; + virBitmapPtr revmap = NULL; + + if ((n = virXPathNodeSet("./virtio", ctxt, &nodes)) < 0) + goto cleanup; + + if (n == 0) { + ret = 0; + goto cleanup; + } + + if (!(revmap = virBitmapNew(VIR_DOMAIN_VIRTIO_REVISION_LAST))) + goto cleanup; + + for (i = 0; i < n; i++) { + int val; + + ctxt->node = nodes[i]; + + if (!(str = virXPathString("string(./@revision)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'revision' attribute in <virtio> element")); + goto cleanup; + } + + if ((val = virDomainVirtioRevisionTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unable to parse virtio revision: '%s'"), + str); + goto cleanup; + } + + ignore_value(virBitmapSetBit(revmap, val)); + } + + ret = 0; + VIR_STEAL_PTR(*res, revmap); + + cleanup: + ctxt->node = save; + virBitmapFree(revmap); + VIR_FREE(nodes); + VIR_FREE(str); + return ret; +} + + +static void +virDomainVirtioRevisionFormatXML(virBufferPtr buf, + virBitmapPtr revmap) +{ + size_t i; + + if (!revmap) + return; + + for (i = VIR_DOMAIN_VIRTIO_REVISION_DEFAULT + 1; + i < VIR_DOMAIN_VIRTIO_REVISION_LAST; + i++) { + if (virBitmapIsBitSet(revmap, i)) { + virBufferAsprintf(buf, "<virtio revision='%s'/>\n", + virDomainVirtioRevisionTypeToString(i)); + } + } +} + void virBlkioDeviceArrayClear(virBlkioDevicePtr devices, @@ -12061,6 +12142,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + cleanup: VIR_FREE(model); VIR_FREE(deflate); @@ -21492,6 +21576,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return -1; } + virDomainVirtioRevisionFormatXML(&childrenBuf, def->virtio_rev); + if (!virBufferUse(&childrenBuf)) { virBufferAddLit(buf, "/>\n"); } else { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8b26724..2b39f41 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -154,6 +154,13 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr; +typedef enum { + VIR_DOMAIN_VIRTIO_REVISION_DEFAULT, + VIR_DOMAIN_VIRTIO_REVISION_HERITAGE, + VIR_DOMAIN_VIRTIO_REVISION_CONTEMPORARY, + VIR_DOMAIN_VIRTIO_REVISION_LAST, +} virDomainVirtioRevision; + /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1546,6 +1553,7 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; int period; /* seconds between collections */ int autodeflate; /* enum virTristateSwitch */ + virBitmapPtr virtio_rev; }; struct _virDomainNVRAMDef { @@ -3022,6 +3030,7 @@ VIR_ENUM_DECL(virDomainTPMBackend) VIR_ENUM_DECL(virDomainMemoryModel) VIR_ENUM_DECL(virDomainMemoryBackingModel) VIR_ENUM_DECL(virDomainIOMMUModel) +VIR_ENUM_DECL(virDomainVirtioRevision) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml new file mode 100644 index 0000000..d971e89 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + </input> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <virtio revision='0.9'/> + <virtio revision='1.0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml new file mode 100644 index 0000000..d971e89 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + </input> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <virtio revision='0.9'/> + <virtio revision='1.0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7601a5f..6952abe 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -862,6 +862,8 @@ mymain(void) DO_TEST("virtio-input", NONE); DO_TEST("virtio-input-passthrough", NONE); + DO_TEST_FULL("virtio-revision", WHEN_BOTH, GIC_NONE, QEMU_CAPS_VIRTIO_SCSI, NONE); + virObjectUnref(cfg); DO_TEST("acpi-table", NONE); -- 2.7.3
On 08/08/2016 12:35 PM, Ján Tomko wrote:
<memballoon model='virtio'> <virtio revision='0.9'/> </memballoon>
https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 9 +++ docs/schemas/domaincommon.rng | 16 ++++ src/conf/domain_conf.c | 86 ++++++++++++++++++++++ src/conf/domain_conf.h | 9 +++ .../qemuxml2argv-virtio-revision.xml | 54 ++++++++++++++ .../qemuxml2xmlout-virtio-revision.xml | 54 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 230 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 5acb3b9..d01f92d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6415,6 +6415,15 @@ qemu-kvm -net nic,model=? /dev/null <span class='since'>Since 1.1.1, requires QEMU 1.5</span> </p> </dd> + <dt><code>virtio</code></dt> + <dd> + <p> + An optional <code>virtio</code> can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + </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..64088ba 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3612,6 +3612,9 @@ </attribute> </element> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </interleave> </element> </define> @@ -4830,6 +4833,19 @@ </element> </define>
+ <define name="virtioRevision"> + <zeroOrMore> + <element name="virtio"> + <attribute name="revision"> + <choice> + <value>0.9</value> + <value>1.0</value> + </choice> + </attribute> + </element> + </zeroOrMore> + </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 5dbeb1a..b006bc9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -834,6 +834,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash")
+VIR_ENUM_IMPL(virDomainVirtioRevision, + VIR_DOMAIN_VIRTIO_REVISION_LAST, + "default", + "0.9", + "1.0") + /* 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 +1065,81 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) return &xmlopt->ns; }
+static int +virDomainVirtioRevisionParseXML(xmlXPathContextPtr ctxt, + virBitmapPtr *res) +{ + xmlNodePtr save = ctxt->node; + xmlNodePtr *nodes = NULL; + char *str = NULL; + int ret = -1; + size_t i; + int n; + virBitmapPtr revmap = NULL; + + if ((n = virXPathNodeSet("./virtio", ctxt, &nodes)) < 0) + goto cleanup; + + if (n == 0) { + ret = 0; + goto cleanup; + } + + if (!(revmap = virBitmapNew(VIR_DOMAIN_VIRTIO_REVISION_LAST))) + goto cleanup; + + for (i = 0; i < n; i++) { + int val; + + ctxt->node = nodes[i]; + + if (!(str = virXPathString("string(./@revision)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'revision' attribute in <virtio> element")); + goto cleanup; + } + + if ((val = virDomainVirtioRevisionTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unable to parse virtio revision: '%s'"), + str); + goto cleanup; + } + + ignore_value(virBitmapSetBit(revmap, val));
It is just really.... odd that you are storing an attribute that can have one of two possible values as a bitmap. Unless you have some specific future plan for that, it really should just be stored as, well, the value itself. (Maybe your idea is to maybe someday allow forcing support for both? If so, then how about an enum value called "ALL" that gets turned into "disable-legacy=off,disable-modern=off"?)
+ } + + ret = 0; + VIR_STEAL_PTR(*res, revmap); + + cleanup: + ctxt->node = save; + virBitmapFree(revmap); + VIR_FREE(nodes); + VIR_FREE(str); + return ret; +} + + +static void +virDomainVirtioRevisionFormatXML(virBufferPtr buf, + virBitmapPtr revmap) +{ + size_t i; + + if (!revmap) + return; + + for (i = VIR_DOMAIN_VIRTIO_REVISION_DEFAULT + 1; + i < VIR_DOMAIN_VIRTIO_REVISION_LAST; + i++) { + if (virBitmapIsBitSet(revmap, i)) { + virBufferAsprintf(buf, "<virtio revision='%s'/>\n", + virDomainVirtioRevisionTypeToString(i)); + } + } +} +
void virBlkioDeviceArrayClear(virBlkioDevicePtr devices, @@ -12061,6 +12142,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node, else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error;
+ if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + cleanup: VIR_FREE(model); VIR_FREE(deflate); @@ -21492,6 +21576,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return -1; }
+ virDomainVirtioRevisionFormatXML(&childrenBuf, def->virtio_rev); + if (!virBufferUse(&childrenBuf)) { virBufferAddLit(buf, "/>\n"); } else { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8b26724..2b39f41 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -154,6 +154,13 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+typedef enum { + VIR_DOMAIN_VIRTIO_REVISION_DEFAULT, + VIR_DOMAIN_VIRTIO_REVISION_HERITAGE, + VIR_DOMAIN_VIRTIO_REVISION_CONTEMPORARY, + VIR_DOMAIN_VIRTIO_REVISION_LAST, +} virDomainVirtioRevision;
And beyond storing it in a bitmap, why do you invent *yet another* name for these things. It was confusing enough that virtio 0.9 is also known as "legacy", and 1.0 as "modern", but now you've come up with a 3rd way to say the same thing. I would suggest giving them the same names as the strings that they represent: VIR_DOMAIN_VIRTIO_REVISION_0_9 VIR_DOMAIN_VIRTIO_REVISION_1_0
+ /* Flags for the 'type' field in virDomainDeviceDef */ typedef enum { VIR_DOMAIN_DEVICE_NONE = 0, @@ -1546,6 +1553,7 @@ struct _virDomainMemballoonDef { virDomainDeviceInfo info; int period; /* seconds between collections */ int autodeflate; /* enum virTristateSwitch */ + virBitmapPtr virtio_rev; };
struct _virDomainNVRAMDef { @@ -3022,6 +3030,7 @@ VIR_ENUM_DECL(virDomainTPMBackend) VIR_ENUM_DECL(virDomainMemoryModel) VIR_ENUM_DECL(virDomainMemoryBackingModel) VIR_ENUM_DECL(virDomainIOMMUModel) +VIR_ENUM_DECL(virDomainVirtioRevision) /* from libvirt.h */ VIR_ENUM_DECL(virDomainState) VIR_ENUM_DECL(virDomainNostateReason) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml new file mode 100644 index 0000000..d971e89 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + </input> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <virtio revision='0.9'/> + <virtio revision='1.0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml new file mode 100644 index 0000000..d971e89 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <input type='mouse' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + </input> + <input type='keyboard' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + </input> + <input type='tablet' bus='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + </input> + <input type='passthrough' bus='virtio'> + <source evdev='/dev/input/event1234'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='virtio' heads='1' primary='yes'> + <acceleration accel3d='yes'/> + </model> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + <virtio revision='0.9'/> + <virtio revision='1.0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7601a5f..6952abe 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -862,6 +862,8 @@ mymain(void) DO_TEST("virtio-input", NONE); DO_TEST("virtio-input-passthrough", NONE);
+ DO_TEST_FULL("virtio-revision", WHEN_BOTH, GIC_NONE, QEMU_CAPS_VIRTIO_SCSI, NONE); + virObjectUnref(cfg);
DO_TEST("acpi-table", NONE);
On Tue, Aug 09, 2016 at 10:37:30PM -0400, Laine Stump wrote:
On 08/08/2016 12:35 PM, Ján Tomko wrote:
<memballoon model='virtio'> <virtio revision='0.9'/> </memballoon>
https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 9 +++ docs/schemas/domaincommon.rng | 16 ++++ src/conf/domain_conf.c | 86 ++++++++++++++++++++++ src/conf/domain_conf.h | 9 +++ .../qemuxml2argv-virtio-revision.xml | 54 ++++++++++++++ .../qemuxml2xmlout-virtio-revision.xml | 54 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 230 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml
+ if (!(revmap = virBitmapNew(VIR_DOMAIN_VIRTIO_REVISION_LAST))) + goto cleanup; + + for (i = 0; i < n; i++) { + int val; + + ctxt->node = nodes[i]; + + if (!(str = virXPathString("string(./@revision)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'revision' attribute in <virtio> element")); + goto cleanup; + } + + if ((val = virDomainVirtioRevisionTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unable to parse virtio revision: '%s'"), + str); + goto cleanup; + } + + ignore_value(virBitmapSetBit(revmap, val));
It is just really.... odd that you are storing an attribute that can have one of two possible values as a bitmap. Unless you have some specific future plan for that, it really should just be stored as, well, the value itself.
Both can be set at the same time. I was thinking it would be useful for libvirt to understand both if someone wants to migrate from a newer libvirt version. But to be useful, they would need to have a QEMU that supports a machine type that does not have 0.9+1.0 as the default and they are using it with an old libvirt, so it probably does not make sense. I still have the single-attribute version on a branch, I could revert back to that.
(Maybe your idea is to maybe someday allow forcing support for both? If so, then how about an enum value called "ALL" that gets turned into "disable-legacy=off,disable-modern=off"?)
ALL meaning all the revisions supported by current libvirt does not seem like a good idea to me - if a new version comes along, do we include it in there or not?
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8b26724..2b39f41 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -154,6 +154,13 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+typedef enum { + VIR_DOMAIN_VIRTIO_REVISION_DEFAULT, + VIR_DOMAIN_VIRTIO_REVISION_HERITAGE, + VIR_DOMAIN_VIRTIO_REVISION_CONTEMPORARY, + VIR_DOMAIN_VIRTIO_REVISION_LAST, +} virDomainVirtioRevision;
And beyond storing it in a bitmap, why do you invent *yet another* name for these things.
I was hoping to amuse the reader.
It was confusing enough that virtio 0.9 is also known as "legacy", and 1.0 as "modern", but now you've come up with a 3rd way to say the same thing. I would suggest giving them the same names as the strings that they represent:
VIR_DOMAIN_VIRTIO_REVISION_0_9 VIR_DOMAIN_VIRTIO_REVISION_1_0
Okay :( Jan
On 08/10/2016 05:56 AM, Ján Tomko wrote:
On Tue, Aug 09, 2016 at 10:37:30PM -0400, Laine Stump wrote:
On 08/08/2016 12:35 PM, Ján Tomko wrote:
<memballoon model='virtio'> <virtio revision='0.9'/> </memballoon>
https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 9 +++ docs/schemas/domaincommon.rng | 16 ++++ src/conf/domain_conf.c | 86 ++++++++++++++++++++++ src/conf/domain_conf.h | 9 +++ .../qemuxml2argv-virtio-revision.xml | 54 ++++++++++++++ .../qemuxml2xmlout-virtio-revision.xml | 54 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 230 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml
+ if (!(revmap = virBitmapNew(VIR_DOMAIN_VIRTIO_REVISION_LAST))) + goto cleanup; + + for (i = 0; i < n; i++) { + int val; + + ctxt->node = nodes[i]; + + if (!(str = virXPathString("string(./@revision)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'revision' attribute in <virtio> element")); + goto cleanup; + } + + if ((val = virDomainVirtioRevisionTypeFromString(str)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Unable to parse virtio revision: '%s'"), + str); + goto cleanup; + } + + ignore_value(virBitmapSetBit(revmap, val));
It is just really.... odd that you are storing an attribute that can have one of two possible values as a bitmap. Unless you have some specific future plan for that, it really should just be stored as, well, the value itself.
Both can be set at the same time.
Ah, I missed that.
I was thinking it would be useful for libvirt to understand both if someone wants to migrate from a newer libvirt version. But to be useful, they would need to have a QEMU that supports a machine type that does not have 0.9+1.0 as the default and they are using it with an old libvirt, so it probably does not make sense.
I still have the single-attribute version on a branch, I could revert back to that.
No, keep it this way. I had missed the fact that you could add the element twice and get both. That is a valid use case - as of very recently in upstream qemu, a virtio device that is placed on a pcie-*-port slot defaults to 1.0-only (in order to save IO port space), and we do need a way to get around that in case someone wants such a device on a guest whose OS doesn't support virtio-1.0.
(Maybe your idea is to maybe someday allow forcing support for both? If so, then how about an enum value called "ALL" that gets turned into "disable-legacy=off,disable-modern=off"?)
ALL meaning all the revisions supported by current libvirt does not seem like a good idea to me - if a new version comes along, do we include it in there or not?
I can see a problem with only being able to select specific versions or "default". If someone just wants to turn off virtio-0.9 but allow anything else above, the only way to do that is to individually list all the other versions. Of course right now there's only one other version, so we can probably just ignore that until/unless a newer virtio version comes around.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8b26724..2b39f41 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -154,6 +154,13 @@ typedef virDomainTPMDef *virDomainTPMDefPtr; typedef struct _virDomainIOMMUDef virDomainIOMMUDef; typedef virDomainIOMMUDef *virDomainIOMMUDefPtr;
+typedef enum { + VIR_DOMAIN_VIRTIO_REVISION_DEFAULT, + VIR_DOMAIN_VIRTIO_REVISION_HERITAGE, + VIR_DOMAIN_VIRTIO_REVISION_CONTEMPORARY, + VIR_DOMAIN_VIRTIO_REVISION_LAST, +} virDomainVirtioRevision;
And beyond storing it in a bitmap, why do you invent *yet another* name for these things.
I was hoping to amuse the reader.
It was confusing enough that virtio 0.9 is also known as "legacy", and 1.0 as "modern", but now you've come up with a 3rd way to say the same thing. I would suggest giving them the same names as the strings that they represent:
VIR_DOMAIN_VIRTIO_REVISION_0_9 VIR_DOMAIN_VIRTIO_REVISION_1_0
Okay :(
Oh come on, I'm sure you have other methods of amusing us - what about that photoshop project abologna suggested?
<disk ...> <virtio revision='0.9'/> </disk> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 9 +++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 14 ++++++++++++++ .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 14 ++++++++++++++ 6 files changed, 45 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d01f92d..5a5826b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2810,6 +2810,15 @@ </dd> </dl> </dd> + <dt><code>virtio</code></dt> + <dd> + <p> + An optional <code>virtio</code> element can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + </dd> </dl> <h4><a name="elementsFilesystems">Filesystems</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 64088ba..6b40553 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1201,6 +1201,9 @@ </data> </element> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </interleave> </define> <define name="snapshot"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b006bc9..9ab1f5d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7716,6 +7716,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + /* Disk volume types will have authentication information handled in * virStorageTranslateDiskSourcePool */ @@ -20141,6 +20144,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) return -1; + virDomainVirtioRevisionFormatXML(buf, def->virtio_rev); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</disk>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2b39f41..0541a8f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -620,6 +620,7 @@ struct _virDomainDiskDef { unsigned int iothread; /* unused = 0, > 0 specific thread # */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ char *domain_name; /* backend domain name */ + virBitmapPtr virtio_rev; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index d971e89..1a8a3de 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -14,6 +14,20 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img1'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + <virtio revision='0.9'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img2'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + <virtio revision='1.0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index d971e89..1a8a3de 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -14,6 +14,20 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/bin/qemu</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img1'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + <virtio revision='0.9'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img2'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + <virtio revision='1.0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> -- 2.7.3
<controller type='scsi' index='0' model='virtio-scsi'> <virtio revision='0.9'/> </controller> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 8 ++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 10 ++++++++++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 10 ++++++++++ 6 files changed, 36 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 5a5826b..04424b4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3150,6 +3150,14 @@ additional attributes that control specific features, such as: </p> + <p> + For virtio controllers, an optional <code>virtio</code> element + can be used to enforce a particular virtio revision in QEMU. + The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + <dl> <dt><code>virtio-serial</code></dt> <dd>The <code>virtio-serial</code> controller has two additional diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6b40553..4f4db8c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1946,6 +1946,9 @@ </optional> </element> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9ab1f5d..098c853 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8328,6 +8328,9 @@ virDomainControllerDefParseXML(xmlNodePtr node, cur = cur->next; } + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + /* node is parsed differently from target attributes because * someone thought it should be a subelement instead... */ @@ -20296,6 +20299,7 @@ virDomainControllerDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<pcihole64 unit='KiB'>%lu</" "pcihole64>\n", def->opts.pciopts.pcihole64size); } + virDomainVirtioRevisionFormatXML(buf, def->virtio_rev); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</controller>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0541a8f..9b0dad0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -763,6 +763,7 @@ struct _virDomainControllerDef { virDomainUSBControllerOpts usbopts; } opts; virDomainDeviceInfo info; + virBitmapPtr virtio_rev; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index 1a8a3de..7972617 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -28,12 +28,22 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> <virtio revision='1.0'/> </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img3'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + <virtio revision='0.9'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index 1a8a3de..7972617 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -28,12 +28,22 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> <virtio revision='1.0'/> </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/img3'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='ide' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> + <controller type='scsi' index='0' model='virtio-scsi'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + <virtio revision='0.9'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> -- 2.7.3
<filesystem ...> ... <virtio revision='1.0'/> </filesystem> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 10 ++++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 6 ++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 13 +++++++++++++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 13 +++++++++++++ 6 files changed, 46 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 04424b4..ba9e643 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2997,6 +2997,16 @@ hard limit is enforced. <span class="since">Since 0.9.13</span> </dd> + + <dt><code>virtio</code></dt> + <dd> + <p> + An optional <code>virtio</code> element can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + </dd> </dl> <h4><a name="elementsAddress">Device Addresses</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4f4db8c..58fc6e3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2099,6 +2099,9 @@ <ref name='scaledInteger'/> </element> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 098c853..3ab2644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8731,6 +8731,9 @@ virDomainFSDefParseXML(xmlNodePtr node, goto error; } + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + def->src->path = source; source = NULL; def->dst = target; @@ -20420,6 +20423,9 @@ virDomainFSDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<space_soft_limit unit='bytes'>" "%llu</space_soft_limit>\n", def->space_soft_limit); } + + virDomainVirtioRevisionFormatXML(buf, def->virtio_rev); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</filesystem>\n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9b0dad0..0bdd6d6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -826,6 +826,7 @@ struct _virDomainFSDef { unsigned long long space_hard_limit; /* in bytes */ unsigned long long space_soft_limit; /* in bytes */ bool symlinksResolved; + virBitmapPtr virtio_rev; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index 7972617..ad846e1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -48,6 +48,19 @@ <controller type='virtio-serial' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/fs1'/> + <target dir='fs1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + <virtio revision='0.9'/> + </filesystem> + <filesystem type='mount' accessmode='mapped'> + <driver type='path' wrpolicy='immediate'/> + <source dir='/export/fs2'/> + <target dir='fs2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <virtio revision='1.0'/> + </filesystem> <input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index 7972617..ad846e1 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -48,6 +48,19 @@ <controller type='virtio-serial' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> </controller> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/export/fs1'/> + <target dir='fs1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + <virtio revision='0.9'/> + </filesystem> + <filesystem type='mount' accessmode='mapped'> + <driver type='path' wrpolicy='immediate'/> + <source dir='/export/fs2'/> + <target dir='fs2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <virtio revision='1.0'/> + </filesystem> <input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> -- 2.7.3
<interface type='user'> <mac address='52:54:56:5a:5c:5e'/> <model type='virtio'/> <virtio revision='1.0'/> </interface> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 8 +++++++- docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 5 +++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 12 ++++++++++++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 12 ++++++++++++ 6 files changed, 40 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ba9e643..9998447 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4048,6 +4048,12 @@ attribute <code>type='pci'</code> as <a href="#elementsAddress">documented above</a>. </p> + <p> + An optional <code>virtio</code> element can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> <h5><a name="elementsNICSVirtual">Virtual network</a></h5> @@ -6445,7 +6451,7 @@ qemu-kvm -net nic,model=? /dev/null <dt><code>virtio</code></dt> <dd> <p> - An optional <code>virtio</code> can be used to enforce a particular + An optional <code>virtio</code> element can be used to enforce a particular virtio revision in QEMU. The valid values for the <code>revision</code> are <code>0.9</code> and <code>1.0</code>. <span class='since'>Since 2.2.0</span> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 58fc6e3..c106cc1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2414,6 +2414,9 @@ <ref name="virYesNo"/> </attribute> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </element> </define> <!-- diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ab2644..3400833 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9709,6 +9709,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + cleanup: ctxt->node = oldnode; VIR_FREE(macaddr); @@ -21159,6 +21162,8 @@ virDomainNetDefFormat(virBufferPtr buf, | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) return -1; + virDomainVirtioRevisionFormatXML(buf, def->virtio_rev); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</interface>\n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0bdd6d6..0f116b7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -993,6 +993,7 @@ struct _virDomainNetDef { virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ int linkstate; + virBitmapPtr virtio_rev; }; /* Used for prefix of ifname of any network name generated dynamically diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index ad846e1..1f9f26f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -61,6 +61,18 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <virtio revision='1.0'/> </filesystem> + <interface type='user'> + <mac address='52:54:56:58:5a:5c'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + <virtio revision='0.9'/> + </interface> + <interface type='user'> + <mac address='52:54:56:5a:5c:5e'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + <virtio revision='1.0'/> + </interface> <input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index ad846e1..1f9f26f 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -61,6 +61,18 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> <virtio revision='1.0'/> </filesystem> + <interface type='user'> + <mac address='52:54:56:58:5a:5c'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + <virtio revision='0.9'/> + </interface> + <interface type='user'> + <mac address='52:54:56:5a:5c:5e'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + <virtio revision='1.0'/> + </interface> <input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> </input> -- 2.7.3
<rng model='virtio'> <backend model='random'>/dev/random</backend> <virtio revision='1.0'/> </rng> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 9 +++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 5 +++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 5 +++++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 5 +++++ 6 files changed, 28 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9998447..b595674 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6538,6 +6538,15 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> </dd> + <dt><code>virtio</code></dt> + <dd> + <p> + An optional <code>virtio</code> element can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c106cc1..2909edf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4804,6 +4804,9 @@ <optional> <ref name="address"/> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3400833..700b2fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12089,6 +12089,9 @@ virDomainRNGDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + cleanup: VIR_FREE(model); VIR_FREE(backend); @@ -21776,6 +21779,8 @@ virDomainRNGDefFormat(virBufferPtr buf, return -1; } + virDomainVirtioRevisionFormatXML(buf, def->virtio_rev); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</rng>\n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0f116b7..d1c7975 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1946,6 +1946,7 @@ struct _virDomainRNGDef { } source; virDomainDeviceInfo info; + virBitmapPtr virtio_rev; }; typedef enum { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index 1f9f26f..c3a4654 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -99,5 +99,10 @@ <virtio revision='0.9'/> <virtio revision='1.0'/> </memballoon> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + <virtio revision='1.0'/> + </rng> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index 1f9f26f..c3a4654 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -99,5 +99,10 @@ <virtio revision='0.9'/> <virtio revision='1.0'/> </memballoon> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0d' function='0x0'/> + <virtio revision='1.0'/> + </rng> </devices> </domain> -- 2.7.3
<video> <model type='virtio' heads='1' primary='yes'/> <virtio revision='1.0'/> </video> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 10 ++++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 15 +++++++++++++-- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 1 + .../qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 1 + 6 files changed, 29 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b595674..606b56c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5646,6 +5646,16 @@ qemu-kvm -net nic,model=? /dev/null The optional <code>address</code> sub-element can be used to tie the video device to a particular PCI slot. </dd> + + <dt><code>virtio</code></dt> + <dd> + <p> + An optional <code>virtio</code> element can be used to enforce a particular + virtio revision in QEMU. The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + </dd> </dl> <h4><a name="elementsConsole">Consoles, serial, parallel & channel devices</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2909edf..fb92f2e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3150,6 +3150,9 @@ <optional> <ref name="address"/> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </element> </define> <!-- diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 700b2fc..fc344b2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12650,11 +12650,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) static virDomainVideoDefPtr virDomainVideoDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, const virDomainDef *dom, unsigned int flags) { virDomainVideoDefPtr def; xmlNodePtr cur; + xmlNodePtr saved = ctxt->node; char *type = NULL; char *heads = NULL; char *vram = NULL; @@ -12663,6 +12665,8 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *vgamem = NULL; char *primary = NULL; + ctxt->node = node; + if (VIR_ALLOC(def) < 0) return NULL; @@ -12764,7 +12768,12 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + cleanup: + ctxt->node = saved; + VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); @@ -13459,7 +13468,7 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_VIDEO: - if (!(dev->data.video = virDomainVideoDefParseXML(node, def, flags))) + if (!(dev->data.video = virDomainVideoDefParseXML(node, ctxt, def, flags))) goto error; break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -17125,7 +17134,7 @@ virDomainDefParseXML(xmlDocPtr xml, virDomainVideoDefPtr video; ssize_t insertAt = -1; - if (!(video = virDomainVideoDefParseXML(nodes[i], def, flags))) + if (!(video = virDomainVideoDefParseXML(nodes[i], ctxt, def, flags))) goto error; if (video->primary) { @@ -21942,6 +21951,8 @@ virDomainVideoDefFormat(virBufferPtr buf, if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; + virDomainVirtioRevisionFormatXML(buf, def->virtio_rev); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</video>\n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d1c7975..254d3db 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1321,6 +1321,7 @@ struct _virDomainVideoDef { bool primary; virDomainVideoAccelDefPtr accel; virDomainDeviceInfo info; + virBitmapPtr virtio_rev; }; /* graphics console modes */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index c3a4654..cfb8581 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -93,6 +93,7 @@ <acceleration accel3d='yes'/> </model> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + <virtio revision='1.0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index c3a4654..cfb8581 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -93,6 +93,7 @@ <acceleration accel3d='yes'/> </model> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + <virtio revision='1.0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> -- 2.7.3
<input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> <virtio revision='1.0'/> </input> <input type='keyboard' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> <virtio revision='1.0'/> </input> <input type='tablet' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> <virtio revision='1.0'/> </input> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> <virtio revision='1.0'/> </input> https://bugzilla.redhat.com/show_bug.cgi?id=1227354 --- docs/formatdomain.html.in | 8 ++++++++ docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml | 4 ++++ tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml | 4 ++++ 6 files changed, 24 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 606b56c..8c71b8f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5235,6 +5235,14 @@ qemu-kvm -net nic,model=? /dev/null event device passed through to guests. (KVM only) </p> + <p> + For virtio input devices, an optional <code>virtio</code> element + can be used to enforce a particular virtio revision in QEMU. + The valid values for the <code>revision</code> + are <code>0.9</code> and <code>1.0</code>. + <span class='since'>Since 2.2.0</span> + </p> + <h4><a name="elementsHub">Hub devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fb92f2e..2b4fa7c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3828,6 +3828,9 @@ <optional> <ref name="address"/> </optional> + <optional> + <ref name="virtioRevision"/> + </optional> </element> </define> <define name="hub"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fc344b2..cef024c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10775,6 +10775,9 @@ virDomainInputDefParseXML(const virDomainDef *dom, goto error; } + if (virDomainVirtioRevisionParseXML(ctxt, &def->virtio_rev) < 0) + goto error; + cleanup: VIR_FREE(evdev); VIR_FREE(type); @@ -21992,6 +21995,7 @@ virDomainInputDefFormat(virBufferPtr buf, virBufferEscapeString(&childbuf, "<source evdev='%s'/>\n", def->source.evdev); if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0) return -1; + virDomainVirtioRevisionFormatXML(&childbuf, def->virtio_rev); if (!virBufferUse(&childbuf)) { virBufferAddLit(buf, "/>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 254d3db..c864810 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1228,6 +1228,7 @@ struct _virDomainInputDef { char *evdev; } source; virDomainDeviceInfo info; + virBitmapPtr virtio_rev; }; typedef enum { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml index cfb8581..2bdfe82 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.xml @@ -75,16 +75,20 @@ </interface> <input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='keyboard' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='tablet' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml index cfb8581..2bdfe82 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-virtio-revision.xml @@ -75,16 +75,20 @@ </interface> <input type='mouse' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0e' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='keyboard' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='tablet' bus='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x11' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='passthrough' bus='virtio'> <source evdev='/dev/input/event1234'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x12' function='0x0'/> + <virtio revision='1.0'/> </input> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> -- 2.7.3
Check whether the disable-legacy property is present on the following devices: virtio-balloon-pci virtio-blk-pci virtio-scsi-pci virtio-net-pci virtio-gpu-pci Assuming that if QEMU knows other virtio devices where this property is applicable, it will have at least one of these devices. Added in QEMU by: commit e266d421490e0ae83044bbebb209b2d3650c0ba6 virtio-pci: add flags to enable/disable legacy/modern --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + 9 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 43e3ea7..340691a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -340,6 +340,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "display", /* 230 */ "intel-iommu", "smm", + "virtio-revision", ); @@ -1565,6 +1566,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { { "deflate-on-oom", QEMU_CAPS_VIRTIO_BALLOON_AUTODEFLATE }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { @@ -1574,15 +1576,18 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { { "event_idx", QEMU_CAPS_VIRTIO_BLK_EVENT_IDX }, { "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI }, { "logical_block_size", QEMU_CAPS_BLOCKIO }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = { { "tx", QEMU_CAPS_VIRTIO_TX_ALG }, { "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioSCSI[] = { { "iothread", QEMU_CAPS_VIRTIO_SCSI_IOTHREAD }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPCIAssign[] = { @@ -1659,6 +1664,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = { { "virgl", QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsICH9[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d249e2e..15cf19d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -373,6 +373,7 @@ typedef enum { QEMU_CAPS_DISPLAY, /* -display */ QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */ QEMU_CAPS_MACHINE_SMM_OPT, /* -machine xxx,smm=on/off/auto */ + QEMU_CAPS_DEVICE_VIRTIO_REVISION, /* virtio-*pci.disable-{legacy,modern} */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index 339ee1f..5ec9320 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -184,6 +184,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index c1a68d0..33c89ae 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -189,6 +189,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index 85d7d3f..2eefaa0 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -158,6 +158,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index deb1257..581ffaa 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -158,6 +158,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 2b7ea0e..8ed2c49 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -152,6 +152,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 495c114..bf35867 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -195,6 +195,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index fafffa6..de2f3ee 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -192,6 +192,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2006091</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0-rc1-52-g42e0d60)</package> -- 2.7.3
On 08/08/2016 12:35 PM, Ján Tomko wrote:
Check whether the disable-legacy property is present on the following devices: virtio-balloon-pci virtio-blk-pci virtio-scsi-pci virtio-net-pci virtio-gpu-pci
Assuming that if QEMU knows other virtio devices where this property is applicable, it will have at least one of these devices.
Pretty cool idea to look for the presence of disable-legacy in several virtio devices in case the binary has been built without one of them. When I was doing a similar check, I semi-randomly picked "disable-modern", but only checked it on virtio-net. Although in practice that will probably work just as well, I like your idea better. I'm curious though - did you have a reason for checking for disable-legacy rather than disable-modern? Or was your choice also semi-random?
Added in QEMU by: commit e266d421490e0ae83044bbebb209b2d3650c0ba6 virtio-pci: add flags to enable/disable legacy/modern --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + 9 files changed, 14 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 43e3ea7..340691a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -340,6 +340,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "display", /* 230 */ "intel-iommu", "smm", + "virtio-revision",
I had thought about naming my capability flag based on what I used it for as well, but decided it would be better to make the name of the capability as close to the actual option name as possible, to avoid confusion and allow re-use. Maybe an even better idea would be to make capabilities named "virtio-disable-modern" and "virtio-disable-legacy" set directly based on the presence of the options, then create a synthetic capability named virtio-revision that would be set if both of those others were true (you might find that you need to add more checks in the future, e.g. to disable it for certain interim versions of qemu that had bugs in virtio-1.0)
);
@@ -1565,6 +1566,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { { "deflate-on-oom", QEMU_CAPS_VIRTIO_BALLOON_AUTODEFLATE }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { @@ -1574,15 +1576,18 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { { "event_idx", QEMU_CAPS_VIRTIO_BLK_EVENT_IDX }, { "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI }, { "logical_block_size", QEMU_CAPS_BLOCKIO }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = { { "tx", QEMU_CAPS_VIRTIO_TX_ALG }, { "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioSCSI[] = { { "iothread", QEMU_CAPS_VIRTIO_SCSI_IOTHREAD }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPCIAssign[] = { @@ -1659,6 +1664,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = {
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = { { "virgl", QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL }, + { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsICH9[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d249e2e..15cf19d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -373,6 +373,7 @@ typedef enum { QEMU_CAPS_DISPLAY, /* -display */ QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */ QEMU_CAPS_MACHINE_SMM_OPT, /* -machine xxx,smm=on/off/auto */ + QEMU_CAPS_DEVICE_VIRTIO_REVISION, /* virtio-*pci.disable-{legacy,modern} */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml index 339ee1f..5ec9320 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml @@ -184,6 +184,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2004000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml index c1a68d0..33c89ae 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml @@ -189,6 +189,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index 85d7d3f..2eefaa0 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -158,6 +158,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index deb1257..581ffaa 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -158,6 +158,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 2b7ea0e..8ed2c49 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -152,6 +152,7 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 495c114..bf35867 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -195,6 +195,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index fafffa6..de2f3ee 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -192,6 +192,7 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='virtio-revision'/> <version>2006091</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0-rc1-52-g42e0d60)</package>
On Tue, Aug 09, 2016 at 11:08:10PM -0400, Laine Stump wrote:
On 08/08/2016 12:35 PM, Ján Tomko wrote:
Check whether the disable-legacy property is present on the following devices: virtio-balloon-pci virtio-blk-pci virtio-scsi-pci virtio-net-pci virtio-gpu-pci
Assuming that if QEMU knows other virtio devices where this property is applicable, it will have at least one of these devices.
Pretty cool idea to look for the presence of disable-legacy in several virtio devices in case the binary has been built without one of them. When I was doing a similar check, I semi-randomly picked "disable-modern", but only checked it on virtio-net. Although in practice that will probably work just as well, I like your idea better.
I only added it to those devices where we look for other properties.
I'm curious though - did you have a reason for checking for disable-legacy rather than disable-modern? Or was your choice also semi-random?
I don't remember thinking about it, I just picked the first one. But chronologically, virtio 0.9 was introduced first.
Added in QEMU by: commit e266d421490e0ae83044bbebb209b2d3650c0ba6 virtio-pci: add flags to enable/disable legacy/modern --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + 9 files changed, 14 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 43e3ea7..340691a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -340,6 +340,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "display", /* 230 */ "intel-iommu", "smm", + "virtio-revision",
I had thought about naming my capability flag based on what I used it for as well, but decided it would be better to make the name of the capability as close to the actual option name as possible, to avoid confusion and allow re-use.
Okay, this string needs to be stable across libvirt releases, so matching it as close as possible makes sense,
Maybe an even better idea would be to make capabilities named "virtio-disable-modern" and "virtio-disable-legacy" set directly based on the presence of the options, then create a synthetic capability named virtio-revision that would be set if both of those others were true (you might find that you need to add more checks in the future, e.g. to disable it for certain interim versions of qemu that had bugs in virtio-1.0)
but I don't see a point of adding two capabilites for it if we are only using one. The second one can just be added later if needed. Jan
https://bugzilla.redhat.com/show_bug.cgi?id=1227354 Translate the optional <virtio revision> attribute to disable-legacy=on/off and disable-modern=on/off options for the following devices: <memballoon> virtio-balloon-pci <disk> virtio-blk-pci <controller> virtio-scsi-pci virtio-serial-pci <filesystem> virtio-9p-pci <interface> virtio-net-pci <rng> virtio-rng-pci <video> virtio-gpu-pci <input> virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci --- src/qemu/qemu_command.c | 56 +++++++++++++++++++ .../qemuxml2argv-virtio-revision.args | 62 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++ 3 files changed, 129 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 55df23d..765b10f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -378,6 +378,36 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, } static int +qemuBuildVirtioRevisionStr(virBufferPtr buf, + virBitmapPtr revmap, + virQEMUCapsPtr qemuCaps) +{ + if (!revmap) + return 0; + + if (virBitmapLastSetBit(revmap) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_REVISION)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the virtio revision is not supported with " + "this QEMU binary")); + return -1; + } + + virBufferAddLit(buf, ",disable-legacy="); + if (virBitmapIsBitSet(revmap, VIR_DOMAIN_VIRTIO_REVISION_HERITAGE)) + virBufferAddLit(buf, "off"); + else + virBufferAddLit(buf, "on"); + + virBufferAddLit(buf, ",disable-modern="); + if (virBitmapIsBitSet(revmap, VIR_DOMAIN_VIRTIO_REVISION_CONTEMPORARY)) + virBufferAddLit(buf, "off"); + else + virBufferAddLit(buf, "on"); + return 0; +} + +static int qemuBuildRomStr(virBufferPtr buf, virDomainDeviceInfoPtr info) { @@ -1988,6 +2018,10 @@ qemuBuildDriveDevStr(const virDomainDef *def, (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) ? "on" : "off"); } + + if (qemuBuildVirtioRevisionStr(&opt, disk->virtio_rev, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) goto error; break; @@ -2309,6 +2343,8 @@ qemuBuildFSDevStr(const virDomainDef *def, QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + qemuBuildVirtioRevisionStr(&opt, fs->virtio_rev, qemuCaps); + if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) goto error; @@ -2566,6 +2602,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->iothread); } } + if (qemuBuildVirtioRevisionStr(&buf, def->virtio_rev, qemuCaps) < 0) + goto error; break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); @@ -2611,6 +2649,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",vectors=%d", def->opts.vioserial.vectors); } + if (qemuBuildVirtioRevisionStr(&buf, def->virtio_rev, qemuCaps) < 0) + goto error; break; case VIR_DOMAIN_CONTROLLER_TYPE_CCID: @@ -3555,6 +3595,10 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",id=%s", net->info.alias); virBufferAsprintf(&buf, ",mac=%s", virMacAddrFormat(&net->mac, macaddr)); + + if (qemuBuildVirtioRevisionStr(&buf, net->virtio_rev, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&buf, def, &net->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, &net->info) < 0) @@ -3825,6 +3869,9 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, virTristateSwitchTypeToString(def->memballoon->autodeflate)); } + if (qemuBuildVirtioRevisionStr(&buf, def->memballoon->virtio_rev, qemuCaps) < 0) + goto error; + virCommandAddArg(cmd, "-device"); virCommandAddArgBuffer(cmd, &buf); return 0; @@ -3955,6 +4002,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) goto error; + if (qemuBuildVirtioRevisionStr(&buf, dev->virtio_rev, qemuCaps) < 0) + goto error; + if (virBufferCheckError(&buf) < 0) goto error; @@ -4320,6 +4370,9 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) goto error; + if (qemuBuildVirtioRevisionStr(&buf, video->virtio_rev, qemuCaps) < 0) + goto error; + if (virBufferCheckError(&buf) < 0) goto error; @@ -5566,6 +5619,9 @@ qemuBuildRNGDevStr(const virDomainDef *def, virBufferAddLit(&buf, ",period=1000"); } + if (qemuBuildVirtioRevisionStr(&buf, dev->virtio_rev, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) goto error; if (virBufferCheckError(&buf) < 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args new file mode 100644 index 0000000..383476a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args @@ -0,0 +1,62 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,disable-legacy=off,disable-modern=on,id=scsi0,\ +bus=pci.0,addr=0x8 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x9 \ +-usb \ +-drive file=/var/lib/libvirt/images/img1,format=raw,if=none,\ +id=drive-virtio-disk0 \ +-device virtio-blk-pci,disable-legacy=off,disable-modern=on,bus=pci.0,addr=0xa,\ +drive=drive-virtio-disk0,id=virtio-disk0 \ +-drive file=/var/lib/libvirt/images/img2,format=raw,if=none,\ +id=drive-virtio-disk1 \ +-device virtio-blk-pci,disable-legacy=on,disable-modern=off,bus=pci.0,addr=0xb,\ +drive=drive-virtio-disk1,id=virtio-disk1 \ +-drive file=/var/lib/libvirt/images/img3,format=raw,if=none,\ +id=drive-scsi0-0-0-0 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/fs1 \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,disable-legacy=off,\ +disable-modern=on,bus=pci.0,addr=0x3 \ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ +path=/export/fs2 \ +-device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,disable-legacy=on,\ +disable-modern=off,bus=pci.0,addr=0x4 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:56:58:5a:5c,disable-legacy=off,\ +disable-modern=on,bus=pci.0,addr=0x6 \ +-net user,vlan=0,name=hostnet0 \ +-device virtio-net-pci,vlan=1,id=net1,mac=52:54:56:5a:5c:5e,disable-legacy=on,\ +disable-modern=off,bus=pci.0,addr=0x7 \ +-net user,vlan=1,name=hostnet1 \ +-device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe,disable-legacy=on,\ +disable-modern=off \ +-device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10,disable-legacy=on,\ +disable-modern=off \ +-device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11,disable-legacy=on,\ +disable-modern=off \ +-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ +addr=0x12,disable-legacy=on,disable-modern=off \ +-device virtio-vga,id=video0,virgl=on,bus=pci.0,addr=0x2,disable-legacy=on,\ +disable-modern=off \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,disable-legacy=off,\ +disable-modern=off \ +-object rng-random,id=objrng0,filename=/dev/random \ +-device virtio-rng-pci,rng=objrng0,id=rng0,disable-legacy=on,\ +disable-modern=off,bus=pci.0,addr=0xd diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d594836..c1e964e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2086,6 +2086,17 @@ mymain(void) DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU); + DO_TEST("virtio-revision", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_VIRTIO_REVISION); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.7.3
On 08/08/2016 12:35 PM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1227354
Translate the optional <virtio revision> attribute to disable-legacy=on/off and disable-modern=on/off options for the following devices:
<memballoon> virtio-balloon-pci <disk> virtio-blk-pci <controller> virtio-scsi-pci virtio-serial-pci <filesystem> virtio-9p-pci <interface> virtio-net-pci <rng> virtio-rng-pci <video> virtio-gpu-pci <input> virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci --- src/qemu/qemu_command.c | 56 +++++++++++++++++++ .../qemuxml2argv-virtio-revision.args | 62 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++ 3 files changed, 129 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 55df23d..765b10f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -378,6 +378,36 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, }
static int +qemuBuildVirtioRevisionStr(virBufferPtr buf, + virBitmapPtr revmap, + virQEMUCapsPtr qemuCaps) +{ + if (!revmap) + return 0; + + if (virBitmapLastSetBit(revmap) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_REVISION)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the virtio revision is not supported with " + "this QEMU binary")); + return -1; + } + + virBufferAddLit(buf, ",disable-legacy="); + if (virBitmapIsBitSet(revmap, VIR_DOMAIN_VIRTIO_REVISION_HERITAGE)) + virBufferAddLit(buf, "off"); + else + virBufferAddLit(buf, "on"); + + virBufferAddLit(buf, ",disable-modern="); + if (virBitmapIsBitSet(revmap, VIR_DOMAIN_VIRTIO_REVISION_CONTEMPORARY)) + virBufferAddLit(buf, "off"); + else + virBufferAddLit(buf, "on"); + return 0; +} + +static int qemuBuildRomStr(virBufferPtr buf, virDomainDeviceInfoPtr info) { @@ -1988,6 +2018,10 @@ qemuBuildDriveDevStr(const virDomainDef *def, (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) ? "on" : "off"); } + + if (qemuBuildVirtioRevisionStr(&opt, disk->virtio_rev, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) goto error; break; @@ -2309,6 +2343,8 @@ qemuBuildFSDevStr(const virDomainDef *def, QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst);
+ qemuBuildVirtioRevisionStr(&opt, fs->virtio_rev, qemuCaps); + if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) goto error;
@@ -2566,6 +2602,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->iothread); } } + if (qemuBuildVirtioRevisionStr(&buf, def->virtio_rev, qemuCaps) < 0) + goto error; break; case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC: virBufferAddLit(&buf, "lsi"); @@ -2611,6 +2649,8 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, virBufferAsprintf(&buf, ",vectors=%d", def->opts.vioserial.vectors); } + if (qemuBuildVirtioRevisionStr(&buf, def->virtio_rev, qemuCaps) < 0) + goto error; break;
case VIR_DOMAIN_CONTROLLER_TYPE_CCID: @@ -3555,6 +3595,10 @@ qemuBuildNicDevStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",id=%s", net->info.alias); virBufferAsprintf(&buf, ",mac=%s", virMacAddrFormat(&net->mac, macaddr)); + + if (qemuBuildVirtioRevisionStr(&buf, net->virtio_rev, qemuCaps) < 0) + goto error; +
Just in case someone accidentally gets virtio_rev set for a non-virtio <interface>, this should be if (usingVirtio && qemuBuildVirtioRevisionStr(...)) (Sure, there should be validation prior to this to take care of that not happening, but everything else in this function that is virtio-only is inside "if (usingVirtio)".) Also, in all the other cases where qemuBuildVirtioRevisionStr() and qemuBuildDeviceAddressStr() are next to each other, qemuBuildAddressStr() is first. </obsessivecompulsive>
if (qemuBuildDeviceAddressStr(&buf, def, &net->info, qemuCaps) < 0) goto error; if (qemuBuildRomStr(&buf, &net->info) < 0) @@ -3825,6 +3869,9 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, virTristateSwitchTypeToString(def->memballoon->autodeflate)); }
+ if (qemuBuildVirtioRevisionStr(&buf, def->memballoon->virtio_rev, qemuCaps) < 0) + goto error; + virCommandAddArg(cmd, "-device"); virCommandAddArgBuffer(cmd, &buf); return 0; @@ -3955,6 +4002,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) goto error;
+ if (qemuBuildVirtioRevisionStr(&buf, dev->virtio_rev, qemuCaps) < 0) + goto error; + if (virBufferCheckError(&buf) < 0) goto error;
@@ -4320,6 +4370,9 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) goto error;
+ if (qemuBuildVirtioRevisionStr(&buf, video->virtio_rev, qemuCaps) < 0) + goto error; +
This should be qualified with "if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO)"
if (virBufferCheckError(&buf) < 0) goto error;
@@ -5566,6 +5619,9 @@ qemuBuildRNGDevStr(const virDomainDef *def, virBufferAddLit(&buf, ",period=1000"); }
+ if (qemuBuildVirtioRevisionStr(&buf, dev->virtio_rev, qemuCaps) < 0) + goto error; + if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) goto error; if (virBufferCheckError(&buf) < 0) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args new file mode 100644 index 0000000..383476a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-revision.args @@ -0,0 +1,62 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,disable-legacy=off,disable-modern=on,id=scsi0,\ +bus=pci.0,addr=0x8 \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x9 \ +-usb \ +-drive file=/var/lib/libvirt/images/img1,format=raw,if=none,\ +id=drive-virtio-disk0 \ +-device virtio-blk-pci,disable-legacy=off,disable-modern=on,bus=pci.0,addr=0xa,\ +drive=drive-virtio-disk0,id=virtio-disk0 \ +-drive file=/var/lib/libvirt/images/img2,format=raw,if=none,\ +id=drive-virtio-disk1 \ +-device virtio-blk-pci,disable-legacy=on,disable-modern=off,bus=pci.0,addr=0xb,\ +drive=drive-virtio-disk1,id=virtio-disk1 \ +-drive file=/var/lib/libvirt/images/img3,format=raw,if=none,\ +id=drive-scsi0-0-0-0 \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-fsdev local,security_model=passthrough,id=fsdev-fs0,path=/export/fs1 \ +-device virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=fs1,disable-legacy=off,\ +disable-modern=on,bus=pci.0,addr=0x3 \ +-fsdev local,security_model=mapped,writeout=immediate,id=fsdev-fs1,\ +path=/export/fs2 \ +-device virtio-9p-pci,id=fs1,fsdev=fsdev-fs1,mount_tag=fs2,disable-legacy=on,\ +disable-modern=off,bus=pci.0,addr=0x4 \ +-device virtio-net-pci,vlan=0,id=net0,mac=52:54:56:58:5a:5c,disable-legacy=off,\ +disable-modern=on,bus=pci.0,addr=0x6 \ +-net user,vlan=0,name=hostnet0 \ +-device virtio-net-pci,vlan=1,id=net1,mac=52:54:56:5a:5c:5e,disable-legacy=on,\ +disable-modern=off,bus=pci.0,addr=0x7 \ +-net user,vlan=1,name=hostnet1 \ +-device virtio-mouse-pci,id=input0,bus=pci.0,addr=0xe,disable-legacy=on,\ +disable-modern=off \ +-device virtio-keyboard-pci,id=input1,bus=pci.0,addr=0x10,disable-legacy=on,\ +disable-modern=off \ +-device virtio-tablet-pci,id=input2,bus=pci.0,addr=0x11,disable-legacy=on,\ +disable-modern=off \ +-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\ +addr=0x12,disable-legacy=on,disable-modern=off \ +-device virtio-vga,id=video0,virgl=on,bus=pci.0,addr=0x2,disable-legacy=on,\ +disable-modern=off \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,disable-legacy=off,\ +disable-modern=off \ +-object rng-random,id=objrng0,filename=/dev/random \ +-device virtio-rng-pci,rng=objrng0,id=rng0,disable-legacy=on,\ +disable-modern=off,bus=pci.0,addr=0xd diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d594836..c1e964e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2086,6 +2086,17 @@ mymain(void) DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_INTEL_IOMMU);
+ DO_TEST("virtio-revision", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_VIRTIO_INPUT_HOST, + QEMU_CAPS_FSDEV, QEMU_CAPS_FSDEV_WRITEOUT, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL, + QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_RANDOM, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_VIRTIO_REVISION); + qemuTestDriverFree(&driver);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
On 08/08/2016 06:35 PM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1227354
Translate the optional <virtio revision> attribute to disable-legacy=on/off and disable-modern=on/off options for the following devices:
<memballoon> virtio-balloon-pci <disk> virtio-blk-pci <controller> virtio-scsi-pci virtio-serial-pci <filesystem> virtio-9p-pci <interface> virtio-net-pci <rng> virtio-rng-pci <video> virtio-gpu-pci <input> virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci --- src/qemu/qemu_command.c | 56 +++++++++++++++++++ .../qemuxml2argv-virtio-revision.args | 62 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++ 3 files changed, 129 insertions(+)
Although you mention in all above devices the type of bus in the code that differentiation does not really exist. It originates from the way the capability QEMU_CAPS_DEVICE_VIRTIO_REVISION is sensed and set, e.g. virQEMUCapsObjectPropsVirtioBalloon[] is also used for virtio-balloon-ccw devices. I would suggest to create a QEMU_CAPS_DEVICE_VIRTIO_PCI_REVISION and sense the capability for pci only and also generate the command line parameters for pci only since the virtio pci revision handling does not work at all for virtio ccw. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Wed, Aug 10, 2016 at 04:27:58PM +0200, Boris Fiuczynski wrote:
On 08/08/2016 06:35 PM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1227354
Translate the optional <virtio revision> attribute to disable-legacy=on/off and disable-modern=on/off options for the following devices:
<memballoon> virtio-balloon-pci <disk> virtio-blk-pci <controller> virtio-scsi-pci virtio-serial-pci <filesystem> virtio-9p-pci <interface> virtio-net-pci <rng> virtio-rng-pci <video> virtio-gpu-pci <input> virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci --- src/qemu/qemu_command.c | 56 +++++++++++++++++++ .../qemuxml2argv-virtio-revision.args | 62 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++ 3 files changed, 129 insertions(+)
Although you mention in all above devices the type of bus in the code that differentiation does not really exist. It originates from the way the capability QEMU_CAPS_DEVICE_VIRTIO_REVISION is sensed and set, e.g. virQEMUCapsObjectPropsVirtioBalloon[] is also used for virtio-balloon-ccw devices. I would suggest to create a QEMU_CAPS_DEVICE_VIRTIO_PCI_REVISION and sense the capability for pci only and also generate the command line parameters for pci only since the virtio pci revision handling does not work at all for virtio ccw.
I have sent a new version that only probes the capability on PCI devices: https://www.redhat.com/archives/libvir-list/2016-August/msg00592.html Thanks for catching that. Jan
On 08/11/2016 01:59 PM, Ján Tomko wrote:
On Wed, Aug 10, 2016 at 04:27:58PM +0200, Boris Fiuczynski wrote:
On 08/08/2016 06:35 PM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1227354
Translate the optional <virtio revision> attribute to disable-legacy=on/off and disable-modern=on/off options for the following devices:
<memballoon> virtio-balloon-pci <disk> virtio-blk-pci <controller> virtio-scsi-pci virtio-serial-pci <filesystem> virtio-9p-pci <interface> virtio-net-pci <rng> virtio-rng-pci <video> virtio-gpu-pci <input> virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci --- src/qemu/qemu_command.c | 56 +++++++++++++++++++ .../qemuxml2argv-virtio-revision.args | 62 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 11 ++++ 3 files changed, 129 insertions(+)
Although you mention in all above devices the type of bus in the code that differentiation does not really exist. It originates from the way the capability QEMU_CAPS_DEVICE_VIRTIO_REVISION is sensed and set, e.g. virQEMUCapsObjectPropsVirtioBalloon[] is also used for virtio-balloon-ccw devices. I would suggest to create a QEMU_CAPS_DEVICE_VIRTIO_PCI_REVISION and sense the capability for pci only and also generate the command line parameters for pci only since the virtio pci revision handling does not work at all for virtio ccw.
I have sent a new version that only probes the capability on PCI devices: https://www.redhat.com/archives/libvir-list/2016-August/msg00592.html
Thanks for catching that.
Jan Thanks, and another thanks for adding pci-only in the command line generation with this patch: https://www.redhat.com/archives/libvir-list/2016-August/msg00593.html
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (3)
-
Boris Fiuczynski -
Ján Tomko -
Laine Stump