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

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;