From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47101 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PuIhd-0002Ix-90 for qemu-devel@nongnu.org; Tue, 01 Mar 2011 01:04:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PuIha-0006wU-PE for qemu-devel@nongnu.org; Tue, 01 Mar 2011 01:04:57 -0500 Received: from e28smtp02.in.ibm.com ([122.248.162.2]:49761) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PuIhZ-0006vq-Sb for qemu-devel@nongnu.org; Tue, 01 Mar 2011 01:04:54 -0500 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by e28smtp02.in.ibm.com (8.14.4/8.13.1) with ESMTP id p2164msw008920 for ; Tue, 1 Mar 2011 11:34:48 +0530 Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2164mio1540248 for ; Tue, 1 Mar 2011 11:34:48 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2164oTC003449 for ; Tue, 1 Mar 2011 11:34:50 +0530 From: "Aneesh Kumar K. V" Subject: Re: [Qemu-devel] [RFC PATCH -V1 4/4] hw/9pfs: Add open flag to fid In-Reply-To: <4D6C5358.1000200@linux.vnet.ibm.com> References: <1296929291-11047-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1296929291-11047-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <4D6C5358.1000200@linux.vnet.ibm.com> Date: Tue, 01 Mar 2011 11:34:42 +0530 Message-ID: <87wrkjgyv9.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Venkateswararao Jujjuri (JV)" Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On Mon, 28 Feb 2011 18:00:56 -0800, "Venkateswararao Jujjuri (JV)" wrote: > On 2/5/2011 10:08 AM, Aneesh Kumar K.V wrote: > > We use this flag when we reopen the file. We need > > to track open flag because if the open request have > > flags like O_SYNC, we want to open the file with same flag > > in host too > > > > Signed-off-by: Aneesh Kumar K.V > > --- > > hw/9pfs/virtio-9p.c | 57 ++++++++++++++++++++++++++++++++++++++++++--------- > > hw/9pfs/virtio-9p.h | 1 + > > 2 files changed, 48 insertions(+), 10 deletions(-) > > > > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > > index 1518e00..cbdf705 100644 > > --- a/hw/9pfs/virtio-9p.c > > +++ b/hw/9pfs/virtio-9p.c > > @@ -22,6 +22,8 @@ > > int debug_9p_pdu; > > static void v9fs_reclaim_fd(V9fsState *s); > > > > +#define PASS_OPEN_FLAG (O_SYNC | O_DSYNC | O_RSYNC | \ > > + O_EXCL) > > enum { > > Oread = 0x00, > > Owrite = 0x01, > > @@ -68,6 +70,24 @@ static int omode_to_uflags(int8_t mode) > > return ret; > > } > > > > +static int get_dotl_openflags(int oflags) > > +{ > > + int flags; > > + /* > > + * Since we can share the fd between multiple fids, > > + * open the file in read write mode > > + */ > > + flags = O_RDWR; > > + /* > > + * If the client asked for any of the below flags we > > + * should open the file with same open flag > > + */ > > + if (oflags & PASS_OPEN_FLAG) { > > + flags = oflags & PASS_OPEN_FLAG; > Isn't it > > flags |= oflags & PASS_OPEN_FLAG; ? Already fixed in my local tree. > > + } > > + return flags; > > +} > > + > > void cred_init(FsCred *credp) > > { > > credp->fc_uid = -1; > > @@ -452,9 +472,9 @@ static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid) > > * descriptors. > > */ > > if (f->fsmap.fid_type == P9_FID_FILE) { > > - /* FIXME!! should we remember the open flags ?*/ > > if (f->fsmap.fs.fd == -1) { > > - f->fsmap.fs.fd = v9fs_do_open(s, &f->fsmap.path, O_RDWR); > > + f->fsmap.fs.fd = v9fs_do_open(s, &f->fsmap.path, > > + f->fsmap.open_flags); > > } > > } > > /* > > @@ -1811,14 +1831,19 @@ static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err) > > v9fs_open_post_opendir(s, vs, err); > > } else { > > if (s->proto_version == V9FS_PROTO_2000L) { > > - flags = vs->mode; > > - flags &= ~(O_NOCTTY | O_ASYNC | O_CREAT); > > - /* Ignore direct disk access hint until the server supports it. */ > > - flags &= ~O_DIRECT; > > This is more drastic change. So far we are ignoring only few flags..but with > this we are allowing only few flags. Need to understand the impact of this on > dotl protocol. The goal is to make sure we use only those flags that make sense for the host file system. That would enable us to share the fd between multiple fids. We cannot share fd with different open flags. > > Also I would put this patch ahead of introducing fid reclaim patch. > > > + flags = get_dotl_openflags(vs->mode); > > } else { > > flags = omode_to_uflags(vs->mode); > > } > > vs->fidp->fsmap.fs.fd = v9fs_do_open(s, &vs->fidp->fsmap.path, flags); > > + vs->fidp->fsmap.open_flags = flags; > > + if (flags & O_EXCL) { > > + /* > > + * We let the host file system do O_EXCL check > > + * We should not reclaim such fd > > + */ > > + vs->fidp->fsmap.flags |= FID_NON_RECLAIMABLE; > > + } > > v9fs_open_post_open(s, vs, err); > > } > > return; > > @@ -1937,11 +1962,22 @@ static void v9fs_lcreate(V9fsState *s, V9fsPDU *pdu) > > v9fs_string_sprintf(&vs->fullname, "%s/%s", vs->fidp->fsmap.path.data, > > vs->name.data); > > > > - /* Ignore direct disk access hint until the server supports it. */ > > - flags &= ~O_DIRECT; > > - > > + flags = get_dotl_openflags(flags); > > vs->fidp->fsmap.fs.fd = v9fs_do_open2(s, vs->fullname.data, vs->fidp->uid, > > - gid, flags, mode); > > + gid, flags, mode); > > + /* > > + * We don't want to recreate the in reclaim path. So remove > > + * create flag > > + */ > > + flags &= ~O_CREAT; > > + vs->fidp->fsmap.open_flags = flags; > > + if (flags & O_EXCL) { > > + /* > > + * We let the host file system do O_EXCL check > > + * We should not reclaim such fd > > + */ > > + vs->fidp->fsmap.flags |= FID_NON_RECLAIMABLE; > > + } > You can move above two steps to here by... > vs->fidp->fsmap.open_flags = flags & ~O_CREAT > > v9fs_lcreate_post_do_open2(s, vs, err); > > return; > > > > @@ -2653,6 +2689,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err) > > -1, > > omode_to_uflags(vs->mode)|O_CREAT, > > vs->perm); > > + vs->fidp->fsmap.open_flags = omode_to_uflags(vs->mode); > > why are we not doing ~O_CREAT here? > We don't have O_CREATE in open_flags. I explicitly pass that to the above call. I also have another fix to make it work correct for dotl. May be i should post the new version i have -aneesh