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

[Bandan Das]

Daniel P. Berrangé <address@hidden> writes:

> 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.
>
No, it doesn't atm and is limited by memory. This does sound right
to prevent an overflow but I am not sure what would be a good limit
for a directory hierarchy with multiple files and folders.

Bandan

> Regards,
> Daniel