On 12/21/2011 08:11 PM, Daniel P. Berrange wrote:
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@xxxxxxxxxxxxxxxxxx>
---
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.
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 ?
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.
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
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list