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

Re: [libvirt] [PATCH v4 6/8] virFDStream: Add option for delete file after it's opening



On Thu, May 12, 2011 at 06:29:13PM +0200, Michal Privoznik wrote:
> This is needed if we want to transfer a temporary file. If the
> transfer is done with iohelper, we might run into a race condition,
> where we unlink() file before iohelper is executed.
> 
> * src/fdstream.c, src/fdstream.h,
>   src/util/iohelper.c: Add new option
> * src/lxc/lxc_driver.c, src/qemu/qemu_driver.c,
>   src/storage/storage_driver.c, src/uml/uml_driver.c,
>   src/xen/xen_driver.c: Expand existing function calls
> ---
>  src/fdstream.c               |   29 ++++++++++++++++++++++-------
>  src/fdstream.h               |    6 ++++--
>  src/lxc/lxc_driver.c         |    3 ++-
>  src/qemu/qemu_driver.c       |    3 ++-
>  src/storage/storage_driver.c |    4 ++--
>  src/uml/uml_driver.c         |    3 ++-
>  src/util/iohelper.c          |   12 ++++++++++--
>  src/xen/xen_driver.c         |    3 ++-
>  8 files changed, 46 insertions(+), 17 deletions(-)
> 
> diff --git a/src/fdstream.c b/src/fdstream.c
> index d2325ae..2702ad7 100644
> --- a/src/fdstream.c
> +++ b/src/fdstream.c
> @@ -493,7 +493,8 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>                              unsigned long long offset,
>                              unsigned long long length,
>                              int flags,
> -                            int mode)
> +                            int mode,
> +                            bool delete)
>  {
>      int fd = -1;
>      int fds[2] = { -1, -1 };
> @@ -502,8 +503,8 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>      int errfd = -1;
>      pid_t pid = 0;
>  
> -    VIR_DEBUG("st=%p path=%s flags=%d offset=%llu length=%llu mode=%d",
> -              st, path, flags, offset, length, mode);
> +    VIR_DEBUG("st=%p path=%s flags=%d offset=%llu length=%llu mode=%d delete=%d",
> +              st, path, flags, offset, length, mode, delete);
>  
>      if (flags & O_CREAT)
>          fd = open(path, flags, mode);
> @@ -554,6 +555,14 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>          virCommandAddArgFormat(cmd, "%d", mode);
>          virCommandAddArgFormat(cmd, "%llu", offset);
>          virCommandAddArgFormat(cmd, "%llu", length);
> +        virCommandAddArgFormat(cmd, "%u", delete);
> +
> +        /* when running iohelper we don't want to delete file now,
> +         * because a race condition may occur in which we delete it
> +         * before iohelper even opens it. We want iohelper to remove
> +         * the file instead.
> +         */
> +        delete = false;
>  
>          if (flags == O_RDONLY) {
>              childfd = fds[1];
> @@ -583,6 +592,9 @@ virFDStreamOpenFileInternal(virStreamPtr st,
>      if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0)
>          goto error;
>  
> +    if (delete)
> +        unlink(path);
> +
>      return 0;
>  
>  error:
> @@ -601,7 +613,8 @@ int virFDStreamOpenFile(virStreamPtr st,
>                          const char *path,
>                          unsigned long long offset,
>                          unsigned long long length,
> -                        int flags)
> +                        int flags,
> +                        bool delete)
>  {
>      if (flags & O_CREAT) {
>          streamsReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -611,7 +624,7 @@ int virFDStreamOpenFile(virStreamPtr st,
>      }
>      return virFDStreamOpenFileInternal(st, path,
>                                         offset, length,
> -                                       flags, 0);
> +                                       flags, 0, delete);
>  }
>  
>  int virFDStreamCreateFile(virStreamPtr st,
> @@ -619,9 +632,11 @@ int virFDStreamCreateFile(virStreamPtr st,
>                            unsigned long long offset,
>                            unsigned long long length,
>                            int flags,
> -                          mode_t mode)
> +                          mode_t mode,
> +                          bool delete)
>  {
>      return virFDStreamOpenFileInternal(st, path,
>                                         offset, length,
> -                                       flags | O_CREAT, mode);
> +                                       flags | O_CREAT,
> +                                       mode, delete);
>  }
> diff --git a/src/fdstream.h b/src/fdstream.h
> index 6b395b6..a66902b 100644
> --- a/src/fdstream.h
> +++ b/src/fdstream.h
> @@ -37,12 +37,14 @@ int virFDStreamOpenFile(virStreamPtr st,
>                          const char *path,
>                          unsigned long long offset,
>                          unsigned long long length,
> -                        int flags);
> +                        int flags,
> +                        bool delete);
>  int virFDStreamCreateFile(virStreamPtr st,
>                            const char *path,
>                            unsigned long long offset,
>                            unsigned long long length,
>                            int flags,
> -                          mode_t mode);
> +                          mode_t mode,
> +                          bool delete);
>  
>  #endif /* __VIR_FDSTREAM_H_ */
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 1aadb02..0939a1d 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -2691,7 +2691,8 @@ lxcDomainOpenConsole(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0)
> +    if (virFDStreamOpenFile(st, chr->source.data.file.path,
> +                            0, 0, O_RDWR, false) < 0)
>          goto cleanup;
>  
>      ret = 0;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7a3556f..482f177 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7120,7 +7120,8 @@ qemuDomainOpenConsole(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0)
> +    if (virFDStreamOpenFile(st, chr->source.data.file.path,
> +                            0, 0, O_RDWR, false) < 0)
>          goto cleanup;
>  
>      ret = 0;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 903ecee..6542579 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1592,7 +1592,7 @@ storageVolumeDownload(virStorageVolPtr obj,
>      if (virFDStreamOpenFile(stream,
>                              vol->target.path,
>                              offset, length,
> -                            O_RDONLY) < 0)
> +                            O_RDONLY, false) < 0)
>          goto out;
>  
>      ret = 0;
> @@ -1656,7 +1656,7 @@ storageVolumeUpload(virStorageVolPtr obj,
>      if (virFDStreamOpenFile(stream,
>                              vol->target.path,
>                              offset, length,
> -                            O_WRONLY) < 0)
> +                            O_WRONLY, false) < 0)
>          goto out;
>  
>      ret = 0;
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index baef86c..e7cd77a 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -2130,7 +2130,8 @@ umlDomainOpenConsole(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0)
> +    if (virFDStreamOpenFile(st, chr->source.data.file.path,
> +                            0, 0, O_RDWR, false) < 0)
>          goto cleanup;
>  
>      ret = 0;
> diff --git a/src/util/iohelper.c b/src/util/iohelper.c
> index d5821b9..f519d5a 100644
> --- a/src/util/iohelper.c
> +++ b/src/util/iohelper.c
> @@ -146,6 +146,7 @@ int main(int argc, char **argv)
>      unsigned long long length;
>      int flags;
>      int mode;
> +    unsigned int delete;
>  
>      if (setlocale(LC_ALL, "") == NULL ||
>          bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
> @@ -161,8 +162,8 @@ int main(int argc, char **argv)
>          exit(EXIT_FAILURE);
>      }
>  
> -    if (argc != 6) {
> -        fprintf(stderr, _("%s: syntax FILENAME FLAGS MODE OFFSET LENGTH\n"), argv[0]);
> +    if (argc != 7) {
> +        fprintf(stderr, _("%s: syntax FILENAME FLAGS MODE OFFSET LENGTH DELETE\n"), argv[0]);
>          exit(EXIT_FAILURE);
>      }
>  
> @@ -186,10 +187,17 @@ int main(int argc, char **argv)
>          fprintf(stderr, _("%s: malformed file length %s"), argv[0], argv[5]);
>          exit(EXIT_FAILURE);
>      }
> +    if (virStrToLong_ui(argv[6], NULL, 10, &delete) < 0) {
> +        fprintf(stderr, _("%s: malformed delete flag %s"), argv[0],argv[6]);
> +        exit(EXIT_FAILURE);
> +    }
>  
>      if (runIO(path, flags, mode, offset, length) < 0)
>          goto error;
>  
> +    if (delete)
> +        unlink(path);
> +
>      return 0;
>  
>  error:
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 0f19d27..5bafb73 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -2086,7 +2086,8 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0)
> +    if (virFDStreamOpenFile(st, chr->source.data.file.path,
> +                            0, 0, O_RDWR, false) < 0)
>          goto cleanup;
>  
>      ret = 0;

ACK


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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