On Sun, 13 Mar 2011 16:08: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: > > @@ -107,7 +108,12 @@ static int v9fs_do_closedir(V9fsState *s, DIR *dir) > > > > static int v9fs_do_open(V9fsState *s, V9fsString *path, int flags) > > { > > - return s->ops->open(&s->ctx, path->data, flags); > > + int fd; > > + fd = s->ops->open(&s->ctx, path->data, flags); > > + if (fd > P9_FD_RECLAIM_THRES) { > > + v9fs_reclaim_fd(s); > > + } > > I think the threshold should depend on the file descriptor ulimit. > The hardcoded constant doesn't work if the ulimit is set to 1000 or > less (it would cause other users in QEMU to hit EMFILE errors).
Yes. That is suppose to be a follow up patch. I had that set to 100 for all the early testing. > > > + 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); > > + } > > Please address the FIXME. I think the case where O_RDWR breaks is if > QEMU has permissions to open the file for read only. The the client > is able to open the file for read but when the file descriptor is > resurrected we'll get EPERM here. The FIXME is fixed in the follow up patch (patch 5) > > > @@ -516,7 +600,10 @@ static int free_fid(V9fsState *s, int32_t fid) > > *fidpp = fidp->next; > > > > if (fidp->fsmap.fid_type == P9_FID_FILE) { > > - v9fs_do_close(s, fidp->fsmap.fs.fd); > > + /* I we reclaimed the fd no need to close */ > > s/I // > > > + if (fidp->fsmap.fs.fd != -1) { > > + v9fs_do_close(s, fidp->fsmap.fs.fd); > > + } > > } else if (fidp->fsmap.fid_type == P9_FID_DIR) { > > v9fs_do_closedir(s, fidp->fsmap.fs.dir); > > } else if (fidp->fsmap.fid_type == P9_FID_XATTR) { > > @@ -2719,7 +2806,11 @@ static void v9fs_remove(V9fsState *s, V9fsPDU *pdu) > > err = -EINVAL; > > goto out; > > } > > - > > + /* > > + * IF the file is unlinked, we cannot reopen > > + * the file later. So don't reclaim fd > > + */ > > + v9fs_mark_fids_unreclaim(s, &vs->fidp->fsmap.path); > > This poses a problem for the case where guest and host are both > accessing the file system. If the fd is reclaimed and the host > deletes the file, then the guest cannot access its open file anymore. > > The same issue also affects rename and has not been covered by this patch. > Currently virtFS don't handle the host rename/unlink. That we walk a name and get the fid and then use the fid to open the file. In between if the file get removed/renamed we will get an EINVAL. All that will go away once we switch to handle based open. -aneesh