Eric Blake
2021-Mar-04 15:30 UTC
[Libguestfs] [libnbd PATCH] info: Let exit status reflect any failures during NBD_OPT_INFO
It turns out that at least nbdkit's testsuite was relying on a
non-zero exit status from nbdinfo when purposefully attempting to get
info on an invalid export name. Printing as much information as
possible instead of going silent becaus of one error is good, but any
time we print to stderr, the exit status should reflect that.
Fixes: 5473e34fc1 (info: Don't kill --list early just because one opt_info
fails)
Reported-by: Rich Jones <rjones at redhat.com>
---
This fixes the regression we were seeing in nbdkit.
info/nbdinfo.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 4b18ab27..3dfc463f 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -58,9 +58,9 @@ DEFINE_VECTOR_TYPE (uint32_vector, uint32_t)
static int collect_context (void *opaque, const char *name);
static int collect_export (void *opaque, const char *name,
const char *desc);
-static void list_one_export (struct nbd_handle *nbd, const char *desc,
+static bool list_one_export (struct nbd_handle *nbd, const char *desc,
bool first, bool last);
-static void list_all_exports (struct nbd_handle *nbd1, const char *uri);
+static bool list_all_exports (struct nbd_handle *nbd1, const char *uri);
static void print_json_string (const char *);
static char *get_content (struct nbd_handle *, int64_t size);
static int extent_callback (void *user_data, const char *metacontext,
@@ -124,6 +124,7 @@ main (int argc, char *argv[])
int tls_negotiated;
char *output = NULL;
size_t output_len = 0;
+ bool list_okay = true;
progname = argv[0];
@@ -336,9 +337,9 @@ main (int argc, char *argv[])
}
if (!list_all)
- list_one_export (nbd, NULL, true, true);
+ list_okay = list_one_export (nbd, NULL, true, true);
else
- list_all_exports (nbd, argv[optind]);
+ list_okay = list_all_exports (nbd, argv[optind]);
if (json_output)
fprintf (fp, "}\n");
@@ -365,7 +366,7 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
- exit (EXIT_SUCCESS);
+ exit (list_okay ? EXIT_SUCCESS : EXIT_FAILURE);
}
static int
@@ -398,7 +399,7 @@ collect_export (void *opaque, const char *name, const char
*desc)
return 0;
}
-static void
+static bool
list_one_export (struct nbd_handle *nbd, const char *desc,
bool first, bool last)
{
@@ -424,7 +425,7 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
nbd_opt_go (nbd) == -1) {
fprintf (stderr, "%s: %s: %s\n", progname, nbd_get_export_name
(nbd),
nbd_get_error ());
- return;
+ return false;
}
size = nbd_get_size (nbd);
if (size == -1) {
@@ -599,12 +600,14 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
free (content);
free (export_name);
free (export_desc);
+ return true;
}
-static void
+static bool
list_all_exports (struct nbd_handle *nbd1, const char *uri)
{
size_t i;
+ bool list_okay = true;
if (export_list.size == 0 && json_output)
fprintf (fp, "\"exports\": []\n");
@@ -639,14 +642,16 @@ list_all_exports (struct nbd_handle *nbd1, const char
*uri)
}
/* List the metadata of this export. */
- list_one_export (nbd2, export_list.ptr[i].desc, i == 0,
- i + 1 == export_list.size);
+ if (!list_one_export (nbd2, export_list.ptr[i].desc, i == 0,
+ i + 1 == export_list.size))
+ list_okay = false;
if (probe_content) {
nbd_shutdown (nbd2, 0);
nbd_close (nbd2);
}
}
+ return list_okay;
}
static void
--
2.30.1
Richard W.M. Jones
2021-Mar-04 15:56 UTC
[Libguestfs] [libnbd PATCH] info: Let exit status reflect any failures during NBD_OPT_INFO
On Thu, Mar 04, 2021 at 09:30:53AM -0600, Eric Blake wrote:> It turns out that at least nbdkit's testsuite was relying on a > non-zero exit status from nbdinfo when purposefully attempting to get > info on an invalid export name. Printing as much information as > possible instead of going silent becaus of one error is good, but any > time we print to stderr, the exit status should reflect that. > > Fixes: 5473e34fc1 (info: Don't kill --list early just because one opt_info fails) > Reported-by: Rich Jones <rjones at redhat.com>Just tested it locally and it worked for me to fix nbdkit. I see it's been pushed so I'll add it to Fedora and rebuild both libnbd and nbdkit there now. Thanks, Rich.> > This fixes the regression we were seeing in nbdkit. > > info/nbdinfo.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/info/nbdinfo.c b/info/nbdinfo.c > index 4b18ab27..3dfc463f 100644 > --- a/info/nbdinfo.c > +++ b/info/nbdinfo.c > @@ -58,9 +58,9 @@ DEFINE_VECTOR_TYPE (uint32_vector, uint32_t) > static int collect_context (void *opaque, const char *name); > static int collect_export (void *opaque, const char *name, > const char *desc); > -static void list_one_export (struct nbd_handle *nbd, const char *desc, > +static bool list_one_export (struct nbd_handle *nbd, const char *desc, > bool first, bool last); > -static void list_all_exports (struct nbd_handle *nbd1, const char *uri); > +static bool list_all_exports (struct nbd_handle *nbd1, const char *uri); > static void print_json_string (const char *); > static char *get_content (struct nbd_handle *, int64_t size); > static int extent_callback (void *user_data, const char *metacontext, > @@ -124,6 +124,7 @@ main (int argc, char *argv[]) > int tls_negotiated; > char *output = NULL; > size_t output_len = 0; > + bool list_okay = true; > > progname = argv[0]; > > @@ -336,9 +337,9 @@ main (int argc, char *argv[]) > } > > if (!list_all) > - list_one_export (nbd, NULL, true, true); > + list_okay = list_one_export (nbd, NULL, true, true); > else > - list_all_exports (nbd, argv[optind]); > + list_okay = list_all_exports (nbd, argv[optind]); > > if (json_output) > fprintf (fp, "}\n"); > @@ -365,7 +366,7 @@ main (int argc, char *argv[]) > exit (EXIT_FAILURE); > } > > - exit (EXIT_SUCCESS); > + exit (list_okay ? EXIT_SUCCESS : EXIT_FAILURE); > } > > static int > @@ -398,7 +399,7 @@ collect_export (void *opaque, const char *name, const char *desc) > return 0; > } > > -static void > +static bool > list_one_export (struct nbd_handle *nbd, const char *desc, > bool first, bool last) > { > @@ -424,7 +425,7 @@ list_one_export (struct nbd_handle *nbd, const char *desc, > nbd_opt_go (nbd) == -1) { > fprintf (stderr, "%s: %s: %s\n", progname, nbd_get_export_name (nbd), > nbd_get_error ()); > - return; > + return false; > } > size = nbd_get_size (nbd); > if (size == -1) { > @@ -599,12 +600,14 @@ list_one_export (struct nbd_handle *nbd, const char *desc, > free (content); > free (export_name); > free (export_desc); > + return true; > } > > -static void > +static bool > list_all_exports (struct nbd_handle *nbd1, const char *uri) > { > size_t i; > + bool list_okay = true; > > if (export_list.size == 0 && json_output) > fprintf (fp, "\"exports\": []\n"); > @@ -639,14 +642,16 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri) > } > > /* List the metadata of this export. */ > - list_one_export (nbd2, export_list.ptr[i].desc, i == 0, > - i + 1 == export_list.size); > + if (!list_one_export (nbd2, export_list.ptr[i].desc, i == 0, > + i + 1 == export_list.size)) > + list_okay = false; > > if (probe_content) { > nbd_shutdown (nbd2, 0); > nbd_close (nbd2); > } > } > + return list_okay; > } > > static void > -- > 2.30.1-- 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