[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/5] qemu-io: Don't die on second open
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/5] qemu-io: Don't die on second open |
Date: |
Thu, 25 May 2017 08:50:37 +0800 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
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 <address@hidden>
>
> ---
> 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 <address@hidden>