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

[Deepak C Shetty]

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