From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54991 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pyqcb-0000eV-Lj for qemu-devel@nongnu.org; Sun, 13 Mar 2011 15:06:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pyqca-0001y4-Er for qemu-devel@nongnu.org; Sun, 13 Mar 2011 15:06:33 -0400 Received: from e28smtp03.in.ibm.com ([122.248.162.3]:54476) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PyqcZ-0001xc-LJ for qemu-devel@nongnu.org; Sun, 13 Mar 2011 15:06:32 -0400 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by e28smtp03.in.ibm.com (8.14.4/8.13.1) with ESMTP id p2DJ6S2i032571 for ; Mon, 14 Mar 2011 00:36:28 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2DJ6RYF4436054 for ; Mon, 14 Mar 2011 00:36:27 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2DJ6Rl6015578 for ; Mon, 14 Mar 2011 06:06:27 +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> Date: Mon, 14 Mar 2011 00:36:25 +0530 Message-ID: <87vczmq1qm.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 Sun, 13 Mar 2011 15:46:29 +0000, Stefan Hajnoczi wr= ote: > On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V > wrote: > > Signed-off-by: Aneesh Kumar K.V > > --- > > =C2=A0hw/9pfs/virtio-9p.c | =C2=A0327 ++++++++++++++++++++++++++-------= ------------------ > > =C2=A0hw/9pfs/virtio-9p.h | =C2=A0 22 +++- > > =C2=A02 files changed, 184 insertions(+), 165 deletions(-) >=20 > 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? >=20 > > @@ -185,17 +188,22 @@ typedef struct V9fsXattr > > =C2=A0 =C2=A0 int flags; > > =C2=A0} V9fsXattr; > > > > +typedef struct V9fsfidmap { >=20 > V9fsFidMap (naming convention) >=20 > > + =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; >=20 > The name "fs" is not meaningful. >=20 > > + =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; >=20 > 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