Re: [Qemu-devel] [PATCH -V3 1/8] hw/9pfs: Add V9fsfidmap in preparation for adding fd reclaim

[Aneesh Kumar K. V]

On Sun, 13 Mar 2011 20:53:41 +0000, Stefan Hajnoczi <address@hidden> wrote:
> On Sun, Mar 13, 2011 at 7:06 PM, Aneesh Kumar K. V
> <address@hidden> wrote:
> > On Sun, 13 Mar 2011 15:46:29 +0000, Stefan Hajnoczi <address@hidden> wrote:
> >> On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V
> >> <address@hidden> wrote:
> >> > @@ -185,17 +188,22 @@ typedef struct V9fsXattr
> >> >     int flags;
> >> >  } V9fsXattr;
> >> >
> >> > +typedef struct V9fsfidmap {
> >>
> >> V9fsFidMap (naming convention)
> >>
> >> > +    union {
> >> > +        int fd;
> >> > +        DIR *dir;
> >> > +        V9fsXattr xattr;
> >> > +    } fs;
> >>
> >> The name "fs" is not meaningful.
> >>
> >> > +    int fid_type;
> >> > +    V9fsString path;
> >> > +    int flags;
> >> > +} V9fsFidMap;
> >> > +
> >> >  struct V9fsFidState
> >> >  {
> >> > -    int fid_type;
> >> >     int32_t fid;
> >> > -    V9fsString path;
> >> > -    union {
> >> > -       int fd;
> >> > -       DIR *dir;
> >> > -       V9fsXattr xattr;
> >> > -    } fs;
> >> >     uid_t uid;
> >> > +    V9fsFidMap fsmap;
> >>
> >> This name is confusing.  A "map" is usually a container that stores
> >> key/value pairs.  V9fsFidMapEntry would be clearer.  But then I
> >> thought that is what V9fsFidState is?
> >
> > I am bad at naming. I wanted to indicate something that can be shared
> > across multiple fids and also indicate the local file system
> > "mapping"/data. I will take any suggestion.
> 
> Where does sharing happen, I didn't notice any code that shares fds
> between fids?

That patch is not yet there. We can only share fd if they open flags
match. Hence making sure we open files on host with limited set of open
flags which enables us much better sharing.

-aneesh