[Qemu-devel] [PATCH 0/2] Support for i_generation / st_gen in 9p server

[Qemu-devel] [PATCH 0/2] Support for i_generation / st_gen in 9p server

[Harsh Prateek Bora]

This patch series provide support for i_generation / st_gen in 9p server.
First patch provide support for local fs type, second patch adds support
for handle based fs driver.

Harsh Prateek Bora (2):
  i_generation / st_gen support for local fs type
  i_generation / st_gen support for handle based fs driver

 fsdev/file-op-9p.h         |    7 +++++++
 hw/9pfs/cofile.c           |   22 ++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h   |    2 ++
 hw/9pfs/virtio-9p-device.c |    1 +
 hw/9pfs/virtio-9p-handle.c |   30 ++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-local.c  |   32 +++++++++++++++++++++++++++++++-
 hw/9pfs/virtio-9p.c        |    9 +++++++++
 7 files changed, 102 insertions(+), 1 deletions(-)

[Qemu-devel] [PATCH 1/2] i_generation / st_gen support for local fs type

[Harsh Prateek Bora]

This patch provides support for st_gen for local 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>
---
 fsdev/file-op-9p.h         |    7 +++++++
 hw/9pfs/cofile.c           |   22 ++++++++++++++++++++++
 hw/9pfs/virtio-9p-coth.h   |    2 ++
 hw/9pfs/virtio-9p-device.c |    1 +
 hw/9pfs/virtio-9p-local.c  |   32 +++++++++++++++++++++++++++++++-
 hw/9pfs/virtio-9p.c        |    9 +++++++++
 6 files changed, 72 insertions(+), 1 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index a6940bb..e6a5055 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -49,6 +49,12 @@ typedef struct FsCred
 } FsCred;
 
 struct xattr_operations;
+struct FsContext;
+struct V9fsPath;
+
+typedef struct extended_ops {
+    int (*get_st_gen)(struct FsContext *, struct V9fsPath *, uint64_t *);
+} extended_ops;
 
 /* FsContext flag values */
 #define PATHNAME_FSCONTEXT 0x1
@@ -62,6 +68,7 @@ typedef struct FsContext
     int open_flags;
     int cache_model;
     struct xattr_operations **xops;
+    struct extended_ops exops;
     /* fs driver specific data */
     void *private;
 } FsContext;
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 7ad4bec..bf7c44b 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -17,6 +17,28 @@
 #include "qemu-coroutine.h"
 #include "virtio-9p-coth.h"
 
+int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, V9fsStatDotl *v9stat)
+{
+    int err;
+    V9fsState *s = pdu->s;
+
+    if (v9fs_request_cancelled(pdu)) {
+        return -EINTR;
+    }
+    if (s->ctx.exops.get_st_gen) {
+        v9fs_path_read_lock(s);
+        v9fs_co_run_in_worker(
+            {
+                err = s->ctx.exops.get_st_gen(&s->ctx, path, &v9stat->st_gen);
+                if (err < 0) {
+                    err = -errno;
+                }
+            });
+        v9fs_path_unlock(s);
+    }
+    return err;
+}
+
 int v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf)
 {
     int err;
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 4630080..c9e9325 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -101,4 +101,6 @@ extern int v9fs_co_preadv(V9fsPDU *, V9fsFidState *,
                           struct iovec *, int, int64_t);
 extern int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *,
                                 const char *, V9fsPath *);
+extern int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, V9fsStatDotl *v9stat);
+
 #endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 246da23..6a969ba 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -122,6 +122,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
     }
     s->ctx.cache_model = fse->cache_model;
     s->ctx.fs_root = qemu_strdup(fse->path);
+    s->ctx.exops.get_st_gen = NULL;
     len = strlen(conf->tag);
     if (len > MAX_TAG_LEN) {
         len = MAX_TAG_LEN;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 1b6c323..2a2ddfa 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -20,6 +20,9 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#include <linux/fs.h>
+#include <linux/magic.h>
+#include <sys/ioctl.h>
 
 static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
 {
@@ -645,10 +648,37 @@ static int local_unlinkat(FsContext *ctx, V9fsPath *dir,
     return ret;
 }
 
+static int local_ioc_getversion(FsContext *ctx, V9fsPath *path, uint64_t *st_gen)
+{
+    int mode = 0600;
+    int fd;
+
+    fd = local_open(ctx, path, mode);
+    if(fd < 0) {
+        return fd;
+    }
+    return ioctl(fd, FS_IOC_GETVERSION, st_gen);
+
+}
+/* XFS_SUPER_MAGIC not available in linux/fs.h */
+#define XFS_SUPER_MAGIC 0x58465342
 static int local_init(FsContext *ctx)
 {
+    int err;
+    struct statfs stbuf;
     ctx->flags |= PATHNAME_FSCONTEXT;
-    return 0;
+    err = statfs(ctx->fs_root, &stbuf);
+    if(!err) {
+        switch (stbuf.f_type) {
+        case EXT4_SUPER_MAGIC: /* same magic val for ext2/3 */
+        case BTRFS_SUPER_MAGIC:
+        case REISERFS_SUPER_MAGIC:
+        case XFS_SUPER_MAGIC:
+            ctx->exops.get_st_gen = local_ioc_getversion;
+            break;
+        }
+    }
+    return err;
 }
 
 FileOperations local_ops = {
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 8b5e711..469668f 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1357,6 +1357,15 @@ static void v9fs_getattr(void *opaque)
         goto out;
     }
     stat_to_v9stat_dotl(s, &stbuf, &v9stat_dotl);
+
+    /*  fill st_gen if requested and supported by underlying fs */
+    if (request_mask & P9_STATS_GEN) {
+        retval = v9fs_co_st_gen(pdu, &fidp->path, &v9stat_dotl);
+        if (retval < 0) {
+            goto out;
+        }
+        v9stat_dotl.st_result_mask |= P9_STATS_GEN;
+    }
     retval = offset;
     retval += pdu_marshal(pdu, offset, "A", &v9stat_dotl);
 out:
-- 
1.7.1.1

Re: [Qemu-devel] [PATCH 1/2] i_generation / st_gen support for local fs type

[Stefan Hajnoczi]

On Thu, Aug 4, 2011 at 11:06 AM, Harsh Prateek Bora
<harsh@linux.vnet.ibm.com> wrote:
> +static int local_ioc_getversion(FsContext *ctx, V9fsPath *path, uint64_t *st_gen)
> +{
> +    int mode = 0600;
> +    int fd;
> +
> +    fd = local_open(ctx, path, mode);
> +    if(fd < 0) {
> +        return fd;
> +    }
> +    return ioctl(fd, FS_IOC_GETVERSION, st_gen);

fd is leaked.

Stefan

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

[Harsh Prateek Bora]

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

diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
index 548a841..8dff662 100644
--- a/hw/9pfs/virtio-9p-handle.c
+++ b/hw/9pfs/virtio-9p-handle.c
@@ -20,6 +20,9 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#include <linux/fs.h>
+#include <linux/magic.h>
+#include <sys/ioctl.h>
 
 struct handle_data {
     int mountfd;
@@ -543,9 +546,25 @@ static int handle_unlinkat(FsContext *ctx, V9fsPath *dir,
     return ret;
 }
 
+static int handle_ioc_getversion(FsContext *ctx, V9fsPath *path, uint64_t *st_gen)
+{
+    int mode = 0600;
+    int fd;
+
+    fd = handle_open(ctx, path, mode);
+    if(fd < 0) {
+        return fd;
+    }
+    return ioctl(fd, FS_IOC_GETVERSION, st_gen);
+
+}
+
+/*  XFS_SUPER_MAGIC not available in linux/fs.h */
+#define XFS_SUPER_MAGIC 0x58465342
 static int handle_init(FsContext *ctx)
 {
     int ret, mnt_id;
+    struct statfs stbuf;
     struct file_handle fh;
     struct handle_data *data = qemu_malloc(sizeof(struct handle_data));
     data->mountfd = open(ctx->fs_root, O_DIRECTORY);
@@ -553,6 +572,17 @@ static int handle_init(FsContext *ctx)
         ret = data->mountfd;
         goto err_out;
     }
+    ret = statfs(ctx->fs_root, &stbuf);
+    if(!ret) {
+        switch (stbuf.f_type) {
+        case EXT4_SUPER_MAGIC: /*  same magic val for ext2/3 */
+        case BTRFS_SUPER_MAGIC:
+        case REISERFS_SUPER_MAGIC:
+        case XFS_SUPER_MAGIC:
+            ctx->exops.get_st_gen = handle_ioc_getversion;
+            break;
+        }
+    }
     memset(&fh, 0, sizeof(struct file_handle));
     ret = name_to_handle(data->mountfd, ".", &fh, &mnt_id, 0);
     if (ret && errno == EOVERFLOW) {
-- 
1.7.1.1

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

[Stefan Hajnoczi]

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?

> diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
> index 548a841..8dff662 100644
> --- a/hw/9pfs/virtio-9p-handle.c
> +++ b/hw/9pfs/virtio-9p-handle.c
> @@ -20,6 +20,9 @@
>  #include <sys/socket.h>
>  #include <sys/un.h>
>  #include <attr/xattr.h>
> +#include <linux/fs.h>
> +#include <linux/magic.h>
> +#include <sys/ioctl.h>
>
>  struct handle_data {
>     int mountfd;
> @@ -543,9 +546,25 @@ static int handle_unlinkat(FsContext *ctx, V9fsPath *dir,
>     return ret;
>  }
>
> +static int handle_ioc_getversion(FsContext *ctx, V9fsPath *path, uint64_t *st_gen)
> +{
> +    int mode = 0600;
> +    int fd;
> +
> +    fd = handle_open(ctx, path, mode);
> +    if(fd < 0) {
> +        return fd;
> +    }
> +    return ioctl(fd, FS_IOC_GETVERSION, st_gen);

fd is leaked here.

Stefan

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

[Aneesh Kumar K.V]

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

> 
> > diff --git a/hw/9pfs/virtio-9p-handle.c b/hw/9pfs/virtio-9p-handle.c
> > index 548a841..8dff662 100644
> > --- a/hw/9pfs/virtio-9p-handle.c
> > +++ b/hw/9pfs/virtio-9p-handle.c
> > @@ -20,6 +20,9 @@
> >  #include <sys/socket.h>
> >  #include <sys/un.h>
> >  #include <attr/xattr.h>
> > +#include <linux/fs.h>
> > +#include <linux/magic.h>
> > +#include <sys/ioctl.h>
> >
> >  struct handle_data {
> >     int mountfd;
> > @@ -543,9 +546,25 @@ static int handle_unlinkat(FsContext *ctx, V9fsPath *dir,
> >     return ret;
> >  }
> >
> > +static int handle_ioc_getversion(FsContext *ctx, V9fsPath *path, uint64_t *st_gen)
> > +{
> > +    int mode = 0600;
> > +    int fd;
> > +
> > +    fd = handle_open(ctx, path, mode);
> > +    if(fd < 0) {
> > +        return fd;
> > +    }
> > +    return ioctl(fd, FS_IOC_GETVERSION, st_gen);
> 
> fd is leaked here.
> 
> Stefan

Both handle and local use V9fsPath to encode file details, the former
use the file handle returned by open-by-handle and the later file
names. It is difficult to find out which callback to use to open the
file unless we look at the fs driver used(local/handle argument passed
to -fsdev command line option), hence the idea of splitting
these into two different callbacks.

-aneesh

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

[Stefan Hajnoczi]

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.

Stefan

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

[Aneesh Kumar K.V]

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. 

-aneesh

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

[Stefan Hajnoczi]

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.

Stefan

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

[Aneesh Kumar K.V]

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.

-aneesh

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

[Stefan Hajnoczi]

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?

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?

Stefan

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

[Aneesh Kumar K.V]

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.

-aneesh

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

[Stefan Hajnoczi]

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.

The none model is handy for developers but what is the real plan for
production?  Maybe QEMU as root can be isolated using SELinux but
basically asking to run QEMU as root isn't an option IMO.

If virtfs runs a separate 9P server in userspace then that can use
whatever privileges it needs because it's scope is much more limited.
Unfortunately this results in communication overhead since the guest
talks through QEMU.  If guest RAM is shared memory it would be
possible to use eventfds and shared memory to get direct
9p-server<->guest data flow.

At the point where you have a userspace 9p server it would also be
thinkable to merge with diod and combine development efforts.

I know this would be a big change but the alternative is to use the
mapped security model (and maybe handle-based open with
CAP_DAC_READ_SEARCH).  Spending effort on features that require QEMU
to run as root isn't going to pay off if they cannot be used.

Stefan

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

[Aneesh Kumar K.V]

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 ?. My understanding was passthrough will be preferred
option. But i may be mistaken.

-aneesh

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

[Stefan Hajnoczi]

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.

Stefan

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

Stefan

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

[Stefan Hajnoczi]

On Wed, Aug 10, 2011 at 4:17 PM, Stefan Hajnoczi <stefanha@gmail.com> 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?

+ Aneesh :)

Stefan

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

[Aneesh Kumar K.V]

On Wed, 10 Aug 2011 16:17:22 +0100, Stefan Hajnoczi <stefanha@gmail.com> 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?

How do we achieve this ? Qemu binary should be setuid and then later
drop privileges to "nobody:kvm"  user:group ? or use file based
capabilities because distros are removing setuid binaries ?.

One nice detail about "handle" fs driver is, it won't require
chroot helper at all. "handle" fs driver also work nicely with
file renames on host. File rename on the guest also become much
easier with "handle" tracking the file rather than the name.

If we agree that running qemu as root do have security implication, then
we should look at what you suggested above. I am trying to understand
how distros would want to ship qemu for achieving the above goal.

-aneesh

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

[Harsh Bora]

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