Re: [libvirt] [PATCH] qemudDomainAttachSCSIDisk: handle empty controller list

However, there's more to it than that. The controller index, while technically "unsigned", may be derived from an expression like -1 / 7, since virDomainDiskDefAssignAddress does this: void virDomainDiskDefAssignAddress(virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def->dst); switch (def->bus) { case VIR_DOMAIN_DISK_BUS_SCSI: ... def->info.addr.drive.controller = idx / 7; def->info.addr.drive.bus = 0; def->info.addr.drive.unit = idx % 7; break; case VIR_DOMAIN_DISK_BUS_IDE: ... case VIR_DOMAIN_DISK_BUS_FDC: ... ... and virDiskNameToIndex may return -1. And "idx" is then -1. While we might have gotten lucky with the -1 -> 0 mapping for the .controller member, I doubt a .unit (also "unsigned int") value that's derived from "-1 % 7" (not portable, btw) will be safe. I will propose a patch to change the above function to return an indication of success or failure and update the few callers. They're all in functions that already return success or failure, so this is the only interface that would change.

Here's the revert: From 4d7423fc0e27e86c937dde1a5d62f3b7b6e49451 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Mon, 15 Mar 2010 16:43:23 +0100 Subject: [PATCH] Revert f5a6ce44ce8368d4183b69a31b77f67688d9af43 * src/qemu/qemu_driver.c (qemudDomainAttachSCSIDisk): The ".controller" member is an index, and *may* be 0. Reported by Wolfgang Mauerer. --- src/qemu/qemu_driver.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f8ab545..04344a8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6057,12 +6057,6 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, if (!(drivestr = qemuBuildDriveStr(disk, 0, qemuCmdFlags))) goto error; - if (disk->info.addr.drive.controller <= 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("no drive controller for %s"), disk->dst); - goto error; - } - for (i = 0 ; i <= disk->info.addr.drive.controller ; i++) { cont = qemuDomainFindOrCreateSCSIDiskController(driver, vm, i, qemuCmdFlags); if (!cont)