qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH -V3 1/8] hw/9pfs: Add V9fsfidmap in preparation for adding fd reclaim
Date: Tue, 15 Mar 2011 10:38:31 +0000

On Tue, Mar 15, 2011 at 9:20 AM, Aneesh Kumar K. V
<address@hidden> wrote:
> 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.

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.

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]