Re: [PATCH RFC 28/39] qemu: hotplug: Prepare for blockdev-add/blockdev-del with backing chains

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Wed, Jul 25, 2018 at 05:57:59PM +0200, Peter Krempa wrote:
> Initialize data for the whole backing chain when plugging in or removing
> disks when a machine supports -blockdev.
>
> Similarly to startup we need to prepare the structures for the whole
> backing chain and take care of the copy-on-read feature.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> src/qemu/qemu_hotplug.c | 75 ++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 61 insertions(+), 14 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 87efd3b411..3eddb0043e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -380,6 +380,10 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver,
> struct _qemuHotplugDiskSourceData {
>     qemuBlockStorageSourceAttachDataPtr *backends;
>     size_t nbackends;
> +
> +    /* disk copy-on-read object */
> +    virJSONValuePtr corProps;
> +    char *corAlias;
> };
> typedef struct _qemuHotplugDiskSourceData qemuHotplugDiskSourceData;
> typedef qemuHotplugDiskSourceData *qemuHotplugDiskSourceDataPtr;
> @@ -393,6 +397,9 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data)
>     if (!data)
>         return;
>
> +    virJSONValueFree(data->corProps);
> +    VIR_FREE(data->corAlias);
> +
>     for (i = 0; i < data->nbackends; i++)
>         qemuBlockStorageSourceAttachDataFree(data->backends[i]);
>
> @@ -461,25 +468,40 @@ qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src,
>
> static qemuHotplugDiskSourceDataPtr
> qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
> -                                   virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
> +                                   virQEMUCapsPtr qemuCaps)
> {
> +    qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>     qemuBlockStorageSourceAttachDataPtr backend = NULL;
>     qemuHotplugDiskSourceDataPtr data = NULL;
>     qemuHotplugDiskSourceDataPtr ret = NULL;
>     char *drivealias = NULL;
> +    virStorageSourcePtr n;
>
>     if (VIR_ALLOC(data) < 0)
>         return NULL;
>
> -    if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
> -        goto cleanup;
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
> +        if (VIR_STRDUP(data->corAlias, diskPriv->nodeCopyOnRead) < 0)
> +            goto cleanup;
>
> -    if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src,
> -                                                              drivealias)))
> -        goto cleanup;
> +        for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
> +            if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(n, NULL)))
> +                goto cleanup;
>
> -    if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
> -        goto cleanup;
> +            if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
> +                goto cleanup;
> +        }
> +    } else {
> +        if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
> +            goto cleanup;
> +
> +        if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src,
> +                                                                  drivealias)))
> +            goto cleanup;
> +
> +        if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
> +            goto cleanup;
> +    }
>
>     VIR_STEAL_PTR(ret, data);
>
> @@ -505,18 +527,36 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
>     qemuBlockStorageSourceAttachDataPtr backend;
>     qemuHotplugDiskSourceDataPtr data;
>     qemuHotplugDiskSourceDataPtr ret = NULL;
> +    virStorageSourcePtr n;
>
>     if (VIR_ALLOC(data) < 0)
>         return NULL;
>
> -    if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, false)))
> -        goto cleanup;
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
> +        if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
> +            !(data->corProps = qemuBlockStorageGetCopyOnReadProps(disk)))
> +            goto cleanup;

CC       qemu/libvirt_driver_qemu_impl_la-qemu_hotplug.lo
qemu/qemu_hotplug.c:536:13: error: variable 'backend' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
       if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
qemu/qemu_hotplug.c:564:42: note: uninitialized use occurs here
   qemuBlockStorageSourceAttachDataFree(backend);
                                        ^~~~~~~
qemu/qemu_hotplug.c:536:9: note: remove the 'if' if its condition is always false
       if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
qemu/qemu_hotplug.c:527:48: note: initialize the variable 'backend' to silence this warning
   qemuBlockStorageSourceAttachDataPtr backend;
                                              ^
                                               = NULL

> -    if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend, qemuCaps) < 0)
> -        goto cleanup;
> +        for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
> +            if (!(backend = qemuBlockStorageSourceAttachPrepareBlockdev(n)))
> +                goto cleanup;

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list