On Wed, 05/24 15:28, Eric Blake wrote: > Most callback commands in qemu-io return 0 to keep the interpreter > loop running, or 1 to quit immediately. However, open_f() just > passed through the return value of openfile(), which has different > semantics of returning 0 if a file was opened, or 1 on any failure. > > As a result of mixing the return semantics, we are forcing the > qemu-io interpreter to exit early on any failures, which is rather > annoying when some of the failures are obviously trying to give > the user a hint of how to proceed (if we didn't then kill qemu-io > out from under the user's feet): > > $ qemu-io > qemu-io> open foo > qemu-io> open foo > file open already, try 'help close' > $ echo $? > 0 > > Meanwhile, we WANT openfile() to report failures, as it is the > way that 'qemu-io -c "$something" no_such_file' knows to exit > early rather than attempting $something. So the solution is to > fix open_f() to always return 0 (when we are in interactive mode, > even failure to open should not end the session), and save the > return value of openfile() for command line use in main(). > > This has been awkward since at least as far back as commit > e3aff4f, in 2009. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v2: fix open_f(), not openfile() > --- > qemu-io.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/qemu-io.c b/qemu-io.c > index 34fa8a1..b3febc2 100644 > --- a/qemu-io.c > +++ b/qemu-io.c > @@ -230,13 +230,14 @@ static int open_f(BlockBackend *blk, int argc, char > **argv) > qemu_opts_reset(&empty_opts); > > if (optind == argc - 1) { > - return openfile(argv[optind], flags, writethrough, force_share, > opts); > + openfile(argv[optind], flags, writethrough, force_share, opts); > } else if (optind == argc) { > - return openfile(NULL, flags, writethrough, force_share, opts); > + openfile(NULL, flags, writethrough, force_share, opts); > } else { > QDECREF(opts); > - return qemuio_command_usage(&open_cmd); > + qemuio_command_usage(&open_cmd); > } > + return 0; > } > > static int quit_f(BlockBackend *blk, int argc, char **argv) > -- > 2.9.4 > >
This is better, thanks! Reviewed-by: Fam Zheng <f...@redhat.com>