From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46759) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQYKM-0006Nb-M3 for qemu-devel@nongnu.org; Wed, 25 Feb 2015 04:32:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQYKG-0006WV-9s for qemu-devel@nongnu.org; Wed, 25 Feb 2015 04:32:22 -0500 From: Markus Armbruster References: <1423128889-18260-1-git-send-email-armbru@redhat.com> <1423128889-18260-2-git-send-email-armbru@redhat.com> <87egpnz3eq.fsf@blackfin.pond.sub.org> <1424822666.4698.37.camel@freescale.com> Date: Wed, 25 Feb 2015 10:32:09 +0100 In-Reply-To: <1424822666.4698.37.camel@freescale.com> (Scott Wood's message of "Tue, 24 Feb 2015 18:04:26 -0600") Message-ID: <87fv9ugwva.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/3] PPC: Clean up misuse of qdev_init() in kvm-openpic creation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Scott Wood Cc: qemu-ppc@nongnu.org, Alexander Graf , afaerber@suse.de, qemu-devel@nongnu.org Scott Wood writes: > On Wed, 2015-02-18 at 15:43 +0100, Markus Armbruster wrote: >> Scott, can you review? >> >> Markus Armbruster writes: >> >> > We call ppce500_init_mpic_kvm() to create a "kvm-openpic". If it >> > fails, we call ppce500_init_mpic_qemu() to fall back to plain >> > "openpic". >> > >> > ppce500_init_mpic_kvm() uses qdev_init(). qdev_init()'s error >> > handling has an unwanted side effect: it calls qerror_report_err(), >> > which prints to stderr. Looks like an error, but isn't. >> > >> > In QMP context, it would stash the error in the monitor instead, >> > making the QMP command fail. Fortunately, it's only called from board >> > initialization, never in QMP context. >> > >> > Clean up by cutting out the qdev_init() middle-man: set property >> > "realized" directly. >> > >> > While there, improve the error message when we can't satisfy an >> > explicit user request for "kvm-openpic", and exit(1) instead of >> > abort(). > > I'm OK with this if setting the realized property directly is considered > good practice, I'm following precedence set by new code, and I double-checked with Andreas to make sure it's how he wants it done. > but if we're not supposed to call qdev_init() in cases > where it could legitimately fail, why is it distinct from > qdev_init_nofail()? Fair question! Both qdev_init() and qdev_init_nofail() are deprecated according to qdev-core.h: * As an interim step, the #DeviceState:realized property is set by deprecated * functions qdev_init() and qdev_init_nofail(). qdev_init() needs to go away, because it fundamentally relies on qerror_report() to get the errors out, and that one needs to go away, because it doesn't coexist nicely with the newer Error API. I'm working towards getting rid of both, and this patch is part of the effort. I'm not working towards getting rid of qdev_init_nofail(). As long as board code has to realize devices, having a helper that exits when realization fails makes sense. It may make sense to rename it once qdev_init() is gone.