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

[Nathan Baum]

On Thu, 24 Mar 2011 16:46:14 +0800
Hu Tao <hutao at cn.fujitsu.com> wrote:

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

It doesn't seem that virDomainDiskConflict observes the "shared"
property of a disk definition.

If all the disks are marked as shared, we should say there is no
conflict.

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