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

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
v1:       https://listman.redhat.com/archives/libguestfs/2023-April/031258.html

Please refer to the Notes section in each patch for the v2 updates.

BR,
Laszlo

Laszlo Ersek (3):
  copy: rename DESTINATION_IS_ZERO_OPTION to TARGET_IS_ZERO_OPTION
  copy: fix layout of "long_options" table
  copy: rewrap error message about stuck NBD server

 copy/file-ops.c             |  4 +-
 copy/main.c                 | 44 ++++++++++----------
 copy/multi-thread-copying.c |  2 +-
 copy/nbd-ops.c              |  5 ++-
 copy/nbdcopy.h              |  2 +-
 5 files changed, 30 insertions(+), 27 deletions(-)


base-commit: 30d8e6414bdeff079394552e4227d80304b08532

Both "--destination-is-zero" and "--target-is-zero" are
parsed as
DESTINATION_IS_ZERO_OPTION internally. By renaming the enum constant to
TARGET_IS_ZERO_OPTION, we reduce the max width of the "copy/main.c"
file
to 79 characters.

Rename the variable "destination_is_zero" to
"target_is_zero" as well, for
consistency.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - new patch, replaces v1 patches #1 and #2 [Rich]
    
    - also rename "destination_is_zero" to "target_is_zero"

 copy/file-ops.c             |  4 ++--
 copy/main.c                 | 12 ++++++------
 copy/multi-thread-copying.c |  2 +-
 copy/nbdcopy.h              |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/copy/file-ops.c b/copy/file-ops.c
index 8233737dbe0b..710fc68a8407 100644
--- a/copy/file-ops.c
+++ b/copy/file-ops.c
@@ -351,8 +351,8 @@ file_truncate (struct rw *rw, int64_t size)
   }
   rwf->rw.size = size;
 
-  /* We can assume the destination is zero. */
-  destination_is_zero = true;
+  /* We can assume the target is zero. */
+  target_is_zero = true;
 }
 
 static void
diff --git a/copy/main.c b/copy/main.c
index f9a714c4f677..34e6943b07e3 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -46,7 +46,7 @@
 
 bool allocated;                     /* --allocated flag */
 unsigned connections = 4;           /* --connections */
-bool destination_is_zero;           /* --destination-is-zero flag */
+bool target_is_zero;                /* --target-is-zero flag */
 bool extents = true;                /* ! --no-extents flag */
 bool flush;                         /* --flush flag */
 unsigned max_requests = 64;         /* --requests */
@@ -110,7 +110,7 @@ main (int argc, char *argv[])
     LONG_OPTIONS,
     SHORT_OPTIONS,
     ALLOCATED_OPTION,
-    DESTINATION_IS_ZERO_OPTION,
+    TARGET_IS_ZERO_OPTION,
     FLUSH_OPTION,
     NO_EXTENTS_OPTION,
     QUEUE_SIZE_OPTION,
@@ -123,7 +123,7 @@ main (int argc, char *argv[])
     { "long-options",       no_argument,       NULL, LONG_OPTIONS },
     { "allocated",          no_argument,       NULL, ALLOCATED_OPTION
},
     { "connections",        required_argument, NULL, 'C' },
-    { "destination-is-zero",no_argument,       NULL,
DESTINATION_IS_ZERO_OPTION },
+    { "destination-is-zero",no_argument,       NULL,
TARGET_IS_ZERO_OPTION },
     { "flush",              no_argument,       NULL, FLUSH_OPTION },
     { "no-extents",         no_argument,       NULL,
NO_EXTENTS_OPTION },
     { "progress",           optional_argument, NULL, 'p' },
@@ -133,7 +133,7 @@ main (int argc, char *argv[])
     { "short-options",      no_argument,       NULL, SHORT_OPTIONS },
     { "sparse",             required_argument, NULL, 'S' },
     { "synchronous",        no_argument,       NULL,
SYNCHRONOUS_OPTION },
-    { "target-is-zero",     no_argument,       NULL,
DESTINATION_IS_ZERO_OPTION },
+    { "target-is-zero",     no_argument,       NULL,
TARGET_IS_ZERO_OPTION },
     { "threads",            required_argument, NULL, 'T' },
     { "verbose",            no_argument,       NULL, 'v' },
     { "version",            no_argument,       NULL, 'V' },
@@ -174,8 +174,8 @@ main (int argc, char *argv[])
       allocated = true;
       break;
 
-    case DESTINATION_IS_ZERO_OPTION:
-      destination_is_zero = true;
+    case TARGET_IS_ZERO_OPTION:
+      target_is_zero = true;
       break;
 
     case FLUSH_OPTION:
diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
index 00eec511d015..a75fb2657bca 100644
--- a/copy/multi-thread-copying.c
+++ b/copy/multi-thread-copying.c
@@ -634,7 +634,7 @@ fill_dst_range_with_zeroes (struct command *command)
   char *data;
   size_t data_size;
 
-  if (destination_is_zero)
+  if (target_is_zero)
     goto free_and_return;
 
   /* Try efficient zeroing. */
diff --git a/copy/nbdcopy.h b/copy/nbdcopy.h
index 28306d6bf289..465b7052e799 100644
--- a/copy/nbdcopy.h
+++ b/copy/nbdcopy.h
@@ -228,7 +228,7 @@ extern void asynch_notify_read_write_not_supported (struct
rw *rw,
 
 extern bool allocated;
 extern unsigned connections;
-extern bool destination_is_zero;
+extern bool target_is_zero;
 extern bool extents;
 extern bool flush;
 extern unsigned max_requests;

The "destination-is-zero" row is missing a space character between the
first two columns, in the "long_options" table; supply it. The max
width
of "copy/main.c" remains 79 chars.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Acked-by: Richard W.M. Jones <rjones at redhat.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - redo (rebase would mean a bunch of conflicts)
    
    - pick up Rich's ACK

 copy/main.c | 36 ++++++++++----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/copy/main.c b/copy/main.c
index 34e6943b07e3..9c53842ff7cf 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, HELP_OPTION },
-    { "long-options",       no_argument,       NULL, LONG_OPTIONS },
-    { "allocated",          no_argument,       NULL, ALLOCATED_OPTION
},
-    { "connections",        required_argument, NULL, 'C' },
-    { "destination-is-zero",no_argument,       NULL,
TARGET_IS_ZERO_OPTION },
-    { "flush",              no_argument,       NULL, FLUSH_OPTION },
-    { "no-extents",         no_argument,       NULL,
NO_EXTENTS_OPTION },
-    { "progress",           optional_argument, NULL, 'p' },
-    { "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 },
-    { "sparse",             required_argument, NULL, 'S' },
-    { "synchronous",        no_argument,       NULL,
SYNCHRONOUS_OPTION },
-    { "target-is-zero",     no_argument,       NULL,
TARGET_IS_ZERO_OPTION },
-    { "threads",            required_argument, NULL, 'T' },
-    { "verbose",            no_argument,       NULL, 'v' },
-    { "version",            no_argument,       NULL, 'V' },
+    { "help",                no_argument,       NULL, HELP_OPTION },
+    { "long-options",        no_argument,       NULL, LONG_OPTIONS },
+    { "allocated",           no_argument,       NULL,
ALLOCATED_OPTION },
+    { "connections",         required_argument, NULL, 'C' },
+    { "destination-is-zero", no_argument,       NULL,
TARGET_IS_ZERO_OPTION },
+    { "flush",               no_argument,       NULL, FLUSH_OPTION },
+    { "no-extents",          no_argument,       NULL,
NO_EXTENTS_OPTION },
+    { "progress",            optional_argument, NULL, 'p' },
+    { "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
},
+    { "sparse",              required_argument, NULL, 'S' },
+    { "synchronous",         no_argument,       NULL,
SYNCHRONOUS_OPTION },
+    { "target-is-zero",      no_argument,       NULL,
TARGET_IS_ZERO_OPTION },
+    { "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
Acked-by: Richard W.M. Jones <rjones at redhat.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - pick up Rich's ACK

 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);
     }