From a9d9b7d3cf9fa9928498273974830e9ba2002e5f Mon Sep 17 00:00:00 2001

From: "Richard W.M. Jones" <rjones@redhat.com>

Date: Fri, 24 Jun 2016 14:45:05 +0100

Subject: [PATCH] p2v: Refactor into get_blockdev_size and get_blockdev_model

 functions.

This is just refactoring, but it reveals and fixes a bug too.  The

size_gb field was left as NULL when the --test-disk option was used.

Apparently passing NULL to gtk_list_store_set is fine, but just in

case I replaced it with "".

(cherry picked from commit 68ff3ffd1de7b72255ee5098c0bfef90e6236cb5)

---

 p2v/gui.c   | 40 ++++++++-----------------------------

 p2v/p2v.h   |  2 ++

 p2v/utils.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

 3 files changed, 75 insertions(+), 32 deletions(-)

diff --git a/p2v/gui.c b/p2v/gui.c

index 7414037..1bae4d8 100644

--- a/p2v/gui.c

+++ b/p2v/gui.c

@@ -869,47 +869,23 @@ populate_disks (GtkTreeView *disks_list)

                                     G_TYPE_STRING, G_TYPE_STRING);

   if (all_disks != NULL) {

     for (i = 0; all_disks[i] != NULL; ++i) {

-      CLEANUP_FREE char *size_filename = NULL;

-      CLEANUP_FREE char *model_filename = NULL;

-      CLEANUP_FREE char *size_str = NULL;

+      uint64_t size;

       CLEANUP_FREE char *size_gb = NULL;

       CLEANUP_FREE char *model = NULL;

-      uint64_t size;

-      if (asprintf (&size_filename, "/sys/block/%s/size",

-                    all_disks[i]) == -1) {

-        perror ("asprintf");

-        exit (EXIT_FAILURE);

-      }

-      if (g_file_get_contents (size_filename, &size_str, NULL, NULL) &&

-          sscanf (size_str, "%" SCNu64, &size) == 1) {

-        size /= 2*1024*1024; /* size from kernel is given in sectors? */

-        if (asprintf (&size_gb, "%" PRIu64, size) == -1) {

-          perror ("asprintf");

-          exit (EXIT_FAILURE);

-        }

-      }

-

-      if (asprintf (&model_filename, "/sys/block/%s/device/model",

-                    all_disks[i]) == -1) {

-        perror ("asprintf");

-        exit (EXIT_FAILURE);

-      }

-      if (g_file_get_contents (model_filename, &model, NULL, NULL)) {

-        /* Need to chomp trailing \n from the content. */

-        size_t len = strlen (model);

-        if (len > 0 && model[len-1] == '\n')

-          model[len-1] = '\0';

-      } else {

-        model = strdup ("");

+      if (all_disks[i][0] != '/') { /* not using --test-disk */

+        size = get_blockdev_size (all_disks[i]);

+        if (asprintf (&size_gb, "%" PRIu64, size) == -1)

+          error (EXIT_FAILURE, errno, "asprintf");

+        model = get_blockdev_model (all_disks[i]);

       }

       gtk_list_store_append (disks_store, &iter);

       gtk_list_store_set (disks_store, &iter,

                           DISKS_COL_CONVERT, TRUE,

                           DISKS_COL_DEVICE, all_disks[i],

-                          DISKS_COL_SIZE, size_gb,

-                          DISKS_COL_MODEL, model,

+                          DISKS_COL_SIZE, size_gb ? size_gb : "",

+                          DISKS_COL_MODEL, model ? model : "",

                           -1);

     }

   }

diff --git a/p2v/p2v.h b/p2v/p2v.h

index 49c97b6..40b05b8 100644

--- a/p2v/p2v.h

+++ b/p2v/p2v.h

@@ -131,6 +131,8 @@ extern mexp_h *start_remote_connection (struct config *, const char *remote_dir,

 extern const char *get_ssh_error (void);

 /* utils.c */

+extern uint64_t get_blockdev_size (const char *dev);

+extern char *get_blockdev_model (const char *dev);

 extern char *get_if_addr (const char *if_name);

 extern char *get_if_vendor (const char *if_name, int truncate);

 extern void wait_network_online (const struct config *);

diff --git a/p2v/utils.c b/p2v/utils.c

index 3781a8d..18c2b7c 100644

--- a/p2v/utils.c

+++ b/p2v/utils.c

@@ -20,9 +20,12 @@

 #include <stdio.h>

 #include <stdlib.h>

+#include <inttypes.h>

 #include <string.h>

 #include <ctype.h>

 #include <unistd.h>

+#include <errno.h>

+#include <error.h>

 #include <locale.h>

 #include <libintl.h>

@@ -38,6 +41,68 @@

     }                                           \

   } while (0)

+/**

+ * Return size of a block device, from F</sys/block/I<dev>/size>.

+ *

+ * This function always succeeds, or else exits (since we expect

+ * C<dev> to always be valid and the C<size> file to always exist).

+ */

+uint64_t

+get_blockdev_size (const char *dev)

+{

+  CLEANUP_FCLOSE FILE *fp = NULL;

+  CLEANUP_FREE char *path = NULL;

+  CLEANUP_FREE char *size_str = NULL;

+  size_t len;

+  uint64_t size;

+

+  if (asprintf (&path, "/sys/block/%s/size", dev) == -1)

+    error (EXIT_FAILURE, errno, "asprintf");

+

+  fp = fopen (path, "r");

+  if (fp == NULL)

+    error (EXIT_FAILURE, errno, "fopen: %s", path);

+  if (getline (&size_str, &len, fp) == -1)

+    error (EXIT_FAILURE, errno, "getline: %s", path);

+

+  if (sscanf (size_str, "%" SCNu64, &size) != 1)

+    error (EXIT_FAILURE, 0, "cannot parse %s: %s", path, size_str);

+

+  size /= 2*1024*1024;     /* size from kernel is given in sectors? */

+  return size;

+}

+

+/**

+ * Return model of a block device, from F</sys/block/I<dev>/device/model>.

+ *

+ * Returns C<NULL> if the file was not found.  The caller must

+ * free the returned string.

+ */

+char *

+get_blockdev_model (const char *dev)

+{

+  CLEANUP_FCLOSE FILE *fp = NULL;

+  CLEANUP_FREE char *path = NULL;

+  char *model = NULL;

+  size_t len = 0;

+  ssize_t n;

+

+  if (asprintf (&path, "/sys/block/%s/device/model", dev) == -1)

+    error (EXIT_FAILURE, errno, "asprintf");

+  fp = fopen (path, "r");

+  if (fp == NULL) {

+    perror (path);

+    return NULL;

+  }

+  if ((n = getline (&model, &len, fp)) == -1) {

+    perror (path);

+    free (model);

+    return NULL;

+  }

+  CHOMP (model, n);

+  return model;

+}

+

 /* Return contents of /sys/class/net/<if_name>/address (if found). */

 char *

 get_if_addr (const char *if_name)

-- 

1.8.3.1