Re: [libvirt] [PATCHv2 45/62] qemu: hotplug: Implement removable media change for -blockdev

2018-08-22 Thread John Ferlan
[...]

> +static int
> +qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
> +  virDomainObjPtr vm,
> +  virDomainDiskDefPtr disk,
> +  virStorageSourcePtr newsrc,
> +  bool force)
> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> +qemuHotplugDiskSourceDataPtr newbackend = NULL;
> +qemuHotplugDiskSourceDataPtr oldbackend = NULL;
> +virStorageSourcePtr oldsrc = disk->src;
> +char *nodename = NULL;
> +int rc;
> +int ret = -1;
> +
> +if (!virStorageSourceIsEmpty(disk->src) &&
> +!(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, 
> priv->qemuCaps)))
> +goto cleanup;
> +
> +disk->src = newsrc;
> +if (!virStorageSourceIsEmpty(disk->src)) {
> +if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk,
> +  
> priv->qemuCaps)))
> +goto cleanup;
> +
> +if (qemuDomainDiskGetBackendAlias(disk, priv->qemuCaps, ) < 
> 0)

Coverity notes @nodename is leaked at cleanup:

John

BTW: Only 2 Coverity whines for a large series and you got rid of one of
my false positives by removing the detach->info.alias creation done in
qemuDomainDetachVirtioDiskDevice. Something similar I suppose could be
done for qemuDomainDetachControllerDevice using the same reasoning.


> +goto cleanup;
> +}
> +
> +if (diskPriv->tray && disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) {
> +qemuDomainObjEnterMonitor(driver, vm);
> +rc = qemuMonitorBlockdevTrayOpen(priv->mon, 
> diskPriv->backendQomName, force);
> +if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
> +goto cleanup;
> +
> +if (!force && qemuHotplugWaitForTrayEject(vm, disk) < 0)
> +goto cleanup;
> +}
> +
> +qemuDomainObjEnterMonitor(driver, vm);
> +
> +rc = qemuMonitorBlockdevMediumRemove(priv->mon, 
> diskPriv->backendQomName);
> +
> +if (rc == 0 && oldbackend)
> +qemuHotplugDiskSourceRemove(priv->mon, oldbackend);
> +
> +if (newbackend && nodename) {
> +if (rc == 0)
> +rc = qemuHotplugDiskSourceAttach(priv->mon, newbackend);
> +
> +if (rc == 0)
> +rc = qemuMonitorBlockdevMediumInsert(priv->mon,
> + diskPriv->backendQomName,
> + nodename);
> +}
> +
> +if (rc == 0)
> +rc = qemuMonitorBlockdevTrayClose(priv->mon, 
> diskPriv->backendQomName);
> +
> +if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
> +goto cleanup;
> +
> +ret = 0;
> +
> + cleanup:
> +qemuHotplugDiskSourceDataFree(newbackend);
> +qemuHotplugDiskSourceDataFree(oldbackend);
> +/* caller handles correct exchange of sources */
> +disk->src = oldsrc;
> +return ret;
> +}
> +
> +

[...]

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


Re: [libvirt] [PATCHv2 45/62] qemu: hotplug: Implement removable media change for -blockdev

2018-08-16 Thread Ján Tomko

On Mon, Aug 13, 2018 at 06:00:19PM +0200, Peter Krempa wrote:

Use the new APIs which allow to manipulate the tray and media separately
and also allow using a nodename to refer to a media to implement media
changing.

With the new approach we don't have to call eject twice as the media is
removed by calling qemuMonitorBlockdevMediumRemove.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_hotplug.c | 95 -
1 file changed, 94 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv2 45/62] qemu: hotplug: Implement removable media change for -blockdev

2018-08-13 Thread Peter Krempa
Use the new APIs which allow to manipulate the tray and media separately
and also allow using a nodename to refer to a media to implement media
changing.

With the new approach we don't have to call eject twice as the media is
removed by calling qemuMonitorBlockdevMediumRemove.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_hotplug.c | 95 -
 1 file changed, 94 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6fde7308f7..9e57bffa48 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -618,6 +618,95 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon,
 }


+/**
+ * qemuDomainChangeMediaBlockdev:
+ * @driver: qemu driver structure
+ * @vm: domain definition
+ * @disk: disk definition to change the source of
+ * @newsrc: new disk source to change to
+ * @force: force the change of media
+ *
+ * Change the media in an ejectable device to the one described by
+ * @newsrc. This function also removes the old source from the
+ * shared device table if appropriate. Note that newsrc is consumed
+ * on success and the old source is freed on success.
+ *
+ * Returns 0 on success, -1 on error and reports libvirt error
+ */
+static int
+qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virDomainDiskDefPtr disk,
+  virStorageSourcePtr newsrc,
+  bool force)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+qemuHotplugDiskSourceDataPtr newbackend = NULL;
+qemuHotplugDiskSourceDataPtr oldbackend = NULL;
+virStorageSourcePtr oldsrc = disk->src;
+char *nodename = NULL;
+int rc;
+int ret = -1;
+
+if (!virStorageSourceIsEmpty(disk->src) &&
+!(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, 
priv->qemuCaps)))
+goto cleanup;
+
+disk->src = newsrc;
+if (!virStorageSourceIsEmpty(disk->src)) {
+if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk,
+  priv->qemuCaps)))
+goto cleanup;
+
+if (qemuDomainDiskGetBackendAlias(disk, priv->qemuCaps, ) < 0)
+goto cleanup;
+}
+
+if (diskPriv->tray && disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) {
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorBlockdevTrayOpen(priv->mon, diskPriv->backendQomName, 
force);
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+goto cleanup;
+
+if (!force && qemuHotplugWaitForTrayEject(vm, disk) < 0)
+goto cleanup;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+rc = qemuMonitorBlockdevMediumRemove(priv->mon, diskPriv->backendQomName);
+
+if (rc == 0 && oldbackend)
+qemuHotplugDiskSourceRemove(priv->mon, oldbackend);
+
+if (newbackend && nodename) {
+if (rc == 0)
+rc = qemuHotplugDiskSourceAttach(priv->mon, newbackend);
+
+if (rc == 0)
+rc = qemuMonitorBlockdevMediumInsert(priv->mon,
+ diskPriv->backendQomName,
+ nodename);
+}
+
+if (rc == 0)
+rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->backendQomName);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+qemuHotplugDiskSourceDataFree(newbackend);
+qemuHotplugDiskSourceDataFree(oldbackend);
+/* caller handles correct exchange of sources */
+disk->src = oldsrc;
+return ret;
+}
+
+
 /**
  * qemuDomainChangeEjectableMedia:
  * @driver: qemu driver structure
@@ -640,6 +729,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virStorageSourcePtr newsrc,
bool force)
 {
+qemuDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1;
 int rc;

@@ -649,7 +739,10 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 
0)
 goto cleanup;

-rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
+rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, newsrc, force);
+else
+rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);

 virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);

-- 
2.16.2

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