Re: [Qemu-devel] [PATCH 2/2] i_generation / st_gen support for handle based fs driver

[Harsh Bora <harsh@linux.vnet.ibm.com>]

On 08/10/2011 08:47 PM, Stefan Hajnoczi wrote:
> On Fri, Aug 5, 2011 at 1:53 PM, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>> On Fri, Aug 5, 2011 at 12:32 PM, Aneesh Kumar K.V
>> <aneesh.kumar@linux.vnet.ibm.com>  wrote:
>>> On Fri, 5 Aug 2011 10:24:42 +0100, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>>>> On Fri, Aug 5, 2011 at 7:40 AM, Aneesh Kumar K.V
>>>> <aneesh.kumar@linux.vnet.ibm.com>  wrote:
>>>>> On Thu, 4 Aug 2011 22:57:34 +0100, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>>>>>> On Thu, Aug 4, 2011 at 7:45 PM, Aneesh Kumar K.V
>>>>>> <aneesh.kumar@linux.vnet.ibm.com>  wrote:
>>>>>>> On Thu, 4 Aug 2011 15:31:08 +0100, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>>>>>>>> On Thu, Aug 4, 2011 at 1:03 PM, Aneesh Kumar K.V
>>>>>>>> <aneesh.kumar@linux.vnet.ibm.com>  wrote:
>>>>>>>>> On Thu, 4 Aug 2011 12:47:42 +0100, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>>>>>>>>>> On Thu, Aug 4, 2011 at 12:20 PM, Aneesh Kumar K.V
>>>>>>>>>> <aneesh.kumar@linux.vnet.ibm.com>  wrote:
>>>>>>>>>>> On Thu, 4 Aug 2011 11:21:05 +0100, Stefan Hajnoczi<stefanha@gmail.com>  wrote:
>>>>>>>>>>>> On Thu, Aug 4, 2011 at 11:06 AM, Harsh Prateek Bora
>>>>>>>>>>>> <harsh@linux.vnet.ibm.com>  wrote:
>>>>>>>>>>>>> This patch provides support for st_gen for handle based fs type server.
>>>>>>>>>>>>> Currently the support is provided for ext4, btrfs, reiserfs and xfs.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Signed-off-by: Harsh Prateek Bora<harsh@linux.vnet.ibm.com>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>   hw/9pfs/virtio-9p-handle.c |   30 ++++++++++++++++++++++++++++++
>>>>>>>>>>>>>   1 files changed, 30 insertions(+), 0 deletions(-)
>>>>>>>>>>>>
>>>>>>>>>>>> Does handle-based file I/O really need to duplicate all this code?  Is
>>>>>>>>>>>> it possible to use either regular open or handle-based open from a
>>>>>>>>>>>> single local fs codebase?
>>>>>>>>>>>
>>>>>>>>>>> The only details common between handle based and local based getversion
>>>>>>>>>>> callback is the ioctl. Moving that into a helper may not really help in
>>>>>>>>>>> this case ?.
>>>>>>>>>>
>>>>>>>>>> Aneesh, do you have a public virtfs tree that I can look at?  In
>>>>>>>>>> qemu.git we don't have virtio-9p-handle.c yet, so I can't give any
>>>>>>>>>> specific feedback.
>>>>>>>>>
>>>>>>>>> http://repo.or.cz/w/qemu/v9fs.git for-upstream
>>>>>>>>>
>>>>>>>>> I should send the patchset to qemu list soon. Was waiting for the
>>>>>>>>> co-routine patches to go upstream.
>>>>>>>>
>>>>>>>> The handle code looks like a copy of the local backend minus security
>>>>>>>> models.  It just needs to use handle syscalls instead of using paths.
>>>>>>>>
>>>>>>>> If you treat the path as the "handle" and use regular openat(2), then
>>>>>>>> the handle code could do what the local backend does today.  Except
>>>>>>>> compared to the local backend it would not have security models and be
>>>>>>>> a bit slower due to extra syscalls.
>>>>>>>>
>>>>>>>> Is the plan to add security models to the handle backend?  If so, then
>>>>>>>> handle and local will be equivalent and duplicate code.
>>>>>>>>
>>>>>>>
>>>>>>> handle require root user privileges to run. So security model with
>>>>>>> handle fs driver doesn't make sense. We added mapped security model to
>>>>>>> avoid requiring user to run as root.
>>>>>>
>>>>>> Does it really require root or is a specific set of capabilities
>>>>>> enough?
>>>>>
>>>>> CAP_DAC_READ_SEARCH  is needed.
>>>>>
>>>>>>
>>>>>> A feature that requires QEMU to run as root has really limited value.
>>>>>> Unprivileged users cannot use the feature, so ad-hoc QEMU users are
>>>>>> left behind.  People don't want to deploy production guests as root,
>>>>>> may not be allowed to, or might find that their management tool
>>>>>> doesn't support that.  So who will be able to use this feature?
>>>>>>
>>>>>
>>>>> One of the main issue that handle based backend fix is the complexity
>>>>> involved in handling renames, both on the guest and on the host. I am
>>>>> also not sure how effective it would be to run the qemu as non root user
>>>>> when exporting a directory with VirtFS. In the mapped security model the
>>>>> user credentials with which the files are created are stored in xattr
>>>>> and that mostly implies host cannot look at the files the same way.
>>>>>
>>>>> My understanding is passthrough security model (which require qemu to
>>>>> run as root) will be used if somebody wants to export a directory on the
>>>>> host to guest. In my case I use none security model, simply because i
>>>>> don't want new xattr on the file created and I am ok even the files
>>>>> get created on the host with the credentials on qemu.
>>>>
>>>> With xattrs you have to mount the directory on the host in order to
>>>> see the same view as the guest.
>>>
>>> How will that help ? There is nothing on the host that maps those xattr
>>> to mode/ownership bits currently. We will have to do something similar to fuse to
>>> make that work ?
>>
>> Sorry, what I suggested is not actually possible today.  We only have
>> a virtio-9p transport in the QEMU 9pfs code, not a TCP transport.  I
>> meant mount -t 9p on the host - don't access the backing directory
>> directly, instead mount it using 9p on localhost.
>>
>>> My understanding was passthrough will be preferred
>>> option. But i may be mistaken.
>>
>> If passthrough requires all of QEMU to run as root, then we need to
>> find a way to run that code separately and drop privileges in QEMU.
>>
>> The chroot helper process patches that Mohan posted might be a
>> solution.  The chroot helper does all path and permissions-related
>> operations in a separate process.  File descriptor passing is used so
>> that QEMU can perform read/write operations itself without copying
>> data.
>>
>> Then we just need to make sure that QEMU itself runs unprivileged and
>> the chroot helper is able to run as root for the passthrough security
>> model.
>
> Harsh, any thoughts on this?

Hi Stefan,
I am still not sure if it is really a big concern for VirtFS users, and 
if really required, we can move the functionality to a privileged 
process and but that would require Qemu to be initially run as root and 
then drop privileges to a certain non-root user. However, lets take this 
discussion separately and see what community thinks about it. If the 
community agrees, we can do it when we merge the chroot patch series. I 
therefore posted v2 for the st_gen patch to keep that isolated.

- Harsh
>
> Stefan