On Fri, Jul 31, 2020 at 05:11:38PM -0500, Eric Blake wrote: > Place the tracing of return values after any generated 'out:' label, > as it is important to see even the errors caused by calling a function > in the wrong handle state. > > Fixes: 9df088bfa9 > --- > > Multiple bugs found today, but only time to submit one patch so far. > I found them all while trying to implement .list_exports in nbdkit. > > Bug 1: 'nbdinfo --list --json $uri' produces invalid JSON if there are > 0 or more than 1 exports. We need to move the '"exports":[' and ']' > into list_all_exports(), not list_one_export(), and manage correct > trailing comma output. (We only tested nbdinfo with qemu-nbd, which > only lists one export, explaining how we missed this) > > Bug 2: nbd_get_nr_list_exports() fails if the handle is still in READY > state, even though there were exports listed and available in that > case. I see no reason to forbid this during READY; but I've also > mentioned that there are probably more changes coming to the way we do > list export handling in libnbd to allow the reuse of a single handle > rather than our current hacks of one handle per export. (We only > tested nbdinfo with qemu-nbd, which generally does NOT allow > NBD_OPT_GO on the '' export if it was advertising some other export; > nbdkit is more forgiving, explaining why I hit a case where nbdinfo > was calling this function in the wrong state) > > Bug 3: this patch. When a function fails due to use in the wrong > state, the debug output was useless in telling me why. > > generator/C.ml | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/generator/C.ml b/generator/C.ml > index 4a0b45d..2c62d4e 100644 > --- a/generator/C.ml > +++ b/generator/C.ml > @@ -542,13 +542,13 @@ let generate_lib_api_c () = > pr " ret = nbd_unlocked_%s " name; > print_arg_list ~wrap:true ~types:false ~handle:true args optargs; > pr ";\n"; > - if may_set_error then ( > - pr "\n"; > - print_trace_leave ret; > - pr "\n" > - ); > + pr "\n"; > if !need_out_label then > pr " out:\n"; > + if may_set_error then ( > + print_trace_leave ret; > + pr "\n" > + ); > if is_locked then ( > pr " if (h->public_state != get_next_state (h))\n"; > pr " h->public_state = get_next_state (h);\n";
ACK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
