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

[Daniel P. Berrange]

On Thu, Dec 22, 2011 at 10:24:57AM +0530, Deepak C Shetty wrote:
> 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 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.
> 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 ?

You are missing my point. For every filesystem we pass through to
the guest, there is an access mode. Some drivers only support
one access mode, other drivers support several access modes. The
XML should show the access mode at all times, even if there is only
a choice of one mode for certain drivers. The fact that there is
only a choice of one mode, is an implementation detail of QEMU.
Individual driver impl details should not leak up into the XML
parsing code, which is why we should be dealing with this by
reporting an error in the QEMU driver.

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 :|