[libvirt] [PATCHv2 0/2] Swap order of AddImplicitControllers and DomainDefPostParse

Implicit controllers may be dependent on device definitions altered in a post-parse callback. E.g., if a console device is defined without the target type, the type will be set in QEMU's callback. In the case of s390, this is virtio, which requires an implicit virtio-serial controller. By moving the implicit controller definition after the post-parse procssing this can be fixed. As Martin pointed out, implicit controllers should not need post-parsing, so the rearranging should not hurt. Probably this is only affecting the S390 virtio console anyway. V2 Changes: - Promoted from RFC to Patch Series - Added an qemuxml2xml testcase highlighting the issue: applying the first patch only will fail make check as the implicit controller is missing. Viktor Mihajlovski (2): S390: Testcase for console default target type (virtio) conf: Swap order of AddImplicitControllers and DomainDefPostParse src/conf/domain_conf.c | 8 +++---- .../qemuxml2argv-s390-defaultconsole.xml | 20 ++++++++++++++++ .../qemuxml2xmlout-balloon-device-auto.xml | 2 +- .../qemuxml2xmlout-channel-virtio-auto.xml | 2 +- .../qemuxml2xmlout-console-virtio.xml | 2 +- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 2 +- .../qemuxml2xmlout-s390-defaultconsole.xml | 24 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 8 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml -- 1.7.9.5

For s390 the default console target type is virtio. This also requires that an implicit virtio-serial controller is instantiated. This testcase verifies that the target type of virtio is correctly set in the generated XML if no target element was given and that the corresponding virtio-serial element is generated too. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- .../qemuxml2argv-s390-defaultconsole.xml | 20 ++++++++++++++++ .../qemuxml2xmlout-s390-defaultconsole.xml | 24 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 3 files changed, 46 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml b/tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml new file mode 100644 index 0000000..036027c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml @@ -0,0 +1,20 @@ +<domain type='kvm'> + <name>test</name> + <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid> + <memory unit='KiB'>262144</memory> + <currentMemory unit='KiB'>262144</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-virtio'>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-kvm</emulator> + <console type='pty'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml new file mode 100644 index 0000000..9a609f8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml @@ -0,0 +1,24 @@ +<domain type='kvm'> + <name>test</name> + <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid> + <memory unit='KiB'>262144</memory> + <currentMemory unit='KiB'>262144</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-virtio'>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-kvm</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='virtio-serial' index='0'/> + <console type='pty'> + <target type='virtio' port='0'/> + </console> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 06e4206..65e9591 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -300,6 +300,8 @@ mymain(void) DO_TEST_DIFFERENT("hostdev-scsi-autogen-address"); + DO_TEST_DIFFERENT("s390-defaultconsole"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.7.9.5

Implicit controllers may be dependent on device definitions altered in a post-parse callback. Specifically, if a console device is defined without the target type, the type will be set in QEMU's callback. In the case of s390, this is virtio, which requires an implicit virtio-serial controller. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 8 ++++---- .../qemuxml2xmlout-balloon-device-auto.xml | 2 +- .../qemuxml2xmlout-channel-virtio-auto.xml | 2 +- .../qemuxml2xmlout-console-virtio.xml | 2 +- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2373397..a5c2315 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12058,14 +12058,14 @@ virDomainDefParseXML(xmlDocPtr xml, (def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) goto error; - /* Auto-add any implied controllers which aren't present */ - if (virDomainDefAddImplicitControllers(def) < 0) - goto error; - /* callback to fill driver specific domain aspects */ if (virDomainDefPostParse(def, caps, 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/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml index 6aed326..380b70f 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-balloon-device-auto.xml @@ -20,8 +20,8 @@ <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='usb' index='0'/> - <controller type='ide' index='0'/> <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> </domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml index 0175272..fd6b852 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-virtio-auto.xml @@ -25,8 +25,8 @@ <controller type='virtio-serial' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </controller> - <controller type='virtio-serial' index='2'/> <controller type='pci' index='0' model='pci-root'/> + <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'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml index 3c865c3..340430e 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml @@ -21,8 +21,8 @@ </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> - <controller type='virtio-serial' index='0'/> <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'/> <console type='pty'> <target type='virtio' port='0'/> </console> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml index 7d152bc..5ec1e94 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-scsi-device-auto.xml @@ -26,8 +26,8 @@ </disk> <controller type='usb' index='0'/> <controller type='ide' index='0'/> - <controller type='scsi' index='0'/> <controller type='pci' index='0' model='pci-root'/> + <controller type='scsi' index='0'/> <memballoon model='virtio'/> </devices> </domain> -- 1.7.9.5

Hi all: Work with virsh CLI, I can easy use the CLI " virsh -c qemu+tcp://127.0.0.1/system domdisplay instance-00000001" successfully: [root@webmintest ~]# virsh -c qemu+tcp://127.0.0.1/system domdisplay instance-00000001 Please enter your authentication name: fred Please enter your password: vnc://127.0.0.1:0 However, I can find any libvit-python API related to this CLI, does it exist ? Thanks. 2013-06-17 mzawdx

On 17.06.2013 17:19, mzawdx wrote:
Hi all: Work with virsh CLI, I can easy use the CLI " virsh -c qemu+tcp://127.0.0.1/system domdisplay instance-00000001" successfully:
[root@webmintest ~]# virsh -c qemu+tcp://127.0.0.1/system domdisplay instance-00000001 Please enter your authentication name: fred Please enter your password: vnc://127.0.0.1:0
However, I can find any libvit-python API related to this CLI, does it exist ?
No. The domdisplay command is pure virsh with a XPATH magic. The algorithm is as follows: 1) dump the XML of desired domain 2) for i in "vnc", "spice", "rdp"; do 2a) xpath = string(/domain/devices/graphics[@type='%s']/@port), %i 2b) port = xpath_query($domxml, $xpath) 2c) if !port continue 2d) xpath = string(/domain/devices/graphics[@type='%s']/@listen), %i 2e) listen = xpath_query($domxml, $xpath) 2f) if ($i == "vnc) port -= 5900; 3) print %s://%s:%d, %i %listen %port The whole source is here: http://libvirt.org/git/?p=libvirt.git;a=blob;f=tools/virsh-domain.c;h=955e88... Michal

On 06/17/2013 04:17 PM, Viktor Mihajlovski wrote:
Implicit controllers may be dependent on device definitions altered in a post-parse callback. E.g., if a console device is defined without the target type, the type will be set in QEMU's callback. In the case of s390, this is virtio, which requires an implicit virtio-serial controller.
By moving the implicit controller definition after the post-parse procssing this can be fixed. As Martin pointed out, implicit controllers should not need post-parsing, so the rearranging should not hurt. Probably this is only affecting the S390 virtio console anyway.
V2 Changes: - Promoted from RFC to Patch Series - Added an qemuxml2xml testcase highlighting the issue: applying the first patch only will fail make check as the implicit controller is missing.
Viktor Mihajlovski (2): S390: Testcase for console default target type (virtio) conf: Swap order of AddImplicitControllers and DomainDefPostParse
src/conf/domain_conf.c | 8 +++---- .../qemuxml2argv-s390-defaultconsole.xml | 20 ++++++++++++++++ .../qemuxml2xmlout-balloon-device-auto.xml | 2 +- .../qemuxml2xmlout-channel-virtio-auto.xml | 2 +- .../qemuxml2xmlout-console-virtio.xml | 2 +- .../qemuxml2xmlout-disk-scsi-device-auto.xml | 2 +- .../qemuxml2xmlout-s390-defaultconsole.xml | 24 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 8 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-s390-defaultconsole.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-s390-defaultconsole.xml
ACK to both. Now pushed. Jan

On 06/28/2013 09:56 AM, Ján Tomko wrote:
ACK to both. Now pushed.
Jan
Thanks! -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (4)
-
Ján Tomko
-
Michal Privoznik
-
mzawdx
-
Viktor Mihajlovski