On Tue, 15 Mar 2011 10:38:31 +0000, Stefan Hajnoczi <stefa...@gmail.com> wrote: > On Tue, Mar 15, 2011 at 9:20 AM, Aneesh Kumar K. V > <aneesh.ku...@linux.vnet.ibm.com> wrote: > > On Sun, 13 Mar 2011 20:53:41 +0000, Stefan Hajnoczi <stefa...@gmail.com> > > wrote: > >> On Sun, Mar 13, 2011 at 7:06 PM, Aneesh Kumar K. V > >> <aneesh.ku...@linux.vnet.ibm.com> wrote: > >> > 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: > >> >> > @@ -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. > > Tracking open flags is fine and is needed for fd reclaim. But > splitting V9fsFidState into the V9fsFidMap structure and all the churn > that causes to the code isn't necessary yet. Please wait with that > until you submit patches fd sharing. The reason I make this > suggestion is that everyone reading or working on the code until fd > sharing is added now needs to deal with the V9fsFidMap structure which > (currently) serves no purpose.
taken. will remove the patch from the series. -aneesh