qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH -V3 2/8] hw/9pfs: Add file descriptor reclaim support
Date: Mon, 14 Mar 2011 10:13:59 +0000

On Sun, Mar 13, 2011 at 6:57 PM, Aneesh Kumar K. V
<address@hidden> wrote:
> On Sun, 13 Mar 2011 16:08:29 +0000, Stefan Hajnoczi <address@hidden> wrote:
>> On Sat, Mar 5, 2011 at 5:52 PM, Aneesh Kumar K.V
>> <address@hidden> 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.

>> > @@ -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)?

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]