From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:57148)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1ZxHyH-0000tI-1B
	for qemu-devel@nongnu.org; Fri, 13 Nov 2015 12:17:11 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1ZxHyA-0006mi-Ng
	for qemu-devel@nongnu.org; Fri, 13 Nov 2015 12:17:08 -0500
Received: from mx1.redhat.com ([209.132.183.28]:56041)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bsd@redhat.com>) id 1ZxHyA-0006mZ-IP
	for qemu-devel@nongnu.org; Fri, 13 Nov 2015 12:17:02 -0500
Received: from int-mx09.intmail.prod.int.phx2.redhat.com
	(int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (Postfix) with ESMTPS id AA2ECF804E
	for <qemu-devel@nongnu.org>; Fri, 13 Nov 2015 17:17:01 +0000 (UTC)
From: Bandan Das <bsd@redhat.com>
References: <1447196299-16091-1-git-send-email-bsd@redhat.com>
	<1447196299-16091-4-git-send-email-bsd@redhat.com>
	<1447418075.1400.110.camel@redhat.com>
Date: Fri, 13 Nov 2015 12:16:59 -0500
In-Reply-To: <1447418075.1400.110.camel@redhat.com> (Gerd Hoffmann's message
	of "Fri, 13 Nov 2015 13:34:35 +0100")
Message-ID: <jpgr3jt95v8.fsf@linux.bootlegged.copy>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v2 3/4] usb-mtp: Add support for inotify
	based file monitoring
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: qemu-devel@nongnu.org

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