Libguestfs - May 2015 - Re: [PATCH 1/2] p2v: Factor out code for parsing vcpus & memory from conversion dialog.

https://bugzilla.redhat.com/show_bug.cgi?id=823758

No functional change, just code refactoring.
---
 p2v/gui.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/p2v/gui.c b/p2v/gui.c
index 7e9de0d..bc762c2 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -375,6 +375,8 @@ static void set_interfaces_from_ui (struct config *);
 static void conversion_back_clicked (GtkWidget *w, gpointer data);
 static void start_conversion_clicked (GtkWidget *w, gpointer data);
 static void notify_ui_callback (int type, const char *data);
+static int get_vcpus_from_conv_dlg (void);
+static uint64_t get_memory_from_conv_dlg (void);
 
 enum {
   DISKS_COL_CONVERT = 0,
@@ -1051,6 +1053,32 @@ conversion_back_clicked (GtkWidget *w, gpointer data)
   gtk_widget_set_sensitive (next_button, FALSE);
 }
 
+static int
+get_vcpus_from_conv_dlg (void)
+{
+  const char *str;
+  int i;
+
+  str = gtk_entry_get_text (GTK_ENTRY (vcpus_entry));
+  if (sscanf (str, "%d", &i) == 1 && i > 0)
+    return i;
+  else
+    return 1;
+}
+
+static uint64_t
+get_memory_from_conv_dlg (void)
+{
+  const char *str;
+  uint64_t i;
+
+  str = gtk_entry_get_text (GTK_ENTRY (memory_entry));
+  if (sscanf (str, "%" SCNu64, &i) == 1 && i >= 256)
+    return i * 1024 * 1024;
+  else
+    return UINT64_C (1024) * 1024 * 1024;
+}
+
 /*----------------------------------------------------------------------*/
 /* Running dialog. */
 
@@ -1174,7 +1202,6 @@ static void
 start_conversion_clicked (GtkWidget *w, gpointer data)
 {
   struct config *config = data;
-  int i;
   const char *str;
   char *str2;
   GtkWidget *dlg;
@@ -1200,17 +1227,8 @@ start_conversion_clicked (GtkWidget *w, gpointer data)
     return;
   }
 
-  str = gtk_entry_get_text (GTK_ENTRY (vcpus_entry));
-  if (sscanf (str, "%d", &i) == 1 && i > 0)
-    config->vcpus = i;
-  else
-    config->vcpus = 1;
-
-  str = gtk_entry_get_text (GTK_ENTRY (memory_entry));
-  if (sscanf (str, "%d", &i) == 1 && i >= 256)
-    config->memory = (uint64_t) i * 1024 * 1024;
-  else
-    config->memory = 1024 * 1024 * 1024;
+  config->vcpus = get_vcpus_from_conv_dlg ();
+  config->memory = get_memory_from_conv_dlg ();
 
   config->verbose      gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON
(debug_button));
-- 
2.3.1

Display a warning if the number of vCPUs or memory selected in the GUI
would be larger than the limits currently supported on RHEL 6 and RHEL 7.

This is just a warning -- the user is free to ignore it and continue
anyway.

This warning does not apply if the selections were made through the
kernel command line, because there is no place to display the warning
for automated conversions.
---
 p2v/gui.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 113 insertions(+), 2 deletions(-)

diff --git a/p2v/gui.c b/p2v/gui.c
index bc762c2..391d3f0 100644
--- a/p2v/gui.c
+++ b/p2v/gui.c
@@ -20,6 +20,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdarg.h>
 #include <string.h>
 #include <inttypes.h>
 #include <unistd.h>
@@ -58,6 +59,7 @@ static GtkWidget *conn_dlg,
 /* The conversion dialog. */
 static GtkWidget *conv_dlg,
   *guestname_entry, *vcpus_entry, *memory_entry,
+  *vcpus_warning, *memory_warning, *target_warning_label,
   *o_combo, *oc_entry, *os_entry, *of_entry, *oa_combo,
   *info_label,
   *debug_button,
@@ -374,6 +376,7 @@ static void set_removable_from_ui (struct config *);
 static void set_interfaces_from_ui (struct config *);
 static void conversion_back_clicked (GtkWidget *w, gpointer data);
 static void start_conversion_clicked (GtkWidget *w, gpointer data);
+static void vcpus_or_memory_check_callback (GtkWidget *w, gpointer data);
 static void notify_ui_callback (int type, const char *data);
 static int get_vcpus_from_conv_dlg (void);
 static uint64_t get_memory_from_conv_dlg (void);
@@ -421,7 +424,7 @@ create_conversion_dialog (struct config *config)
   /* XXX It would be nice not to have to set this explicitly, but
    * if we don't then Gtk chooses a very small window.
    */
-  gtk_widget_set_size_request (conv_dlg, 800, 500);
+  gtk_widget_set_size_request (conv_dlg, 800, 560);
 
   /* The main dialog area. */
   hbox = gtk_hbox_new (TRUE, 1);
@@ -434,7 +437,7 @@ create_conversion_dialog (struct config *config)
 
   target_vbox = gtk_vbox_new (FALSE, 1);
 
-  target_tbl = gtk_table_new (3, 2, FALSE);
+  target_tbl = gtk_table_new (3, 3, FALSE);
   guestname_label = gtk_label_new (_("Name:"));
   gtk_misc_set_alignment (GTK_MISC (guestname_label), 1., 0.5);
   gtk_table_attach (GTK_TABLE (target_tbl), guestname_label,
@@ -454,6 +457,10 @@ create_conversion_dialog (struct config *config)
   gtk_entry_set_text (GTK_ENTRY (vcpus_entry), vcpus_str);
   gtk_table_attach (GTK_TABLE (target_tbl), vcpus_entry,
                     1, 2, 1, 2, GTK_FILL, GTK_FILL, 1, 1);
+  vcpus_warning = gtk_image_new_from_stock (GTK_STOCK_DIALOG_WARNING,
+                                            GTK_ICON_SIZE_BUTTON);
+  gtk_table_attach (GTK_TABLE (target_tbl), vcpus_warning,
+                    2, 3, 1, 2, 0, 0, 1, 1);
 
   memory_label = gtk_label_new (_("Memory (MB):"));
   gtk_misc_set_alignment (GTK_MISC (memory_label), 1., 0.5);
@@ -465,8 +472,20 @@ create_conversion_dialog (struct config *config)
   gtk_entry_set_text (GTK_ENTRY (memory_entry), memory_str);
   gtk_table_attach (GTK_TABLE (target_tbl), memory_entry,
                     1, 2, 2, 3, GTK_FILL, GTK_FILL, 1, 1);
+  memory_warning = gtk_image_new_from_stock (GTK_STOCK_DIALOG_WARNING,
+                                             GTK_ICON_SIZE_BUTTON);
+  gtk_table_attach (GTK_TABLE (target_tbl), memory_warning,
+                    2, 3, 2, 3, 0, 0, 1, 1);
 
   gtk_box_pack_start (GTK_BOX (target_vbox), target_tbl, TRUE, TRUE, 0);
+
+  target_warning_label = gtk_label_new ("");
+  gtk_label_set_line_wrap (GTK_LABEL (target_warning_label), TRUE);
+  gtk_label_set_line_wrap_mode (GTK_LABEL (target_warning_label),
+                                GTK_WRAP_WORD);
+  gtk_widget_set_size_request (target_warning_label, -1, 7 * 16);
+  gtk_box_pack_end (GTK_BOX (target_vbox), target_warning_label, TRUE, TRUE,
0);
+
   gtk_container_add (GTK_CONTAINER (target_frame), target_vbox);
 
   output_frame = gtk_frame_new (_("Virt-v2v output options"));
@@ -619,6 +638,10 @@ create_conversion_dialog (struct config *config)
                     G_CALLBACK (conversion_back_clicked), NULL);
   g_signal_connect (G_OBJECT (start_button), "clicked",
                     G_CALLBACK (start_conversion_clicked), config);
+  g_signal_connect (G_OBJECT (vcpus_entry), "changed",
+                    G_CALLBACK (vcpus_or_memory_check_callback), NULL);
+  g_signal_connect (G_OBJECT (memory_entry), "changed",
+                    G_CALLBACK (vcpus_or_memory_check_callback), NULL);
 }
 
 static void
@@ -630,6 +653,8 @@ show_conversion_dialog (void)
 
   /* Show the conversion dialog. */
   gtk_widget_show_all (conv_dlg);
+  gtk_widget_hide (vcpus_warning);
+  gtk_widget_hide (memory_warning);
 
   /* output_drivers may have been updated, so repopulate o_combo. */
   repopulate_output_combo (NULL);
@@ -1053,6 +1078,92 @@ conversion_back_clicked (GtkWidget *w, gpointer data)
   gtk_widget_set_sensitive (next_button, FALSE);
 }
 
+/* Display a warning if the vCPUs or memory is outside the supported
+ * range.  (RHBZ#823758).  See also:
+ * https://access.redhat.com/articles/rhel-kvm-limits
+ */
+#define MAX_SUPPORTED_VCPUS 160
+#define MAX_SUPPORTED_MEMORY_MB (UINT64_C (4000 * 1024))
+
+static char *concat_warning (char *warning, const char *fs, ...)
+  __attribute__((format (printf,2,3)));
+
+static char *
+concat_warning (char *warning, const char *fs, ...)
+{
+  va_list args;
+  char *msg;
+  size_t len, len2;
+  int r;
+
+  if (warning == NULL) {
+    warning = strdup ("");
+    if (warning == NULL) {
+    malloc_fail:
+      perror ("malloc");
+      exit (EXIT_FAILURE);
+    }
+  }
+
+  len = strlen (warning);
+  if (len > 0 && warning[len-1] != '\n' && fs[0] !=
'\n') {
+    warning = concat_warning (warning, "\n");
+    len = strlen (warning);
+  }
+
+  va_start (args, fs);
+  r = vasprintf (&msg, fs, args);
+  va_end (args);
+  if (r == -1) goto malloc_fail;
+
+  len2 = strlen (msg);
+  warning = realloc (warning, len + len2 + 1);
+  if (warning == NULL) goto malloc_fail;
+  memcpy (&warning[len], msg, len2 + 1);
+  free (msg);
+
+  return warning;
+}
+
+static void
+vcpus_or_memory_check_callback (GtkWidget *w, gpointer data)
+{
+  int vcpus;
+  uint64_t memory;
+  CLEANUP_FREE char *warning = NULL;
+
+  vcpus = get_vcpus_from_conv_dlg ();
+  memory = get_memory_from_conv_dlg ();
+
+  if (vcpus > MAX_SUPPORTED_VCPUS) {
+    gtk_widget_show (vcpus_warning);
+
+    warning = concat_warning (warning,
+                              _("Number of virtual CPUs is larger than
what is supported for KVM (max: %d)."),
+                              MAX_SUPPORTED_VCPUS);
+  }
+  else
+    gtk_widget_hide (vcpus_warning);
+
+  if (memory > MAX_SUPPORTED_MEMORY_MB * 1024 * 1024) {
+    gtk_widget_show (memory_warning);
+
+    warning = concat_warning (warning,
+                              _("Memory size is larger than what is
supported for KVM (max: %" PRIu64 ")."),
+                              MAX_SUPPORTED_MEMORY_MB);
+  }
+  else
+    gtk_widget_hide (memory_warning);
+
+  if (warning != NULL) {
+    warning = concat_warning (warning,
+                              _("If you ignore this warning, conversion
can still succeed, but the guest may not work or may not be supported on the
target."));
+    gtk_label_set_text (GTK_LABEL (target_warning_label), warning);
+  }
+  else
+    gtk_label_set_text (GTK_LABEL (target_warning_label), "");
+}
+
 static int
 get_vcpus_from_conv_dlg (void)
 {
-- 
2.3.1

On Tuesday 05 May 2015 15:13:16 Richard W.M. Jones
wrote:
> No functional change, just code refactoring.
> ---
>  p2v/gui.c | 42 ++++++++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/p2v/gui.c b/p2v/gui.c
> index 7e9de0d..bc762c2 100644
> --- a/p2v/gui.c
> +++ b/p2v/gui.c
> @@ -375,6 +375,8 @@ static void set_interfaces_from_ui (struct config *);
>  static void conversion_back_clicked (GtkWidget *w, gpointer data);
>  static void start_conversion_clicked (GtkWidget *w, gpointer data);
>  static void notify_ui_callback (int type, const char *data);
> +static int get_vcpus_from_conv_dlg (void);
> +static uint64_t get_memory_from_conv_dlg (void);
>  
>  enum {
>    DISKS_COL_CONVERT = 0,
> @@ -1051,6 +1053,32 @@ conversion_back_clicked (GtkWidget *w, gpointer
data)
>    gtk_widget_set_sensitive (next_button, FALSE);
>  }
>  
> +static int
> +get_vcpus_from_conv_dlg (void)
> +{
> +  const char *str;
> +  int i;
> +
> +  str = gtk_entry_get_text (GTK_ENTRY (vcpus_entry));
> +  if (sscanf (str, "%d", &i) == 1 && i > 0)
> +    return i;
> +  else
> +    return 1;
> +}
> +
> +static uint64_t
> +get_memory_from_conv_dlg (void)
> +{
> +  const char *str;
> +  uint64_t i;
> +
> +  str = gtk_entry_get_text (GTK_ENTRY (memory_entry));
> +  if (sscanf (str, "%" SCNu64, &i) == 1 && i >=
256)
> +    return i * 1024 * 1024;
> +  else
> +    return UINT64_C (1024) * 1024 * 1024;
> +}
> +
>  /*----------------------------------------------------------------------*/
>  /* Running dialog. */
>  
> @@ -1174,7 +1202,6 @@ static void
>  start_conversion_clicked (GtkWidget *w, gpointer data)
>  {
>    struct config *config = data;
> -  int i;
>    const char *str;
>    char *str2;
>    GtkWidget *dlg;
> @@ -1200,17 +1227,8 @@ start_conversion_clicked (GtkWidget *w, gpointer
data)
>      return;
>    }
>  
> -  str = gtk_entry_get_text (GTK_ENTRY (vcpus_entry));
> -  if (sscanf (str, "%d", &i) == 1 && i > 0)
> -    config->vcpus = i;
> -  else
> -    config->vcpus = 1;
> -
> -  str = gtk_entry_get_text (GTK_ENTRY (memory_entry));
> -  if (sscanf (str, "%d", &i) == 1 && i >= 256)
> -    config->memory = (uint64_t) i * 1024 * 1024;
> -  else
> -    config->memory = 1024 * 1024 * 1024;
> +  config->vcpus = get_vcpus_from_conv_dlg ();
> +  config->memory = get_memory_from_conv_dlg ();
>  
>    config->verbose >      gtk_toggle_button_get_active
(GTK_TOGGLE_BUTTON (debug_button));
Hm shouldn't vcpus_entry and memory_entry be some kind of numeric input
widgets, then? Something like GtkSpinButton.

-- 
Pino Toscano