Tweak MDRaid:ActiveDevices some more
authorDavid Zeuthen <zeuthen@gmail.com>
Fri, 14 Sep 2012 17:04:51 +0000 (13:04 -0400)
committerDavid Zeuthen <zeuthen@gmail.com>
Fri, 14 Sep 2012 17:04:51 +0000 (13:04 -0400)
We need to iterate over md/dev-*, not just md/rd%d, otherwise we miss
spares and failed devices. With this change, Disks can show the user
what really is going on

http://people.freedesktop.org/~david/gnome-disks-mdraid-20120914-1.png

Signed-off-by: David Zeuthen <zeuthen@gmail.com>
data/org.freedesktop.UDisks2.xml
src/udiskslinuxmdraid.c

index b427d35..941d90c 100644 (file)
     <property name="Degraded" type="u" access="read"/>
 
     <!-- ActiveDevices:
-         This property is an array with #org.freedesktop.UDisks2.MDRaid:NumDevices elements if
-         the array is running, zero elements otherwise. Each element of the array is a struct
-         with the following members:
+         This property is an array with block devices that are
+         currently associated with the with the array. It is empty if
+         the array is not running.
+
+         Each element of the array is a struct with the following members:
          <variablelist>
          <varlistentry><term>block (type 'o')</term>
-         <listitem><para>If there is no member in the given position this is set to '/', otherwise it is the object path for the underlying block device (guaranteed to implement the #org.freedesktop.UDisks2.Block interface)</para></listitem></varlistentry>
+         <listitem><para>The object path for the underlying block device (guaranteed to implement the #org.freedesktop.UDisks2.Block interface)</para></listitem></varlistentry>
+         <varlistentry><term>slot (type 'i')</term>
+         <listitem><para>-1 if the device is not currently part of the array (ie. <literal>spare</literal> or <literal>faulty</literal>), otherwise the slot number the device currently fills (between 0 and #org.freedesktop.UDisks2.MDRaid:NumDevices)</para></listitem></varlistentry>
          <varlistentry><term>state (type 's')</term>
-         <listitem><para>Blank if there is no member in the position, otherwise the state of the member, e.g. one of <literal>faulty</literal>, <literal>in_sync</literal>, <literal>writemostly</literal>, <literal>blocked</literal> and <literal>spare</literal></para></listitem></varlistentry>
+         <listitem><para>The state of the device - known values include <literal>faulty</literal>, <literal>in_sync</literal>, <literal>writemostly</literal>, <literal>blocked</literal> and <literal>spare</literal></para></listitem></varlistentry>
          <varlistentry><term>num_read_errors (type 't')</term>
-         <listitem><para>Zero if there is no member in the position in question, otherwise an ongoing count of read errors that have been detected on this device but have not caused the device to be evicted from the array</para></listitem></varlistentry>
+         <listitem><para>An ongoing count of read errors that have been detected on this device but have not caused the device to be evicted from the array</para></listitem></varlistentry>
          <varlistentry><term>expansion (type 'a{sv}')</term>
          <listitem><para>Currently unused. Intended for future expansion.</para></listitem></varlistentry>
          </variablelist>
          This property correspond to the
-         <literal>rdNN</literal> symlinks in sysfs and the sysfs files in each directory.
+         <filename>/sys/block/mdN/md/dev-*</filename> directories in sysfs and the sysfs files in each directory.
          See the
          <filename><ulink url="http://www.kernel.org/doc/Documentation/md.txt">Documentation/md.txt</ulink></filename>
          file shipped with the kernel sources.
     -->
-    <property name="ActiveDevices" type="a(osta{sv})" access="read"/>
+    <property name="ActiveDevices" type="a(oista{sv})" access="read"/>
 
     <!--
         Start:
index 155857b..b0ea4d6 100644 (file)
@@ -227,6 +227,27 @@ ensure_polling (UDisksLinuxMDRaid  *mdraid,
     }
 }
 
+static gint
+member_cmpfunc (GVariant **a,
+                GVariant **b)
+{
+  gint slot_a;
+  gint slot_b;
+  const gchar *objpath_a;
+  const gchar *objpath_b;
+
+  g_return_val_if_fail (a != NULL, 0);
+  g_return_val_if_fail (b != NULL, 0);
+
+  g_variant_get (*a, "(&oista{sv})", &objpath_a, &slot_a, NULL, NULL, NULL);
+  g_variant_get (*b, "(&oista{sv})", &objpath_b, &slot_b, NULL, NULL, NULL);
+
+  if (slot_a == slot_b)
+    return g_strcmp0 (objpath_a, objpath_b);
+
+  return slot_a - slot_b;
+}
+
 
 /**
  * udisks_linux_mdraid_update:
@@ -337,59 +358,98 @@ udisks_linux_mdraid_update (UDisksLinuxMDRaid       *mdraid,
     }
 
   /* figure out active devices */
-  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(osta{sv})"));
+  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(oista{sv})"));
   if (raid_device != NULL)
     {
-      const gchar *raid_sysfs_path = g_udev_device_get_sysfs_path (raid_device);
-      gchar p[256];
+      gchar *md_dir_name = NULL;
+      GDir *md_dir;
+      GPtrArray *p;
       guint n;
 
-      for (n = 0; n < num_devices; n++)
+      /* First build an array of variants, then sort it, then build
+       *
+       * Why sort it? Because directory traversal does not preserve
+       * the order and we want the same order every time to avoid
+       * spurious property changes on MDRaid:ActiveDevices
+       */
+      p = g_ptr_array_new ();
+      md_dir_name = g_strdup_printf ("%s/md", g_udev_device_get_sysfs_path (raid_device));
+      md_dir = g_dir_open (md_dir_name, 0, NULL);
+      if (md_dir != NULL)
         {
-          gchar *block_sysfs_path = NULL;
-          UDisksObject *member_object = NULL;
-          gchar *member_state = NULL;
-          guint64 member_errors = 0;
-
-          snprintf (p, sizeof (p), "md/rd%d/block", n);
-          block_sysfs_path = udisks_daemon_util_resolve_link (raid_sysfs_path, p);
-          if (block_sysfs_path == NULL)
+          gchar buf[256];
+          const gchar *name;
+          while ((name = g_dir_read_name (md_dir)) != NULL)
             {
-              udisks_warning ("Unable to resolve %s/%s symlink",
-                              raid_sysfs_path, p);
-              g_variant_builder_add (&builder, "(osta{sv})",
-                                     "/", "", 0, NULL);
-              goto member_done;
-            }
-
-          member_object = udisks_daemon_find_block_by_sysfs_path (daemon, block_sysfs_path);
-          if (member_object == NULL)
-            {
-              /* TODO: only warn on !coldplug */
-              /* udisks_warning ("No object for block device with sysfs path %s", block_sysfs_path); */
-              g_variant_builder_add (&builder, "(osta{sv})",
-                                     "/", "", 0, NULL);
-              goto member_done;
-            }
-
-          snprintf (p, sizeof (p), "md/rd%d/state", n);
-          member_state = read_sysfs_attr (raid_device, p);
-          if (member_state != NULL)
-            g_strstrip (member_state);
-
-          snprintf (p, sizeof (p), "md/rd%d/errors", n);
-          member_errors = read_sysfs_attr_as_uint64 (raid_device, p);
-
-          g_variant_builder_add (&builder, "(osta{sv})",
-                                 g_dbus_object_get_object_path (G_DBUS_OBJECT (member_object)),
-                                 member_state,
-                                 member_errors,
-                                 NULL); /* expansion, unused for now */
-
-        member_done:
-          g_clear_object (&member_object);
-          g_free (block_sysfs_path);
+              gchar *block_sysfs_path = NULL;
+              UDisksObject *member_object = NULL;
+              gchar *member_state = NULL;
+              gchar *member_slot = NULL;
+              gint member_slot_as_int = -1;
+              guint64 member_errors = 0;
+
+              if (!g_str_has_prefix (name, "dev-"))
+                goto member_done;
+
+              snprintf (buf, sizeof (buf), "%s/block", name);
+              block_sysfs_path = udisks_daemon_util_resolve_link (md_dir_name, buf);
+              if (block_sysfs_path == NULL)
+                {
+                  udisks_warning ("Unable to resolve %s/%s symlink", md_dir_name, buf);
+                  goto member_done;
+                }
+
+              member_object = udisks_daemon_find_block_by_sysfs_path (daemon, block_sysfs_path);
+              if (member_object == NULL)
+                {
+                  /* TODO: only warn on !coldplug */
+                  /* udisks_warning ("No object for block device with sysfs path %s", block_sysfs_path); */
+                  goto member_done;
+                }
+
+              snprintf (buf, sizeof (buf), "md/%s/state", name);
+              member_state = read_sysfs_attr (raid_device, buf);
+              if (member_state != NULL)
+                g_strstrip (member_state);
+
+              snprintf (buf, sizeof (buf), "md/%s/slot", name);
+              member_slot = read_sysfs_attr (raid_device, buf);
+              if (member_slot != NULL)
+                g_strstrip (member_slot);
+              if (g_strcmp0 (member_slot, "none") != 0)
+                member_slot_as_int = atoi (member_slot);
+
+              snprintf (buf, sizeof (buf), "md/%s/errors", name);
+              member_errors = read_sysfs_attr_as_uint64 (raid_device, buf);
+
+              g_ptr_array_add (p,
+                               g_variant_new ("(oista{sv})",
+                                              g_dbus_object_get_object_path (G_DBUS_OBJECT (member_object)),
+                                              member_slot_as_int,
+                                              member_state,
+                                              member_errors,
+                                              NULL)); /* expansion, unused for now */
+
+            member_done:
+              g_free (member_slot);
+              g_free (member_state);
+              g_clear_object (&member_object);
+              g_free (block_sysfs_path);
+
+            } /* for all dev- directories */
+
+          /* ... and sort */
+          g_ptr_array_sort (p, (GCompareFunc) member_cmpfunc);
+
+          /* ... and finally build (builder consumes each GVariant instance) */
+          for (n = 0; n < p->len; n++)
+            g_variant_builder_add_value (&builder, p->pdata[n]);
+          g_ptr_array_free (p, TRUE);
+
+          g_dir_close (md_dir);
         }
+      g_free (md_dir_name);
+
     }
   udisks_mdraid_set_active_devices (iface, g_variant_builder_end (&builder));