On 02/13/2013 09:58 AM, Osier Yang wrote:
The disk def could be free'ed by qemuDomainChangeEjectableMedia,
which can thus cause crash if we reference the disk pointer. On
the other hand, we have to remove the added shared disk entry from
the table on error codepath.
---
src/qemu/qemu_driver.c | 25 +++++++++++--------------
1 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 527c671..48852ad 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5837,6 +5837,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
goto end;
}
+ if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
+ goto end;
+
+ if (qemuSetUnprivSGIO(disk) < 0)
+ goto end;
+
if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
goto end;
@@ -5850,6 +5856,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
if (qemuSetupDiskCgroup(vm, cgroup, disk) < 0)
goto end;
}
+
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -5888,16 +5895,9 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
NULLSTR(disk->src));
}
- if (ret == 0) {
- if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
- VIR_WARN("Failed to add disk '%s' to shared disk table",
- disk->src);
-
- if (qemuSetUnprivSGIO(disk) < 0)
- VIR_WARN("Failed to set unpriv_sgio of disk '%s'",
disk->src);
- }
-
end:
+ if (ret != 0)
+ ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
I can understand why not to log here - you didn't add and I think we
message those cases, so why say we couldn't remove if we couldn't add.
if (cgroup)
virCgroupFree(&cgroup);
return ret;
@@ -6012,11 +6012,8 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver,
break;
}
- if (ret == 0) {
- if (qemuRemoveSharedDisk(driver, disk, vm->def->name) < 0)
- VIR_WARN("Failed to remove disk '%s' from shared disk
table",
- disk->src);
- }
+ if (ret == 0)
+ ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
However, it seems we should log this.
return ret;
}