Re: [Qemu-devel] [PATCH v4 01/17] util: add helper APIs for dealing with inotify in portable manner

[Daniel P . Berrangé]

On Tue, Mar 12, 2019 at 07:07:42PM -0400, Bandan Das wrote:
> Daniel P. Berrangé <address@hidden> writes:
> ...
> > +
> > +int
> > +qemu_file_monitor_add_watch(QFileMonitor *mon,
> > +                            const char *dirpath,
> > +                            const char *filename,
> > +                            QFileMonitorHandler cb,
> > +                            void *opaque,
> > +                            Error **errp)
> > +{
> > +    QFileMonitorDir *dir;
> > +    QFileMonitorWatch watch;
> > +    int ret = -1;
> > +
> > +    qemu_mutex_lock(&mon->lock);
> > +    dir = g_hash_table_lookup(mon->dirs, dirpath);
> > +    if (!dir) {
> > +        int rv = inotify_add_watch(mon->fd, dirpath,
> > +                                   IN_CREATE | IN_DELETE | IN_MODIFY |
> > +                                   IN_MOVED_TO | IN_MOVED_FROM | 
> > IN_ATTRIB);
> > +
> > +        if (rv < 0) {
> > +            error_setg_errno(errp, errno, "Unable to watch '%s'", dirpath);
> > +            goto cleanup;
> > +        }
> > +
> > +        trace_qemu_file_monitor_enable_watch(mon, dirpath, rv);
> > +
> > +        dir = g_new0(QFileMonitorDir, 1);
> > +        dir->path = g_strdup(dirpath);
> > +        dir->id = rv;
> > +        dir->watches = g_array_new(FALSE, TRUE, sizeof(QFileMonitorWatch));
> > +
> > +        g_hash_table_insert(mon->dirs, dir->path, dir);
> > +        g_hash_table_insert(mon->idmap, GINT_TO_POINTER(rv), dir);
> > +
> > +        if (g_hash_table_size(mon->dirs) == 1) {
> > +            qemu_set_fd_handler(mon->fd, qemu_file_monitor_watch, NULL, 
> > mon);
> > +        }
> > +    }
> > +
> > +    watch.id = dir->nextid++;
> > +    watch.filename = g_strdup(filename);
> > +    watch.cb = cb;
> > +    watch.opaque = opaque;
> > +
> > +    g_array_append_val(dir->watches, watch);
> > +
> > +    trace_qemu_file_monitor_add_watch(mon, dirpath,
> > +                                      filename ? filename : "<none>",
> > +                                      cb, opaque, watch.id);
> > +
> > +    ret = watch.id;
> > +
> 
> This seems to break usb-mtp since we are returning watch.id.
> Why are we not returning dir->id here ? usb-mtp relies on the watch
> descriptor to handle events.

dir->id is the low level inotify identifier. This is not supposed to be
visible to any calling code, since that should instead be using the
QEMU generated watch.id value.

Can you give more info on how usb-mtp broke ?  I tested USB MTP before
sending this series and I saw it correctly detecting create / delete
of files & reflecting this in the guest.


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 :|