[libvirt] [PATCH 03/17] qemu: Move qemuDomainAttachDeviceDiskLive to qemu_hotplug.c

[John Ferlan]

On 08/01/2013 03:28 PM, Jiri Denemark wrote:
> ---
>  src/qemu/qemu_driver.c  | 114 --------------------------------------
>  src/qemu/qemu_hotplug.c | 142 ++++++++++++++++++++++++++++++++++++++++++++----
>  src/qemu/qemu_hotplug.h |  16 ++----
>  3 files changed, 134 insertions(+), 138 deletions(-)
> 

Although it seems it already existed, the code motion seems to have
awoken Coverity into determining that qemuDomainAttachDeviceDiskLive()
has unreachable code.

The 'cgroup' definition starts out as NULL, is never changed, and then
the condition to make the qemuTeardownDiskCgroup call is if cgroup has
been set.

In any case, considering it never is reached - the question remains
under what circumstances should the call be made.


John

...
> +int
> +qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> +                               virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainDeviceDefPtr dev)
> +{
> +    virDomainDiskDefPtr disk = dev->data.disk;
> +    virDomainDiskDefPtr orig_disk = NULL;
> +    virDomainDeviceDefPtr dev_copy = NULL;
> +    virDomainDiskDefPtr tmp = NULL;

685  	    virDomainDiskDefPtr tmp = NULL;

(1) Event assignment: 	Assigning: "cgroup" = "NULL".
Also see events: 	[null][dead_error_condition][dead_error_line]

686  	    virCgroupPtr cgroup = NULL;


> +    virCgroupPtr cgroup = NULL;
> +    virCapsPtr caps = NULL;
> +    int ret = -1;
> +
> +    if (disk->driverName != NULL && !STREQ(disk->driverName, "qemu")) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("unsupported driver name '%s' for disk '%s'"),
> +                       disk->driverName, disk->src);
> +        goto end;
> +    }
> +
> +    if (qemuTranslateDiskSourcePool(conn, disk) < 0)
> +        goto end;
> +
> +    if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
> +        goto end;
> +
> +    if (qemuSetUnprivSGIO(dev) < 0)
> +        goto end;
> +
> +    if (qemuDomainDetermineDiskChain(driver, disk, false) < 0)
> +        goto end;
> +
> +    if (qemuSetupDiskCgroup(vm, disk) < 0)
> +        goto end;
> +
> +    switch (disk->device)  {
> +    case VIR_DOMAIN_DISK_DEVICE_CDROM:
> +    case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> +        if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
> +                                                       disk->bus, disk->dst))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("No device with bus '%s' and target '%s'"),
> +                           virDomainDiskBusTypeToString(disk->bus),
> +                           disk->dst);
> +            goto end;
> +        }
> +
> +        if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> +            goto end;
> +
> +        tmp = dev->data.disk;
> +        dev->data.disk = orig_disk;
> +
> +        if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def,
> +                                                caps, driver->xmlopt))) {
> +            dev->data.disk = tmp;
> +            goto end;
> +        }
> +        dev->data.disk = tmp;
> +
> +        ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, false);
> +        /* 'disk' must not be accessed now - it has been free'd.
> +         * 'orig_disk' now points to the new disk, while 'dev_copy'
> +         * now points to the old disk */
> +
> +        /* Need to remove the shared disk entry for the original disk src
> +         * if the operation is either ejecting or updating.
> +         */
> +        if (ret == 0)
> +            ignore_value(qemuRemoveSharedDevice(driver, dev_copy,
> +                                                vm->def->name));
> +        break;
> +    case VIR_DOMAIN_DISK_DEVICE_DISK:
> +    case VIR_DOMAIN_DISK_DEVICE_LUN:
> +        if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
> +            if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("disk device='lun' is not supported for usb bus"));
> +                break;
> +            }
> +            ret = qemuDomainAttachUsbMassstorageDevice(conn, driver, vm,
> +                                                       disk);
> +        } else if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
> +            ret = qemuDomainAttachVirtioDiskDevice(conn, driver, vm, disk);
> +        } else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
> +            ret = qemuDomainAttachSCSIDisk(conn, driver, vm, disk);
> +        } else {
> +            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                           _("disk bus '%s' cannot be hotplugged."),
> +                           virDomainDiskBusTypeToString(disk->bus));
> +        }
> +        break;
> +    default:
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                       _("disk device type '%s' cannot be hotplugged"),
> +                       virDomainDiskDeviceTypeToString(disk->device));
> +        break;
> +    }
> +
775  	

(2) Event null: 	At condition "cgroup", the value of "cgroup" must be NULL.
(3) Event dead_error_condition: 	The condition "cgroup" cannot be true.
Also see events: 	[assignment][dead_error_line]

776  	    if (ret != 0 && cgroup) {

(4) Event dead_error_line: 	Execution cannot reach this statement "if
(qemuTeardownDiskCgroup(...".
Also see events: 	[assignment][null][dead_error_condition]

777  	        if (qemuTeardownDiskCgroup(vm, disk) < 0)


> +    if (ret != 0 && cgroup) {
> +        if (qemuTeardownDiskCgroup(vm, disk) < 0)
> +            VIR_WARN("Failed to teardown cgroup for disk path %s",
> +                     NULLSTR(disk->src));
> +    }
> +
> +end:
> +    if (ret != 0)
> +        ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
> +    virObjectUnref(caps);
> +    virDomainDeviceDefFree(dev_copy);
> +    return ret;
> +}
> +
> +

...