From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43053) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VXEKy-0000K0-7y for qemu-devel@nongnu.org; Fri, 18 Oct 2013 13:59:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VXEKs-0008Mv-0o for qemu-devel@nongnu.org; Fri, 18 Oct 2013 13:59:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VXEKr-0008Mh-OJ for qemu-devel@nongnu.org; Fri, 18 Oct 2013 13:59:41 -0400 Message-ID: <52617702.1080906@redhat.com> Date: Fri, 18 Oct 2013 19:59:30 +0200 From: Max Reitz MIME-Version: 1.0 References: <1381803804-4551-1-git-send-email-famz@redhat.com> <525EE171.20208@redhat.com> <20131017124928.GI10774@stefanha-thinkpad.redhat.com> <20131017130023.GD2942@dhcp-200-207.str.redhat.com> <20131018085150.GA27034@T430s.nay.redhat.com> In-Reply-To: <20131018085150.GA27034@T430s.nay.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: famz@redhat.com, Kevin Wolf Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, stefanha@redhat.com On 2013-10-18 10:51, Fam Zheng wrote: > On Thu, 10/17 15:00, Kevin Wolf wrote: >> Am 17.10.2013 um 14:49 hat Stefan Hajnoczi geschrieben: >>> On Wed, Oct 16, 2013 at 08:56:49PM +0200, Max Reitz wrote: >>>> On 2013-10-15 04:23, Fam Zheng wrote: >>>> The reason I object it here is that error_propagate *currently* is a >>>> no-op. But this may change in the future: I have already sent an RFC >>>> which extends error_propagate so it can generate an error backtrace >>>> if enabled through configure. If this (or something similar which >>>> extends error_propagate to do more than basically just *errp =3D >>>> local_err) is merged to/implemented in qemu later on, I believe we >>>> want to call error_propagate in every function that touches an error >>>> variable in order to generate a backtrace with maximum verbosity. >>> Did you check if glib's backtrace(3) APIs can be used in error_set() >>> instead of rolling our own backtrace? >>> >>> Also, what is the purpose of the backtrace? If the problem is that >>> error messages don't identify unique errors, then we should fix that >>> instead of relying on backtraces. For example, a function that opens >>> two separate files shouldn't just fail with "File not found" since we >>> don't know which of the two files wasn't found. >> Mostly debugging, I guess. Even if you have unique error messages that >> can only come from a single place, finding that single place by lookin= g >> at all called functions from a given QMP command can take quite a bit = of >> time. I can see it useful. >> >> And we don't even have the unique error messages yet, so we shouldn't >> fall in the trap of rejecting an improvement because it's not perfect. > Stacktrace already provides useful information for debugging, so I'm wo= ndering > if it makes sense to add support (a framework) to catch or dump the sta= cktrace, > which can be used in error_set(), abort() and tracing framework. Well, it seems like a hack to me, but if it works=85 Okay, that's why I=20 sent that series as an RFC with the comment =93Since I do not know whethe= r=20 I am the only one considering it useful in the first place, this is just=20 an RFC for now.=94 Now I know. ;-) > Manually calling error_propagate as above sounds a unnecessary reinvent= ion of > this. Then there's still the problem that I'm not the one who introduced=20 error_propagate. Someone obviously intended some purpose for it, so even=20 if it doesn't make a difference now (and my RFC is unneeded), I'd still=20 use it to propagate errors (instead of passing the error pointer). My=20 point being that there *is* a function for propagating errors and I=20 think we should therefore use it. The current qemu code seems to=20 alternate between the two alternatives, although using error_propagate=20 seems more common to me (at least, that was the result when I looked=20 through the code trying to decide whether to use it or not). Generally, we need a proper discussion about whether error_propagate is=20 obsolete or not. Afterwards, we can adapt the current codebase to the=20 result of that discussion; but until then, I oppose changing existing=20 code without actual need to do so but personal preference. Max PS: I wrote my error_propagate RFC in part because I was disappointed=20 about how much of a no-op error_propagate actually is and was trying to=20 change that. ;-)