From 00ef048f62c9789e5d2f8848e16627cc30248259 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 2 Aug 2011 11:19:53 -0600 Subject: [PATCH] fdstream: drop delete argument Revert 6a1f5f568f8. Now that libvirt_iohelper takes fds by inheritance rather than by open() (commit 1eb66479), there is no longer a race where the parent can unlink() a file prior to the iohelper open()ing the same file. From there, it makes more sense to have the callers both create and unlink, rather than the caller create and the stream unlink, since the latter was only needed when iohelper had to do the unlink. * src/fdstream.h (virFDStreamOpenFile, virFDStreamCreateFile): Callers are responsible for deletion. * src/fdstream.c (virFDStreamOpenFileInternal): Don't leak created file on failure. (virFDStreamOpenFile, virFDStreamCreateFile): Drop parameter. * src/lxc/lxc_driver.c (lxcDomainOpenConsole): Update callers. * src/qemu/qemu_driver.c (qemuDomainScreenshot) (qemuDomainOpenConsole): Likewise. * src/storage/storage_driver.c (storageVolumeDownload) (storageVolumeUpload): Likewise. * src/uml/uml_driver.c (umlDomainOpenConsole): Likewise. * src/vbox/vbox_tmpl.c (vboxDomainScreenshot): Likewise. * src/xen/xen_driver.c (xenUnifiedDomainOpenConsole): Likewise. --- src/fdstream.c | 23 +++++++++-------------- src/fdstream.h | 6 ++---- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 11 ++++++----- src/storage/storage_driver.c | 4 ++-- src/uml/uml_driver.c | 2 +- src/vbox/vbox_tmpl.c | 8 ++------ src/xen/xen_driver.c | 2 +- 8 files changed, 24 insertions(+), 34 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index 2b7812ff04..b60162c6a1 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -505,8 +505,7 @@ virFDStreamOpenFileInternal(virStreamPtr st, unsigned long long offset, unsigned long long length, int oflags, - int mode, - bool delete) + int mode) { int fd = -1; int fds[2] = { -1, -1 }; @@ -514,8 +513,8 @@ virFDStreamOpenFileInternal(virStreamPtr st, virCommandPtr cmd = NULL; int errfd = -1; - VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o delete=%d", - st, path, oflags, offset, length, mode, delete); + VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o", + st, path, oflags, offset, length, mode); if (oflags & O_CREAT) fd = open(path, oflags, mode); @@ -593,9 +592,6 @@ virFDStreamOpenFileInternal(virStreamPtr st, if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0) goto error; - if (delete) - unlink(path); - return 0; error: @@ -603,6 +599,8 @@ error: VIR_FORCE_CLOSE(fds[0]); VIR_FORCE_CLOSE(fds[1]); VIR_FORCE_CLOSE(fd); + if (oflags & O_CREAT) + unlink(path); return -1; } @@ -610,8 +608,7 @@ int virFDStreamOpenFile(virStreamPtr st, const char *path, unsigned long long offset, unsigned long long length, - int oflags, - bool delete) + int oflags) { if (oflags & O_CREAT) { streamsReportError(VIR_ERR_INTERNAL_ERROR, @@ -621,7 +618,7 @@ int virFDStreamOpenFile(virStreamPtr st, } return virFDStreamOpenFileInternal(st, path, offset, length, - oflags, 0, delete); + oflags, 0); } int virFDStreamCreateFile(virStreamPtr st, @@ -629,11 +626,9 @@ int virFDStreamCreateFile(virStreamPtr st, unsigned long long offset, unsigned long long length, int oflags, - mode_t mode, - bool delete) + mode_t mode) { return virFDStreamOpenFileInternal(st, path, offset, length, - oflags | O_CREAT, - mode, delete); + oflags | O_CREAT, mode); } diff --git a/src/fdstream.h b/src/fdstream.h index 76f0cd030f..f9edb23e9e 100644 --- a/src/fdstream.h +++ b/src/fdstream.h @@ -37,14 +37,12 @@ int virFDStreamOpenFile(virStreamPtr st, const char *path, unsigned long long offset, unsigned long long length, - int oflags, - bool delete); + int oflags); int virFDStreamCreateFile(virStreamPtr st, const char *path, unsigned long long offset, unsigned long long length, int oflags, - mode_t mode, - bool delete); + mode_t mode); #endif /* __VIR_FDSTREAM_H_ */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 7d99d27ee8..2d94309399 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2889,7 +2889,7 @@ lxcDomainOpenConsole(virDomainPtr dom, } if (virFDStreamOpenFile(st, chr->source.data.file.path, - 0, 0, O_RDWR, false) < 0) + 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 02971592d6..5c6d1b876f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2862,6 +2862,7 @@ qemuDomainScreenshot(virDomainPtr dom, char *tmp = NULL; int tmp_fd = -1; char *ret = NULL; + bool unlink_tmp = false; virCheckFlags(0, NULL); @@ -2906,27 +2907,25 @@ qemuDomainScreenshot(virDomainPtr dom, virReportSystemError(errno, _("mkstemp(\"%s\") failed"), tmp); goto endjob; } + unlink_tmp = true; virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, vm, tmp); qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorScreendump(priv->mon, tmp) < 0) { qemuDomainObjExitMonitor(driver, vm); - unlink(tmp); goto endjob; } qemuDomainObjExitMonitor(driver, vm); if (VIR_CLOSE(tmp_fd) < 0) { virReportSystemError(errno, _("unable to close %s"), tmp); - unlink(tmp); goto endjob; } - if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY, true) < 0) { + if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to open stream")); - unlink(tmp); goto endjob; } @@ -2934,6 +2933,8 @@ qemuDomainScreenshot(virDomainPtr dom, endjob: VIR_FORCE_CLOSE(tmp_fd); + if (unlink_tmp) + unlink(tmp); VIR_FREE(tmp); if (qemuDomainObjEndJob(driver, vm) == 0) @@ -9259,7 +9260,7 @@ qemuDomainOpenConsole(virDomainPtr dom, } if (virFDStreamOpenFile(st, chr->source.data.file.path, - 0, 0, O_RDWR, false) < 0) + 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 9c353e394e..6715790a6f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1614,7 +1614,7 @@ storageVolumeDownload(virStorageVolPtr obj, if (virFDStreamOpenFile(stream, vol->target.path, offset, length, - O_RDONLY, false) < 0) + O_RDONLY) < 0) goto out; ret = 0; @@ -1678,7 +1678,7 @@ storageVolumeUpload(virStorageVolPtr obj, if (virFDStreamOpenFile(stream, vol->target.path, offset, length, - O_WRONLY, false) < 0) + O_WRONLY) < 0) goto out; ret = 0; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 6d04120156..a9cb7efa1a 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2295,7 +2295,7 @@ umlDomainOpenConsole(virDomainPtr dom, } if (virFDStreamOpenFile(st, chr->source.data.file.path, - 0, 0, O_RDWR, false) < 0) + 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 66a0fe9365..5c717299c0 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8713,7 +8713,6 @@ vboxDomainScreenshot(virDomainPtr dom, if (NS_FAILED(rc) || !width || !height) { vboxError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to get screen resolution")); - unlink(tmp); goto endjob; } @@ -8724,7 +8723,6 @@ vboxDomainScreenshot(virDomainPtr dom, if (NS_FAILED(rc)) { vboxError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to take screenshot")); - unlink(tmp); goto endjob; } @@ -8732,20 +8730,17 @@ vboxDomainScreenshot(virDomainPtr dom, screenDataSize) < 0) { virReportSystemError(errno, _("unable to write data " "to '%s'"), tmp); - unlink(tmp); goto endjob; } if (VIR_CLOSE(tmp_fd) < 0) { virReportSystemError(errno, _("unable to close %s"), tmp); - unlink(tmp); goto endjob; } - if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY, true) < 0) { + if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY) < 0) { vboxError(VIR_ERR_OPERATION_FAILED, "%s", _("unable to open stream")); - unlink(tmp); goto endjob; } @@ -8761,6 +8756,7 @@ endjob: } VIR_FORCE_CLOSE(tmp_fd); + unlink(tmp); VIR_FREE(tmp); VBOX_RELEASE(machine); vboxIIDUnalloc(&iid); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 76506fb9c8..2c66143c08 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2164,7 +2164,7 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom, } if (virFDStreamOpenFile(st, chr->source.data.file.path, - 0, 0, O_RDWR, false) < 0) + 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; -- 2.39.5