[PATCHv5 02/13] qemu: fix restoring a compressed save image

[Eric Blake <eblake@xxxxxxxxxx>]

Latent bug introduced in commit 2d6a581960 (Aug 2009), but not exposed
until commit 1859939a (Jan 2011).  Basically, when virExec creates a
pipe, it always marks libvirt's side as cloexec.  If libvirt then
wants to hand that pipe to another child process, things work great if
the fd is dup2()'d onto stdin or stdout (as with stdin: or exec:
migration), but if the pipe is instead used as-is (such as with fd:
migration) then qemu sees EBADF because the fd was closed at exec().

This is a minimal fix for the problem at hand; it is slightly racy,
but no more racy than the rest of libvirt fd handling, including the
case of uncompressed save images.  A more invasive fix, but ultimately
safer at avoiding leaking unintended fds, would be to _always and
atomically_ open all fds as cloexec in libvirt (thanks to primitives
like open(O_CLOEXEC), pipe2(), accept4(), ...), then teach virExec to
clear that bit for all fds explicitly marked to be handed to the child
only after forking.

* src/qemu/qemu_command.c (qemuBuildCommandLine): Clear cloexec
flag.
* tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Tweak test.
---

v5: new patch

 src/qemu/qemu_command.c  |   16 ++++++++++++++++
 tests/qemuxml2argvtest.c |    2 +-
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c9b9850..3138943 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4329,6 +4329,14 @@ qemuBuildCommandLine(virConnectPtr conn,
         } else if (STREQ(migrateFrom, "stdio")) {
             if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
                 virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
+                /* migrateFd might be cloexec, but qemu must inherit
+                 * it if vmop indicates qemu will be executed */
+                if (vmop != VIR_VM_OP_NO_OP &&
+                    virSetInherit(migrateFd, true) < 0) {
+                    qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                    _("Failed to clear cloexec flag"));
+                    goto error;
+                }
                 virCommandPreserveFD(cmd, migrateFd);
             } else if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) {
                 virCommandAddArg(cmd, "exec:cat");
@@ -4358,6 +4366,14 @@ qemuBuildCommandLine(virConnectPtr conn,
                 goto error;
             }
             virCommandAddArg(cmd, migrateFrom);
+            /* migrateFd might be cloexec, but qemu must inherit
+             * it if vmop indicates qemu will be executed */
+            if (vmop != VIR_VM_OP_NO_OP &&
+                virSetInherit(migrateFd, true) < 0) {
+                qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                _("Failed to clear cloexec flag"));
+                goto error;
+            }
             virCommandPreserveFD(cmd, migrateFd);
         } else if (STRPREFIX(migrateFrom, "unix")) {
             if (!qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX)) {
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c1329fa..02de8de 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -122,7 +122,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
     if (!(cmd = qemuBuildCommandLine(conn, &driver,
                                      vmdef, &monitor_chr, false, extraFlags,
                                      migrateFrom, migrateFd, NULL,
-                                     VIR_VM_OP_CREATE)))
+                                     VIR_VM_OP_NO_OP)))
         goto fail;

     if (!!virGetLastError() != expectError) {
-- 
1.7.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list