On Tue, May 18, 2010 at 04:02:44PM +0200, Jim Meyering wrote:
> 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)
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list