From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60659 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Px01z-0005xy-PN for qemu-devel@nongnu.org; Tue, 08 Mar 2011 11:45:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Px01y-00074S-9m for qemu-devel@nongnu.org; Tue, 08 Mar 2011 11:45:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52683) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Px01x-000747-SO for qemu-devel@nongnu.org; Tue, 08 Mar 2011 11:45:06 -0500 Message-ID: <4D765CFF.3090909@redhat.com> Date: Tue, 08 Mar 2011 17:44:47 +0100 From: Jes Sorensen MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v3] Improve error handling in do_snapshot_blkdev() References: <1299511653-11357-1-git-send-email-Jes.Sorensen@redhat.com> <4D75092F.1020107@codemonkey.ws> <4D750A4B.1070304@redhat.com> <4D751A2E.7030309@codemonkey.ws> <4D75E7D0.90900@redhat.com> <4D76323D.8050906@codemonkey.ws> In-Reply-To: <4D76323D.8050906@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com 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