On Sun, Mar 13, 2011 at 8:53 PM, 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?
I saw your response to a later patch in this series that fd sharing is
not implemented. In that case this patch doesn't add value, it just
makes the code harder to understand by introducing an unused level of
indirection for which we don't have a good name yet.
Perhaps drop this patch from the series and send it later if you
implement fd sharing.
Stefan
