Re: [Qemu-devel] [PATCH 2/2] filemon: ensure watch IDs are unique to QFileMonitor scope

[Daniel P . Berrangé]

On Fri, Mar 15, 2019 at 01:24:42PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <address@hidden> writes:
> ...
> >> Thanks, this fixes it! I had a related question about the way
> >> qemu_file_monitor_add_watch works.
> >> 
> >> Am I correct in understanding that if there is already a watch on a dir,
> >> we return back mon->nextid++ i.e the next free id. Why don't we return
> >> back the originally assigned watchid ?
> >
> > inotify watches are a per-directory scope thing, but this API is aiming
> > to present the ability to have per-file scope watches as it is more
> > convenient to have those semantics for some users in QEMU. Thus many
> > QEMU level watches are mapped to the same low level inotify watch.
> >
> > The watch ID is used to unregister a watch later, and while I could have
> > done ref counting against a common watch ID, I find it clearer to maintain
> > separate watch IDs for every thing the caller watches. If nothing else it
> > makes the debugging easier as you can see the relation between events
> > received and the original watch registration.
> >
> Thanks, this makes sense. I think an advantage of having a refcounting
> mechanism is that we won't run out of watchids because some caller
> ended up adding a watch on the same dir repeatedly.

The watchid number is global to the QFileMonitor object, so if you
repeatedly create & delete watches on the same dir you're still going
to use up watch ids. You'll only save if you create & delete per-file
watches within the dir. Given that the guest OS can trigger create
/ delete of directories though, which in turn triggers create/delete
of watches, it looks like we should at the very least check for
overflow.

BTW, does USB-MTP have any limit on the number of objects it will
permit the guest to access ? something ought to limit the memory
usage of resources allocated per dir, including the watch state.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|