Use VIR_AUTOFREE and remove the cleanup label.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_migration.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1a557851bb..82625b2261 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -868,36 +868,31 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver,
                                           unsigned long long mirror_speed,
                                           bool mirror_shallow)
 {
-    char *nbd_dest = NULL;
+    VIR_AUTOFREE(char *) nbd_dest = NULL;
     int mon_ret;
-    int ret = -1;

     if (strchr(host, ':')) {
         if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s",
                         host, port, diskAlias) < 0)
-            goto cleanup;
+            return -1;
     } else {
         if (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s",
                         host, port, diskAlias) < 0)
-            goto cleanup;
+            return -1;
     }

     if (qemuDomainObjEnterMonitorAsync(driver, vm,
                                        QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
-        goto cleanup;
+        return -1;

     mon_ret = qemuMonitorDriveMirror(qemuDomainGetMonitor(vm),
                                      diskAlias, nbd_dest, "raw",
                                      mirror_speed, 0, 0, mirror_shallow, true);

     if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0)
-        goto cleanup;
-
-    ret = 0;
+        return -1;

- cleanup:
-    VIR_FREE(nbd_dest);
-    return ret;
+    return 0;
 }


-- 
2.21.0

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

On 9/4/19 10:39 AM, Peter Krempa wrote:
> Use VIR_AUTOFREE and remove the cleanup label.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_migration.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 1a557851bb..82625b2261 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -868,36 +868,31 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver,
>                                            unsigned long long mirror_speed,
>                                            bool mirror_shallow)
>  {
> -    char *nbd_dest = NULL;
> +    VIR_AUTOFREE(char *) nbd_dest = NULL;
>      int mon_ret;
> -    int ret = -1;
> 
>      if (strchr(host, ':')) {
>          if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s",
>                          host, port, diskAlias) < 0)

Should we prefer "nbd://[%s]:%d/%s" here, now that the NBD URI spec is
available (and since qemu supports that for quite some time now)?
https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md

> -            goto cleanup;
> +            return -1;
>      } else {
>          if (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s",

and similar here?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

On Wed, Sep 04, 2019 at 12:01:27 -0500, Eric Blake wrote:
> On 9/4/19 10:39 AM, Peter Krempa wrote:
> > Use VIR_AUTOFREE and remove the cleanup label.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/qemu/qemu_migration.c | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 1a557851bb..82625b2261 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -868,36 +868,31 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver,
> >                                            unsigned long long mirror_speed,
> >                                            bool mirror_shallow)
> >  {
> > -    char *nbd_dest = NULL;
> > +    VIR_AUTOFREE(char *) nbd_dest = NULL;
> >      int mon_ret;
> > -    int ret = -1;
> > 
> >      if (strchr(host, ':')) {
> >          if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s",
> >                          host, port, diskAlias) < 0)
> 
> Should we prefer "nbd://[%s]:%d/%s" here, now that the NBD URI spec is
> available (and since qemu supports that for quite some time now)?
> https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md

That commit is literally 2 days old. Also this code is executed on all
supported qemus (starting from 1.5.3) so you'd need to be more specific
on the 'quite some time now'.

Additionally this patchset actually stops calling this code path when
blockdev is used so I'm not going to invest into investigating whether
the URI format is really supported on ancient qemus if it's not going to
be used in the future anyways.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list