On Sun, 13 Mar 2011 15:46:29 +0000, Stefan Hajnoczi <stefa...@gmail.com> wrote: > On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V > <aneesh.ku...@linux.vnet.ibm.com> wrote: > > Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com> > > --- > > hw/9pfs/virtio-9p.c | 327 > > ++++++++++++++++++++++++++------------------------- > > hw/9pfs/virtio-9p.h | 22 +++- > > 2 files changed, 184 insertions(+), 165 deletions(-) > > I don't understand why this patch is necessary. It introduces an > intermediate structure that doesn't seem to be useful by itself. > Don't we always need the V9fsFidState? > > > @@ -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. -aneesh