Re: [Qemu-devel] [PATCH -V3 2/8] hw/9pfs: Add file descriptor reclaim support

["Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>]

On Mon, 14 Mar 2011 10:13:59 +0000, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sun, Mar 13, 2011 at 6:57 PM, Aneesh Kumar K. V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > On Sun, 13 Mar 2011 16:08:29 +0000, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >> On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V
> >> <aneesh.kumar@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.
> 
> Using getrlimit(2) to choose a good threshold at runtime shouldn't be
> a lot of code.  Please add it to this patch so the threshold isn't
> arbitrary and possibly ineffective due to ulimit.

ok.

> 
> >> > @@ -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.
> 
> Can you explain this more?  Will multiple entities be able to safely
> use the file system (e.g. host and guest)?

handles are stable across renames. So even if host rename the file, qemu
will be able to access it. But we still won't be able to handle unlink
on host. But that is true with even other file servers. They do get
ESTALE in that case.


-aneesh