Libguestfs - Apr 2023 - [libnbd PATCH 4/4] copy: rewrap error message about stuck NBD server

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

I figured I'd work on the libnbd line wrapping in shorter waves than how
long the nbdkit patch series was.

Laszlo

Laszlo Ersek (4):
  copy: rename (LONG|SHORT)_OPTIONS to (LONG|SHORT)_OPTIONS_OPTION
  copy: rename <purpose>_OPTION to OPT_<purpose>
  copy: fix layout of "long_options" table
  copy: rewrap error message about stuck NBD server

 copy/main.c    | 76 ++++++++++----------
 copy/nbd-ops.c |  5 +-
 2 files changed, 42 insertions(+), 39 deletions(-)


base-commit: 30d8e6414bdeff079394552e4227d80304b08532

Two of the enum constants that denote command line options are
inconsistently named with the rest: all identifiers should be
<purpose>_OPTION, but LONG_OPTIONS and SHORT_OPTIONS (which are supposed
to list the long and short options) don't conform. Rename them.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 copy/main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/copy/main.c b/copy/main.c
index f9a714c4f677..bb67e97ff97a 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -107,8 +107,8 @@ main (int argc, char *argv[])
 {
   enum {
     HELP_OPTION = CHAR_MAX + 1,
-    LONG_OPTIONS,
-    SHORT_OPTIONS,
+    LONG_OPTIONS_OPTION,
+    SHORT_OPTIONS_OPTION,
     ALLOCATED_OPTION,
     DESTINATION_IS_ZERO_OPTION,
     FLUSH_OPTION,
@@ -120,7 +120,7 @@ main (int argc, char *argv[])
   const char *short_options = "C:pR:S:T:vV";
   const struct option long_options[] = {
     { "help",               no_argument,       NULL, HELP_OPTION },
-    { "long-options",       no_argument,       NULL, LONG_OPTIONS },
+    { "long-options",       no_argument,       NULL,
LONG_OPTIONS_OPTION },
     { "allocated",          no_argument,       NULL, ALLOCATED_OPTION
},
     { "connections",        required_argument, NULL, 'C' },
     { "destination-is-zero",no_argument,       NULL,
DESTINATION_IS_ZERO_OPTION },
@@ -130,7 +130,7 @@ main (int argc, char *argv[])
     { "queue-size",         required_argument, NULL,
QUEUE_SIZE_OPTION },
     { "request-size",       required_argument, NULL,
REQUEST_SIZE_OPTION },
     { "requests",           required_argument, NULL, 'R' },
-    { "short-options",      no_argument,       NULL, SHORT_OPTIONS },
+    { "short-options",      no_argument,       NULL,
SHORT_OPTIONS_OPTION },
     { "sparse",             required_argument, NULL, 'S' },
     { "synchronous",        no_argument,       NULL,
SYNCHRONOUS_OPTION },
     { "target-is-zero",     no_argument,       NULL,
DESTINATION_IS_ZERO_OPTION },
@@ -155,7 +155,7 @@ main (int argc, char *argv[])
     case HELP_OPTION:
       usage (stdout, EXIT_SUCCESS);
 
-    case LONG_OPTIONS:
+    case LONG_OPTIONS_OPTION:
       for (i = 0; long_options[i].name != NULL; ++i) {
         if (strcmp (long_options[i].name, "long-options") != 0
&&
             strcmp (long_options[i].name, "short-options") != 0)
@@ -163,7 +163,7 @@ main (int argc, char *argv[])
       }
       exit (EXIT_SUCCESS);
 
-    case SHORT_OPTIONS:
+    case SHORT_OPTIONS_OPTION:
       for (i = 0; short_options[i]; ++i) {
         if (short_options[i] != ':' && short_options[i] !=
'+')
           printf ("-%c\n", short_options[i]);

Rename <purpose>_OPTION to OPT_<purpose> for two reasons:

- it is more idiomatic for enum constants to have the same prefix than for
  them to have the same suffix,

- we hew away three characters (length-wise) from each option name, which
  happens to reduce the max width of "copy/main.c" to 79 characters.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 copy/main.c | 62 ++++++++++----------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/copy/main.c b/copy/main.c
index bb67e97ff97a..8055b3a656ba 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -106,34 +106,34 @@ int
 main (int argc, char *argv[])
 {
   enum {
-    HELP_OPTION = CHAR_MAX + 1,
-    LONG_OPTIONS_OPTION,
-    SHORT_OPTIONS_OPTION,
-    ALLOCATED_OPTION,
-    DESTINATION_IS_ZERO_OPTION,
-    FLUSH_OPTION,
-    NO_EXTENTS_OPTION,
-    QUEUE_SIZE_OPTION,
-    REQUEST_SIZE_OPTION,
-    SYNCHRONOUS_OPTION,
+    OPT_HELP = CHAR_MAX + 1,
+    OPT_LONG_OPTIONS,
+    OPT_SHORT_OPTIONS,
+    OPT_ALLOCATED,
+    OPT_DESTINATION_IS_ZERO,
+    OPT_FLUSH,
+    OPT_NO_EXTENTS,
+    OPT_QUEUE_SIZE,
+    OPT_REQUEST_SIZE,
+    OPT_SYNCHRONOUS,
   };
   const char *short_options = "C:pR:S:T:vV";
   const struct option long_options[] = {
-    { "help",               no_argument,       NULL, HELP_OPTION },
-    { "long-options",       no_argument,       NULL,
LONG_OPTIONS_OPTION },
-    { "allocated",          no_argument,       NULL, ALLOCATED_OPTION
},
+    { "help",               no_argument,       NULL, OPT_HELP },
+    { "long-options",       no_argument,       NULL, OPT_LONG_OPTIONS
},
+    { "allocated",          no_argument,       NULL, OPT_ALLOCATED },
     { "connections",        required_argument, NULL, 'C' },
-    { "destination-is-zero",no_argument,       NULL,
DESTINATION_IS_ZERO_OPTION },
-    { "flush",              no_argument,       NULL, FLUSH_OPTION },
-    { "no-extents",         no_argument,       NULL,
NO_EXTENTS_OPTION },
+    { "destination-is-zero",no_argument,       NULL,
OPT_DESTINATION_IS_ZERO },
+    { "flush",              no_argument,       NULL, OPT_FLUSH },
+    { "no-extents",         no_argument,       NULL, OPT_NO_EXTENTS
},
     { "progress",           optional_argument, NULL, 'p' },
-    { "queue-size",         required_argument, NULL,
QUEUE_SIZE_OPTION },
-    { "request-size",       required_argument, NULL,
REQUEST_SIZE_OPTION },
+    { "queue-size",         required_argument, NULL, OPT_QUEUE_SIZE
},
+    { "request-size",       required_argument, NULL, OPT_REQUEST_SIZE
},
     { "requests",           required_argument, NULL, 'R' },
-    { "short-options",      no_argument,       NULL,
SHORT_OPTIONS_OPTION },
+    { "short-options",      no_argument,       NULL,
OPT_SHORT_OPTIONS },
     { "sparse",             required_argument, NULL, 'S' },
-    { "synchronous",        no_argument,       NULL,
SYNCHRONOUS_OPTION },
-    { "target-is-zero",     no_argument,       NULL,
DESTINATION_IS_ZERO_OPTION },
+    { "synchronous",        no_argument,       NULL, OPT_SYNCHRONOUS
},
+    { "target-is-zero",     no_argument,       NULL,
OPT_DESTINATION_IS_ZERO },
     { "threads",            required_argument, NULL, 'T' },
     { "verbose",            no_argument,       NULL, 'v' },
     { "version",            no_argument,       NULL, 'V' },
@@ -152,10 +152,10 @@ main (int argc, char *argv[])
       break;
 
     switch (c) {
-    case HELP_OPTION:
+    case OPT_HELP:
       usage (stdout, EXIT_SUCCESS);
 
-    case LONG_OPTIONS_OPTION:
+    case OPT_LONG_OPTIONS:
       for (i = 0; long_options[i].name != NULL; ++i) {
         if (strcmp (long_options[i].name, "long-options") != 0
&&
             strcmp (long_options[i].name, "short-options") != 0)
@@ -163,30 +163,30 @@ main (int argc, char *argv[])
       }
       exit (EXIT_SUCCESS);
 
-    case SHORT_OPTIONS_OPTION:
+    case OPT_SHORT_OPTIONS:
       for (i = 0; short_options[i]; ++i) {
         if (short_options[i] != ':' && short_options[i] !=
'+')
           printf ("-%c\n", short_options[i]);
       }
       exit (EXIT_SUCCESS);
 
-    case ALLOCATED_OPTION:
+    case OPT_ALLOCATED:
       allocated = true;
       break;
 
-    case DESTINATION_IS_ZERO_OPTION:
+    case OPT_DESTINATION_IS_ZERO:
       destination_is_zero = true;
       break;
 
-    case FLUSH_OPTION:
+    case OPT_FLUSH:
       flush = true;
       break;
 
-    case NO_EXTENTS_OPTION:
+    case OPT_NO_EXTENTS:
       extents = false;
       break;
 
-    case SYNCHRONOUS_OPTION:
+    case OPT_SYNCHRONOUS:
       synchronous = true;
       break;
 
@@ -209,7 +209,7 @@ main (int argc, char *argv[])
       }
       break;
 
-    case QUEUE_SIZE_OPTION:
+    case OPT_QUEUE_SIZE:
       if (sscanf (optarg, "%u", &queue_size) != 1) {
         fprintf (stderr, "%s: --queue-size: could not parse: %s\n",
                  prog, optarg);
@@ -217,7 +217,7 @@ main (int argc, char *argv[])
       }
       break;
 
-    case REQUEST_SIZE_OPTION:
+    case OPT_REQUEST_SIZE:
       if (sscanf (optarg, "%u", &request_size) != 1) {
         fprintf (stderr, "%s: --request-size: could not parse: %s\n",
                  prog, optarg);

The "destination-is-zero" row is missing a space character between the
first two columns, in the "long_options" table; supply it. This
increases
the max width of "copy/main.c" to 80 chars, so we're still good
like that.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 copy/main.c | 36 ++++++++++----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/copy/main.c b/copy/main.c
index 8055b3a656ba..28ac0f319676 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -119,24 +119,24 @@ main (int argc, char *argv[])
   };
   const char *short_options = "C:pR:S:T:vV";
   const struct option long_options[] = {
-    { "help",               no_argument,       NULL, OPT_HELP },
-    { "long-options",       no_argument,       NULL, OPT_LONG_OPTIONS
},
-    { "allocated",          no_argument,       NULL, OPT_ALLOCATED },
-    { "connections",        required_argument, NULL, 'C' },
-    { "destination-is-zero",no_argument,       NULL,
OPT_DESTINATION_IS_ZERO },
-    { "flush",              no_argument,       NULL, OPT_FLUSH },
-    { "no-extents",         no_argument,       NULL, OPT_NO_EXTENTS
},
-    { "progress",           optional_argument, NULL, 'p' },
-    { "queue-size",         required_argument, NULL, OPT_QUEUE_SIZE
},
-    { "request-size",       required_argument, NULL, OPT_REQUEST_SIZE
},
-    { "requests",           required_argument, NULL, 'R' },
-    { "short-options",      no_argument,       NULL,
OPT_SHORT_OPTIONS },
-    { "sparse",             required_argument, NULL, 'S' },
-    { "synchronous",        no_argument,       NULL, OPT_SYNCHRONOUS
},
-    { "target-is-zero",     no_argument,       NULL,
OPT_DESTINATION_IS_ZERO },
-    { "threads",            required_argument, NULL, 'T' },
-    { "verbose",            no_argument,       NULL, 'v' },
-    { "version",            no_argument,       NULL, 'V' },
+    { "help",                no_argument,       NULL, OPT_HELP },
+    { "long-options",        no_argument,       NULL,
OPT_LONG_OPTIONS },
+    { "allocated",           no_argument,       NULL, OPT_ALLOCATED
},
+    { "connections",         required_argument, NULL, 'C' },
+    { "destination-is-zero", no_argument,       NULL,
OPT_DESTINATION_IS_ZERO },
+    { "flush",               no_argument,       NULL, OPT_FLUSH },
+    { "no-extents",          no_argument,       NULL, OPT_NO_EXTENTS
},
+    { "progress",            optional_argument, NULL, 'p' },
+    { "queue-size",          required_argument, NULL, OPT_QUEUE_SIZE
},
+    { "request-size",        required_argument, NULL,
OPT_REQUEST_SIZE },
+    { "requests",            required_argument, NULL, 'R' },
+    { "short-options",       no_argument,       NULL,
OPT_SHORT_OPTIONS },
+    { "sparse",              required_argument, NULL, 'S' },
+    { "synchronous",         no_argument,       NULL, OPT_SYNCHRONOUS
},
+    { "target-is-zero",      no_argument,       NULL,
OPT_DESTINATION_IS_ZERO },
+    { "threads",             required_argument, NULL, 'T' },
+    { "verbose",             no_argument,       NULL, 'v' },
+    { "version",             no_argument,       NULL, 'V' },
     { NULL }
   };
   int c;

Wrap "copy/nbd-ops.c" at 80 characters.

I couldn't find a way to test that this change is unobservable.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 copy/nbd-ops.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c
index d3e50864125f..843b7c1746e3 100644
--- a/copy/nbd-ops.c
+++ b/copy/nbd-ops.c
@@ -482,7 +482,10 @@ nbd_ops_get_extents (struct rw *rw, size_t index,
 
     /* The server should always make progress. */
     if (offset == old_offset) {
-      fprintf (stderr, "%s: NBD server is broken: it is not returning
extent information.\nTry nbdcopy --no-extents as a workaround.\n",
+      fprintf (stderr,
+               "%s: NBD server is broken: it is not returning extent
"
+               "information.\n"
+               "Try nbdcopy --no-extents as a workaround.\n",
                rw->name);
       exit (EXIT_FAILURE);
     }