qemu: Don't overwrite security labels

Currently, if qemuProcessStart fail at some point, e.g. because
domain being started wants a PCI/USB device already assigned to
a different domain, we jump to cleanup label where qemuProcessStop
is performed. This unconditionally calls virSecurityManagerRestoreAllLabel
which is wrong because the other domain is still using those devices.

However, once we successfully label all devices/paths in
qemuProcessStart() from that point on, we have to perform a rollback
on failure - that is - we have to virSecurityManagerRestoreAllLabel.

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4cb0a831a35fa2207aa9039464c7ec5f4729b4ed..5f3aa99db6af58cd480b38a2e23468a3c6b00e52 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3281,6 +3281,7 @@ int qemuProcessStart(virConnectPtr conn,
     int i;
     char *nodeset = NULL;
     char *nodemask = NULL;
+    unsigned int stop_flags;
 
     /* Okay, these are just internal flags,
      * but doesn't hurt to check */
@@ -3288,6 +3289,12 @@ int qemuProcessStart(virConnectPtr conn,
                   VIR_QEMU_PROCESS_START_PAUSED |
                   VIR_QEMU_PROCESS_START_AUTODESROY, -1);
 
+    /* From now on until domain security labeling is done:
+     * if any operation fails and we goto cleanup, we must not
+     * restore any security label as we would overwrite labels
+     * we did not set. */
+    stop_flags = VIR_QEMU_PROCESS_STOP_NO_RELABEL;
+
     hookData.conn = conn;
     hookData.vm = vm;
     hookData.driver = driver;
@@ -3631,6 +3638,12 @@ int qemuProcessStart(virConnectPtr conn,
                                       vm->def, stdin_path) < 0)
         goto cleanup;
 
+    /* Security manager labeled all devices, therefore
+     * if any operation from now on fails and we goto cleanup,
+     * where virSecurityManagerRestoreAllLabel() is called
+     * (hidden under qemuProcessStop) we need to restore labels. */
+    stop_flags &= ~VIR_QEMU_PROCESS_STOP_NO_RELABEL;
+
     if (stdin_fd != -1) {
         /* if there's an fd to migrate from, and it's a pipe, put the
          * proper security label on it
@@ -3770,7 +3783,7 @@ cleanup:
     VIR_FREE(nodemask);
     virCommandFree(cmd);
     VIR_FORCE_CLOSE(logfile);
-    qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
+    qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags);
 
     return -1;
 }
@@ -3983,10 +3996,11 @@ void qemuProcessStop(struct qemud_driver *driver,
         VIR_FREE(xml);
     }
 
-    /* Reset Security Labels */
-    virSecurityManagerRestoreAllLabel(driver->securityManager,
-                                      vm->def,
-                                      flags & VIR_QEMU_PROCESS_STOP_MIGRATED);
+    /* Reset Security Labels unless caller don't want us to */
+    if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL))
+        virSecurityManagerRestoreAllLabel(driver->securityManager,
+                                          vm->def,
+                                          flags & VIR_QEMU_PROCESS_STOP_MIGRATED);
     virSecurityManagerReleaseLabel(driver->securityManager, vm->def);
 
     /* Clear out dynamically assigned labels */

diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index ce592150782a7ad33b4f3e5931944d9f789092f2..d13778d21d3a60c9ccf3ff1651d680e19fc71403 100644 (file)
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -61,7 +61,8 @@ int qemuProcessStart(virConnectPtr conn,
                      unsigned int flags);
 
 typedef enum {
-    VIR_QEMU_PROCESS_STOP_MIGRATED  = 1 << 0,
+    VIR_QEMU_PROCESS_STOP_MIGRATED      = 1 << 0,
+    VIR_QEMU_PROCESS_STOP_NO_RELABEL    = 1 << 1,
 } qemuProcessStopFlags;
 
 void qemuProcessStop(struct qemud_driver *driver,