[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] qemu: avoid double close on domain restore



qemudDomainSaveImageStartVM was evil - it closed the incoming fd
argument on some, but not all, code paths, without informing the
caller about that action.  No wonder that this resulted in
double-closes: https://bugzilla.redhat.com/show_bug.cgi?id=672725

* src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Alter
signature, to avoid double-close.
(qemudDomainRestore, qemudDomainObjRestore): Update callers.
---
 src/qemu/qemu_driver.c |   21 +++++++++++----------
 1 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7fc08e8..5acddcb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3248,7 +3248,7 @@ static int ATTRIBUTE_NONNULL(6)
 qemudDomainSaveImageStartVM(virConnectPtr conn,
                             struct qemud_driver *driver,
                             virDomainObjPtr vm,
-                            int fd,
+                            int *fd,
                             pid_t read_pid,
                             const struct qemud_save_header *header,
                             const char *path)
@@ -3273,20 +3273,21 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,

         if (header->compressed != QEMUD_SAVE_FORMAT_RAW) {
             intermediate_argv[0] = prog;
-            intermediatefd = fd;
-            fd = -1;
+            intermediatefd = *fd;
+            *fd = -1;
             if (virExec(intermediate_argv, NULL, NULL,
-                        &intermediate_pid, intermediatefd, &fd, NULL, 0) < 0) {
+                        &intermediate_pid, intermediatefd, fd, NULL, 0) < 0) {
                 qemuReportError(VIR_ERR_INTERNAL_ERROR,
                                 _("Failed to start decompression binary %s"),
                                 intermediate_argv[0]);
+                *fd = intermediatefd;
                 goto out;
             }
         }
     }

     /* Set the migration source and start it up. */
-    ret = qemuProcessStart(conn, driver, vm, "stdio", true, fd, path,
+    ret = qemuProcessStart(conn, driver, vm, "stdio", true, *fd, path,
                            VIR_VM_OP_RESTORE);

     if (intermediate_pid != -1) {
@@ -3295,7 +3296,7 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
              * wait forever to write to stdout, so we must manually kill it.
              */
             VIR_FORCE_CLOSE(intermediatefd);
-            VIR_FORCE_CLOSE(fd);
+            VIR_FORCE_CLOSE(*fd);
             kill(intermediate_pid, SIGTERM);
         }

@@ -3307,8 +3308,8 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
     }
     VIR_FORCE_CLOSE(intermediatefd);

-    wait_ret = qemudDomainSaveImageClose(fd, read_pid, &status);
-    fd = -1;
+    wait_ret = qemudDomainSaveImageClose(*fd, read_pid, &status);
+    *fd = -1;
     if (read_pid != -1) {
         if (wait_ret == -1) {
             virReportSystemError(errno,
@@ -3398,7 +3399,7 @@ static int qemudDomainRestore(virConnectPtr conn,
     if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
         goto cleanup;

-    ret = qemudDomainSaveImageStartVM(conn, driver, vm, fd,
+    ret = qemudDomainSaveImageStartVM(conn, driver, vm, &fd,
                                       read_pid, &header, path);

     if (qemuDomainObjEndJob(vm) == 0)
@@ -3449,7 +3450,7 @@ static int qemudDomainObjRestore(virConnectPtr conn,
     virDomainObjAssignDef(vm, def, true);
     def = NULL;

-    ret = qemudDomainSaveImageStartVM(conn, driver, vm, fd,
+    ret = qemudDomainSaveImageStartVM(conn, driver, vm, &fd,
                                       read_pid, &header, path);

 cleanup:
-- 
1.7.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]