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@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); + }
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
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. fsdriver type 'handle' does not support 'accessmode', if we include it in the xml, wouldn't it be misleading at the xml level ?. Also when viewed from virsh/VMM, we do not want to show accessmode attribute, if user selected fs driver type as 'handle'. If we end up including accessmode in the xml always, it would be misleading there too, correct ? 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. This typically won't happen when user is using VMM, since I took care not to disable accessmode, when user selects fsdriver 'handle'. But its
On 12/21/2011 08:11 PM, Daniel P. Berrange wrote: possible using virsh, if someone edited the domain xml and added fsdriver 'handle' with accessmode, in which case I agree, there should be an error reported. Will send a v2 of this patch. If we show unsupported error in this case, do we still include accessmode in the xml ?, referring to your comment above, if we always include it it will be misleading here, since virsh would show unsupported error but xml would still include accessmode, is my understanding correct ?
Daniel