Re: [PATCH 3/5] qemu: blockcopy: reuse storage driver APIs to pre-create copy target

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 19, 2017 at 17:44:16 -0400, John Ferlan wrote:
> 
> 
> On 07/11/2017 11:46 AM, Peter Krempa wrote:
> > Rather than using the local-file only implementation 'qemuOpenFile'
> > switch to the imagelabel aware storage driver implementation.
> > ---
> >  src/qemu/qemu_driver.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index d00166f23..48dc5e5cc 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c

[...]

> > @@ -16880,8 +16885,10 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
> >                   vm->def->name);
> > 
> >   endjob:
> > -    if (need_unlink && unlink(mirror->path))
> > -        VIR_WARN("unable to unlink just-created %s", mirror->path);
> > +    if (need_unlink && virStorageFileUnlink(mirror))
> 
> ...Unlink(mirror) < 0)
> 
> > +        VIR_WARN("%s", _("unable to remove just-created copy target"));
> > +    virStorageFileDeinit(mirror);
> > +    virStorageSourceFree(mirror);
> 
> Isn't this duplicitous with the same in the cleanup: label

Yes indeed. It would even crash on the error paths if 'mirror' was not
consumed into the disk definition structure. Good catch.

> 
> >      qemuDomainObjEndJob(driver, vm);
> >      if (monitor_error) {
> >          virSetError(monitor_error);
> > 
> 
> with fixups...
> 
> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>

Thanks

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux