Re: [Qemu-devel] [PATCH v3] Improve error handling in do_snapshot_blkdev()

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/08/2011 10:44 AM, Jes Sorensen wrote:
> On 03/08/11 14:42, Anthony Liguori wrote:
>> On 03/08/2011 02:24 AM, Jes Sorensen wrote:
>>> On 03/07/11 18:47, Anthony Liguori wrote:
>>>> In your case, it's definitely a fatal error for the command.  The
>>>> command is failing and you're just printing out information about the
>>>> rollback information you're taking.
>>> I guess the disconnect here is the definition of fatal. Fatal in my book
>>> means we're dead, toast, gone ..... hardly the case if we manage to fail
>>> back to the previous image.
>> Let me put it another way, you can't call qerror_report twice because
>> there is only one QMP error object sent in the protocol.  You
>> potentially call qerror_report twice which breaks QMP.
>>
>> The way you ought to structure things is to return to the old image, and
>> then throw an error saying that you couldn't open the new image.
> I see, I had the impression QMP would create multiple objects and pass
> them along. Guess not.

No, this is made clearer in QAPI because an error pointer is passed and 
you explicitly set the object.

>> If FileOpenFailed has the filename of the new image, then opening the
>> file failed and we're using the old image.  If FileOpenFailed has the
>> filename of the old image, we're toast.
>>
>> That basically covers it, no?
> It kinda sorta covers it. The problem with that is that you then have to
> do a string match of the return values to determine which of the cases
> happened, which isn't very nice. But I guess we can do that for now.

Right, but this can be done in the HMP command such that the HMP command 
still prints out the warning message.

The key is to have well documented error semantics where the various 
cases can be distinguished because then we can ensure that we can not 
only print out a nice error message in HMP, but that a remote QMP client 
(like libvirt) can also generate a high quality error message.

Regards,

Anthony Liguori

> I'll have a look.
>
> Cheers,
> Jes
>