From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57380) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEc3g-00044P-90 for qemu-devel@nongnu.org; Sat, 15 Feb 2014 05:01:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WEc3a-0001Pv-A4 for qemu-devel@nongnu.org; Sat, 15 Feb 2014 05:01:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35690) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEc3a-0001Pn-2H for qemu-devel@nongnu.org; Sat, 15 Feb 2014 05:01:10 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1FA175Z019166 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sat, 15 Feb 2014 05:01:08 -0500 From: Markus Armbruster References: <1392138233-26407-1-git-send-email-pbonzini@redhat.com> <1392138233-26407-10-git-send-email-pbonzini@redhat.com> <20140214164540.GK32343@dhcp-200-207.str.redhat.com> <20140214165954.GD17514@localhost.localdomain> Date: Sat, 15 Feb 2014 11:01:05 +0100 In-Reply-To: <20140214165954.GD17514@localhost.localdomain> (Jeff Cody's message of "Fri, 14 Feb 2014 11:59:54 -0500") Message-ID: <87d2io7l0e.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2 09/20] cow: correctly propagate errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: Kevin Wolf , Paolo Bonzini , famz@redhat.com, qemu-devel@nongnu.org Jeff Cody writes: > On Fri, Feb 14, 2014 at 05:45:40PM +0100, Kevin Wolf wrote: >> Am 11.02.2014 um 18:03 hat Paolo Bonzini geschrieben: >> > Signed-off-by: Paolo Bonzini >> > --- >> > block/cow.c | 12 +++--------- >> > 1 file changed, 3 insertions(+), 9 deletions(-) >> > >> > diff --git a/block/cow.c b/block/cow.c >> > index 7fc0b12..43a2150 100644 >> > --- a/block/cow.c >> > +++ b/block/cow.c >> > @@ -82,7 +82,7 @@ static int cow_open(BlockDriverState *bs, QDict *options, int flags, >> > char version[64]; >> > snprintf(version, sizeof(version), >> > "COW version %d", cow_header.version); >> > - qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, >> > + error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, >> > bs->device_name, "cow", version); >> > ret = -ENOTSUP; >> > goto fail; >> > @@ -330,7 +330,6 @@ static int cow_create(const char *filename, QEMUOptionParameter *options, >> > struct stat st; >> > int64_t image_sectors = 0; >> > const char *image_filename = NULL; >> > - Error *local_err = NULL; >> > int ret; >> > BlockDriverState *cow_bs; >> > >> > @@ -344,18 +343,13 @@ static int cow_create(const char *filename, QEMUOptionParameter *options, >> > options++; >> > } >> > >> > - ret = bdrv_create_file(filename, options, &local_err); >> > + ret = bdrv_create_file(filename, options, errp); >> > if (ret < 0) { >> > - qerror_report_err(local_err); >> > - error_free(local_err); >> > return ret; >> > } >> > >> > - ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR, >> > - &local_err); >> > + ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR, errp); >> > if (ret < 0) { >> > - qerror_report_err(local_err); >> > - error_free(local_err); >> > return ret; >> > } >> >> This is technically correct, but I think general policy is that using >> the local_err pattern is preferred anyway. >> > > If I recall correct, I think there are several places that pass errp > along. How about this for a rule of thumb policy: use the local_err > method if the function does not indicate error outside of the passed > Error pointer. Use &local_err when you need to examine the error object. Passing errp directly is no good then, because it may be null. When you're forwarding errors without examining them, then passing errp directly is just fine.