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.

diff --git a/src/fdstream.c b/src/fdstream.c
index 2b7812ff04f2bfb91e47741dd3d666a17904b159..b60162c6a148c4f297b32ab56bd4c1f8f2b4cc70 100644 (file)
--- 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 76f0cd030fbf04edbea1bc74ad67ed39a72dcb97..f9edb23e9ea56a9ea126493af1810704e3156fb6 100644 (file)
--- 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 7d99d27ee8d800db7b0c28b78613cd17d3c80c0c..2d94309399129da2208b25f455d2e972417931fe 100644 (file)
--- 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 02971592d65bdeab2f2ed15df07dd52abb787c4c..5c6d1b876f2f1697cb7b09e1acab82dfd2458f71 100644 (file)
--- 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 9c353e394e9b992e4c395a5ab92bfc01518be2c5..6715790a6f99dd6ddc43c99e753b41c188f06f48 100644 (file)
--- 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 6d04120156a20e1b05605d216ed2b57aa0e145c6..a9cb7efa1a4b10e4c7cf92b42df3d1d983d10951 100644 (file)
--- 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 66a0fe9365c13ea4eac613ef4adc4f47c36271bf..5c717299c03652ecb3ebc786c1cdf398bee109d5 100644 (file)
--- 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 76506fb9c878234eb00e27afed03bb46fe6650a0..2c66143c08c2b22fa851e62d6073a8caa6b3b6fc 100644 (file)
--- 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;