[gvfs] mtp: Switch to a stable device uri

[Philip Langdale <philipl src gnome org>]

commit 3a7bb06be72fc62b4ec524afe67febf0f529066d
Author: Philip Langdale <philipl overt org>
Date:   Sun Jan 7 10:46:26 2018 -0800

    mtp: Switch to a stable device uri
    
    Currently, we generate our device uris using the USB bus number and
    device number. The device number is a monotonically increasing number
    that goes up for each new device plugged into a bus, and that
    includes replugging a device.
    
    This is inconvenient for a couple of reasons:
    1: Modern Android devices actually trigger a replug event when you
       switch them into File Transfer mode, so if you, or your file
       manager opens a window before switching modes, that window
       ends up being useless.
    2: You can't write automation that encodes device uris as these
       end up being unpredictable
    
    This commit implements a change to the device uri to use the
    ID_SERIAL field provided by udev. If the device has a serial
    number, it will be combined with the product identity to
    yield this value. If the device has no serial number, then
    udev will fallback to using product identity only.
    
    The end result is that a device with a real serial number
    will always appear with the same uri, regardless of when or
    where it is plugged in. If a device has no serial number, or
    has a broken one (eg: all zeros, all the time), then one such
    device will work fine, but if a second one is plugged in, it
    will be ignored. We have chosen to ignore the device to avoid
    implementing a fallback uri generation scheme.
    
    The code becomes slightly more complicated because you cannot directly
    look up a device in this way, and libmtp works with dev/bus numbers.
    That requires a couple of additional lookups, but it's nothing
    serious.
    
    Note that this does not perfectly fix problem 1) because there's
    no event triggered which causes Nautilus to refresh the device
    window. If you manually refresh, you will see the contents.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=789561
    
    https://bugzilla.gnome.org/show_bug.cgi?id=744267

 daemon/gvfsbackendmtp.c         |  102 +++++++++++++++++++++++----------------
 monitor/mtp/gmtpvolumemonitor.c |   51 ++++++++++++++------
 2 files changed, 96 insertions(+), 57 deletions(-)
---
diff --git a/daemon/gvfsbackendmtp.c b/daemon/gvfsbackendmtp.c
index 3537f4e..ee3c70e 100644
--- a/daemon/gvfsbackendmtp.c
+++ b/daemon/gvfsbackendmtp.c
@@ -586,7 +586,7 @@ fail_job (GVfsJob *job, LIBMTP_mtpdevice_t *device)
  ************************************************/
 
 static LIBMTP_mtpdevice_t *
-get_device (GVfsBackend *backend, const char *id, GVfsJob *job);
+get_device (GVfsBackend *backend, uint32_t bus_num, uint32_t dev_num , GVfsJob *job);
 
 
 static void
@@ -851,40 +851,61 @@ static char *
 get_dev_path_and_device_from_host (GVfsJob *job,
                                    GUdevClient *gudev_client,
                                    const char *host,
+                                   uint32_t *bus_num,
+                                   uint32_t *dev_num,
                                    GUdevDevice **device)
 {
-  /* turn usb:001,041 string into an udev device name */
-  if (!g_str_has_prefix (host, "[usb:")) {
-    g_vfs_job_failed_literal (G_VFS_JOB (job), G_IO_ERROR,
-                              G_IO_ERROR_NOT_SUPPORTED,
-                              _("Unexpected host URI format."));
-    return NULL;
+  GList *devices, *l;
+  g_debug ("(II) get_dev_path_from_host: %s\n", host);
+
+  *device = NULL;
+
+  /* find corresponding GUdevDevice */
+  devices = g_udev_client_query_by_subsystem (gudev_client, "usb");
+  for (l = devices; l != NULL; l = l->next) {
+    const char *id = g_udev_device_get_property (l->data, "ID_SERIAL");
+    if (g_strcmp0 (id, host) == 0) {
+      *device = g_object_ref (l->data);
+      *bus_num = g_ascii_strtoull (g_udev_device_get_property (l->data, "BUSNUM"),
+                                   NULL, 10);
+      *dev_num = g_ascii_strtoull (g_udev_device_get_property (l->data, "DEVNUM"),
+                                   NULL, 10);
+      break;
+    }
   }
+  g_list_free_full (devices, g_object_unref);
 
-  char *comma;
-  char *dev_path = g_strconcat ("/dev/bus/usb/", host + 5, NULL);
-  if ((comma = strchr (dev_path, ',')) == NULL) {
-    g_free (dev_path);
-    g_vfs_job_failed_literal (G_VFS_JOB (job), G_IO_ERROR,
-                              G_IO_ERROR_NOT_SUPPORTED,
-                              _("Malformed host URI."));
-    return NULL;
+  if (*device) {
+    return g_strdup_printf ("/dev/bus/usb/%03u/%03u", *bus_num, *dev_num);
   }
-  *comma = '/';
-  dev_path[strlen (dev_path) -1] = '\0';
-  g_debug ("(II) get_dev_path_from_host: Parsed '%s' into device name %s\n", host, dev_path);
 
-  /* find corresponding GUdevDevice */
-  *device = g_udev_client_query_by_device_file (gudev_client, dev_path);
-  if (!*device) {
-    g_free (dev_path);
-    g_vfs_job_failed_literal (G_VFS_JOB (job),
-                              G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
-                              _("Couldn’t find matching udev device."));
-    return NULL;
+  /* For compatibility, handle old style host specifications. */
+  if (g_str_has_prefix (host, "[usb:")) {
+    /* Split [usb:001,002] into: '[usb', '001', '002', '' */
+    char **elements = g_strsplit_set (host, ":,]", -1);
+    if (g_strv_length (elements) == 4 && elements[3][0] == '\0') {
+      *bus_num = g_ascii_strtoull (elements[1], NULL, 10);
+      *dev_num = g_ascii_strtoull (elements[2], NULL, 10);
+
+      /* These values are non-zero, so zero means a parsing error. */
+      if (*bus_num != 0 && *dev_num != 0) {
+        char *dev_path = g_strdup_printf ("/dev/bus/usb/%s/%s",
+                                          elements[1], elements[2]);
+        *device = g_udev_client_query_by_device_file (gudev_client, dev_path);
+        if (*device) {
+          g_strfreev (elements);
+          return dev_path;
+        }
+        g_free (dev_path);
+      }
+    }
+    g_strfreev (elements);
   }
 
-  return dev_path;
+  g_vfs_job_failed_literal (G_VFS_JOB (job),
+                            G_IO_ERROR, G_IO_ERROR_NOT_FOUND,
+                            _("Couldn’t find matching udev device."));
+  return NULL;
 }
 
 static void
@@ -896,6 +917,7 @@ do_mount (GVfsBackend *backend,
 {
   GVfsBackendMtp *op_backend = G_VFS_BACKEND_MTP (backend);
   GUdevDevice *device;
+  uint32_t bus_num, dev_num;
 
   g_debug ("(I) do_mount\n");
 
@@ -918,6 +940,8 @@ do_mount (GVfsBackend *backend,
   char *dev_path = get_dev_path_and_device_from_host (G_VFS_JOB (job),
                                                       op_backend->gudev_client,
                                                       host,
+                                                      &bus_num,
+                                                      &dev_num,
                                                       &device);
   if (dev_path == NULL) {
     g_object_unref (op_backend->gudev_client);
@@ -933,7 +957,7 @@ do_mount (GVfsBackend *backend,
 
   LIBMTP_Init ();
 
-  get_device (backend, host, G_VFS_JOB (job));
+  get_device (backend, bus_num, dev_num, G_VFS_JOB (job));
   if (!G_VFS_JOB (job)->failed) {
     g_signal_connect (op_backend->gudev_client, "uevent", G_CALLBACK (on_uevent), op_backend);
 
@@ -1041,8 +1065,9 @@ do_unmount (GVfsBackend *backend, GVfsJobUnmount *job,
  * Called with backend mutex lock held.
  */
 LIBMTP_mtpdevice_t *
-get_device (GVfsBackend *backend, const char *id, GVfsJob *job) {
-  g_debug ("(II) get_device: %s\n", id);
+get_device (GVfsBackend *backend, uint32_t bus_num, uint32_t dev_num,
+            GVfsJob *job) {
+  g_debug ("(II) get_device: %u,%u\n", bus_num, dev_num);
 
   LIBMTP_mtpdevice_t *device = NULL;
 
@@ -1085,30 +1110,23 @@ get_device (GVfsBackend *backend, const char *id, GVfsJob *job) {
   /* Iterate over connected MTP devices */
   int i;
   for (i = 0; i < numrawdevices; i++) {
-    char *name;
-    name = g_strdup_printf ("[usb:%03u,%03u]",
-                            rawdevices[i].bus_location,
-                            rawdevices[i].devnum);
-
-    if (strcmp (id, name) == 0) {
+    if (rawdevices[i].bus_location == bus_num &&
+        rawdevices[i].devnum == dev_num) {
       device = LIBMTP_Open_Raw_Device_Uncached (&rawdevices[i]);
       if (device == NULL) {
         g_vfs_job_failed (G_VFS_JOB (job),
                           G_IO_ERROR, G_IO_ERROR_FAILED,
-                          _("Unable to open MTP device “%s”"), name);
-        g_free (name);
+                          _("Unable to open MTP device “%03u,%03u”"),
+                          bus_num, dev_num);
         goto exit;
       }
 
-      g_debug ("(II) get_device: Storing device %s\n", name);
+      g_debug ("(II) get_device: Storing device %03u,%03u\n", bus_num, dev_num);
       G_VFS_BACKEND_MTP (backend)->device = device;
 
       LIBMTP_Dump_Errorstack (device);
       LIBMTP_Clear_Errorstack (device);
-      g_free (name);
       break;
-    } else {
-      g_free (name);
     }
   }
 
diff --git a/monitor/mtp/gmtpvolumemonitor.c b/monitor/mtp/gmtpvolumemonitor.c
index 894bcd5..d8055f7 100644
--- a/monitor/mtp/gmtpvolumemonitor.c
+++ b/monitor/mtp/gmtpvolumemonitor.c
@@ -136,10 +136,12 @@ get_mount_for_uuid (GVolumeMonitor *volume_monitor, const char *uuid)
 static void
 gudev_add_device (GMtpVolumeMonitor *monitor, GUdevDevice *device, gboolean do_emit)
 {
+  GList *l;
   GMtpVolume *volume;
-  const char *usb_bus_num, *usb_device_num, *device_path;
+  const char *usb_serial_id, *device_path;
   char *uri;
   GFile *activation_mount_root;
+  gboolean serial_conflict = FALSE;
 
   device_path = g_udev_device_get_device_file (device);
   if (!device_path) {
@@ -148,26 +150,45 @@ gudev_add_device (GMtpVolumeMonitor *monitor, GUdevDevice *device, gboolean do_e
     return;
   }
 
-  usb_bus_num = g_udev_device_get_property (device, "BUSNUM");
-  if (usb_bus_num == NULL) {
-    g_warning ("device %s has no BUSNUM property, ignoring", device_path);
-    return;
-  }
-
-  usb_device_num = g_udev_device_get_property (device, "DEVNUM");
-  if (usb_device_num == NULL) {
-    g_warning ("device %s has no DEVNUM property, ignoring", device_path);
+  /*
+   * We do not use ID_SERIAL_SHORT (the actualy device serial value) as
+   * this field is not populated when an ID_SERIAL has to be synthesized.
+   */
+  usb_serial_id = g_udev_device_get_property (device, "ID_SERIAL");
+  if (usb_serial_id == NULL) {
+    g_warning ("device %s has no ID_SERIAL property, ignoring", device_path);
     return;
   }
 
-  g_debug ("gudev_add_device: device %s (bus: %s, device: %s)",
-           g_udev_device_get_device_file (device),
-           usb_bus_num, usb_device_num);
-
-  uri = g_strdup_printf ("mtp://[usb:%s,%s]", usb_bus_num, usb_device_num);
+  uri = g_strdup_printf ("mtp://%s", usb_serial_id);
   activation_mount_root = g_file_new_for_uri (uri);
   g_free (uri);
 
+  /*
+   * We do not support plugging in multiple devices that lack proper serial
+   * numbers. Linux will attempt to synthesize an ID based on the device
+   * product information, which will avoid collisions between different
+   * types of device, but two identical, serial-less, devices will still
+   * conflict.
+   */
+  for (l = monitor->device_volumes; l != NULL; l = l->next) {
+    GMtpVolume *volume = G_MTP_VOLUME (l->data);
+
+    GFile *existing_root = g_volume_get_activation_root (G_VOLUME (volume));
+    if (g_file_equal (activation_mount_root, existing_root)) {
+      serial_conflict = TRUE;
+    }
+    g_object_unref (existing_root);
+
+    if (serial_conflict) {
+      g_warning ("device %s has an identical ID_SERIAL value to an "
+                 "existing device. Multiple devices are not supported.",
+                 g_udev_device_get_device_file (device));
+      g_object_unref (activation_mount_root);
+      return;
+    }
+  }
+
   volume = g_mtp_volume_new (G_VOLUME_MONITOR (monitor),
                              device,
                              monitor->gudev_client,