On 12/03/2015 05:56 PM, John Ferlan wrote:
On 11/30/2015 06:06 AM, Boris Fiuczynski wrote:
When a SCSI disk is hotplugged to a domain that does not have the required
scsi controller already defined the following internal error occurs
error: Failed to attach device from scsi_disk.xml
error: internal error: Could not find scsi controller with index 0 required for device
Commit 0260506c added in method qemuBuildDriveDevStr a lookup of the controller
alias. The internal error occurs because in method qemuDomainAttachSCSIDisk
the automatic creation of the potentially missing SCSI controller occurs after
calling qemuBuildDriveDevStr.
This patch reverses the calling sequence.
Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Stefan Zimmermann <stzi@xxxxxxxxxxxxxxxxxx>
---
src/qemu/qemu_hotplug.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8804d3d..210d485 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -607,6 +607,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
goto error;
}
Would you say the following is an accurate description to add?
/* Let's make sure our disk has a controller defined and loaded
* before trying add the disk. The virDomainDiskDefAssignAddress
* doesn't try to add a controller if one doesn't exist, it just
* assigns the disk to the calculated spot.
*/
First sentence I can agree to. The second sentence I am not sure I
understand. Did you mean: The controller the disk is going to use must
exist before a qemu command line string is being generated.
+ for (i = 0; i <= disk->info.addr.drive.controller; i++) {
+ cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i);
+ if (!cont)
+ goto error;
+ }
+
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
@@ -617,12 +623,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
goto error;
- 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. */
Is there a reason you chose to not grab the clang check too?
yes, it's a miss... good catch! ;-)
John
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list