Libguestfs - Jan 2016 - Re: [PATCH] p2v: User can click on an interface name to identify the physical interface.

When the user clicks on the second column of the list of network
interfaces, run 'ethtool --identify <if_name> 10', which (on
supported
cards) flashes a light on the physical interface for 10 seconds,
allowing it to be identified by the operator.

This has a nasty bit of hidden UI, basically because Gtk makes it near
impossible to put a regular button into a GtkTreeView.
---
 p2v/gui.c        | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 p2v/virt-p2v.pod |  4 ++++
 2 files changed, 71 insertions(+)

diff --git a/p2v/gui.c b/p2v/gui.c
index 92a7bc4..ed23753 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -414,6 +414,7 @@ static void populate_removable (GtkTreeView
*removable_list);
 static void populate_interfaces (GtkTreeView *interfaces_list);
 static void toggled (GtkCellRendererToggle *cell, gchar *path_str, gpointer
data);
 static void network_edited_callback (GtkCellRendererToggle *cell, gchar
*path_str, gchar *new_text, gpointer data);
+static gboolean maybe_identify_click (GtkWidget *interfaces_list,
GdkEventButton *event, gpointer data);
 static void set_disks_from_ui (struct config *);
 static void set_removable_from_ui (struct config *);
 static void set_interfaces_from_ui (struct config *);
@@ -649,6 +650,10 @@ create_conversion_dialog (struct config *config)
   gtk_scrolled_window_set_policy (GTK_SCROLLED_WINDOW (interfaces_sw),
                                   GTK_POLICY_NEVER, GTK_POLICY_AUTOMATIC);
   interfaces_list = gtk_tree_view_new ();
+  /* See maybe_identify_click below for what we're doing. */
+  g_signal_connect (interfaces_list, "button-press-event",
+                    G_CALLBACK (maybe_identify_click), NULL);
+  gtk_widget_set_tooltip_markup (interfaces_list, _("Left click on an
interface name to flash the light on the physical interface."));
   populate_interfaces (GTK_TREE_VIEW (interfaces_list));
   gtk_scrolled_window_add_with_viewport (GTK_SCROLLED_WINDOW (interfaces_sw),
                                          interfaces_list);
@@ -1022,6 +1027,68 @@ network_edited_callback (GtkCellRendererToggle *cell,
gchar *path_str,
   gtk_tree_path_free (path);
 }
 
+/* When the user clicks on the interface name on the list of
+ * interfaces, we want to run 'ethtool --identify', which usually
+ * makes some lights flash on the physical interface.  We cannot catch
+ * clicks on the cell itself, so we have to go via a more obscure
+ * route.  See http://stackoverflow.com/a/27207433 and
+ * https://en.wikibooks.org/wiki/GTK%2B_By_Example/Tree_View/Events
+ */
+static gboolean
+maybe_identify_click (GtkWidget *interfaces_list, GdkEventButton *event,
+                      gpointer data)
+{
+  gboolean ret = FALSE;         /* Did we handle this event? */
+
+  /* Single left click only. */
+  if (event->type == GDK_BUTTON_PRESS && event->button == 1) {
+    GtkTreePath *path;
+    GtkTreeViewColumn *column;
+
+    if (gtk_tree_view_get_path_at_pos (GTK_TREE_VIEW (interfaces_list),
+                                       event->x, event->y,
+                                       &path, &column, NULL, NULL)) {
+      GList *cols;
+      gint column_index;
+
+      /* Get column index. */
+      cols = gtk_tree_view_get_columns (GTK_TREE_VIEW (interfaces_list));
+      column_index = g_list_index (cols, (gpointer) column);
+      g_list_free (cols);
+
+      if (column_index == INTERFACES_COL_DEVICE) {
+        const gint *indices;
+        gint row_index;
+        const char *if_name;
+        char *cmd;
+
+        /* Get the row index. */
+        indices = gtk_tree_path_get_indices (path);
+        row_index = indices[0];
+
+        /* And the interface name. */
+        if_name = all_interfaces[row_index];
+
+        /* Issue the ethtool command in the background. */
+        if (asprintf (&cmd, "ethtool --identify '%s' 10
&", if_name) == -1) {
+          perror ("asprintf");
+          exit (EXIT_FAILURE);
+        }
+        printf ("%s\n", cmd);
+        ignore_value (system (cmd));
+
+        free (cmd);
+
+        ret = TRUE;             /* We handled this event. */
+      }
+
+      gtk_tree_path_free (path);
+    }
+  }
+
+  return ret;
+}
+
 static void
 set_from_ui_generic (char **all, char ***ret, GtkTreeView *list)
 {
diff --git a/p2v/virt-p2v.pod b/p2v/virt-p2v.pod
index 6193ca8..b4598d9 100644
--- a/p2v/virt-p2v.pod
+++ b/p2v/virt-p2v.pod
@@ -232,6 +232,10 @@ should be created in the guest after conversion.  You can
also connect
 these to target hypervisor networks (for further information about
 this feature, see L<virt-v2v(1)/NETWORKS AND BRIDGES>).
 
+On supported hardware, left-clicking on the device name (eg. C<em1>)
+causes a light to start flashing on the physical interface, allowing
+the interface to be identified by the operator.
+
 When you are ready to begin the conversion, press the
 C<Start conversion> button:
 
-- 
2.5.0

On Tuesday 26 January 2016 15:38:07 Richard W.M. Jones
wrote:
> When the user clicks on the second column of the list of network
> interfaces, run 'ethtool --identify <if_name> 10', which (on
supported
> cards) flashes a light on the physical interface for 10 seconds,
> allowing it to be identified by the operator.
> 
> This has a nasty bit of hidden UI, basically because Gtk makes it near
> impossible to put a regular button into a GtkTreeView.
> ---
Wouldn't it better than add a simple "Identify interface" next to
the
list? IMHO would be more explicit, and avoid spawning ethtool commands
when not wanted.

-- 
Pino Toscano

On Tue, Jan 26, 2016 at 06:14:18PM +0100, Pino Toscano
wrote:
> On Tuesday 26 January 2016 15:38:07 Richard W.M. Jones wrote:
> > When the user clicks on the second column of the list of network
> > interfaces, run 'ethtool --identify <if_name> 10', which
(on supported
> > cards) flashes a light on the physical interface for 10 seconds,
> > allowing it to be identified by the operator.
> > 
> > This has a nasty bit of hidden UI, basically because Gtk makes it near
> > impossible to put a regular button into a GtkTreeView.
> > ---
> 
> Wouldn't it better than add a simple "Identify interface"
next to the
> list? IMHO would be more explicit, and avoid spawning ethtool commands
> when not wanted.
I think do you mean another column with a label [not a button,
unfortunately] marked "identify interface"?  That is possible.  Also
we could add that text to the Device column.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v