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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list