Don't reset user/group/security label on shared filesystems during migrate

When QEMU runs with its disk on NFS, and as a non-root user, the
disk is chownd to that non-root user. When migration completes
the last step is shutting down the QEMU on the source host. THis
normally resets user/group/security label. This is bad when the
VM was just migrated because the file is still in use on the dest
host. It is thus neccessary to skip the reset step for any files
found to be on a shared filesystem

* src/libvirt_private.syms: Export virStorageFileIsSharedFS
* src/util/storage_file.c, src/util/storage_file.h: Add a new
  method virStorageFileIsSharedFS() to determine if a file is
  on a shared filesystem (NFS, GFS, OCFS2, etc)
* src/qemu/qemu_driver.c: Tell security driver not to reset
  disk labels on migration completion
* src/qemu/qemu_security_dac.c, src/qemu/qemu_security_stacked.c,
  src/security/security_selinux.c, src/security/security_driver.h,
  src/security/security_apparmor.c: Add ability to skip disk
  restore step for files on shared filesystems.

src/libvirt_private.syms

@@ -613,6 +613,7 @@ virStorageFileFormatTypeToString;
 virStorageFileFormatTypeFromString;
 virStorageFileGetMetadata;
 virStorageFileGetMetadataFromFD;
+virStorageFileIsSharedFS;
 
 # threads.h
 virMutexInit;

src/qemu/qemu_driver.c

@@ -150,7 +150,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
                               int stdin_fd);
 
 static void qemudShutdownVMDaemon(struct qemud_driver *driver,
-                                  virDomainObjPtr vm);
+                                  virDomainObjPtr vm,
+                                  int migrated);
 
 static int qemudDomainGetMaxVcpus(virDomainPtr dom);
 
@@ -723,7 +724,7 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                                      VIR_DOMAIN_EVENT_STOPPED_FAILED :
                                      VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
 
-    qemudShutdownVMDaemon(driver, vm);
+    qemudShutdownVMDaemon(driver, vm, 0);
     if (!vm->persistent)
         virDomainRemoveInactive(&driver->domains, vm);
     else
@@ -1254,7 +1255,7 @@ error:
     /* We can't get the monitor back, so must kill the VM
      * to remove danger of it ending up running twice if
      * user tries to start it again later */
-    qemudShutdownVMDaemon(driver, obj);
+    qemudShutdownVMDaemon(driver, obj, 0);
     if (!obj->persistent)
         virDomainRemoveInactive(&driver->domains, obj);
     else
@@ -3550,7 +3551,7 @@ cleanup:
 
     if (driver->securityDriver &&
         driver->securityDriver->domainRestoreSecurityAllLabel)
-        driver->securityDriver->domainRestoreSecurityAllLabel(vm);
+        driver->securityDriver->domainRestoreSecurityAllLabel(vm, 0);
     if (driver->securityDriver &&
         driver->securityDriver->domainReleaseSecurityLabel)
         driver->securityDriver->domainReleaseSecurityLabel(vm);
@@ -3567,7 +3568,7 @@ cleanup:
 abort:
     /* We jump here if we failed to initialize the now running VM
      * killing it off and pretend we never started it */
-    qemudShutdownVMDaemon(driver, vm);
+    qemudShutdownVMDaemon(driver, vm, 0);
 
     if (logfile != -1)
         close(logfile);
@@ -3577,7 +3578,8 @@ abort:
 
 
 static void qemudShutdownVMDaemon(struct qemud_driver *driver,
-                                  virDomainObjPtr vm) {
+                                  virDomainObjPtr vm,
+                                  int migrated) {
     int ret;
     int retries = 0;
     qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -3588,7 +3590,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
     if (!virDomainObjIsActive(vm))
         return;
 
-    VIR_DEBUG("Shutting down VM '%s'", vm->def->name);
+    VIR_DEBUG("Shutting down VM '%s' migrated=%d", vm->def->name, migrated);
 
     /* This method is routinely used in clean up paths. Disable error
      * reporting so we don't squash a legit error. */
@@ -3646,7 +3648,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
     /* Reset Security Labels */
     if (driver->securityDriver &&
         driver->securityDriver->domainRestoreSecurityAllLabel)
-        driver->securityDriver->domainRestoreSecurityAllLabel(vm);
+        driver->securityDriver->domainRestoreSecurityAllLabel(vm, migrated);
     if (driver->securityDriver &&
         driver->securityDriver->domainReleaseSecurityLabel)
         driver->securityDriver->domainReleaseSecurityLabel(vm);
@@ -4370,7 +4372,7 @@ static int qemudDomainDestroy(virDomainPtr dom) {
         goto endjob;
     }
 
-    qemudShutdownVMDaemon(driver, vm);
+    qemudShutdownVMDaemon(driver, vm, 0);
     event = virDomainEventNewFromObj(vm,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
@@ -5085,7 +5087,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path,
     ret = 0;
 
     /* Shut it down */
-    qemudShutdownVMDaemon(driver, vm);
+    qemudShutdownVMDaemon(driver, vm, 0);
     event = virDomainEventNewFromObj(vm,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_SAVED);
@@ -5396,7 +5398,7 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 
 endjob:
     if ((ret == 0) && (flags & VIR_DUMP_CRASH)) {
-        qemudShutdownVMDaemon(driver, vm);
+        qemudShutdownVMDaemon(driver, vm, 0);
         event = virDomainEventNewFromObj(vm,
                                          VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_CRASHED);
@@ -9868,7 +9870,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
 
     qemust = qemuStreamMigOpen(st, unixfile);
     if (qemust == NULL) {
-        qemudShutdownVMDaemon(driver, vm);
+        qemudShutdownVMDaemon(driver, vm, 0);
         if (!vm->persistent) {
             if (qemuDomainObjEndJob(vm) > 0)
                 virDomainRemoveInactive(&driver->domains, vm);
@@ -10567,7 +10569,9 @@ qemudDomainMigratePerform (virDomainPtr dom,
     }
 
     /* Clean up the source domain. */
-    qemudShutdownVMDaemon(driver, vm);
+    fprintf(stderr, "******************* MIG \n");
+    qemudShutdownVMDaemon(driver, vm, 1);
+    fprintf(stderr, "******************* YEEHAAA\n");
     resume = 0;
 
     event = virDomainEventNewFromObj(vm,
@@ -10709,7 +10713,7 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn,
         }
         virDomainSaveStatus(driver->caps, driver->stateDir, vm);
     } else {
-        qemudShutdownVMDaemon(driver, vm);
+        qemudShutdownVMDaemon(driver, vm, 0);
         event = virDomainEventNewFromObj(vm,
                                          VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_FAILED);
@@ -11555,7 +11559,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
          */
 
         if (virDomainObjIsActive(vm)) {
-            qemudShutdownVMDaemon(driver, vm);
+            qemudShutdownVMDaemon(driver, vm, 0);
             event = virDomainEventNewFromObj(vm,
                                              VIR_DOMAIN_EVENT_STOPPED,
                                              VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);

src/qemu/qemu_security_dac.c

@@ -142,8 +142,9 @@ qemuSecurityDACSetSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED,
 
 
 static int
-qemuSecurityDACRestoreSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED,
-                                         virDomainDiskDefPtr disk)
+qemuSecurityDACRestoreSecurityImageLabelInt(virDomainObjPtr vm ATTRIBUTE_UNUSED,
+                                            virDomainDiskDefPtr disk,
+                                            int migrated)
 {
     if (!driver->privileged || !driver->dynamicOwnership)
         return 0;
@@ -162,10 +163,34 @@ qemuSecurityDACRestoreSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED,
     if (!disk->src)
         return 0;
 
+    /* If we have a shared FS & doing migrated, we must not
+     * change ownership, because that kills access on the
+     * destination host which is sub-optimal for the guest
+     * VM's I/O attempts :-)
+     */
+    if (migrated) {
+        int rc = virStorageFileIsSharedFS(disk->src);
+        if (rc < 0)
+            return -1;
+        if (rc == 1) {
+            VIR_DEBUG("Skipping image label restore on %s because FS is shared",
+                      disk->src);
+            return 0;
+        }
+    }
+
     return qemuSecurityDACRestoreSecurityFileLabel(disk->src);
 }
 
 
+static int
+qemuSecurityDACRestoreSecurityImageLabel(virDomainObjPtr vm,
+                                         virDomainDiskDefPtr disk)
+{
+    return qemuSecurityDACRestoreSecurityImageLabelInt(vm, disk, 0);
+}
+
+
 static int
 qemuSecurityDACSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED,
                                    const char *file,
@@ -306,7 +331,8 @@ done:
 
 
 static int
-qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm)
+qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm,
+                                       int migrated)
 {
     int i;
     int rc = 0;
@@ -314,7 +340,8 @@ qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm)
     if (!driver->privileged || !driver->dynamicOwnership)
         return 0;
 
-    VIR_DEBUG("Restoring security label on %s", vm->def->name);
+    VIR_DEBUG("Restoring security label on %s migrated=%d",
+              vm->def->name, migrated);
 
     for (i = 0 ; i < vm->def->nhostdevs ; i++) {
         if (qemuSecurityDACRestoreSecurityHostdevLabel(vm,
@@ -322,8 +349,9 @@ qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm)
             rc = -1;
     }
     for (i = 0 ; i < vm->def->ndisks ; i++) {
-        if (qemuSecurityDACRestoreSecurityImageLabel(vm,
-                                                     vm->def->disks[i]) < 0)
+        if (qemuSecurityDACRestoreSecurityImageLabelInt(vm,
+                                                        vm->def->disks[i],
+                                                        migrated) < 0)
             rc = -1;
     }
 

src/qemu/qemu_security_stacked.c

@@ -215,18 +215,19 @@ qemuSecurityStackedSetSecurityAllLabel(virDomainObjPtr vm)
 
 
 static int
-qemuSecurityStackedRestoreSecurityAllLabel(virDomainObjPtr vm)
+qemuSecurityStackedRestoreSecurityAllLabel(virDomainObjPtr vm,
+                                           int migrated)
 {
     int rc = 0;
 
     if (driver->securitySecondaryDriver &&
         driver->securitySecondaryDriver->domainRestoreSecurityAllLabel &&
-        driver->securitySecondaryDriver->domainRestoreSecurityAllLabel(vm) < 0)
+        driver->securitySecondaryDriver->domainRestoreSecurityAllLabel(vm, migrated) < 0)
         rc = -1;
 
     if (driver->securityPrimaryDriver &&
         driver->securityPrimaryDriver->domainRestoreSecurityAllLabel &&
-        driver->securityPrimaryDriver->domainRestoreSecurityAllLabel(vm) < 0)
+        driver->securityPrimaryDriver->domainRestoreSecurityAllLabel(vm, migrated) < 0)
         rc = -1;
 
     return rc;

src/security/security_apparmor.c

@@ -444,7 +444,8 @@ AppArmorReleaseSecurityLabel(virDomainObjPtr vm)
 
 
 static int
-AppArmorRestoreSecurityAllLabel(virDomainObjPtr vm)
+AppArmorRestoreSecurityAllLabel(virDomainObjPtr vm,
+                                int migrated ATTRIBUTE_UNUSED)
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
     int rc = 0;

src/security/security_driver.h

@@ -46,7 +46,8 @@ typedef int (*virSecurityDomainGenLabel) (virDomainObjPtr sec);
 typedef int (*virSecurityDomainReserveLabel) (virDomainObjPtr sec);
 typedef int (*virSecurityDomainReleaseLabel) (virDomainObjPtr sec);
 typedef int (*virSecurityDomainSetAllLabel) (virDomainObjPtr sec);
-typedef int (*virSecurityDomainRestoreAllLabel) (virDomainObjPtr vm);
+typedef int (*virSecurityDomainRestoreAllLabel) (virDomainObjPtr vm,
+                                                 int migrated);
 typedef int (*virSecurityDomainGetProcessLabel) (virDomainObjPtr vm,
                                                  virSecurityLabelPtr sec);
 typedef int (*virSecurityDomainSetProcessLabel) (virSecurityDriverPtr drv,

src/security/security_selinux.c

@@ -385,8 +385,9 @@ err:
 }
 
 static int
-SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm,
-                                 virDomainDiskDefPtr disk)
+SELinuxRestoreSecurityImageLabelInt(virDomainObjPtr vm,
+                                    virDomainDiskDefPtr disk,
+                                    int migrated)
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
 
@@ -407,9 +408,34 @@ SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm,
     if (!disk->src)
         return 0;
 
+    /* If we have a shared FS & doing migrated, we must not
+     * change ownership, because that kills access on the
+     * destination host which is sub-optimal for the guest
+     * VM's I/O attempts :-)
+     */
+    if (migrated) {
+        int rc = virStorageFileIsSharedFS(disk->src);
+        if (rc < 0)
+            return -1;
+        if (rc == 1) {
+            VIR_DEBUG("Skipping image label restore on %s because FS is shared",
+                      disk->src);
+            return 0;
+        }
+    }
+
     return SELinuxRestoreSecurityFileLabel(disk->src);
 }
 
+
+static int
+SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm,
+                                 virDomainDiskDefPtr disk)
+{
+    return SELinuxRestoreSecurityImageLabelInt(vm, disk, 0);
+}
+
+
 static int
 SELinuxSetSecurityImageLabel(virDomainObjPtr vm,
                              virDomainDiskDefPtr disk)
@@ -603,7 +629,8 @@ done:
 }
 
 static int
-SELinuxRestoreSecurityAllLabel(virDomainObjPtr vm)
+SELinuxRestoreSecurityAllLabel(virDomainObjPtr vm,
+                               int migrated ATTRIBUTE_UNUSED)
 {
     const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
     int i;
@@ -619,8 +646,9 @@ SELinuxRestoreSecurityAllLabel(virDomainObjPtr vm)
             rc = -1;
     }
     for (i = 0 ; i < vm->def->ndisks ; i++) {
-        if (SELinuxRestoreSecurityImageLabel(vm,
-                                             vm->def->disks[i]) < 0)
+        if (SELinuxRestoreSecurityImageLabelInt(vm,
+                                                vm->def->disks[i],
+                                                migrated) < 0)
             rc = -1;
     }
 

src/util/storage_file.c

@@ -26,9 +26,14 @@
 
 #include <unistd.h>
 #include <fcntl.h>
+#ifdef __linux__
+# include <linux/magic.h>
+# include <sys/statfs.h>
+#endif
 #include "dirname.h"
 #include "memory.h"
 #include "virterror_internal.h"
+#include "logging.h"
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
@@ -407,3 +412,48 @@ virStorageFileGetMetadata(const char *path,
 
     return ret;
 }
+
+
+#ifdef __linux__
+
+# ifndef OCFS2_SUPER_MAGIC
+#  define OCFS2_SUPER_MAGIC 0x7461636f
+# endif
+# ifndef GFS2_MAGIC
+#  define GFS2_MAGIC 0x01161970
+# endif
+# ifndef AFS_FS_MAGIC
+#  define AFS_FS_MAGIC 0x6B414653
+# endif
+
+
+int virStorageFileIsSharedFS(const char *path)
+{
+    struct statfs sb;
+
+    if (statfs(path, &sb) < 0) {
+        virReportSystemError(errno,
+                             _("cannot determine filesystem for '%s'"),
+                             path);
+        return -1;
+    }
+
+    VIR_DEBUG("Check if path %s with FS magic %lld is shared",
+              path, (long long int)sb.f_type);
+
+    if (sb.f_type == NFS_SUPER_MAGIC ||
+        sb.f_type == GFS2_MAGIC ||
+        sb.f_type == OCFS2_SUPER_MAGIC ||
+        sb.f_type == AFS_FS_MAGIC) {
+        return 1;
+    }
+
+    return 0;
+}
+#else
+int virStorageFileIsSharedFS(const char *path ATTRIBUTE_UNUSED)
+{
+    /* XXX implement me :-) */
+    return 0;
+}
+#endif

src/util/storage_file.h

@@ -61,4 +61,6 @@ int virStorageFileGetMetadataFromFD(const char *path,
                                     int fd,
                                     virStorageFileMetadata *meta);
 
+int virStorageFileIsSharedFS(const char *path);
+
 #endif /* __VIR_STORAGE_FILE_H__ */