Daniel P. Berrange wrote:
> On Tue, May 18, 2010 at 03:23:23PM +0200, Jim Meyering wrote:
>> In src/qemu/qemu_driver.c, coverity gripes (rightly) about this:
>>
>> 6912 qemuDomainObjEnterMonitorWithDriver(driver, vm);
>> 6913 if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
>> 6914 ret = qemuMonitorAddDrive(priv->mon, drivestr);
>> 6915 if (ret == 0)
>> No check of the return value of "qemuMonitorAddDevice(priv->mon, devstr)".
>> Calling function "qemuMonitorAddDevice" without checking return value.
>> 6916 qemuMonitorAddDevice(priv->mon, devstr);
>> 6917 /* XXX remove the drive upon fail */
>> 6918 } else {
>>
>> Does anyone have a preference on how to deal with it
>> while we wait for a drive-removal function?
>> I think it deserves at least a diagnostic.
>
> Add a VIR_WARN message i guess
Here you go:
>From 62ece506ead7534ac37a70a6750a97e69809d088 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Tue, 18 May 2010 16:02:12 +0200
Subject: [PATCH] do not ignore qemuMonitorAddDrive failure; make uses identical
There were three very similar uses of qemuMonitorAddDrive.
This change makes the three 17-line sequences identical.
* src/qemu/qemu_driver.c (qemudDomainAttachPciDiskDevice): Detect
failure. Add VIR_WARN and braces.
(qemudDomainAttachSCSIDisk): Add VIR_WARN and braces.
(qemudDomainAttachUsbMassstorageDevice): Likewise.
---
src/qemu/qemu_driver.c | 44 +++++++++++++++++++++++++++++---------------
1 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5649a20..9d23191 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6912,15 +6912,21 @@ static int qemudDomainAttachPciDiskDevice(struct qemud_driver *driver,
goto error;
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0)
- qemuMonitorAddDevice(priv->mon, devstr);
- /* XXX remove the drive upon fail */
+ if (ret == 0) {
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+ if (ret < 0) {
+ VIR_WARN(_("qemuMonitorAddDevice failed on %s (%s)"),
+ drivestr, devstr);
+ /* XXX should call 'drive_del' on error but this does not
+ exist yet */
+ }
+ }
} else {
virDomainDevicePCIAddress guestAddr;
ret = qemuMonitorAddPCIDisk(priv->mon,
disk->src,
type,
&guestAddr);
@@ -7126,18 +7132,22 @@ static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver,
virReportOOMError();
goto error;
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
- ret = qemuMonitorAddDrive(priv->mon,
- drivestr);
- if (ret == 0)
- ret = qemuMonitorAddDevice(priv->mon,
- devstr);
- /* XXX should call 'drive_del' on error but this does not exist yet */
+ ret = qemuMonitorAddDrive(priv->mon, drivestr);
+ if (ret == 0) {
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+ if (ret < 0) {
+ VIR_WARN(_("qemuMonitorAddDevice failed on %s (%s)"),
+ drivestr, devstr);
+ /* XXX should call 'drive_del' on error but this does not
+ exist yet */
+ }
+ }
} else {
virDomainDeviceDriveAddress driveAddr;
ret = qemuMonitorAttachDrive(priv->mon,
drivestr,
&cont->info.addr.pci,
&driveAddr);
@@ -7215,18 +7225,22 @@ static int qemudDomainAttachUsbMassstorageDevice(struct qemud_driver *driver,
virReportOOMError();
goto error;
}
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
- ret = qemuMonitorAddDrive(priv->mon,
- drivestr);
- if (ret == 0)
- ret = qemuMonitorAddDevice(priv->mon,
- devstr);
- /* XXX should call 'drive_del' on error but this does not exist yet */
+ ret = qemuMonitorAddDrive(priv->mon, drivestr);
+ if (ret == 0) {
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+ if (ret < 0) {
+ VIR_WARN(_("qemuMonitorAddDevice failed on %s (%s)"),
+ drivestr, devstr);
+ /* XXX should call 'drive_del' on error but this does not
+ exist yet */
+ }
+ }
} else {
ret = qemuMonitorAddUSBDisk(priv->mon, disk->src);
}
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (ret < 0)
--
1.7.1.250.g7d1e8
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list