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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list