| Message ID | 1312452371-10375-3-git-send-email-harsh@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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) {
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(-)