On 2017-05-31 17:12, Eric Blake wrote: > On 05/31/2017 09:18 AM, Max Reitz wrote: >> On 2017-05-24 22: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> >> >> This still makes qemu-io -c "$something" no_such_file fail, right. But >> now qemu-io -c "open no_such_file" -c "$something" will execute >> $something; and I'm not sure we can convert all -c open users to not use >> that command (maybe we can, now that we have --image-opts). > > Oh, so we do have some uses of -c "open..." -c "$something" (iotest 103 > for example). > > What happens during "$something" if there was no file currently open? It > may be a command-by-command behavior. But as long as we get graceful > secondary failures rather than crashes for those subsequent $somethings, > we may be okay.
Sure, but this is arguing against "we want [qemu-io no_such_file] to exit early". :-) >> I don't think it's absolutely necessary for -c open to exit on error, >> but you seem to do, if I understand your penultimate paragraph >> correctly. :-) > > I don't think it's absolutely necessary either. You're right that this > patch is just worried about 'qemu-img name', not 'qemu-img -c "open > name"'. So maybe all I need to do is update the commit message to > document that the change in semantics to -c "open..." is a side-effect, > that may or may not be changed in a followup patch? If you're OK with that, I am, too. Max
signature.asc
Description: OpenPGP digital signature