[PATCH 0/7] Fix two bugs in XML schema
Patches 1/7 and 6/7 outline the individual bugs. Peter Krempa (7): schema: nodedev: Fix schema attribute value for the 'vport_ops' capability nodedevschematest: Add example file for a HBA with 'vport_ops' capability qemudomainsnapshotxml2xmltest: Allow regenerating into non-existing output file schemas: Extract overrides for the domain element from 'domain.rng' schemas: domaincommon: Extract contents of the 'domain' element definition schema: Add schema for '<inactiveDomain>' element used in the snapshot definition qemudomainsnapshotxml2xmltest: Add test case for a snapshot with 'inactiveDomain' element docs/formatnode.rst | 2 +- src/conf/schemas/domain.rng | 12 +- src/conf/schemas/domaincommon.rng | 150 ++++++++++-------- src/conf/schemas/domainoverrides.rng | 16 ++ src/conf/schemas/domainsnapshot.rng | 5 + src/conf/schemas/inactiveDomain.rng | 10 ++ src/conf/schemas/nodedev.rng | 2 +- tests/nodedevschemadata/hba_vport_ops.xml | 18 +++ tests/nodedevxml2xmlout/hba_vport_ops.xml | 18 +++ tests/nodedevxml2xmltest.c | 1 + .../memory-snapshot-inactivedomain.xml | 148 +++++++++++++++++ tests/qemudomainsnapshotxml2xmltest.c | 10 +- 12 files changed, 303 insertions(+), 89 deletions(-) create mode 100644 src/conf/schemas/domainoverrides.rng create mode 100644 src/conf/schemas/inactiveDomain.rng create mode 100644 tests/nodedevschemadata/hba_vport_ops.xml create mode 100644 tests/nodedevxml2xmlout/hba_vport_ops.xml create mode 100644 tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml -- 2.37.1
The code originally (since 76ddf2d2e350c3a879819) formatted 'vport_ops'. Later commit cc17f09246212ef added 'vports_ops' into the schema. Fix it and the corresponding docs. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2121262 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatnode.rst | 2 +- src/conf/schemas/nodedev.rng | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatnode.rst b/docs/formatnode.rst index 5903cac7fe..61c2a0965f 100644 --- a/docs/formatnode.rst +++ b/docs/formatnode.rst @@ -250,7 +250,7 @@ Describes a SCSI host device. Sub-elements include: scsi_host adapter in a `Storage Pool <formatstorage.html>`__. :since:`Since 1.2.7` ``capability`` - Current capabilities include "vports_ops" (indicates vport operations + Current capabilities include "vport_ops" (indicates vport operations are supported) and "fc_host". "vport_ops" could contain two optional sub-elements: ``vports``, and ``max_vports``. ``vports`` shows the number of vport in use. ``max_vports`` shows the maximum vports the HBA diff --git a/src/conf/schemas/nodedev.rng b/src/conf/schemas/nodedev.rng index e40243e257..6299c52671 100644 --- a/src/conf/schemas/nodedev.rng +++ b/src/conf/schemas/nodedev.rng @@ -416,7 +416,7 @@ <define name="capsvports"> <attribute name="type"> - <value>vports_ops</value> + <value>vport_ops</value> </attribute> <element name="max_vports"> <ref name="unsignedInt"/> -- 2.37.1
On a Thursday in 2022, Peter Krempa wrote:
The code originally (since 76ddf2d2e350c3a879819) formatted 'vport_ops'.
commit 448be8f706693327d77756fb47aa275edfb14977 was the one adding 'vport_ops' Jano
Later commit cc17f09246212ef added 'vports_ops' into the schema. Fix it and the corresponding docs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2121262 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatnode.rst | 2 +- src/conf/schemas/nodedev.rng | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Illustrate the problem in the schema fixed by previous commit. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/nodedevschemadata/hba_vport_ops.xml | 18 ++++++++++++++++++ tests/nodedevxml2xmlout/hba_vport_ops.xml | 18 ++++++++++++++++++ tests/nodedevxml2xmltest.c | 1 + 3 files changed, 37 insertions(+) create mode 100644 tests/nodedevschemadata/hba_vport_ops.xml create mode 100644 tests/nodedevxml2xmlout/hba_vport_ops.xml diff --git a/tests/nodedevschemadata/hba_vport_ops.xml b/tests/nodedevschemadata/hba_vport_ops.xml new file mode 100644 index 0000000000..4abc2e5161 --- /dev/null +++ b/tests/nodedevschemadata/hba_vport_ops.xml @@ -0,0 +1,18 @@ +<device> + <name>scsi_host12</name> + <path>/sys/devices/pci0000:00/0000:00:03.2/0000:06:00.1/host12</path> + <parent>pci_0000_06_00_1</parent> + <capability type='scsi_host'> + <host>12</host> + <unique_id>12</unique_id> + <capability type='fc_host'> + <wwnn>2000f4e9d4eb02c9</wwnn> + <wwpn>2001f4e9d4eb02c9</wwpn> + <fabric_wwn>2001547feebe79c1</fabric_wwn> + </capability> + <capability type='vport_ops'> + <max_vports>254</max_vports> + <vports>0</vports> + </capability> + </capability> +</device> diff --git a/tests/nodedevxml2xmlout/hba_vport_ops.xml b/tests/nodedevxml2xmlout/hba_vport_ops.xml new file mode 100644 index 0000000000..6a680d467e --- /dev/null +++ b/tests/nodedevxml2xmlout/hba_vport_ops.xml @@ -0,0 +1,18 @@ +<device> + <name>scsi_host12</name> + <path>/sys/devices/pci0000:00/0000:00:03.2/0000:06:00.1/host12</path> + <parent>pci_0000_06_00_1</parent> + <capability type='scsi_host'> + <host>12</host> + <unique_id>12</unique_id> + <capability type='fc_host'> + <wwnn>2000f4e9d4eb02c9</wwnn> + <wwpn>2001f4e9d4eb02c9</wwpn> + <fabric_wwn>2001547feebe79c1</fabric_wwn> + </capability> + <capability type='vport_ops'> + <max_vports>0</max_vports> + <vports>0</vports> + </capability> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index f82027fb87..68a4041d8c 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -134,6 +134,7 @@ mymain(void) DO_TEST("mdev_d069d019_36ea_4111_8f0a_8c9a70e21366"); DO_TEST("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9"); DO_TEST("mdev_fedc4916_1ca8_49ac_b176_871d16c13076"); + DO_TEST("hba_vport_ops"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.37.1
Replacing the 'virTestLoadFile' + 'virTestDifferenceFull' by ' virTestCompareToFile' allows to use the VIR_TEST_REGENERATE_OUTPUT=1 option to also generate the output file if it doesn't exist. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemudomainsnapshotxml2xmltest.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/qemudomainsnapshotxml2xmltest.c b/tests/qemudomainsnapshotxml2xmltest.c index 2f66839ae8..6881d692ee 100644 --- a/tests/qemudomainsnapshotxml2xmltest.c +++ b/tests/qemudomainsnapshotxml2xmltest.c @@ -30,7 +30,6 @@ testCompareXMLToXMLFiles(const char *inxml, unsigned int flags) { g_autofree char *inXmlData = NULL; - g_autofree char *outXmlData = NULL; g_autofree char *actual = NULL; unsigned int parseflags = 0; unsigned int formatflags = VIR_DOMAIN_SNAPSHOT_FORMAT_SECURE; @@ -48,9 +47,6 @@ testCompareXMLToXMLFiles(const char *inxml, if (virTestLoadFile(inxml, &inXmlData) < 0) return -1; - if (virTestLoadFile(outxml, &outXmlData) < 0) - return -1; - if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.xmlopt, NULL, &cur, parseflags))) @@ -71,10 +67,8 @@ testCompareXMLToXMLFiles(const char *inxml, formatflags))) return -1; - if (STRNEQ(outXmlData, actual)) { - virTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml); + if (virTestCompareToFile(actual, outxml) < 0) return -1; - } return 0; } -- 2.37.1
Move the overrides into a single file so that later patches can add another top level element 'inactiveDomain' used in snapshots. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/schemas/domain.rng | 12 +----------- src/conf/schemas/domainoverrides.rng | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 src/conf/schemas/domainoverrides.rng diff --git a/src/conf/schemas/domain.rng b/src/conf/schemas/domain.rng index b93bbed959..adc4386a27 100644 --- a/src/conf/schemas/domain.rng +++ b/src/conf/schemas/domain.rng @@ -6,16 +6,6 @@ <ref name="domain"/> </start> - <include href="domaincommon.rng"/> - - <define name="storageStartupPolicy" combine="choice"> - <!-- overrides the no-op version in storagecommon.rng --> - <ref name="startupPolicy"/> - </define> - - <define name="storageSourceExtra" combine="choice"> - <!-- overrides the no-op version in storagecommon.rng --> - <ref name="diskspec"/> - </define> + <include href="domainoverrides.rng"/> </grammar> diff --git a/src/conf/schemas/domainoverrides.rng b/src/conf/schemas/domainoverrides.rng new file mode 100644 index 0000000000..13b18a28ea --- /dev/null +++ b/src/conf/schemas/domainoverrides.rng @@ -0,0 +1,16 @@ +<?xml version="1.0"?> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <!-- Overrides for the no-op versions of storage elements for use with domain XMLs. --> + <include href="domaincommon.rng"/> + + <define name="storageStartupPolicy" combine="choice"> + <!-- overrides the no-op version in storagecommon.rng --> + <ref name="startupPolicy"/> + </define> + + <define name="storageSourceExtra" combine="choice"> + <!-- overrides the no-op version in storagecommon.rng --> + <ref name="diskspec"/> + </define> + +</grammar> -- 2.37.1
On 8/25/22 11:46, Peter Krempa wrote:
Move the overrides into a single file so that later patches can add another top level element 'inactiveDomain' used in snapshots.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/schemas/domain.rng | 12 +----------- src/conf/schemas/domainoverrides.rng | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 src/conf/schemas/domainoverrides.rng
Don't forget to list this new file in meson.build: diff --git i/src/conf/schemas/meson.build w/src/conf/schemas/meson.build index 700161bf75..1bb09b4c5d 100644 --- i/src/conf/schemas/meson.build +++ w/src/conf/schemas/meson.build @@ -8,6 +8,7 @@ schema_files = [ 'domaincheckpoint.rng', 'domaincommon.rng', 'domain.rng', + 'domainoverrides.rng', 'domainsnapshot.rng', 'interface.rng', 'networkcommon.rng', Michal
Move all definition under the <domain> element into a separate definition so that it can be referenced from elements with other names. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/schemas/domaincommon.rng | 143 ++++++++++++++++-------------- 1 file changed, 74 insertions(+), 69 deletions(-) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 7f6ea1d888..cc6a3475c8 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -33,77 +33,82 @@ --> <define name="domain"> <element name="domain"> - <ref name="hvs"/> - <interleave> - <ref name="ids"/> - <optional> - <ref name="title"/> - </optional> - <optional> - <ref name="description"/> - </optional> - <optional> - <ref name="metadata"/> - </optional> - <optional> - <ref name="guestcpu"/> - </optional> - <zeroOrMore> - <ref name="sysinfo"/> - </zeroOrMore> - <ref name="os"/> - <ref name="clock"/> - <ref name="resources"/> - <ref name="features"/> - <ref name="events"/> - <optional> - <ref name="pm"/> - </optional> - <optional> - <ref name="perf"/> - </optional> - <optional> - <ref name="idmap"/> - </optional> - <optional> - <ref name="devices"/> - </optional> - <zeroOrMore> - <ref name="seclabel"/> - </zeroOrMore> - <optional> - <ref name="qemucmdline"/> - </optional> - <optional> - <ref name="qemucapabilities"/> - </optional> - <optional> - <ref name="qemudeprecation"/> - </optional> - <optional> - <ref name="qemuoverride"/> - </optional> - <optional> - <ref name="lxcsharens"/> - </optional> - <optional> - <ref name="keywrap"/> - </optional> - <optional> - <ref name="launchSecurity"/> - </optional> - <optional> - <ref name="bhyvecmdline"/> - </optional> - <optional> - <ref name="xencmdline"/> - </optional> - <optional> - <ref name="vmwaredatacenterpath"/> - </optional> - </interleave> + <ref name="domaincontents"/> </element> </define> + + <define name="domaincontents"> + <ref name="hvs"/> + <interleave> + <ref name="ids"/> + <optional> + <ref name="title"/> + </optional> + <optional> + <ref name="description"/> + </optional> + <optional> + <ref name="metadata"/> + </optional> + <optional> + <ref name="guestcpu"/> + </optional> + <zeroOrMore> + <ref name="sysinfo"/> + </zeroOrMore> + <ref name="os"/> + <ref name="clock"/> + <ref name="resources"/> + <ref name="features"/> + <ref name="events"/> + <optional> + <ref name="pm"/> + </optional> + <optional> + <ref name="perf"/> + </optional> + <optional> + <ref name="idmap"/> + </optional> + <optional> + <ref name="devices"/> + </optional> + <zeroOrMore> + <ref name="seclabel"/> + </zeroOrMore> + <optional> + <ref name="qemucmdline"/> + </optional> + <optional> + <ref name="qemucapabilities"/> + </optional> + <optional> + <ref name="qemudeprecation"/> + </optional> + <optional> + <ref name="qemuoverride"/> + </optional> + <optional> + <ref name="lxcsharens"/> + </optional> + <optional> + <ref name="keywrap"/> + </optional> + <optional> + <ref name="launchSecurity"/> + </optional> + <optional> + <ref name="bhyvecmdline"/> + </optional> + <optional> + <ref name="xencmdline"/> + </optional> + <optional> + <ref name="vmwaredatacenterpath"/> + </optional> + </interleave> + </define> + <define name="seclabel"> <element name="seclabel"> <optional> -- 2.37.1
The '<inactiveDomain>' element stores the next-start definition of a VM on snapshot. It was not covered by the schema when it was introduced. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2121276 Fixes: 152c165d34cb6dcd21d08427422850f406cd0643 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/schemas/domaincommon.rng | 7 +++++++ src/conf/schemas/domainsnapshot.rng | 5 +++++ src/conf/schemas/inactiveDomain.rng | 10 ++++++++++ 3 files changed, 22 insertions(+) create mode 100644 src/conf/schemas/inactiveDomain.rng diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index cc6a3475c8..ae04e0ea7d 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -37,6 +37,13 @@ </element> </define> + <!-- this element is used as a child of a snapshot definition --> + <define name="inactiveDomain"> + <element name="inactiveDomain"> + <ref name="domaincontents"/> + </element> + </define> + <define name="domaincontents"> <ref name="hvs"/> <interleave> diff --git a/src/conf/schemas/domainsnapshot.rng b/src/conf/schemas/domainsnapshot.rng index a5d1a40493..3db9f458ba 100644 --- a/src/conf/schemas/domainsnapshot.rng +++ b/src/conf/schemas/domainsnapshot.rng @@ -83,6 +83,11 @@ </grammar> </choice> </optional> + <optional> + <grammar> + <include href="inactiveDomain.rng"/> + </grammar> + </optional> <optional> <element name="parent"> <element name="name"> diff --git a/src/conf/schemas/inactiveDomain.rng b/src/conf/schemas/inactiveDomain.rng new file mode 100644 index 0000000000..ae1207d978 --- /dev/null +++ b/src/conf/schemas/inactiveDomain.rng @@ -0,0 +1,10 @@ +<?xml version="1.0"?> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + <!-- inactiveDomain element grammar is included into domainsnapshot.rng --> + <start> + <ref name="inactiveDomain"/> + </start> + + <include href="domainoverrides.rng"/> + +</grammar> -- 2.37.1
On 8/25/22 11:46, Peter Krempa wrote:
The '<inactiveDomain>' element stores the next-start definition of a VM on snapshot. It was not covered by the schema when it was introduced.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2121276 Fixes: 152c165d34cb6dcd21d08427422850f406cd0643 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/schemas/domaincommon.rng | 7 +++++++ src/conf/schemas/domainsnapshot.rng | 5 +++++ src/conf/schemas/inactiveDomain.rng | 10 ++++++++++ 3 files changed, 22 insertions(+) create mode 100644 src/conf/schemas/inactiveDomain.rng
Again, meson.build might be also interested in learning about this new file: diff --git i/src/conf/schemas/meson.build w/src/conf/schemas/meson.build index 1bb09b4c5d..9ec4c010d6 100644 --- i/src/conf/schemas/meson.build +++ w/src/conf/schemas/meson.build @@ -10,6 +10,7 @@ schema_files = [ 'domain.rng', 'domainoverrides.rng', 'domainsnapshot.rng', + 'inactiveDomain.rng', 'interface.rng', 'networkcommon.rng', 'networkport.rng', Michal
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../memory-snapshot-inactivedomain.xml | 148 ++++++++++++++++++ tests/qemudomainsnapshotxml2xmltest.c | 2 + 2 files changed, 150 insertions(+) create mode 100644 tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml diff --git a/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml b/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml new file mode 100644 index 0000000000..3acc6d952b --- /dev/null +++ b/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml @@ -0,0 +1,148 @@ +<domainsnapshot> + <name>test-snap</name> + <state>running</state> + <creationTime>1661417356</creationTime> + <memory snapshot='external' file='/tmp/ble.img'/> + <domain type='kvm'> + <name>snapshot-inactivedomain</name> + <uuid>14beef2c-8cae-4ea8-bf55-e48fe0cd4b73</uuid> + <memory unit='KiB'>1024000</memory> + <currentMemory unit='KiB'>1024000</currentMemory> + <vcpu placement='static' current='1'>8</vcpu> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64' machine='pc-i440fx-7.0'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x2'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </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='0x09' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> + <seclabel type='dynamic' model='selinux' relabel='yes'/> + </domain> + <inactiveDomain type='kvm'> + <name>snapshot-inactivedomain</name> + <uuid>14beef2c-8cae-4ea8-bf55-e48fe0cd4b73</uuid> + <memory unit='KiB'>1024000</memory> + <currentMemory unit='KiB'>1024000</currentMemory> + <vcpu placement='static' current='1'>8</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-7.0'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </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='0x09' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> + </inactiveDomain> + <cookie> + <cpu mode='custom' match='exact' check='full'> + <model fallback='forbid'>qemu64</model> + <feature policy='require' name='x2apic'/> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='lahf_lm'/> + <feature policy='disable' name='svm'/> + </cpu> + <slirpHelper/> + </cookie> +</domainsnapshot> diff --git a/tests/qemudomainsnapshotxml2xmltest.c b/tests/qemudomainsnapshotxml2xmltest.c index 6881d692ee..a09404a9d5 100644 --- a/tests/qemudomainsnapshotxml2xmltest.c +++ b/tests/qemudomainsnapshotxml2xmltest.c @@ -160,6 +160,8 @@ mymain(void) DO_TEST_OUT("external_vm_redefine", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 0); + DO_TEST_OUT("memory-snapshot-inactivedomain", "14beef2c-8cae-4ea8-bf55-e48fe0cd4b73", 0); + DO_TEST_INOUT("empty", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 1386166249, 0); DO_TEST_INOUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", -- 2.37.1
On Thu, Aug 25, 2022 at 11:46:10AM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../memory-snapshot-inactivedomain.xml | 148 ++++++++++++++++++ tests/qemudomainsnapshotxml2xmltest.c | 2 + 2 files changed, 150 insertions(+) create mode 100644 tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml
snip
diff --git a/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml b/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml new file mode 100644 index 0000000000..3acc6d952b --- /dev/null +++ b/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml @@ -0,0 +1,148 @@
+ <cookie> + <cpu mode='custom' match='exact' check='full'> + <model fallback='forbid'>qemu64</model> + <feature policy='require' name='x2apic'/> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='lahf_lm'/> + <feature policy='disable' name='svm'/> + </cpu> + <slirpHelper/> + </cookie> +</domainsnapshot>
As best I can tell this <cookie> element is undocumented and further contains arbitrary undocumented hypervisor specific XML content :-( This is really horrible and really should have been hidden behind a driver specific XML namespace. Can someone explain more about the <cookie> ? If this is a strictly output only element, which applications are not expected to parse or interpret, just pass along "as is", then it is possible we can fix/improve this, to clearly demarcate what is our public XML schema and what is opaque driver specific schema. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Sep 26, 2022 at 12:07:07 +0100, Daniel P. Berrangé wrote:
On Thu, Aug 25, 2022 at 11:46:10AM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../memory-snapshot-inactivedomain.xml | 148 ++++++++++++++++++ tests/qemudomainsnapshotxml2xmltest.c | 2 + 2 files changed, 150 insertions(+) create mode 100644 tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml
snip
diff --git a/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml b/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml new file mode 100644 index 0000000000..3acc6d952b --- /dev/null +++ b/tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml @@ -0,0 +1,148 @@
+ <cookie> + <cpu mode='custom' match='exact' check='full'> + <model fallback='forbid'>qemu64</model> + <feature policy='require' name='x2apic'/> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='lahf_lm'/> + <feature policy='disable' name='svm'/> + </cpu> + <slirpHelper/> + </cookie> +</domainsnapshot>
As best I can tell this <cookie> element is undocumented and further contains arbitrary undocumented hypervisor specific XML content :-(
This is really horrible and really should have been hidden behind a driver specific XML namespace.
Can someone explain more about the <cookie> ?
AFAIK it's something akin to the migration cookie, but for the save image/snapshot it's stored inside the snapshot XML, rather than passed via a side-band. IIUC it's required also in the snapshot XML (not just in the save image XML) as internal snapshots don't store the XML in-band.
If this is a strictly output only element, which applications are not expected to parse or interpret, just pass along "as is", then it is possible we can fix/improve this, to clearly demarcate what is our public XML schema and what is opaque driver specific schema.
It's indeed private and driver specific. There's a driver specific callback to format it.
On a Thursday in 2022, Peter Krempa wrote:
Patches 1/7 and 6/7 outline the individual bugs.
Peter Krempa (7): schema: nodedev: Fix schema attribute value for the 'vport_ops' capability nodedevschematest: Add example file for a HBA with 'vport_ops' capability qemudomainsnapshotxml2xmltest: Allow regenerating into non-existing output file schemas: Extract overrides for the domain element from 'domain.rng' schemas: domaincommon: Extract contents of the 'domain' element definition schema: Add schema for '<inactiveDomain>' element used in the snapshot definition qemudomainsnapshotxml2xmltest: Add test case for a snapshot with 'inactiveDomain' element
docs/formatnode.rst | 2 +- src/conf/schemas/domain.rng | 12 +- src/conf/schemas/domaincommon.rng | 150 ++++++++++-------- src/conf/schemas/domainoverrides.rng | 16 ++ src/conf/schemas/domainsnapshot.rng | 5 + src/conf/schemas/inactiveDomain.rng | 10 ++ src/conf/schemas/nodedev.rng | 2 +- tests/nodedevschemadata/hba_vport_ops.xml | 18 +++ tests/nodedevxml2xmlout/hba_vport_ops.xml | 18 +++ tests/nodedevxml2xmltest.c | 1 + .../memory-snapshot-inactivedomain.xml | 148 +++++++++++++++++ tests/qemudomainsnapshotxml2xmltest.c | 10 +- 12 files changed, 303 insertions(+), 89 deletions(-) create mode 100644 src/conf/schemas/domainoverrides.rng create mode 100644 src/conf/schemas/inactiveDomain.rng create mode 100644 tests/nodedevschemadata/hba_vport_ops.xml create mode 100644 tests/nodedevxml2xmlout/hba_vport_ops.xml create mode 100644 tests/qemudomainsnapshotxml2xmlout/memory-snapshot-inactivedomain.xml
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (4)
-
Daniel P. Berrangé -
Ján Tomko -
Michal Prívozník -
Peter Krempa