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

[Wen Congyang]

At 03/24/2011 03:02 PM, Hu Tao Write:
> 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.

If two disks are mapped to the same image and these two disks are read
only, operating on these two disks at the same is OK. Should we allow
this behavior?

> ---
>  src/conf/domain_conf.c   |   22 ++++++++++++++++++++++
>  src/conf/domain_conf.h   |    2 ++
>  src/libvirt_private.syms |    1 +
>  src/qemu/qemu_driver.c   |    6 ++++++
>  4 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1b02c25..93a081d 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,17 @@ cleanup:
>  
>      return ret;
>  }
> +
> +bool virDomainDiskConflict(virDomainDiskDefPtr disk, virDomainDefPtr def)
> +{
> +    int i;
> +
> +    if (!disk->src)
> +        return false;
> +
> +    for (i = 0; i < def->ndisks; i++) {
> +        if (STREQ(disk->src, def->disks[i]->src))

If disk->src is '/var/lib/libvirt/images/test.img' and
def->disk[i]->src is '/var/libv//libvirt/images/test.img',
they are the same images, but STREQ can not work.

Comparing device id and inode number is a better way to check whether
two images are the same.

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