From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NwF6g-0003D3-GH for qemu-devel@nongnu.org; Mon, 29 Mar 2010 09:34:18 -0400 Received: from [140.186.70.92] (port=40655 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NwF4q-0002EY-JA for qemu-devel@nongnu.org; Mon, 29 Mar 2010 09:34:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1NwF3o-0000pN-Q5 for qemu-devel@nongnu.org; Mon, 29 Mar 2010 09:32:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64943) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1NwF3k-0000ob-Jt for qemu-devel@nongnu.org; Mon, 29 Mar 2010 09:31:19 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2TDVEpD027434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 29 Mar 2010 09:31:14 -0400 Date: Mon, 29 Mar 2010 10:31:08 -0300 From: Luiz Capitulino Message-ID: <20100329103108.5608bbee@redhat.com> In-Reply-To: References: <1269367641-6241-1-git-send-email-armbru@redhat.com> <1269367641-6241-4-git-send-email-armbru@redhat.com> <20100324164018.713345f6@redhat.com> <20100325144921.05e4a27c@redhat.com> <20100326093147.235772e1@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 3/3] monitor: Convert do_migrate() to QError List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Mon, 29 Mar 2010 14:38:35 +0200 Markus Armbruster wrote: > Luiz Capitulino writes: > > > On Thu, 25 Mar 2010 20:30:33 +0100 > > Markus Armbruster wrote: > > > >> Luiz Capitulino writes: > >> > >> > On Thu, 25 Mar 2010 18:37:25 +0100 > >> > Markus Armbruster wrote: > >> > > >> > [...] > >> > > >> >> >> @@ -86,12 +86,13 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data) > >> >> >> (int)qdict_get_int(qdict, "inc")); > >> >> >> #endif > >> >> >> } else { > >> >> >> - monitor_printf(mon, "unknown migration protocol: %s\n", uri); > >> >> >> + qerror_report(QERR_INVALID_PARAMETER, "uri"); > >> >> >> return -1; > >> >> >> } > >> >> >> > >> >> >> if (s == NULL) { > >> >> >> - monitor_printf(mon, "migration failed\n"); > >> >> >> + /* TODO push error reporting into the FOO_start_outgoing_migration() */ > >> >> >> + qerror_report(QERR_MIGRATION_FAILED); > >> >> >> return -1; > >> >> >> } > >> >> > > >> >> > I think this one is no better than the automatic UndefinedError > >> >> > which is going to be triggered. I would only touch this when/if > >> >> > we get the migration functions converted. > >> >> > >> >> I feel it is a bit better, because: > >> >> > >> >> * It doesn't dilute the nice "this is a bug, and I should report it" > >> >> property of UndefinedError. > >> >> > >> >> * It avoids the "returned failure but did not pass an error" message. > >> >> Minor, because it's under CONFIG_DEBUG_MONITOR. > >> > > >> > But this is exactly what we want because having QERR_MIGRATION_FAILED > >> > there doesn't fix the problems those warnings are making us aware of. > >> > >> Except we are already aware of the problem, and additional warnings are > >> just noise. > > > > Our memory is not the best place for it, not only because we can forget, > > but also because it's limited on you and me. > > > > Anyone who enables QMP's debugging support should be able to know what's > > there to be done. > > That's what TODO comments are for. It's case by case and in this very case I'd only add QERR_MIGRATION_FAILED if it makes difference for clients, because besides silencing the warning w/o the proper fix, this error is probably going to be dropped when the *_outgoing_migration() functions are converted.