[libvirt] [RFC PATCH] Prevent defining a domain has disk used by other domain

[Osier Yang]

于 2011年09月23日 15:59, Osier Yang 写道:
> $subject + "If the disk is shared and readonly."

s/disk is/disk is not/

>
> If the disk is not shared or readonly, the later started domain
> will relabel the disk, thus the first domain will lose the permission
> to write to the disk and be corrupt.
>
> --
> I'm not sure if it's the design to allow multiple domains use
> same disk not shared and readonly. So this patch just gives a
> demo of the implementation (it skips the checking of nbd disk,
> and only changes qemu driver), 

Hmm, preventing the relabeling in security driver instead might be the
more proper way? (If the disk source is used by other *running* domain,
then quit relabeling and exit with error).

However, this won't prevent one using same disk source for multiple domains
if security_driver is disabled.

And if security_driver is disabled, there will be no permission problem, all
the domains can write to the same disk source, thus it might cause
inconsistency
between the domains or corrupt.

> to see whether if the pricinple
> is right or not.
> ---
>  src/conf/domain_conf.c   |   34 ++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h   |    2 ++
>  src/libvirt_private.syms |    1 +
>  src/qemu/qemu_driver.c   |   24 ++++++++++++++++++++++++
>  4 files changed, 61 insertions(+), 0 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7463d7c..b64450b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -655,6 +655,40 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
>      return obj;
>  }
>  
> +static int
> +virDomainObjListSearchDiskPath(const void *payload,
> +                               const void *name ATTRIBUTE_UNUSED,
> +                               const void *data)
> +{
> +    virDomainObjPtr obj = (virDomainObjPtr)payload;
> +    int i;
> +    int want = 0;
> +    const char *path = NULL;
> +
> +    path = (const char *)data;
> +
> +    virDomainObjLock(obj);
> +    for (i = 0; obj->def->ndisks; i++) {
> +        if (STREQ(obj->def->disks[i]->src, path)) {
> +            want = 1;
> +            break;
> +        }
> +    }
> +    virDomainObjUnlock(obj);
> +
> +    return want;
> +}
> +
> +virDomainObjPtr
> +virDomainFindByDiskPath(const virDomainObjListPtr doms,
> +                        const char *path)
> +{
> +    virDomainObjPtr obj = NULL;
> +    obj = virHashSearch(doms->objs, virDomainObjListSearchDiskPath, path);
> +    if (obj)
> +        virDomainObjLock(obj);
> +    return obj;
> +}
>  
>  bool virDomainObjTaint(virDomainObjPtr obj,
>                         enum virDomainTaintFlags taint)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 371f270..d7d42dd 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1549,6 +1549,8 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms,
>                                      const unsigned char *uuid);
>  virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
>                                      const char *name);
> +virDomainObjPtr virDomainFindByDiskPath(const virDomainObjListPtr doms,
> +                                        const char *path);
>  
>  bool virDomainObjTaint(virDomainObjPtr obj,
>                         enum virDomainTaintFlags taint);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8235ea1..c33cb2d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -302,6 +302,7 @@ virDomainFSTypeToString;
>  virDomainFindByID;
>  virDomainFindByName;
>  virDomainFindByUUID;
> +virDomainFindByDiskPath;
>  virDomainGetRootFilesystem;
>  virDomainGraphicsAuthConnectedTypeFromString;
>  virDomainGraphicsAuthConnectedTypeToString;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0d0bea2..a448a0b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4796,6 +4796,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
>      virDomainPtr dom = NULL;
>      virDomainEventPtr event = NULL;
>      int dupVM;
> +    int i;
>  
>      qemuDriverLock(driver);
>      if (!(def = virDomainDefParseString(driver->caps, xml,
> @@ -4809,6 +4810,29 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
>      if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0)
>          goto cleanup;
>  
> +    /* Make sure no disk is used by other domain, if it's not
> +     * either shareable or readonly.
> +     */
> +    for (i = 0; i < def->ndisks; i++) {
> +        /* XXX: Do we also need to check if same host&port is used by
> +         * other domain for disk of nbd type?
> +         */
> +        if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_NETWORK &&
> +            def->disks[i]->protocol == VIR_DOMAIN_DISK_PROTOCOL_NBD)
> +            continue;
> +
> +        if (!def->disks[i]->shared &&
> +            !def->disks[i]->readonly &&
> +            (vm = virDomainFindByDiskPath(&driver->domains,
> +                                          def->disks[i]->src))
> ) {
> +            qemuReportError(VIR_ERR_INVALID_ARG,
> +                            _("disk '%s' already used by domain '%s'"),
> +                            def->disks[i]->src,
> +                            vm->def->name);
> +            goto cleanup;
> +        }
> +    }
> +
>      if (qemudCanonicalizeMachine(driver, def) < 0)
>          goto cleanup;
>