Re: [Qemu-block] [Qemu-devel] [PATCH 1/4] qemu-io: Don't die on second open

[Eric Blake]

On 05/24/2017 01:28 AM, Fam Zheng wrote:
> On Thu, 05/18 21:32, Eric Blake wrote:
>> Failure to open a file in qemu-io should normally return 1 on
>> failure to end the command loop, on the presumption that when
>> batching commands all on the command line, failure to open means
>> nothing further can be attempted. But when executing qemu-io
>> interactively, there is a special case: if open is executed a
>> second time, we print a hint that the user should try the
>> interactive 'close' first.  But the hint is useless if we don't
>> actually LET them try 'close'.
>>
>> This has been awkward since at least as far back as commit
>> 43642b3, in 2011 (probably earlier, but git blame has a harder
>> time going past the file renames at that point).
>>

>> @@ -63,7 +63,7 @@ static int openfile(char *name, int flags, bool 
>> writethrough, bool force_share,
>>      if (qemuio_blk) {
>>          error_report("file open already, try 'help close'");
>>          QDECREF(opts);
>> -        return 1;
>> +        return 0;
>>      }
>>
>>      if (force_share) {
>> -- 
>> 2.9.4
>>
>>
> 
> Hmm, failure is failure, why is "return 0" better than "return 1"?

"return 0" is what tells the caller to keep interpreting commands.
"return 1" is what tells the caller to exit immediately.  Most qemu-io
commands return 0 always.  Only a few need to return 1 ("quit"
absolutely needs to, and we are arguing about whether "open" should do
so).  Pre-patch, "open" always failed with 1, exiting the interpreter
immediately with nonzero status.  Which is wrong if we print a help
message on the second attempt at open (since a second attempt is only
possible in interactive mode), but DOES match what you want to have
happen on a first open attempt from the command line:

$ qemu-io unreadable_file

But you are arguing that in batch mode, when you do:

$ qemu-io
qemu-io> open unreadable_file

that you want to keep things in qemu-io, rather than exiting
immediately.  If so, then we need to make openfile() return 0 always,
and instead teach the caller to check whether it is invoked in command
line mode (cause non-zero exit status if nothing was opened) or in
interactive mode (keep qemu-io running, to let the user try to open
something else).  In command line mode, the caller should then use the
status of whether a file was actually opened (rather than the return
code) when dealing with the call to openfile() for the command line
argument.

That's a bigger change; but I can try it if you think it is worth it.

> 
> If the control flow in the caller has a problem, fix it there? Specifically, I
> don't think failed interactive open need to exit program, at all.
> 
> Fam
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature