[libvirt] [PATCH 00/12] Centralize more more bits in XML PostParse
There are several qemu and generic domain XML validating/populating routines that callers need to invoke manually after parsing XML. This series moves some of these calls into the PostParse handling routines. Functionally there shouldn't be much change, except for more complete XML in a few cases where drivers convert their native config formats. Most of the patches are just moving functions around and verifying things don't break, but there's some test suite improvements sprinkled in. The initial motivation for this series is to put qemuDomainAssignAddresses into the PostParse call path, since I want that for future patches tp autofill a user requested <address type='pci'/> Cole Robinson (12): domain: separate out function for post parse console compat domain: separate out function for post parse timer validation domain: add implicit controllers from post parse domain: Add virDomainDefAddImplicitDevices qemu: domain: split out post parse default device handling qemu: Handle CanonicalizeMachine in post parse qemu: Handle SecurityManagerVerify in post parse domain: conf: Export virDomainDefPostParseDevices tests: qemuxml2xml: Use standard file comparison helpers tests: qemuxml2xml: Convert some fprintf to VIR_TEST_DEBUG tests: qemuxml2xml: Wire up QEMUCaps usage qemu: Assign device addresses in PostParse src/conf/domain_conf.c | 104 ++++++++++++++------- src/conf/domain_conf.h | 6 +- src/libvirt_private.syms | 3 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 89 ++++++++++++++---- src/qemu/qemu_driver.c | 50 +--------- src/vmx/vmx.c | 2 +- src/vz/vz_sdk.c | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 4 +- tests/qemuxml2argvtest.c | 12 +-- .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +- .../qemuxml2xmlout-disk-scsi-vscsi.xml | 35 +++++++ .../qemuxml2xmlout-panic-pseries.xml | 30 ++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 4 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 4 +- tests/qemuxml2xmltest.c | 39 +++++--- tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 1 + .../sexpr2xml-fv-serial-dev-2-ports.xml | 1 + .../sexpr2xml-fv-serial-dev-2nd-port.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 1 + .../sexpr2xml-fv-serial-tcp-telnet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv.xml | 1 + tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 1 + .../test-fullvirt-direct-kernel-boot.xml | 1 + tests/xlconfigdata/test-fullvirt-multiusb.xml | 1 + tests/xlconfigdata/test-new-disk.xml | 1 + tests/xlconfigdata/test-spice-features.xml | 1 + tests/xlconfigdata/test-spice.xml | 1 + tests/xmconfigdata/test-escape-paths.xml | 1 + .../xmconfigdata/test-fullvirt-default-feature.xml | 1 + tests/xmconfigdata/test-fullvirt-force-hpet.xml | 1 + tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 1 + tests/xmconfigdata/test-fullvirt-localtime.xml | 1 + tests/xmconfigdata/test-fullvirt-net-netfront.xml | 1 + tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 1 + tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 1 + .../test-fullvirt-serial-dev-2-ports.xml | 1 + .../test-fullvirt-serial-dev-2nd-port.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-file.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-null.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-pty.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 1 + .../test-fullvirt-serial-tcp-telnet.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-tcp.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-udp.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-unix.xml | 1 + tests/xmconfigdata/test-fullvirt-sound.xml | 1 + tests/xmconfigdata/test-fullvirt-usbmouse.xml | 1 + tests/xmconfigdata/test-fullvirt-usbtablet.xml | 1 + tests/xmconfigdata/test-fullvirt-utc.xml | 1 + tests/xmconfigdata/test-no-source-cdrom.xml | 1 + tests/xmconfigdata/test-pci-devs.xml | 1 + 72 files changed, 315 insertions(+), 136 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml -- 2.5.0
This should be a no-op --- src/conf/domain_conf.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9d47846..ab22322 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3664,24 +3664,11 @@ virDomainDefPostParseMemory(virDomainDefPtr def, return 0; } - static int -virDomainDefPostParseInternal(virDomainDefPtr def, - virCapsPtr caps ATTRIBUTE_UNUSED, - unsigned int parseFlags) +virDomainDefAddConsoleCompat(virDomainDefPtr def) { size_t i; - /* verify init path for container based domains */ - if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("init binary must be specified")); - return -1; - } - - if (virDomainDefPostParseMemory(def, parseFlags) < 0) - return -1; - /* * Some really crazy backcompat stuff for consoles * @@ -3774,6 +3761,29 @@ virDomainDefPostParseInternal(virDomainDefPtr def, def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } + return 0; +} + +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags) +{ + size_t i; + + /* verify init path for container based domains */ + if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + return -1; + } + + if (virDomainDefPostParseMemory(def, parseFlags) < 0) + return -1; + + if (virDomainDefAddConsoleCompat(def) < 0) + return -1; + if (virDomainDefRejectDuplicateControllers(def) < 0) return -1; -- 2.5.0
On Thu, Jan 07, 2016 at 22:49:55 -0500, Cole Robinson wrote:
This should be a no-op --- src/conf/domain_conf.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9d47846..ab22322 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3664,24 +3664,11 @@ virDomainDefPostParseMemory(virDomainDefPtr def, return 0; }
-
This file uses two newlines to separate functions ..
static int -virDomainDefPostParseInternal(virDomainDefPtr def, - virCapsPtr caps ATTRIBUTE_UNUSED, - unsigned int parseFlags) +virDomainDefAddConsoleCompat(virDomainDefPtr def) { size_t i;
- /* verify init path for container based domains */ - if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("init binary must be specified")); - return -1; - } - - if (virDomainDefPostParseMemory(def, parseFlags) < 0) - return -1; - /* * Some really crazy backcompat stuff for consoles * @@ -3774,6 +3761,29 @@ virDomainDefPostParseInternal(virDomainDefPtr def, def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; }
+ return 0; +}
... here too ...
+ +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags) +{ + size_t i; + + /* verify init path for container based domains */ + if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + return -1; + } + + if (virDomainDefPostParseMemory(def, parseFlags) < 0) + return -1; + + if (virDomainDefAddConsoleCompat(def) < 0) + return -1; + if (virDomainDefRejectDuplicateControllers(def) < 0) return -1;
ACK with whitespace fixed.
On 01/08/2016 02:00 AM, Peter Krempa wrote:
On Thu, Jan 07, 2016 at 22:49:55 -0500, Cole Robinson wrote:
This should be a no-op --- src/conf/domain_conf.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9d47846..ab22322 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3664,24 +3664,11 @@ virDomainDefPostParseMemory(virDomainDefPtr def, return 0; }
-
This file uses two newlines to separate functions ..
static int -virDomainDefPostParseInternal(virDomainDefPtr def, - virCapsPtr caps ATTRIBUTE_UNUSED, - unsigned int parseFlags) +virDomainDefAddConsoleCompat(virDomainDefPtr def) { size_t i;
- /* verify init path for container based domains */ - if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("init binary must be specified")); - return -1; - } - - if (virDomainDefPostParseMemory(def, parseFlags) < 0) - return -1; - /* * Some really crazy backcompat stuff for consoles * @@ -3774,6 +3761,29 @@ virDomainDefPostParseInternal(virDomainDefPtr def, def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; }
+ return 0; +}
... here too ...
+ +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags) +{ + size_t i; + + /* verify init path for container based domains */ + if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + return -1; + } + + if (virDomainDefPostParseMemory(def, parseFlags) < 0) + return -1; + + if (virDomainDefAddConsoleCompat(def) < 0) + return -1; + if (virDomainDefRejectDuplicateControllers(def) < 0) return -1;
ACK with whitespace fixed.
I pushed patches 1, 2, 5, 6, 7, and 10, with the whitespace bits fixed. Thanks for the review. I'll follow up on the other bits later - Cole
This should be a no-op --- src/conf/domain_conf.c | 53 +++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ab22322..2570f94 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3765,31 +3765,10 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) } static int -virDomainDefPostParseInternal(virDomainDefPtr def, - virCapsPtr caps ATTRIBUTE_UNUSED, - unsigned int parseFlags) +virDomainDefPostParseTimer(virDomainDefPtr def) { size_t i; - /* verify init path for container based domains */ - if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("init binary must be specified")); - return -1; - } - - if (virDomainDefPostParseMemory(def, parseFlags) < 0) - return -1; - - if (virDomainDefAddConsoleCompat(def) < 0) - return -1; - - if (virDomainDefRejectDuplicateControllers(def) < 0) - return -1; - - if (virDomainDefRejectDuplicatePanics(def) < 0) - return -1; - /* verify settings of guest timers */ for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDefPtr timer = def->clock.timers[i]; @@ -3845,6 +3824,36 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } } + return 0; +} + +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags) +{ + /* verify init path for container based domains */ + if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + return -1; + } + + if (virDomainDefPostParseMemory(def, parseFlags) < 0) + return -1; + + if (virDomainDefAddConsoleCompat(def) < 0) + return -1; + + if (virDomainDefRejectDuplicateControllers(def) < 0) + return -1; + + if (virDomainDefRejectDuplicatePanics(def) < 0) + return -1; + + if (virDomainDefPostParseTimer(def) < 0) + return -1; + /* clean up possibly duplicated metadata entries */ virDomainDefMetadataSanitize(def); -- 2.5.0
On Thu, Jan 07, 2016 at 22:49:56 -0500, Cole Robinson wrote:
This should be a no-op --- src/conf/domain_conf.c | 53 +++++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ab22322..2570f94 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[...]
@@ -3845,6 +3824,36 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } }
+ return 0; +}
Two newlines please
+ +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + unsigned int parseFlags) +{ + /* verify init path for container based domains */ + if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + return -1;
ACK
Seems like the natural fit, since we are already adding other XML bits in the PostParse routine. Previously AddImplicitControllers was only called at the end of XML parsing, meaning code that builds a DomainDef by hand had to manually call it. Adding it for those sites causes some test suite churn. --- src/conf/domain_conf.c | 7 +++---- tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv-v2.xml | 1 + tests/sexpr2xmldata/sexpr2xml-fv.xml | 1 + tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 1 + tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml | 1 + tests/xlconfigdata/test-fullvirt-multiusb.xml | 1 + tests/xlconfigdata/test-new-disk.xml | 1 + tests/xlconfigdata/test-spice-features.xml | 1 + tests/xlconfigdata/test-spice.xml | 1 + tests/xmconfigdata/test-escape-paths.xml | 1 + tests/xmconfigdata/test-fullvirt-default-feature.xml | 1 + tests/xmconfigdata/test-fullvirt-force-hpet.xml | 1 + tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 1 + tests/xmconfigdata/test-fullvirt-localtime.xml | 1 + tests/xmconfigdata/test-fullvirt-net-netfront.xml | 1 + tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 1 + tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-file.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-null.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-pty.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-tcp.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-udp.xml | 1 + tests/xmconfigdata/test-fullvirt-serial-unix.xml | 1 + tests/xmconfigdata/test-fullvirt-sound.xml | 1 + tests/xmconfigdata/test-fullvirt-usbmouse.xml | 1 + tests/xmconfigdata/test-fullvirt-usbtablet.xml | 1 + tests/xmconfigdata/test-fullvirt-utc.xml | 1 + tests/xmconfigdata/test-no-source-cdrom.xml | 1 + tests/xmconfigdata/test-pci-devs.xml | 1 + 57 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2570f94..5b9dab9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3854,6 +3854,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1; + if (virDomainDefAddImplicitControllers(def) < 0) + return -1; + /* clean up possibly duplicated metadata entries */ virDomainDefMetadataSanitize(def); @@ -16396,10 +16399,6 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainDefPostParse(def, caps, flags, xmlopt) < 0) goto error; - /* Auto-add any implied controllers which aren't present */ - if (virDomainDefAddImplicitControllers(def) < 0) - goto error; - virHashFree(bootHash); return def; diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml index d68782d..17522f1 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:e8:18'/> <source bridge='e1000g0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml b/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml index dbfd603..00907fe 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml index 8aa3250..7e014dc 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml @@ -35,6 +35,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml index fbf155c..1fa8c14 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml @@ -35,6 +35,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml index 1d41979..d82ca38 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml index c0a78e1..d5479dc 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml index 1268416..91afcc1 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml index 4c1c5ce..b5bdf5c 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2-ports.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml index 8cd0f69..aca8f15 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-dev-2nd-port.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml index 1b7ab29..ac754a2 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml index 73361d6..3ed8c77 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml index c96df26..649b5e6 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml index af788fe..ef84402 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml index 6a21390..07bd1fa 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml index 3033685..cd3031b 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp-telnet.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml index 78fbe97..ab23196 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml index f7497f6..fe89b9a 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml b/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml index 3728609..344ea42 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml index 823e402..caa856f 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml index 823e402..caa856f 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-sound.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml b/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml index 446b4ac..bf710ec 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml b/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml index c2671f6..8e9fd80 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml b/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml index 3073806..1d81991 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-utc.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml index 3073806..1d81991 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv-v2.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-fv.xml b/tests/sexpr2xmldata/sexpr2xml-fv.xml index 3073806..1d81991 100644 --- a/tests/sexpr2xmldata/sexpr2xml-fv.xml +++ b/tests/sexpr2xmldata/sexpr2xml-fv.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:1b:b1:47'/> <source bridge='xenbr0'/> diff --git a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml index ede6e27..310279d 100644 --- a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml +++ b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:0a:7b:39'/> <source bridge='xenbr0'/> diff --git a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml index f750e02..1a27be6 100644 --- a/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml +++ b/tests/xlconfigdata/test-fullvirt-direct-kernel-boot.xml @@ -36,6 +36,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.xml b/tests/xlconfigdata/test-fullvirt-multiusb.xml index 9e5cad9..1686807 100644 --- a/tests/xlconfigdata/test-fullvirt-multiusb.xml +++ b/tests/xlconfigdata/test-fullvirt-multiusb.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xlconfigdata/test-new-disk.xml b/tests/xlconfigdata/test-new-disk.xml index 49f6dbe..7fd1899 100644 --- a/tests/xlconfigdata/test-new-disk.xml +++ b/tests/xlconfigdata/test-new-disk.xml @@ -39,6 +39,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xlconfigdata/test-spice-features.xml b/tests/xlconfigdata/test-spice-features.xml index d40a671..0f81f3d 100644 --- a/tests/xlconfigdata/test-spice-features.xml +++ b/tests/xlconfigdata/test-spice-features.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xlconfigdata/test-spice.xml b/tests/xlconfigdata/test-spice.xml index d15557e..e997654 100644 --- a/tests/xlconfigdata/test-spice.xml +++ b/tests/xlconfigdata/test-spice.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-escape-paths.xml b/tests/xmconfigdata/test-escape-paths.xml index db01910..6aae9c7 100644 --- a/tests/xmconfigdata/test-escape-paths.xml +++ b/tests/xmconfigdata/test-escape-paths.xml @@ -39,6 +39,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml b/tests/xmconfigdata/test-fullvirt-default-feature.xml index 231aea6..2eacbd9 100644 --- a/tests/xmconfigdata/test-fullvirt-default-feature.xml +++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml @@ -35,6 +35,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-force-hpet.xml b/tests/xmconfigdata/test-fullvirt-force-hpet.xml index 231aea6..2eacbd9 100644 --- a/tests/xmconfigdata/test-fullvirt-force-hpet.xml +++ b/tests/xmconfigdata/test-fullvirt-force-hpet.xml @@ -35,6 +35,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-force-nohpet.xml b/tests/xmconfigdata/test-fullvirt-force-nohpet.xml index ef216c3..987096d 100644 --- a/tests/xmconfigdata/test-fullvirt-force-nohpet.xml +++ b/tests/xmconfigdata/test-fullvirt-force-nohpet.xml @@ -35,6 +35,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-localtime.xml b/tests/xmconfigdata/test-fullvirt-localtime.xml index 0162246..29fcd56 100644 --- a/tests/xmconfigdata/test-fullvirt-localtime.xml +++ b/tests/xmconfigdata/test-fullvirt-localtime.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-net-netfront.xml b/tests/xmconfigdata/test-fullvirt-net-netfront.xml index 172a530..621955d 100644 --- a/tests/xmconfigdata/test-fullvirt-net-netfront.xml +++ b/tests/xmconfigdata/test-fullvirt-net-netfront.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-new-cdrom.xml b/tests/xmconfigdata/test-fullvirt-new-cdrom.xml index 6544cb3..bc9865b 100644 --- a/tests/xmconfigdata/test-fullvirt-new-cdrom.xml +++ b/tests/xmconfigdata/test-fullvirt-new-cdrom.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml b/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml index 5aef2de..081ad74 100644 --- a/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml +++ b/tests/xmconfigdata/test-fullvirt-parallel-tcp.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml b/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml index a9e68b5..074cbcb 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-dev-2-ports.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml b/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml index 5564fa8..88a0293 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-dev-2nd-port.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-file.xml b/tests/xmconfigdata/test-fullvirt-serial-file.xml index f592d44..7ac994f 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-file.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-file.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-null.xml b/tests/xmconfigdata/test-fullvirt-serial-null.xml index 78142a7..bc47c4b 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-null.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-null.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-pipe.xml b/tests/xmconfigdata/test-fullvirt-serial-pipe.xml index d3f0653..afaa64f 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-pipe.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-pipe.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-pty.xml b/tests/xmconfigdata/test-fullvirt-serial-pty.xml index bc8e182..ac68710 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-pty.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-pty.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-stdio.xml b/tests/xmconfigdata/test-fullvirt-serial-stdio.xml index 9244a7c..cf30786 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-stdio.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-stdio.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml b/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml index 2ffdc8a..4e943c1 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-tcp-telnet.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-tcp.xml b/tests/xmconfigdata/test-fullvirt-serial-tcp.xml index ff7ec9b..f752be8 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-tcp.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-tcp.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-udp.xml b/tests/xmconfigdata/test-fullvirt-serial-udp.xml index f2f2b1f..d70c5d8 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-udp.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-udp.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-serial-unix.xml b/tests/xmconfigdata/test-fullvirt-serial-unix.xml index eaa484b..9900179 100644 --- a/tests/xmconfigdata/test-fullvirt-serial-unix.xml +++ b/tests/xmconfigdata/test-fullvirt-serial-unix.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-sound.xml b/tests/xmconfigdata/test-fullvirt-sound.xml index 6ee3dfd..abbe8df 100644 --- a/tests/xmconfigdata/test-fullvirt-sound.xml +++ b/tests/xmconfigdata/test-fullvirt-sound.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-usbmouse.xml b/tests/xmconfigdata/test-fullvirt-usbmouse.xml index 5d739f1..460ecc7 100644 --- a/tests/xmconfigdata/test-fullvirt-usbmouse.xml +++ b/tests/xmconfigdata/test-fullvirt-usbmouse.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-usbtablet.xml b/tests/xmconfigdata/test-fullvirt-usbtablet.xml index 9933ef7..e6e807a 100644 --- a/tests/xmconfigdata/test-fullvirt-usbtablet.xml +++ b/tests/xmconfigdata/test-fullvirt-usbtablet.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-fullvirt-utc.xml b/tests/xmconfigdata/test-fullvirt-utc.xml index 6544cb3..bc9865b 100644 --- a/tests/xmconfigdata/test-fullvirt-utc.xml +++ b/tests/xmconfigdata/test-fullvirt-utc.xml @@ -33,6 +33,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> diff --git a/tests/xmconfigdata/test-no-source-cdrom.xml b/tests/xmconfigdata/test-no-source-cdrom.xml index 08ee701..7600085 100644 --- a/tests/xmconfigdata/test-no-source-cdrom.xml +++ b/tests/xmconfigdata/test-no-source-cdrom.xml @@ -32,6 +32,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:0a:7b:39'/> <source bridge='xenbr0'/> diff --git a/tests/xmconfigdata/test-pci-devs.xml b/tests/xmconfigdata/test-pci-devs.xml index 31db26f..74f17b0 100644 --- a/tests/xmconfigdata/test-pci-devs.xml +++ b/tests/xmconfigdata/test-pci-devs.xml @@ -32,6 +32,7 @@ <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> + <controller type='ide' index='0'/> <interface type='bridge'> <mac address='00:16:3e:0a:7b:39'/> <source bridge='xenbr0'/> -- 2.5.0
On Thu, Jan 07, 2016 at 22:49:57 -0500, Cole Robinson wrote:
Seems like the natural fit, since we are already adding other XML bits in the PostParse routine.
Previously AddImplicitControllers was only called at the end of XML parsing, meaning code that builds a DomainDef by hand had to manually call it. Adding it for those sites causes some test suite churn. ---
[...]
57 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2570f94..5b9dab9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3854,6 +3854,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1;
+ if (virDomainDefAddImplicitControllers(def) < 0) + return -1; +
Moving it here makes this called twice in case you use qemuParseCommandLine or virVMXParseConfig.
/* clean up possibly duplicated metadata entries */ virDomainDefMetadataSanitize(def);
The changes to the test suite look good to me but a more XEN knowledgeable person could comment on this fact possibly so that we are sure.
On 01/08/2016 07:05 AM, Peter Krempa wrote:
On Thu, Jan 07, 2016 at 22:49:57 -0500, Cole Robinson wrote:
Seems like the natural fit, since we are already adding other XML bits in the PostParse routine.
Previously AddImplicitControllers was only called at the end of XML parsing, meaning code that builds a DomainDef by hand had to manually call it. Adding it for those sites causes some test suite churn. ---
[...]
57 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2570f94..5b9dab9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3854,6 +3854,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1;
+ if (virDomainDefAddImplicitControllers(def) < 0) + return -1; +
Moving it here makes this called twice in case you use qemuParseCommandLine or virVMXParseConfig.
Right, the calling twice should be fine since this operation needs to be idempotent as it's called every time we read the XML off disk for example. Probably an opportunity for me cleanup but dropping some call sites causes test suite churn that I didn't feel like dealing with at this point.
/* clean up possibly duplicated metadata entries */ virDomainDefMetadataSanitize(def);
The changes to the test suite look good to me but a more XEN knowledgeable person could comment on this fact possibly so that we are sure.
ccing jfehlig To expand a bit, I think the changes should be safe, it's only adding bits to the output XML that xl/xm/sexpr code already needs to handle, since they will show up in any XML defined by the user. Thanks, Cole
It's just a combination of AddImplicitControllers, and AddConsoleCompat. Every caller that wants ImplicitControllers also wants the ConsoleCompat AFAICT, so lump them together. We also need it for future patches. --- src/conf/domain_conf.c | 19 ++++++++++++++----- src/conf/domain_conf.h | 2 +- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/vmx/vmx.c | 2 +- src/vz/vz_sdk.c | 2 +- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5b9dab9..61dc650 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3842,9 +3842,6 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseMemory(def, parseFlags) < 0) return -1; - if (virDomainDefAddConsoleCompat(def) < 0) - return -1; - if (virDomainDefRejectDuplicateControllers(def) < 0) return -1; @@ -3854,7 +3851,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseTimer(def) < 0) return -1; - if (virDomainDefAddImplicitControllers(def) < 0) + if (virDomainDefAddImplicitDevices(def) < 0) return -1; /* clean up possibly duplicated metadata entries */ @@ -18216,7 +18213,7 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def) * in the XML. This is for compat with existing apps which will * not know/care about <controller> info in the XML */ -int +static int virDomainDefAddImplicitControllers(virDomainDefPtr def) { if (virDomainDefAddDiskControllersForType(def, @@ -18251,6 +18248,18 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) return 0; } +int +virDomainDefAddImplicitDevices(virDomainDefPtr def) +{ + if (virDomainDefAddConsoleCompat(def) < 0) + return -1; + + if (virDomainDefAddImplicitControllers(def) < 0) + return -1; + + return 0; +} + virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ae6d546..4a91f24 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2718,7 +2718,7 @@ virDomainObjPtr virDomainObjParseFile(const char *filename, bool virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefPtr dst); -int virDomainDefAddImplicitControllers(virDomainDefPtr def); +int virDomainDefAddImplicitDevices(virDomainDefPtr def); virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e51dcf..0f2f66c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -197,7 +197,7 @@ virDomainControllerRemove; virDomainControllerTypeToString; virDomainCpuPlacementModeTypeFromString; virDomainCpuPlacementModeTypeToString; -virDomainDefAddImplicitControllers; +virDomainDefAddImplicitDevices; virDomainDefCheckABIStability; virDomainDefCheckDuplicateDiskInfo; virDomainDefCheckUnsupportedMemoryHotplug; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 66ca111..2ea87cc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -13862,7 +13862,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, VIR_FREE(nics); - if (virDomainDefAddImplicitControllers(def) < 0) + if (virDomainDefAddImplicitDevices(def) < 0) goto error; if (virDomainDefPostParse(def, qemuCaps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1161aa0..51c6950 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8074,7 +8074,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ dev->data.disk = NULL; if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) - if (virDomainDefAddImplicitControllers(vmdef) < 0) + if (virDomainDefAddImplicitDevices(vmdef) < 0) return -1; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) return -1; @@ -8099,7 +8099,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainHostdevInsert(vmdef, hostdev)) return -1; dev->data.hostdev = NULL; - if (virDomainDefAddImplicitControllers(vmdef) < 0) + if (virDomainDefAddImplicitDevices(vmdef) < 0) return -1; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) return -1; @@ -8141,7 +8141,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (qemuDomainChrInsert(vmdef, dev->data.chr) < 0) return -1; dev->data.chr = NULL; - if (virDomainDefAddImplicitControllers(vmdef) < 0) + if (virDomainDefAddImplicitDevices(vmdef) < 0) return -1; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) return -1; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 568b2c7..e641571 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1689,7 +1689,7 @@ virVMXParseConfig(virVMXContext *ctx, } /* def:controllers */ - if (virDomainDefAddImplicitControllers(def) < 0) { + if (virDomainDefAddImplicitDevices(def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add controllers")); goto cleanup; } diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index b78c413..f691713 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1350,7 +1350,7 @@ prlsdkLoadDomain(vzConnPtr privconn, if (prlsdkGetDomainState(sdkdom, &domainState) < 0) goto error; - if (virDomainDefAddImplicitControllers(def) < 0) + if (virDomainDefAddImplicitDevices(def) < 0) goto error; if (def->ngraphics > 0) { -- 2.5.0
Should be a no-op --- src/qemu/qemu_domain.c | 53 +++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bb8d47f..26a29b1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1025,14 +1025,10 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { .href = qemuDomainDefNamespaceHref, }; - static int -qemuDomainDefPostParse(virDomainDefPtr def, - virCapsPtr caps, - void *opaque) +qemuDomainDefAddDefaultDevices(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { - virQEMUDriverPtr driver = opaque; - virQEMUCapsPtr qemuCaps = NULL; bool addDefaultUSB = true; bool addImplicitSATA = false; bool addPCIRoot = false; @@ -1043,20 +1039,6 @@ qemuDomainDefPostParse(virDomainDefPtr def, bool addPanicDevice = false; int ret = -1; - if (def->os.bootloader || def->os.bootloaderArgs) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("bootloader is not supported by QEMU")); - return ret; - } - - /* check for emulator and create a default one if needed */ - if (!def->emulator && - !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) - return ret; - - - qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); - /* Add implicit PCI root controller if the machine has one */ switch (def->os.arch) { case VIR_ARCH_I686: @@ -1212,6 +1194,37 @@ qemuDomainDefPostParse(virDomainDefPtr def, ret = 0; cleanup: + return ret; +} + + +static int +qemuDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + virQEMUCapsPtr qemuCaps = NULL; + int ret = -1; + + if (def->os.bootloader || def->os.bootloaderArgs) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("bootloader is not supported by QEMU")); + return ret; + } + + /* check for emulator and create a default one if needed */ + if (!def->emulator && + !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) + return ret; + + qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); + + if (qemuDomainDefAddDefaultDevices(def, qemuCaps) < 0) + goto cleanup; + + ret = 0; + cleanup: virObjectUnref(qemuCaps); return ret; } -- 2.5.0
On Thu, Jan 07, 2016 at 22:49:59 -0500, Cole Robinson wrote:
Should be a no-op --- src/qemu/qemu_domain.c | 53 +++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bb8d47f..26a29b1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1025,14 +1025,10 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { .href = qemuDomainDefNamespaceHref, };
-
This creates wrong spacing ...
static int -qemuDomainDefPostParse(virDomainDefPtr def, - virCapsPtr caps, - void *opaque) +qemuDomainDefAddDefaultDevices(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { - virQEMUDriverPtr driver = opaque; - virQEMUCapsPtr qemuCaps = NULL; bool addDefaultUSB = true; bool addImplicitSATA = false; bool addPCIRoot = false;
[...]
@@ -1212,6 +1194,37 @@ qemuDomainDefPostParse(virDomainDefPtr def,
ret = 0; cleanup: + return ret; +} + +
While here you add correct one ...
+static int +qemuDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps,
[...] ACK
Rather than open coding calls. I can't see any reason not to --- src/qemu/qemu_domain.c | 23 +++++++++++++++++++++++ src/qemu/qemu_driver.c | 29 ----------------------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 26a29b1..48a2160 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1199,6 +1199,26 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, static int +qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) +{ + const char *canon; + + if (!(canon = virQEMUCapsGetCanonicalMachine(qemuCaps, def->os.machine))) + return 0; + + if (STRNEQ(canon, def->os.machine)) { + char *tmp; + if (VIR_STRDUP(tmp, canon) < 0) + return -1; + VIR_FREE(def->os.machine); + def->os.machine = tmp; + } + + return 0; +} + + +static int qemuDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps, void *opaque) @@ -1223,6 +1243,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuDomainDefAddDefaultDevices(def, qemuCaps) < 0) goto cleanup; + if (qemuCanonicalizeMachine(def, qemuCaps) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 51c6950..50ce514 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1684,26 +1684,6 @@ static int qemuConnectNumOfDomains(virConnectPtr conn) } -static int -qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) -{ - const char *canon; - - if (!(canon = virQEMUCapsGetCanonicalMachine(qemuCaps, def->os.machine))) - return 0; - - if (STRNEQ(canon, def->os.machine)) { - char *tmp; - if (VIR_STRDUP(tmp, canon) < 0) - return -1; - VIR_FREE(def->os.machine); - def->os.machine = tmp; - } - - return 0; -} - - static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) @@ -1749,9 +1729,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; - if (qemuCanonicalizeMachine(def, qemuCaps) < 0) - goto cleanup; - if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) goto cleanup; @@ -7531,9 +7508,6 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; - if (qemuCanonicalizeMachine(def, qemuCaps) < 0) - goto cleanup; - if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) goto cleanup; @@ -15888,9 +15862,6 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; - if (qemuCanonicalizeMachine(def, qemuCaps) < 0) - goto cleanup; - if (qemuAssignDeviceAliases(def, qemuCaps) < 0) goto cleanup; -- 2.5.0
On Thu, Jan 07, 2016 at 22:50:00 -0500, Cole Robinson wrote:
Rather than open coding calls. I can't see any reason not to
Historically, *PostParse wasn't called in the command line parser in the qemu driver, thus it was open-coded.
--- src/qemu/qemu_domain.c | 23 +++++++++++++++++++++++ src/qemu/qemu_driver.c | 29 ----------------------------- 2 files changed, 23 insertions(+), 29 deletions(-)
ACK
Rather than open coding calls. I can't see any reason not to --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_driver.c | 6 ------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 48a2160..a981310 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1246,6 +1246,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (qemuCanonicalizeMachine(def, qemuCaps) < 0) goto cleanup; + if (virSecurityManagerVerify(driver->securityManager, def) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 50ce514..459401a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1723,9 +1723,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (virDomainCreateXMLEnsureACL(conn, def) < 0) goto cleanup; - if (virSecurityManagerVerify(driver->securityManager, def) < 0) - goto cleanup; - if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; @@ -7502,9 +7499,6 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup; - if (virSecurityManagerVerify(driver->securityManager, def) < 0) - goto cleanup; - if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; -- 2.5.0
On Thu, Jan 07, 2016 at 22:50:01 -0500, Cole Robinson wrote:
Rather than open coding calls. I can't see any reason not to --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_driver.c | 6 ------ 2 files changed, 3 insertions(+), 6 deletions(-)
This will make it called in the qemuAttach function, but since that doesn't set any security manager info it will be a no-op. ACK
We will use this in upcoming patches --- src/conf/domain_conf.c | 31 +++++++++++++++++++++---------- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 61dc650..52dd293 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4187,12 +4187,10 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED, return virDomainDeviceDefPostParse(dev, data->def, data->caps, data->xmlopt); } - int -virDomainDefPostParse(virDomainDefPtr def, - virCapsPtr caps, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt) +virDomainDefPostParseDevices(virDomainDefPtr def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt) { int ret; struct virDomainDefPostParseDeviceIteratorData data = { @@ -4201,6 +4199,23 @@ virDomainDefPostParse(virDomainDefPtr def, .xmlopt = xmlopt, }; + if ((ret = virDomainDeviceInfoIterateInternal(def, + virDomainDefPostParseDeviceIterator, + true, + &data)) < 0) + return ret; + + return 0; +} + +int +virDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt) +{ + int ret; + /* call the domain config callback */ if (xmlopt->config.domainPostParseCallback) { ret = xmlopt->config.domainPostParseCallback(def, caps, @@ -4210,13 +4225,9 @@ virDomainDefPostParse(virDomainDefPtr def, } /* iterate the devices */ - if ((ret = virDomainDeviceInfoIterateInternal(def, - virDomainDefPostParseDeviceIterator, - true, - &data)) < 0) + if ((ret = virDomainDefPostParseDevices(def, caps, xmlopt)) < 0) return ret; - if ((ret = virDomainDefPostParseInternal(def, caps, parseFlags)) < 0) return ret; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4a91f24..2bba554 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2497,6 +2497,10 @@ virDomainDefPostParse(virDomainDefPtr def, virCapsPtr caps, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt); +int +virDomainDefPostParseDevices(virDomainDefPtr def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt); static inline bool virDomainObjIsActive(virDomainObjPtr dom) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0f2f66c..58f2d22 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -230,6 +230,7 @@ virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; virDomainDefPostParse; +virDomainDefPostParseDevices; virDomainDefSetMemoryInitial; virDomainDefSetMemoryTotal; virDomainDefSetVcpus; -- 2.5.0
On Thu, Jan 07, 2016 at 22:50:02 -0500, Cole Robinson wrote:
We will use this in upcoming patches --- src/conf/domain_conf.c | 31 +++++++++++++++++++++---------- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 61dc650..52dd293 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4187,12 +4187,10 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED, return virDomainDeviceDefPostParse(dev, data->def, data->caps, data->xmlopt); }
-
White space damage.
int -virDomainDefPostParse(virDomainDefPtr def, - virCapsPtr caps, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt) +virDomainDefPostParseDevices(virDomainDefPtr def, + virCapsPtr caps, + virDomainXMLOptionPtr xmlopt) { int ret; struct virDomainDefPostParseDeviceIteratorData data = { @@ -4201,6 +4199,23 @@ virDomainDefPostParse(virDomainDefPtr def, .xmlopt = xmlopt, };
+ if ((ret = virDomainDeviceInfoIterateInternal(def, + virDomainDefPostParseDeviceIterator, + true, + &data)) < 0) + return ret; + + return 0; +} +
Standard are two newlines.
+int +virDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt) +{ + int ret; + /* call the domain config callback */ if (xmlopt->config.domainPostParseCallback) { ret = xmlopt->config.domainPostParseCallback(def, caps,
ACK
This also makes VIR_TEST_REGENERATE_OUTPUT work --- tests/qemuxml2xmltest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f967ceb..e3bd9c2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -43,7 +43,7 @@ static int testXML2XMLHelper(const char *inxml, const char *inXmlData, const char *outxml, - const char *outXmlData, + const char *outXmlData ATTRIBUTE_UNUSED, bool live) { char *actual = NULL; @@ -66,10 +66,8 @@ testXML2XMLHelper(const char *inxml, if (!(actual = virDomainDefFormat(def, format_flags))) goto fail; - if (STRNEQ(outXmlData, actual)) { - virtTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml); + if (virtTestCompareToFile(actual, outxml) < 0) goto fail; - } ret = 0; -- 2.5.0
On Thu, Jan 07, 2016 at 22:50:03 -0500, Cole Robinson wrote:
This also makes VIR_TEST_REGENERATE_OUTPUT work --- tests/qemuxml2xmltest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f967ceb..e3bd9c2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -43,7 +43,7 @@ static int testXML2XMLHelper(const char *inxml, const char *inXmlData, const char *outxml, - const char *outXmlData, + const char *outXmlData ATTRIBUTE_UNUSED,
Since there are just two callers of this you might as well as remove this argument rather than making it unused.
On 01/08/2016 02:09 AM, Peter Krempa wrote:
On Thu, Jan 07, 2016 at 22:50:03 -0500, Cole Robinson wrote:
This also makes VIR_TEST_REGENERATE_OUTPUT work --- tests/qemuxml2xmltest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index f967ceb..e3bd9c2 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -43,7 +43,7 @@ static int testXML2XMLHelper(const char *inxml, const char *inXmlData, const char *outxml, - const char *outXmlData, + const char *outXmlData ATTRIBUTE_UNUSED,
Since there are just two callers of this you might as well as remove this argument rather than making it unused.
Good point. I dropped this in favor of a bigger cleanup anyways, see just posted patches Thanks, Cole
--- tests/qemuxml2xmltest.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e3bd9c2..c0270d4 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -59,7 +59,7 @@ testXML2XMLHelper(const char *inxml, goto fail; if (!virDomainDefCheckABIStability(def, def)) { - fprintf(stderr, "ABI stability check failed on %s", inxml); + VIR_TEST_DEBUG("ABI stability check failed on %s", inxml); goto fail; } @@ -151,7 +151,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque) virBufferAdd(&buf, testStatusXMLSuffix, -1); if (!(source = virBufferContentAndReset(&buf))) { - fprintf(stderr, "Failed to create the source XML"); + VIR_TEST_DEBUG("Failed to create the source XML"); goto cleanup; } @@ -163,7 +163,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque) virBufferAdd(&buf, testStatusXMLSuffix, -1); if (!(expect = virBufferContentAndReset(&buf))) { - fprintf(stderr, "Failed to create the expect XML"); + VIR_TEST_DEBUG("Failed to create the expect XML"); goto cleanup; } @@ -174,14 +174,14 @@ testCompareStatusXMLToXMLFiles(const void *opaque) VIR_DOMAIN_DEF_PARSE_STATUS | VIR_DOMAIN_DEF_PARSE_ACTUAL_NET | VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES))) { - fprintf(stderr, "Failed to parse domain status XML:\n%s", source); + VIR_TEST_DEBUG("Failed to parse domain status XML:\n%s", source); goto cleanup; } /* format it back */ if (!(actual = virDomainObjFormat(driver.xmlopt, obj, VIR_DOMAIN_DEF_FORMAT_SECURE))) { - fprintf(stderr, "Failed to format domain status XML"); + VIR_TEST_DEBUG("Failed to format domain status XML"); goto cleanup; } @@ -303,7 +303,7 @@ mymain(void) # define DO_TEST_FULL(name, is_different, when) \ do { \ if (testInfoSet(&info, name, is_different, when) < 0) { \ - fprintf(stderr, "Failed to generate test data for '%s'", name); \ + VIR_TEST_DEBUG("Failed to generate test data for '%s'", name); \ return -1; \ } \ \ -- 2.5.0
s/some// in subject, since this patch converts all in the particular test file. On Thu, Jan 07, 2016 at 22:50:04 -0500, Cole Robinson wrote:
--- tests/qemuxml2xmltest.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
ACK
Future changes will make some of these tests dependent on specific QEMUCaps flags, so wire up the basic handling. --- tests/qemuxml2xmltest.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c0270d4..32c9fed 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -37,6 +37,8 @@ struct testInfo { char *outInactiveName; char *outInactiveFile; + + virQEMUCapsPtr qemuCaps; }; static int @@ -216,6 +218,8 @@ testInfoFree(struct testInfo *info) VIR_FREE(info->outInactiveName); VIR_FREE(info->outInactiveFile); + + virObjectUnref(info->qemuCaps); } @@ -225,6 +229,13 @@ testInfoSet(struct testInfo *info, bool different, int when) { + if (!(info->qemuCaps = virQEMUCapsNew())) + goto error; + + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, name, + info->qemuCaps) < 0) + goto error; + if (virAsprintf(&info->inName, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", abs_srcdir, name) < 0) goto error; -- 2.5.0
On Thu, Jan 07, 2016 at 22:50:05 -0500, Cole Robinson wrote:
Future changes will make some of these tests dependent on specific QEMUCaps flags, so wire up the basic handling. --- tests/qemuxml2xmltest.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c0270d4..32c9fed 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c
[...]
@@ -225,6 +229,13 @@ testInfoSet(struct testInfo *info, bool different, int when) { + if (!(info->qemuCaps = virQEMUCapsNew())) + goto error;
This is not necessary, since ...
+ + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, name, + info->qemuCaps) < 0) + goto error;
... this function allocates the caps if it's called with the third argument set to NULL.
On 01/08/2016 06:39 AM, Peter Krempa wrote:
On Thu, Jan 07, 2016 at 22:50:05 -0500, Cole Robinson wrote:
Future changes will make some of these tests dependent on specific QEMUCaps flags, so wire up the basic handling. --- tests/qemuxml2xmltest.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c0270d4..32c9fed 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c
[...]
@@ -225,6 +229,13 @@ testInfoSet(struct testInfo *info, bool different, int when) { + if (!(info->qemuCaps = virQEMUCapsNew())) + goto error;
This is not necessary, since ...
+ + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, name, + info->qemuCaps) < 0) + goto error;
... this function allocates the caps if it's called with the third argument set to NULL.
I didn't see that. However then we don't have a handle to actually set CAPS flags on, we would have to look it up, which is roughly the same amount of code. I reposted this patch unchanged in my latest series Thanks, Cole
In order to make this work, we need to short circuit the normal virDomainDefPostParse ordering, and manually add stock devices ourselves, since we need them in the XML before assigning addresses. There's a bit of test suite churn due to extra XML output, and validation happening at different call sites, but it all looks correct to me. There's still quite a few manual callers of qemuDomainAssignAddresses that could be dropped too but it would need additional testing. --- src/qemu/qemu_domain.c | 10 +++++++ src/qemu/qemu_driver.c | 9 ------ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 4 ++- tests/qemuxml2argvtest.c | 12 ++------ .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +++--- .../qemuxml2xmlout-disk-scsi-vscsi.xml | 35 ++++++++++++++++++++++ .../qemuxml2xmlout-panic-pseries.xml | 30 +++++++++++++++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 4 +-- .../qemuxml2xmlout-pseries-panic-no-address.xml | 4 +-- tests/qemuxml2xmltest.c | 10 +++++-- 10 files changed, 97 insertions(+), 30 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a981310..e0520ab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1249,6 +1249,16 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup; + /* Device defaults are normally set after calling the driver specific + PostParse routine (this function), but we need them earlier. */ + if (virDomainDefPostParseDevices(def, caps, driver->xmlopt) < 0) + goto cleanup; + if (virDomainDefAddImplicitDevices(def) < 0) + goto cleanup; + + if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 459401a..4290c9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1726,9 +1726,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; - if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) - goto cleanup; - if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -7266,9 +7263,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, if (qemuAssignDeviceAliases(def, qemuCaps) < 0) goto cleanup; - if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) - goto cleanup; - /* do fake auto-alloc of graphics ports, if such config is used */ for (i = 0; i < def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = def->graphics[i]; @@ -7502,9 +7496,6 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; - if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0) - goto cleanup; - if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, 0, &oldDef))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml index 39f4a1f..256f8c6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml @@ -28,7 +28,9 @@ <address type='drive' controller='0' bus='0' target='0' unit='2'/> </disk> <controller type='usb' index='0'/> - <controller type='scsi' index='0'/> + <controller type='scsi' index='0'> + <address type='spapr-vio' reg='0x2000'/> + </controller> <controller type='pci' index='0' model='pci-root'/> <input type='keyboard' bus='usb'/> <input type='mouse' bus='usb'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 63480ce..fb9630d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -304,14 +304,6 @@ static int testCompareXMLToArgvFiles(const char *xml, virQEMUCapsFilterByMachineType(extraFlags, vmdef->os.machine); - if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DEVICE)) { - if (qemuDomainAssignAddresses(vmdef, extraFlags, NULL)) { - if (flags & FLAG_EXPECT_ERROR) - goto ok; - goto out; - } - } - log = virtTestLogContentAndReset(); VIR_FREE(log); virResetLastError(); @@ -1373,7 +1365,7 @@ mymain(void) QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PCI_MULTIFUNCTION); DO_TEST("pseries-vio-user-assigned", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); - DO_TEST_ERROR("pseries-vio-address-clash", + DO_TEST_PARSE_ERROR("pseries-vio-address-clash", QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM); DO_TEST("pseries-usb-kbd", QEMU_CAPS_PCI_OHCI, @@ -1499,7 +1491,7 @@ mymain(void) QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL); - DO_TEST_ERROR("pcie-root-port-too-many", + DO_TEST_PARSE_ERROR("pcie-root-port-too-many", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420, diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml index 7a608a8..b300324 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml @@ -29,10 +29,11 @@ <controller type='virtio-serial' index='2'/> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.foo'/> - <address type='virtio-serial' controller='1' bus='0' port='0'/> + <address type='virtio-serial' controller='1' bus='0' port='1'/> </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.bar'/> @@ -40,15 +41,15 @@ </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.wizz'/> - <address type='virtio-serial' controller='0' bus='0' port='0'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.ooh'/> - <address type='virtio-serial' controller='1' bus='0' port='0'/> + <address type='virtio-serial' controller='1' bus='0' port='2'/> </channel> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.lla'/> - <address type='virtio-serial' controller='2' bus='0' port='0'/> + <address type='virtio-serial' controller='2' bus='0' port='1'/> </channel> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml new file mode 100644 index 0000000..6bf4ca5 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml @@ -0,0 +1,35 @@ +<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='i686' 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> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/scsidisk.img'/> + <target dev='sda' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='3' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='scsi' index='0' model='ibmvscsi'> + <address type='spapr-vio' reg='0x2000'/> + </controller> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml new file mode 100644 index 0000000..81d81dd --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>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-system-ppc64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio' reg='0x30000000'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio' reg='0x30000000'/> + </console> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml index 8fcd644..81d81dd 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -18,11 +18,11 @@ <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> <target port='0'/> - <address type='spapr-vio'/> + <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> <target type='serial' port='0'/> - <address type='spapr-vio'/> + <address type='spapr-vio' reg='0x30000000'/> </console> <memballoon model='none'/> <panic model='pseries'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml index 8fcd644..81d81dd 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml @@ -18,11 +18,11 @@ <controller type='pci' index='0' model='pci-root'/> <serial type='pty'> <target port='0'/> - <address type='spapr-vio'/> + <address type='spapr-vio' reg='0x30000000'/> </serial> <console type='pty'> <target type='serial' port='0'/> - <address type='spapr-vio'/> + <address type='spapr-vio' reg='0x30000000'/> </console> <memballoon model='none'/> <panic model='pseries'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 32c9fed..393acb7 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -232,6 +232,12 @@ testInfoSet(struct testInfo *info, if (!(info->qemuCaps = virQEMUCapsNew())) goto error; + virQEMUCapsSetList(info->qemuCaps, + QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_SCSI_MEGASAS, + QEMU_CAPS_LAST); + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, name, info->qemuCaps) < 0) goto error; @@ -417,7 +423,7 @@ mymain(void) DO_TEST("disk-drive-network-iscsi"); DO_TEST("disk-drive-network-iscsi-auth"); DO_TEST("disk-scsi-device"); - DO_TEST("disk-scsi-vscsi"); + DO_TEST_DIFFERENT("disk-scsi-vscsi"); DO_TEST("disk-scsi-virtio-scsi"); DO_TEST("disk-virtio-scsi-num_queues"); DO_TEST("disk-virtio-scsi-cmd_per_lun"); @@ -602,7 +608,7 @@ mymain(void) DO_TEST_DIFFERENT("panic"); DO_TEST("panic-isa"); - DO_TEST("panic-pseries"); + DO_TEST_DIFFERENT("panic-pseries"); DO_TEST("panic-double"); DO_TEST("panic-no-address"); -- 2.5.0
On Thu, Jan 07, 2016 at 22:50:06 -0500, Cole Robinson wrote:
In order to make this work, we need to short circuit the normal virDomainDefPostParse ordering, and manually add stock devices ourselves, since we need them in the XML before assigning addresses.
There's a bit of test suite churn due to extra XML output, and validation happening at different call sites, but it all looks correct to me.
There's still quite a few manual callers of qemuDomainAssignAddresses that could be dropped too but it would need additional testing. --- src/qemu/qemu_domain.c | 10 +++++++ src/qemu/qemu_driver.c | 9 ------ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 4 ++- tests/qemuxml2argvtest.c | 12 ++------ .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +++--- .../qemuxml2xmlout-disk-scsi-vscsi.xml | 35 ++++++++++++++++++++++ .../qemuxml2xmlout-panic-pseries.xml | 30 +++++++++++++++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 4 +-- .../qemuxml2xmlout-pseries-panic-no-address.xml | 4 +-- tests/qemuxml2xmltest.c | 10 +++++-- 10 files changed, 97 insertions(+), 30 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a981310..e0520ab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1249,6 +1249,16 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup;
+ /* Device defaults are normally set after calling the driver specific + PostParse routine (this function), but we need them earlier. */ + if (virDomainDefPostParseDevices(def, caps, driver->xmlopt) < 0) + goto cleanup;
NACK to this, this would create a possibly dangerous situation by encouraging others to call the post parse callback handlers from themeself. If it's necessary to do two passes of post parse stuff, then please add a new callback pointer for it.
On 01/08/2016 06:35 AM, Peter Krempa wrote:
On Thu, Jan 07, 2016 at 22:50:06 -0500, Cole Robinson wrote:
In order to make this work, we need to short circuit the normal virDomainDefPostParse ordering, and manually add stock devices ourselves, since we need them in the XML before assigning addresses.
There's a bit of test suite churn due to extra XML output, and validation happening at different call sites, but it all looks correct to me.
There's still quite a few manual callers of qemuDomainAssignAddresses that could be dropped too but it would need additional testing. --- src/qemu/qemu_domain.c | 10 +++++++ src/qemu/qemu_driver.c | 9 ------ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 4 ++- tests/qemuxml2argvtest.c | 12 ++------ .../qemuxml2xmlout-channel-virtio-auto.xml | 9 +++--- .../qemuxml2xmlout-disk-scsi-vscsi.xml | 35 ++++++++++++++++++++++ .../qemuxml2xmlout-panic-pseries.xml | 30 +++++++++++++++++++ .../qemuxml2xmlout-pseries-panic-missing.xml | 4 +-- .../qemuxml2xmlout-pseries-panic-no-address.xml | 4 +-- tests/qemuxml2xmltest.c | 10 +++++-- 10 files changed, 97 insertions(+), 30 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-vscsi.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a981310..e0520ab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1249,6 +1249,16 @@ qemuDomainDefPostParse(virDomainDefPtr def, if (virSecurityManagerVerify(driver->securityManager, def) < 0) goto cleanup;
+ /* Device defaults are normally set after calling the driver specific + PostParse routine (this function), but we need them earlier. */ + if (virDomainDefPostParseDevices(def, caps, driver->xmlopt) < 0) + goto cleanup;
NACK to this, this would create a possibly dangerous situation by encouraging others to call the post parse callback handlers from themeself.
What is dangerous about this? These operations are idempotent, and if they ever failed to be idempotent, many things would break. Consider that DefPostParseDevices is called every time a VM is redefined, or XML is read from disk at daemon startup. Many xml2xml tests would likely fail as well. Now is this change in good taste? debatable :)
If it's necessary to do two passes of post parse stuff, then please add a new callback pointer for it.
Let's say we change things to do: virDomainDefPostParse() { domainPostParseCallback1(); virDomainDefPostParseDevices(); virDomainDefPostParseInternal(); domainPostParseCallback2(); } ... how do we know the generic bits don't need to be called after Callback2? We don't really. Especially because so much of the generic bits is just about validation checking, Callback2 may have changed some bit that the generic code needs to validate. So then maybe we do: virDomainDefPostParse() { domainPostParseCallback1(); virDomainDefPostParseDevices(); virDomainDefPostParseInternal(); domainPostParseCallback2(); virDomainDefPostParseDevices(); virDomainDefPostParseInternal(); } But that's basically my proposed patch, except encoded generically rather than to the only driver that currently needs. Granted all that is hypothetical, but the point is that there isn't any clear line that we can say 'this is for pass #1, this is for pass #2' except on a case by case driver basis, which suggests to me it's a confusing thing to put in generic code. If we accept the premise that the generic PostParse functions must be idempotent, then it seems fine to me to handle any special ordering in just one driver PostParse callback. sidenote: IMO we _do_ need a separate PostParse entry point though, but strictly to implement something like virDomainDefValidate() which always runs at the end of PostParse. All it does is check for invalid config, and doesn't change any settings. The goal would be to move as much validation as possible out of the current PostParse and XML parsing bits into that function, so it can be shared by impls that populate the DomainDef directly. Things seem like they are already moving in that direction with much of the PostParse bits already, but the validation vs. stateful bit isn't explicit. However that's a much bigger project, and it's completely orthogonal to this patch since it wouldn't solve my issue. Thanks, Cole
participants (2)
-
Cole Robinson -
Peter Krempa