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
@@ -16786,6 +16786,10 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
                        _("non-file destination not supported yet"));
         goto endjob;
     }
+
+    if (qemuDomainStorageFileInit(driver, vm, mirror) < 0)
+        goto endjob;
+
     if (stat(mirror->path, &st) < 0) {
         if (errno != ENOENT) {
             virReportSystemError(errno, _("unable to stat for disk %s: %s"),
@@ -16833,12 +16837,12 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,

     /* pre-create the image file */
     if (!reuse) {
-        int fd = qemuOpenFile(driver, vm, mirror->path,
-                              O_WRONLY | O_TRUNC | O_CREAT,
-                              &need_unlink, NULL);
-        if (fd < 0)
+        if (virStorageFileCreate(mirror) < 0) {
+            virReportSystemError(errno, "%s", _("failed to create copy target"));
             goto endjob;
-        VIR_FORCE_CLOSE(fd);
+        }
+
+        need_unlink = true;
     }

     if (mirror->format > 0)
@@ -16870,6 +16874,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,

     /* Update vm in place to match changes.  */
     need_unlink = false;
+    virStorageFileDeinit(mirror);
     disk->mirror = mirror;
     mirror = NULL;
     disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
@@ -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))
+        VIR_WARN("%s", _("unable to remove just-created copy target"));
+    virStorageFileDeinit(mirror);
+    virStorageSourceFree(mirror);
     qemuDomainObjEndJob(driver, vm);
     if (monitor_error) {
         virSetError(monitor_error);
-- 
2.12.2

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

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
> @@ -16786,6 +16786,10 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
>                         _("non-file destination not supported yet"));
>          goto endjob;
>      }
> +
> +    if (qemuDomainStorageFileInit(driver, vm, mirror) < 0)
> +        goto endjob;
> +
>      if (stat(mirror->path, &st) < 0) {
>          if (errno != ENOENT) {
>              virReportSystemError(errno, _("unable to stat for disk %s: %s"),
> @@ -16833,12 +16837,12 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
> 
>      /* pre-create the image file */
>      if (!reuse) {
> -        int fd = qemuOpenFile(driver, vm, mirror->path,
> -                              O_WRONLY | O_TRUNC | O_CREAT,
> -                              &need_unlink, NULL);
> -        if (fd < 0)
> +        if (virStorageFileCreate(mirror) < 0) {
> +            virReportSystemError(errno, "%s", _("failed to create copy target"));
>              goto endjob;
> -        VIR_FORCE_CLOSE(fd);
> +        }
> +
> +        need_unlink = true;
>      }
> 
>      if (mirror->format > 0)
> @@ -16870,6 +16874,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
> 
>      /* Update vm in place to match changes.  */
>      need_unlink = false;
> +    virStorageFileDeinit(mirror);
>      disk->mirror = mirror;
>      mirror = NULL;
>      disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
> @@ -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

>      qemuDomainObjEndJob(driver, vm);
>      if (monitor_error) {
>          virSetError(monitor_error);
> 

with fixups...

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

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

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@redhat.com>

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