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 <address@hidden> wrote:
> On Thu, Aug 4, 2011 at 11:06 AM, Harsh Prateek Bora
> <address@hidden> wrote:
> > This patch provides support for st_gen for handle based fs type server.
> > Currently the support is provided for ext4, btrfs, reiserfs and xfs.
> >
> > Signed-off-by: Harsh Prateek Bora <address@hidden>
> > ---
> >  hw/9pfs/virtio-9p-handle.c |   30 ++++++++++++++++++++++++++++++
> >  1 files changed, 30 insertions(+), 0 deletions(-)
> 
> Does handle-based file I/O really need to duplicate all this code?  Is
> it possible to use either regular open or handle-based open from a
> single local fs codebase?

The only details common between handle based and local based getversion
callback is the ioctl. Moving that into a helper may not really help in
this case ?.

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