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. Thanks for the explanation. >>>> The printfs are very valuable for the human monitor, but it isn't >>>> really >>>> clear to me what is the ideal return value. >>> But error_printf() is meaningless in the context of QMP. You can >>> reproduce these printfs in HMP based on the errors returned by QMP. >>> >>> But if you're just doing an HMP command (and don't care about QMP) then >>> you shouldn't use qerror_report(). But you need to care about QMP so >>> you should focus on making it a well behaved QMP command. >> The question here is then how to propagate the message back that we >> failed to switch to the new image, but stayed on the old one, as opposed >> to both of them failing? This part of QMP is really black magic and >> there doesn't seem to be a good error for this. Time for a new QMP error? > > 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. I'll have a look. Cheers, Jes
