[libvirt] [PATCH 0/2] VirtFS fixes needed for supporting fs driver in VMM
These are the libvirt changes needed to ensure that the virt-manager changes done for supporting fs driver work well. --- Deepak C Shetty (2): Set default fs driver when not provided in input xml Do not generate security_model when fs driver is anything but 'path' src/conf/domain_conf.c | 15 +++++++++++---- src/qemu/qemu_command.c | 15 +++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) -- Signature
While adding support for fs driver sub-element in virt-manager (vmm) it throws a python exception/error when one tries to list fs device (in details window) that has default driver selected. Root causing that leads to libvirt not setting the default driver when the input xml does not carry one, which is the case when user selects default driver in vmm. As part of vmm's add new fs device flow, libvirt's DefineXML gets called which does not add the <driver...> sub-element in case of default fs driver selected in vmm. This patch fixes that. Signed-off-by: Deepak C Shetty <deepakcs@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4be8fe0..8b89a0b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3318,6 +3318,8 @@ virDomainFSDefParseXML(xmlNodePtr node, _("unknown fs driver type '%s'"), fsdriver); goto error; } + } else { + def->fsdriver = VIR_DOMAIN_FS_DRIVER_TYPE_PATH; } if (source == NULL) {
Now that I think again, I am not sure if the below is really needed ? <driver... being an optional sub-element, it should be ok for it not to be present in the returned XML, which means the consumer should make appropriate checks and handle the presence/absence of <driver....> accordingly.... I think that is correct. On 12/21/2011 11:17 AM, Deepak C Shetty wrote:
While adding support for fs driver sub-element in virt-manager (vmm) it throws a python exception/error when one tries to list fs device (in details window) that has default driver selected.
Root causing that leads to libvirt not setting the default driver when the input xml does not carry one, which is the case when user selects default driver in vmm. As part of vmm's add new fs device flow, libvirt's DefineXML gets called which does not add the<driver...> sub-element in case of default fs driver selected in vmm. This patch fixes that.
Signed-off-by: Deepak C Shetty<deepakcs@linux.vnet.ibm.com> ---
src/conf/domain_conf.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4be8fe0..8b89a0b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3318,6 +3318,8 @@ virDomainFSDefParseXML(xmlNodePtr node, _("unknown fs driver type '%s'"), fsdriver); goto error; } + } else { + def->fsdriver = VIR_DOMAIN_FS_DRIVER_TYPE_PATH; }
if (source == NULL) {
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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); + } if (def->fsdriver) { virBufferAsprintf(buf, " <driver type='%s'/>\n", fsdriver); 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"); + } } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",path=%s", fs->src);
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.
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 :|
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
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. 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 :|
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
On Thu, Dec 22, 2011 at 10:24:57AM +0530, Deepak C Shetty wrote: 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 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
On 12/22/2011 11:00 PM, Daniel P. Berrange wrote: 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 security_model 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. Thanx, deepak
On Mon, Dec 26, 2011 at 02:49:55PM +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
On Thu, Dec 22, 2011 at 10:24:57AM +0530, Deepak C Shetty wrote: 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 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
On 12/22/2011 11:00 PM, Daniel P. Berrange wrote: 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 security_model 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. 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 :|
Posted v2 of the patch here... https://www.redhat.com/archives/libvir-list/2012-January/msg00311.html thanx, deepak On 01/03/2012 05:15 PM, Daniel P. Berrange 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
On Thu, Dec 22, 2011 at 10:24:57AM +0530, Deepak C Shetty wrote: 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 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
On 12/22/2011 11:00 PM, Daniel P. Berrange wrote: 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 security_model 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
On Mon, Dec 26, 2011 at 02:49:55PM +0530, Deepak C Shetty wrote: the QEMU driver code in libvirt should report errors. The XML code should not care what combinations QEMU supports.
Daniel
participants (2)
-
Daniel P. Berrange -
Deepak C Shetty