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

[Stefan Hajnoczi]

On Fri, Aug 5, 2011 at 1:53 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Fri, Aug 5, 2011 at 12:32 PM, Aneesh Kumar K.V
> <address@hidden> wrote:
>> On Fri, 5 Aug 2011 10:24:42 +0100, Stefan Hajnoczi <address@hidden> wrote:
>>> On Fri, Aug 5, 2011 at 7:40 AM, Aneesh Kumar K.V
>>> <address@hidden> wrote:
>>> > On Thu, 4 Aug 2011 22:57:34 +0100, Stefan Hajnoczi <address@hidden> wrote:
>>> >> On Thu, Aug 4, 2011 at 7:45 PM, Aneesh Kumar K.V
>>> >> <address@hidden> wrote:
>>> >> > On Thu, 4 Aug 2011 15:31:08 +0100, Stefan Hajnoczi <address@hidden> 
>>> >> > wrote:
>>> >> >> On Thu, Aug 4, 2011 at 1:03 PM, Aneesh Kumar K.V
>>> >> >> <address@hidden> wrote:
>>> >> >> > On Thu, 4 Aug 2011 12:47:42 +0100, Stefan Hajnoczi <address@hidden> 
>>> >> >> > wrote:
>>> >> >> >> On Thu, Aug 4, 2011 at 12:20 PM, Aneesh Kumar K.V
>>> >> >> >> <address@hidden> wrote:
>>> >> >> >> > On Thu, 4 Aug 2011 11:21:05 +0100, Stefan Hajnoczi 
>>> >> >> >> > <address@hidden> wrote:
>>> >> >> >> >> On Thu, Aug 4, 2011 at 11:06 AM, Harsh Prateek Bora
>>> >> >> >> >> <address@hidden> 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 <address@hidden>
>>> >> >> >> >> > ---
>>> >> >> >> >> >  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?

Stefan