[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

Posted v2 of the patch here...


On 01/03/2012 05:15 PM, Daniel P. Berrange wrote:
On Mon, Dec 26, 2011 at 02:49:55PM +0530, Deepak C Shetty wrote:
On 12/22/2011 11:00 PM, Daniel P. Berrange wrote:
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 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.

Hi Dan,
     Its possible for fs driver to not support accessmode at all. fs
driver type proxy
and synthfs are examples, qemu already supports these today. Proxy
driver is for
passthru + TOCTOU vulnerability fix and synthfs help export
host/hypervisor info
to a trusted guest/domain. In both of these cases, accessmode does not make
any sense. So i feel it should be ok for accessmode not to be supported for
a particular fs driver type.

    For  the 'handle' fs driver type, qemu currently throws error if
is present when fs driver is handle, hence the need to check for
anything but
local/default fs driver in libvirt and not to generate
security_model, else it would just
result in qemu error and user won't be able to create a domain.

    Having said the above, I also think its logical to have
accessmode for 'handle' case
since the behaviour maps to 'passthru' mode, Aneesh can correct me
here if i am
wrong, but since qemu won't allow us to specify security_model when
its handle type,
i was thinking to support a 4th accessmode in libvirt called
'default' and set that in xml
when fs driver is handle, will that work ? If acessmode is set to
'default" it also takes care
of not generating the security_model when fs driver is handle in
libvirt, and qemu
cmdline will be a valid one. Ofcouse will add code libvirt to
generate unsuppported error
for anything but accessmode= 'default'  when fs driver is handle.

Pls let me know your comments, based on which will post v2.
The core requirement here is that the XML parsing/formatting is
not tied to specifics of the QEMU implementation. If certain
combinations of XML attributes do not make sense for QEMU, then
the QEMU driver code in libvirt should report errors. The XML
code should not care what combinations QEMU supports.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]