[libvirt] [PATCH 4/8] qemu: hotplug: Prepare disk source for media changing

Peter Krempa posted 8 patches 7 years, 4 months ago
[libvirt] [PATCH 4/8] qemu: hotplug: Prepare disk source for media changing
Posted by Peter Krempa 7 years, 4 months ago
The disk storage source needs to be prepared if we want to use -blockdev
or secrets for the new media image.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_hotplug.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3043b3257b..1ec696e2aa 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -729,6 +729,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
                                virStorageSourcePtr newsrc,
                                bool force)
 {
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virStorageSourcePtr oldsrc = disk->src;
     int ret = -1;
@@ -736,6 +737,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,

     disk->src = newsrc;

+    if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
+        goto cleanup;
+
     if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0)
         goto cleanup;

@@ -770,6 +774,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     if (oldsrc)
         disk->src = oldsrc;

+    virObjectUnref(cfg);
     return ret;
 }

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] qemu: hotplug: Prepare disk source for media changing
Posted by John Ferlan 7 years, 4 months ago

On 9/27/18 11:09 AM, Peter Krempa wrote:
> The disk storage source needs to be prepared if we want to use -blockdev
> or secrets for the new media image.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 3043b3257b..1ec696e2aa 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -729,6 +729,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>                                 virStorageSourcePtr newsrc,
>                                 bool force)
>  {
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virStorageSourcePtr oldsrc = disk->src;
>      int ret = -1;
> @@ -736,6 +737,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> 
>      disk->src = newsrc;
> 
> +    if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
> +        goto cleanup;
> +

"Assuming" this becomes passing "@newdisk" along to
qemuDomainPrepareDiskSource, then this would seem to be fine.

Compared to the order in AttachDiskGeneric, should this go under
PrepareDiskAccess? And since qemuAssignDeviceDiskAlias doesn't use the
disk->src, then it doesn't need to be called - whether you want to note
that or not, IDC.

I didn't go chasing into qemuHotplugAttachManagedPR to determine whether
perhaps that failure should have "undone" qemuHotplugPrepareDiskAccess,
but looking at qemuDomainAttachDiskGeneric, I think the answer may be
yes.  IOW: Perhaps an error label would be needed.

John

>      if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0)
>          goto cleanup;
> 
> @@ -770,6 +774,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>      if (oldsrc)
>          disk->src = oldsrc;
> 
> +    virObjectUnref(cfg);
>      return ret;
>  }
> 

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