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

[Eric Blake]

On 03/02/2011 05:10 AM, Daniel P. Berrange wrote:
> On Tue, Mar 01, 2011 at 09:02:23PM -0700, Eric Blake wrote:
>> 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(-)
> 
> ACK

Actually, I realized that the read_pid had the same issue (in systems
that rapidly reuse pids, a double wait can end up hanging while it waits
for the wrong pid).  I squashed this in before pushing.

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 5acddcb..c9095bb 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -3249,7 +3249,7 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
                             struct qemud_driver *driver,
                             virDomainObjPtr vm,
                             int *fd,
-                            pid_t read_pid,
+                            pid_t *read_pid,
                             const struct qemud_save_header *header,
                             const char *path)
 {
@@ -3308,9 +3308,9 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
     }
     VIR_FORCE_CLOSE(intermediatefd);

-    wait_ret = qemudDomainSaveImageClose(*fd, read_pid, &status);
+    wait_ret = qemudDomainSaveImageClose(*fd, *read_pid, &status);
     *fd = -1;
-    if (read_pid != -1) {
+    if (*read_pid != -1) {
         if (wait_ret == -1) {
             virReportSystemError(errno,
                                  _("failed to wait for process reading
'%s'"),
@@ -3331,6 +3331,7 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
             }
         }
     }
+    *read_pid = -1;

     if (ret < 0) {
         qemuDomainStartAudit(vm, "restored", false);
@@ -3400,7 +3401,7 @@ static int qemudDomainRestore(virConnectPtr conn,
         goto cleanup;

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

     if (qemuDomainObjEndJob(vm) == 0)
         vm = NULL;
@@ -3451,7 +3452,7 @@ static int qemudDomainObjRestore(virConnectPtr conn,
     def = NULL;

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

 cleanup:
     virDomainDefFree(def);


-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110302/c52ca556/attachment-0001.sig>