From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51971 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzTLq-0004nD-0m for qemu-devel@nongnu.org; Tue, 15 Mar 2011 08:27:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzTLo-0004Y6-2v for qemu-devel@nongnu.org; Tue, 15 Mar 2011 08:27:49 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:48910) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzTLn-0004XE-JL for qemu-devel@nongnu.org; Tue, 15 Mar 2011 08:27:48 -0400 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [202.81.31.246]) by e23smtp04.au.ibm.com (8.14.4/8.13.1) with ESMTP id p2FCLsbP031636 for ; Tue, 15 Mar 2011 23:21:54 +1100 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2FCRQcI2052098 for ; Tue, 15 Mar 2011 23:27:29 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2FCRQKS019621 for ; Tue, 15 Mar 2011 23:27:26 +1100 From: "Aneesh Kumar K. V" Subject: Re: [Qemu-devel] [PATCH -V3 1/8] hw/9pfs: Add V9fsfidmap in preparation for adding fd reclaim In-Reply-To: References: <1299347533-17047-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <87vczmq1qm.fsf@linux.vnet.ibm.com> <87fwqowxi8.fsf@linux.vnet.ibm.com> Date: Tue, 15 Mar 2011 17:57:14 +0530 Message-ID: <87zkowpo0t.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On Tue, 15 Mar 2011 10:38:31 +0000, Stefan Hajnoczi wr= ote: > On Tue, Mar 15, 2011 at 9:20 AM, Aneesh Kumar K. V > wrote: > > On Sun, 13 Mar 2011 20:53:41 +0000, Stefan Hajnoczi wrote: > >> On Sun, Mar 13, 2011 at 7:06 PM, Aneesh Kumar K. V > >> wrote: > >> > On Sun, 13 Mar 2011 15:46:29 +0000, Stefan Hajnoczi wrote: > >> >> On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V > >> >> wrote: > >> >> > @@ -185,17 +188,22 @@ typedef struct V9fsXattr > >> >> > =C2=A0 =C2=A0 int flags; > >> >> > =C2=A0} V9fsXattr; > >> >> > > >> >> > +typedef struct V9fsfidmap { > >> >> > >> >> V9fsFidMap (naming convention) > >> >> > >> >> > + =C2=A0 =C2=A0union { > >> >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0int fd; > >> >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0DIR *dir; > >> >> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0V9fsXattr xattr; > >> >> > + =C2=A0 =C2=A0} fs; > >> >> > >> >> The name "fs" is not meaningful. > >> >> > >> >> > + =C2=A0 =C2=A0int fid_type; > >> >> > + =C2=A0 =C2=A0V9fsString path; > >> >> > + =C2=A0 =C2=A0int flags; > >> >> > +} V9fsFidMap; > >> >> > + > >> >> > =C2=A0struct V9fsFidState > >> >> > =C2=A0{ > >> >> > - =C2=A0 =C2=A0int fid_type; > >> >> > =C2=A0 =C2=A0 int32_t fid; > >> >> > - =C2=A0 =C2=A0V9fsString path; > >> >> > - =C2=A0 =C2=A0union { > >> >> > - =C2=A0 =C2=A0 =C2=A0 int fd; > >> >> > - =C2=A0 =C2=A0 =C2=A0 DIR *dir; > >> >> > - =C2=A0 =C2=A0 =C2=A0 V9fsXattr xattr; > >> >> > - =C2=A0 =C2=A0} fs; > >> >> > =C2=A0 =C2=A0 uid_t uid; > >> >> > + =C2=A0 =C2=A0V9fsFidMap fsmap; > >> >> > >> >> This name is confusing. =C2=A0A "map" is usually a container that s= tores > >> >> key/value pairs. =C2=A0V9fsFidMapEntry would be clearer. =C2=A0But = 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. >=20 > 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