12:31 p.m.
Revert 6a1f5f568f8. Now that libvirt_iohelper no longer has a
race where it can open() a file after the parent process has
unlink()d the same file, it makes more sense to make the callers
both create and unlink, rather than the caller create and the
stream 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.
---
This is the bigger patch, which I'm worried may be too invasive
for 0.9.4 this late in the release cycle, but is cleaner overall.
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 2b7812f..b60162c 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 76f0cd0..f9edb23 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 7d99d27..2d94309 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 0297159..5c6d1b8 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 9c353e3..6715790 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 6d04120..a9cb7ef 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 66a0fe9..5c71729 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 76506fb..2c66143 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;
--
1.7.4.4