[libvirt] [PATCH 0/3] qemu: Fix auto-generation of SCSI controllers for hot-plugged SCSI disks and add tests
This patch series fixes the auto-generation of SCSI controllers for hot-plugged SCSI disks and hot-plugged SCSI hostdevs. Additionally this patch series adds test cases for the auto-generation of SCSI controllers to the test suite. Marc Hartmayer (3): qemu: hot-plug: Fix broken SCSI disk hot-plug qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI controllers tests: Add test cases for SCSI disk hot-plug with QEMU src/qemu/qemu_hotplug.c | 41 +++++++++++++- tests/qemuhotplugtest.c | 29 ++++++++++ .../qemuhotplug-disk-scsi-2.xml | 8 +++ ...-base-with-scsi-controller-live+disk-scsi-2.xml | 51 +++++++++++++++++ ...se-without-scsi-controller-live+disk-scsi-2.xml | 66 ++++++++++++++++++++++ ...argv-hotplug-base-with-scsi-controller-live.xml | 56 ++++++++++++++++++ ...v-hotplug-base-without-scsi-controller-live.xml | 40 +++++++++++++ 7 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml -- 2.5.5
The commit "qemu: hot-plug: Assume support for -device in qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI controller creation used in SCSI disk hot-plugging. If we are hot-plugging a SCSI disk to a domain and there is no proper SCSI controller defined, it results in an "error: internal error: Could not find scsi controller with index X required for device" error. For that reason reverting a hunk of the commit d4d32005d6e8b2cc0a2f26b483ca1de10171db6d. This patch also adds an extra comment to the code to clarify the loop. Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e0b8230..5e6a8cb 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -544,7 +544,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk) { + size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainControllerDefPtr cont = NULL; char *drivestr = NULL; char *devstr = NULL; int ret = -1; @@ -561,6 +563,24 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, goto error; } + /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= disk->info.addr.drive.controller; i++) { + cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i); + if (!cont) + goto error; + } + + /* Tell clang that "cont" is non-NULL. + This is because disk->info.addr.driver.controller is unsigned, + and hence the above loop must iterate at least once. */ + sa_assert(cont); + if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; -- 2.5.5
On Mon, Jun 27, 2016 at 04:43:46PM +0200, Marc Hartmayer wrote:
The commit "qemu: hot-plug: Assume support for -device in qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI controller creation used in SCSI disk hot-plugging. If we are hot-plugging a SCSI disk to a domain and there is no proper SCSI controller defined, it results in an "error: internal error: Could not find scsi controller with index X required for device" error.
For that reason reverting a hunk of the commit d4d32005d6e8b2cc0a2f26b483ca1de10171db6d.
This patch also adds an extra comment to the code to clarify the loop.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Jun 27, 2016 at 16:43:46 +0200, Marc Hartmayer wrote:
The commit "qemu: hot-plug: Assume support for -device in qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI controller creation used in SCSI disk hot-plugging. If we are hot-plugging a SCSI disk to a domain and there is no proper SCSI controller defined, it results in an "error: internal error: Could not find scsi controller with index X required for device" error.
For that reason reverting a hunk of the commit d4d32005d6e8b2cc0a2f26b483ca1de10171db6d.
Uh, you are right. I missed that qemuDomainFindOrCreateSCSIDiskController has a side effect of creating the controller in this case when getting rid of the old code.
This patch also adds an extra comment to the code to clarify the loop.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e0b8230..5e6a8cb 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -544,7 +544,9 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk) { + size_t i; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainControllerDefPtr cont = NULL;
This variable isn't necessary.
char *drivestr = NULL; char *devstr = NULL; int ret = -1; @@ -561,6 +563,24 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, goto error; }
+ /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= disk->info.addr.drive.controller; i++) { + cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i); + if (!cont) + goto error;
This can be checked right away as the value of 'cont' isn't used.
+ } + + /* Tell clang that "cont" is non-NULL. + This is because disk->info.addr.driver.controller is unsigned, + and hence the above loop must iterate at least once. */ + sa_assert(cont);
Also this whole comment and assert can be dropped.
+ if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error;
I'll adjust the patch and push it shortly. Thanks for fixing up the mess I've made. Peter
On Thu, Jun 30, 2016 at 12:34 PM +0200, Peter Krempa <pkrempa@redhat.com> wrote:
qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainControllerDefPtr cont = NULL;
This variable isn't necessary.
+ /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= disk->info.addr.drive.controller; i++) { + cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i); + if (!cont) + goto error;
This can be checked right away as the value of 'cont' isn't used.
+ /* Tell clang that "cont" is non-NULL. + This is because disk->info.addr.driver.controller is unsigned, + and hence the above loop must iterate at least once. */ + sa_assert(cont);
Also this whole comment and assert can be dropped.
You're right.
I'll adjust the patch and push it shortly. Thanks for fixing up the mess I've made.
Okay, thanks. Regards, Marc
On Mon, Jun 27, 2016 at 04:43:46PM +0200, Marc Hartmayer wrote:
The commit "qemu: hot-plug: Assume support for -device in qemuDomainAttachSCSIDisk" dropped the code for the automatic SCSI controller creation used in SCSI disk hot-plugging. If we are hot-plugging a SCSI disk to a domain and there is no proper SCSI controller defined, it results in an "error: internal error: Could not find scsi controller with index X required for device" error.
For that reason reverting a hunk of the commit d4d32005d6e8b2cc0a2f26b483ca1de10171db6d.
This patch also adds an extra comment to the code to clarify the loop.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> ---
I just tested with the attached trivial script, which hot-plugs two SCSI disks on the _second_ controller. With your patch, controlle 2, and all the smaller ones below it (1, 0) are auto-generated: $ sudo ./run ./tools/virsh start cvm1 $ sudo ./run ./tools/virsh list Id Name State ---------------------------------------------------- 6 cvm1 running $ sudo ./run ./tools/virsh domblklist cvm1 Target Source ------------------------------------------------ vda /export/vmimages/sn1.qcow2 $ sudo ./run ./tools/virsh dumpxml cvm1 | grep controller.*scsi $ echo $? 1 $ ./attach-scsi-device.bash $ sudo ./run ./tools/virsh domblklist cvm1 Target Source ------------------------------------------------ vda /export/vmimages/sn1.qcow2 sdb /export/vmimages/b.raw sdc /export/vmimages/c.raw $ sudo ./run ./tools/virsh dumpxml cvm1 | grep controller.*scsi <controller type='scsi' index='0'> <controller type='scsi' index='1'> <controller type='scsi' index='2'> $ sudo ./run ./tools/virsh dumpxml cvm1 | grep scsi <target dev='sdb' bus='scsi'/> <alias name='scsi2-0-2'/> <target dev='sdc' bus='scsi'/> <alias name='scsi2-0-3'/> <controller type='scsi' index='0'> <alias name='scsi0'/> <controller type='scsi' index='1'> <alias name='scsi1'/> <controller type='scsi' index='2'> <alias name='scsi2'/> So: Tested-by: Kashyap Chamarthy <kchamart@redhat.com> [...] -- /kashyap
Ensure that the given controller and all controllers with a smaller index exist; there must not be any missing index in between. Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5e6a8cb..037e601 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1873,6 +1873,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { + size_t i; int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; @@ -1888,9 +1889,23 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; } - cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, hostdev->info->addr.drive.controller); - if (!cont) - return -1; + /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= hostdev->info->addr.drive.controller; i++) { + cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i); + if (!cont) + return -1; + } + + /* Tell clang that "cont" is non-NULL. + This is because disk->info.addr.driver.controller is unsigned, + and hence the above loop must iterate at least once. */ + sa_assert(cont); if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, &hostdev, 1)) { -- 2.5.5
On Mon, Jun 27, 2016 at 04:43:47PM +0200, Marc Hartmayer wrote:
Ensure that the given controller and all controllers with a smaller index exist; there must not be any missing index in between.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Jun 27, 2016 at 16:43:47 +0200, Marc Hartmayer wrote:
Ensure that the given controller and all controllers with a smaller index exist; there must not be any missing index in between.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5e6a8cb..037e601 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1873,6 +1873,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { + size_t i; int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; @@ -1888,9 +1889,23 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; }
- cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, hostdev->info->addr.drive.controller); - if (!cont) - return -1; + /* Let's make sure the disk has a controller defined and loaded before + * trying to add it. The controller used by the disk must exist before a + * qemu command line string is generated. + * + * Ensure that the given controller and all controllers with a smaller index + * exist; there must not be any missing index in between. + */ + for (i = 0; i <= hostdev->info->addr.drive.controller; i++) { + cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i); + if (!cont) + return -1; + } + + /* Tell clang that "cont" is non-NULL. + This is because disk->info.addr.driver.controller is unsigned, + and hence the above loop must iterate at least once. */ + sa_assert(cont);
Same as in the previous patch I'll tweak it to remove the hunk above and the unnecessary variable. Peter
Verify that SCSI controllers get created automatically when a SCSI disk is hot-plugged to a domain that doesn't have a matching SCSI controller defined already. Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- tests/qemuhotplugtest.c | 29 ++++++++++ .../qemuhotplug-disk-scsi-2.xml | 8 +++ ...-base-with-scsi-controller-live+disk-scsi-2.xml | 51 +++++++++++++++++ ...se-without-scsi-controller-live+disk-scsi-2.xml | 66 ++++++++++++++++++++++ ...argv-hotplug-base-with-scsi-controller-live.xml | 56 ++++++++++++++++++ ...v-hotplug-base-without-scsi-controller-live.xml | 40 +++++++++++++ 6 files changed, 250 insertions(+) create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 91bf331..ae57c0a 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -487,6 +487,35 @@ mymain(void) "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK, "human-monitor-command", HMP("")); + DO_TEST_ATTACH("hotplug-base-without-scsi-controller-live", "disk-scsi-2", false, true, + /* Four controllers added */ + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK, + "human-monitor-command", HMP("OK\\r\\n"), + /* Disk added */ + "device_add", QMP_OK); + DO_TEST_DETACH("hotplug-base-with-scsi-controller-live", "disk-scsi-2", false, false, + "device_del", QMP_OK, + "human-monitor-command", HMP("")); + + DO_TEST_ATTACH_EVENT("hotplug-base-without-scsi-controller-live", "disk-scsi-2", false, true, + /* Four controllers added */ + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK, + "device_add", QMP_OK, + "human-monitor-command", HMP("OK\\r\\n"), + /* Disk added */ + "device_add", QMP_OK); + DO_TEST_DETACH("hotplug-base-with-scsi-controller-live", "disk-scsi-2", true, true, + "device_del", QMP_OK, + "human-monitor-command", HMP("")); + DO_TEST_DETACH("hotplug-base-with-scsi-controller-live", "disk-scsi-2", false, false, + "device_del", QMP_DEVICE_DELETED("scsi3-0-5-7") QMP_OK, + "human-monitor-command", HMP("")); + DO_TEST_ATTACH("hotplug-base-live", "qemu-agent", false, true, "chardev-add", QMP_OK, "device_add", QMP_OK); diff --git a/tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml b/tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml new file mode 100644 index 0000000..3a847fb --- /dev/null +++ b/tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml @@ -0,0 +1,8 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source file='/dev/null'/> + <target dev='sdf' bus='scsi'/> + <address type='drive' controller='3' bus='0' target='5' unit='7'/> + <readonly/> + <shareable/> +</disk> diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml new file mode 100644 index 0000000..b2c9a07 --- /dev/null +++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml @@ -0,0 +1,51 @@ +<domain type='kvm'> + <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='x86_64' machine='pc'>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'/> + <target dev='sdf' bus='scsi'/> + <readonly/> + <shareable/> + <address type='drive' controller='3' bus='0' target='5' unit='7'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='scsi' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='scsi' index='1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='scsi' index='2'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='scsi' index='3'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> + <seclabel type='none' model='none'/> +</domain> diff --git a/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml new file mode 100644 index 0000000..2b11f21 --- /dev/null +++ b/tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml @@ -0,0 +1,66 @@ +<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='x86_64' machine='pc'>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='sdf' bus='scsi'/> + <readonly/> + <shareable/> + <alias name='scsi3-0-5-7'/> + <address type='drive' controller='3' bus='0' target='5' unit='7'/> + </disk> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <controller type='scsi' index='0'> + <alias name='scsi0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='scsi' index='1'> + <alias name='scsi1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='scsi' index='2'> + <alias name='scsi2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='scsi' index='3'> + <alias name='scsi3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <memballoon model='none'> + <alias name='balloon0'/> + </memballoon> + </devices> + <seclabel type='none' model='none'/> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml b/tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml new file mode 100644 index 0000000..b2ea144 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml @@ -0,0 +1,56 @@ +<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='x86_64' machine='pc'>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> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <controller type='scsi' index='0'> + <alias name='scsi0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <controller type='scsi' index='1'> + <alias name='scsi1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='scsi' index='2'> + <alias name='scsi2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='scsi' index='3'> + <alias name='scsi3'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <memballoon model='none'> + <alias name='balloon0'/> + </memballoon> + </devices> + <seclabel type='none' model='none'/> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml b/tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml new file mode 100644 index 0000000..66bc2af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml @@ -0,0 +1,40 @@ +<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='x86_64' machine='pc'>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> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <memballoon model='none'> + <alias name='balloon0'/> + </memballoon> + </devices> + <seclabel type='none' model='none'/> +</domain> -- 2.5.5
On Mon, Jun 27, 2016 at 04:43:48PM +0200, Marc Hartmayer wrote:
Verify that SCSI controllers get created automatically when a SCSI disk is hot-plugged to a domain that doesn't have a matching SCSI controller defined already.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- tests/qemuhotplugtest.c | 29 ++++++++++ .../qemuhotplug-disk-scsi-2.xml | 8 +++ ...-base-with-scsi-controller-live+disk-scsi-2.xml | 51 +++++++++++++++++ ...se-without-scsi-controller-live+disk-scsi-2.xml | 66 ++++++++++++++++++++++ ...argv-hotplug-base-with-scsi-controller-live.xml | 56 ++++++++++++++++++ ...v-hotplug-base-without-scsi-controller-live.xml | 40 +++++++++++++ 6 files changed, 250 insertions(+) create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-disk-scsi-2.xml create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-with-scsi-controller-live+disk-scsi-2.xml create mode 100644 tests/qemuhotplugtestdata/qemuhotplug-hotplug-base-without-scsi-controller-live+disk-scsi-2.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-with-scsi-controller-live.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hotplug-base-without-scsi-controller-live.xml
ACK, top marks for adding unit tests ! Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Jun 27, 2016 at 04:43:45PM +0200, Marc Hartmayer wrote:
This patch series fixes the auto-generation of SCSI controllers for hot-plugged SCSI disks and hot-plugged SCSI hostdevs.
Additionally this patch series adds test cases for the auto-generation of SCSI controllers to the test suite.
Marc Hartmayer (3): qemu: hot-plug: Fix broken SCSI disk hot-plug qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI controllers tests: Add test cases for SCSI disk hot-plug with QEMU
All looks fine and I tested it works. Given we're almost at release I want Peter to also ACK it before pushing Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Jun 30, 2016 at 09:44:47AM +0100, Daniel P. Berrange wrote:
On Mon, Jun 27, 2016 at 04:43:45PM +0200, Marc Hartmayer wrote:
This patch series fixes the auto-generation of SCSI controllers for hot-plugged SCSI disks and hot-plugged SCSI hostdevs.
Additionally this patch series adds test cases for the auto-generation of SCSI controllers to the test suite.
Marc Hartmayer (3): qemu: hot-plug: Fix broken SCSI disk hot-plug qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI controllers tests: Add test cases for SCSI disk hot-plug with QEMU
All looks fine and I tested it works. Given we're almost at release I want Peter to also ACK it before pushing
Once pushed to master, is this also desirable to backport to relevant stable branches, given bugs like these (reported by an OpenStack Nova user)? https://bugzilla.redhat.com/show_bug.cgi?id=1343302 -- - libvirt Could not find scsi controller with index 1 required for device -- /kashyap
On Thu, Jun 30, 2016 at 10:57:16 +0200, Kashyap Chamarthy wrote:
On Thu, Jun 30, 2016 at 09:44:47AM +0100, Daniel P. Berrange wrote:
On Mon, Jun 27, 2016 at 04:43:45PM +0200, Marc Hartmayer wrote:
This patch series fixes the auto-generation of SCSI controllers for hot-plugged SCSI disks and hot-plugged SCSI hostdevs.
Additionally this patch series adds test cases for the auto-generation of SCSI controllers to the test suite.
Marc Hartmayer (3): qemu: hot-plug: Fix broken SCSI disk hot-plug qemu: SCSI hostdev hot-plug: Fix automatic creation of SCSI controllers tests: Add test cases for SCSI disk hot-plug with QEMU
All looks fine and I tested it works. Given we're almost at release I want Peter to also ACK it before pushing
Once pushed to master, is this also desirable to backport to relevant stable branches, given bugs like these (reported by an OpenStack Nova user)?
I pushed it now to v1.3.3-main. Other branches were not created yet so I didn't bother. The last patch adding tests didn't work on that branch as Martin refactored the hotplug tests lately. I checked the results but didn't bother fixing the test. Peter
participants (4)
-
Daniel P. Berrange -
Kashyap Chamarthy -
Marc Hartmayer -
Peter Krempa