Re: [PATCH v2] qemuhotplugtest: Add tests for ccw devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 12, 2016 at 03:08:22PM +0200, Tomasz Flendrich wrote:
These are a few simple and one complex testcases.
The simple ones test attaching and detaching ccw devices
with both implicitly and explicitly stated addresses.
In the complex one, attaching and detaching a device
should make the address free to reuse.

I have plan to rework the address handling, so testcases
that verify hotplugging ccw devices will help in avoiding
regression.

In this commit, some device files are duplicated because
of the way qemuhotplug.c calculates the expected xml filenames.
I plan on changing that to explicitly stating the basis domain
xml, the device xml, and the expected xml.
---

Changed in v2:
* Add a newline to the end of qemuhotplug-ccw-virtio-2-explicit.xml,
 so that syntax-check doesn't complain.
* Rename two files: remove the "explicit" word where the address
 is actually implicit.

Link to the previous patch:
https://www.redhat.com/archives/libvir-list/2016-July/msg00264.html



[...]

diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-1-explicit.xml b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-1-explicit.xml
new file mode 100644
index 0000000..74bd6a9
--- /dev/null
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-1-explicit.xml
@@ -0,0 +1,8 @@
+<disk type='file' device='disk'>
+  <driver name='qemu' type='raw' cache='none'/>
+  <source file='/dev/null'/>
+  <target dev='vde' bus='virtio'/>
+  <readonly/>
+  <shareable/>
+  <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
+</disk>

I know you weren't aiming for that, but this got me thinking, are we
checking that unplug works with not fully specified XML?  I mean that
when unplugging, you don't need to specify everything.  Whatever
uniquely identifies the device should be enough.  So for example just
address or target, definitely no need for the readonly and shareable
flags.  But as said, that's not meant to be part of this patch.

[...]

diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-2-explicit.xml b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-2-explicit.xml
new file mode 100644
index 0000000..b2cb161
--- /dev/null
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-ccw-virtio-2-explicit.xml
@@ -0,0 +1,8 @@
+<disk type='file' device='disk'>
+  <driver name='qemu' type='raw' cache='none'/>
+  <source file='/dev/null'/>
+  <target dev='vde2' bus='virtio'/>

vde2 is an address for a partition.  This works?  I don't think it
should.

[...]

diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml
new file mode 100644
index 0000000..ff94b6c
--- /dev/null
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-ccw-live-with-2-ccw-virtio+ccw-virtio-1-explicit.xml
@@ -0,0 +1,73 @@
+<domain type='kvm' id='7'>
+  <name>hotplug</name>
+  <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid>
+  <memory unit='KiB'>4194304</memory>
+  <currentMemory unit='KiB'>4194304</currentMemory>
+  <vcpu placement='static'>4</vcpu>
+  <os>
+    <type arch='s390x' machine='s390-ccw'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/libexec/qemu-kvm</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw' cache='none'/>
+      <source file='/dev/null'/>
+      <backingStore/>
+      <target dev='vde' bus='virtio'/>
+      <readonly/>
+      <shareable/>
+      <alias name='virtio-disk4'/>

This ...

+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw' cache='none'/>
+      <source file='/dev/null'/>
+      <backingStore/>
+      <target dev='vde2' bus='virtio'/>
+      <readonly/>
+      <shareable/>
+      <alias name='virtio-disk4'/>

... and this?  This will never work, so that's another bug right here.

+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>

By the way, attaching a device without explicit address and without
target dev= specified (only bus='virtio')  Should work and that would
actually test address allocation.  By allocating disk with target
dev='XX' the address gets calculated from that device, so no allocation
is being tested.

Other than these problems the approach is fine.

Martin

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]