Re: [Qemu-devel] [PATCH v2 3/4] usb-mtp: Add support for inotify based file monitoring

[Bandan Das <bsd@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> +            switch (mask) {
>> +            case IN_CREATE:
>> +                if (event->mask & IN_ISDIR) {
>> +                    /*
>> +                     * Add the watchpoint first so we
>> +                     * don't miss events in this subdir
>> +                     */
>> +                    name = g_strndup(event->name, event->len);
>> +                    path = g_strdup_printf("%s/%s", parent->path, name);
>> +
>> +                    watchfd = usb_mtp_add_watch(s->inotifyfd, path);
>> +                    g_free(path);
>> +                    g_free(name);
>> +
>> +                    if (watchfd == -1) {
>> +                        continue;
>> +                    }
>> +                }
>
> So, to follow up my mail from today in the morning:
>
> I think this is not needed here ...
>
>>          usb_mtp_object_readdir(s, o);
>> +        if (usb_mtp_inotify_mon(s, o)) {
>> +            fprintf(stderr, "usb-mtp: adding watch for %s failed\n", o->path);
>> +        }
>
> ... but here we have to add the watch first, then read the directory.
> When reading the directory first we'll miss new files which are added
> after readdir() but before add_watch().

Yep, this makes sense. So:

1. No need to add watch as soon as possible. As long as the guest knows there
is a directory(which it would because of the create event), it can request contents
at any time.
2. When it does, the first thing to do is add a watchpoint. Then, readdir does its thing
getting the dir contents and the watchpoint which has already been added will track new
changes.
3. We don't have to worry about  content changes before the guest has requested
the folder contents.

Thanks for the review!

> cheers,
>   Gerd