[PATCHv2] Don't allow two or more disks to be mapped to the same image file

[Hu Tao <hutao@xxxxxxxxxxxxxx>]

If two or more disks are mapped to the same image file, operating
on these disks at the same time may corrupt data stored in the
image file.

changes:

v2:

- allow it for read-only disks
- compare source files by inode number

---
 src/conf/domain_conf.c   |   54 ++++++++++++++++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h   |    2 +
 src/libvirt_private.syms |    1 +
 src/qemu/qemu_driver.c   |    6 +++++
 4 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1b02c25..b43dc4a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5455,6 +5455,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
         if (!disk)
             goto error;
 
+        if (virDomainDiskConflict(disk, def)) {
+            virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s %s %s",
+                                 _("source"),
+                                 disk->src,
+                                 _("is already mapped to another device, "
+                                   "skip this device."));
+            continue;
+        }
         def->disks[def->ndisks++] = disk;
     }
     VIR_FREE(nodes);
@@ -9088,3 +9096,49 @@ cleanup:
 
     return ret;
 }
+
+bool virDomainDiskConflict(virDomainDiskDefPtr disk, virDomainDefPtr def)
+{
+    struct stat stat1, stat2;
+    int i;
+
+    if (!disk->src)
+        return false;
+
+    if (stat(disk->src, &stat1)) {
+        if (errno != ENOENT) {
+            /* Can't stat file, for safety treate it as conflicted */
+            return true;
+        }
+    }
+
+    for (i = 0; i < def->ndisks; i++) {
+        if (disk->readonly && def->disks[i]->readonly)
+            continue;
+
+        if (stat(def->disks[i]->src, &stat2)) {
+            if (errno != ENOENT) {
+                /* Can't stat file, shouldn't happen but for safety treate
+                 * it as conflicted */
+                return true;
+            }
+        }
+
+        if (S_ISREG(stat1.st_mode) && S_ISREG(stat2.st_mode)
+            && (stat1.st_ino == stat2.st_ino)
+            && (stat1.st_dev == stat2.st_dev)) {
+            return true;
+        }
+
+        if (S_ISCHR(stat1.st_mode) && S_ISCHR(stat2.st_mode)
+            && (stat1.st_rdev == stat2.st_rdev)) {
+            return true;
+        }
+
+        if (S_ISBLK(stat1.st_mode) && S_ISBLK(stat2.st_mode)
+            && (stat1.st_rdev == stat2.st_rdev)) {
+            return true;
+        }
+    }
+    return false;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9f595d6..78b2f95 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1350,6 +1350,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
                                 bool ignoreOpenFailure,
                                 virDomainDiskDefPathIterator iter,
                                 void *opaque);
+bool virDomainDiskConflict(virDomainDiskDefPtr disk,
+                           virDomainDefPtr def);
 
 typedef const char* (*virLifecycleToStringFunc)(int type);
 typedef int (*virLifecycleFromStringFunc)(const char *type);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b4b6c63..17e2ec4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -237,6 +237,7 @@ virDomainDeviceTypeToString;
 virDomainDiskBusTypeToString;
 virDomainDiskCacheTypeFromString;
 virDomainDiskCacheTypeToString;
+virDomainDiskConflict;
 virDomainDiskDefAssignAddress;
 virDomainDiskDefForeachPath;
 virDomainDiskDefFree;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6f296c9..6de08d3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4058,6 +4058,12 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
             break;
 
         case VIR_DOMAIN_DISK_DEVICE_DISK:
+            if (virDomainDiskConflict(dev->data.disk, vm->def)) {
+                qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s %s %s",
+                                _("source"), dev->data.disk->src,
+                                _("is already mapped to another device."));
+                break;
+            }
             if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
                 ret = qemuDomainAttachUsbMassstorageDevice(driver, vm,
                                                            dev->data.disk, qemuCaps);
-- 
1.7.3.1


-- 
Thanks,
Hu Tao
2011/03/24

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