[libvirt] [PATCH 2/2] Do not generate security_model when fs driver is anything but 'path'

[Daniel P. Berrange]

On Wed, Dec 21, 2011 at 11:17:17AM +0530, Deepak C Shetty wrote:
> QEMU does not support security_model for anything but 'path' fs driver type.
> Currently in libvirt, when security_model ( accessmode attribute) is not
> specified it auto-generates it irrespective of the fs driver type. Also
> when virt-manager (vmm) adds a new fs device with default security_model
> the input xml passed to libvirt does not contain accessmode attribute, but
> libvirt generates it as part of the virDomainDefine flow, which should
> only be done if fs driver is of type 'path', else not. This patch fixes
> these issues.
> 
> Signed-off-by: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>
> ---
> 
>  src/conf/domain_conf.c  |   13 +++++++++----
>  src/qemu/qemu_command.c |   15 +++++++++------
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8b89a0b..2c91f82 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10019,10 +10019,15 @@ virDomainFSDefFormat(virBufferPtr buf,
>          return -1;
>      }
>  
> -
> -    virBufferAsprintf(buf,
> -                      "    <filesystem type='%s' accessmode='%s'>\n",
> -                      type, accessmode);
> +    if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
> +        def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) {
> +        virBufferAsprintf(buf,
> +                          "    <filesystem type='%s' accessmode='%s'>\n",
> +                          type, accessmode);
> +    } else {
> +        virBufferAsprintf(buf,
> +                          "    <filesystem type='%s'>\n", type);
> +    }


No, this isn't right. We should *always* include the accessmode
in the XML. Only at time of use should we decide whether the
requested accessmode can be supported or not.

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d33d7c8..1f70eb1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2004,12 +2004,15 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs,
>      }
>      virBufferAdd(&opt, driver, -1);
>  
> -    if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) {
> -        virBufferAddLit(&opt, ",security_model=mapped");
> -    } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
> -        virBufferAddLit(&opt, ",security_model=passthrough");
> -    } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) {
> -        virBufferAddLit(&opt, ",security_model=none");
> +    if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_PATH ||
> +        fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT) {
> +        if (fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_MAPPED) {
> +            virBufferAddLit(&opt, ",security_model=mapped");
> +        } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH) {
> +            virBufferAddLit(&opt, ",security_model=passthrough");
> +        } else if(fs->accessmode == VIR_DOMAIN_FS_ACCESSMODE_SQUASH) {
> +            virBufferAddLit(&opt, ",security_model=none");
> +        }
>      }

This is wrong too, because it is silently ignoring the accessmode
that user requested in some cases. If a particular fsdriver does
not support the requested accesmode, it should be raising a
VIR_ERR_CONFIG_UNSUPPORTED error.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|