Re: [libvirt] [PATCH 2/4] qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopyDriveMirror

[Peter Krempa]

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 <pkre...@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.

signature.asc
Description: PGP signature

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