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

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);