From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35710) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGXSx-0006zy-K8 for qemu-devel@nongnu.org; Wed, 28 Jan 2015 13:35:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGXSu-0007Xx-E9 for qemu-devel@nongnu.org; Wed, 28 Jan 2015 13:35:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35355) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGXSu-0007Xc-0A for qemu-devel@nongnu.org; Wed, 28 Jan 2015 13:35:48 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0SIZkvx003730 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 28 Jan 2015 13:35:47 -0500 Message-ID: <54C92C00.6000209@redhat.com> Date: Wed, 28 Jan 2015 13:35:44 -0500 From: Max Reitz MIME-Version: 1.0 References: <1422387983-32153-1-git-send-email-mreitz@redhat.com> <1422387983-32153-18-git-send-email-mreitz@redhat.com> <54C92B14.5090009@redhat.com> In-Reply-To: <54C92B14.5090009@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RESEND 17/50] block: Respect empty BB in bdrv_lookup_bs() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: Kevin Wolf , Jeff Cody , Fam Zheng , Markus Armbruster , Stefan Hajnoczi On 2015-01-28 at 13:31, John Snow wrote: > > > On 01/27/2015 02:45 PM, Max Reitz wrote: >> blk_by_name() may return a BlockBackend for which blk_bs() returns NULL. >> In this case, an error should be returned (instead of just returning >> NULL without modifying *errp). >> >> Signed-off-by: Max Reitz >> --- >> block.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/block.c b/block.c >> index 9a0a510..b7e631c 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3718,6 +3718,11 @@ BlockDriverState *bdrv_lookup_bs(const char >> *device, >> blk = blk_by_name(device); >> >> if (blk) { >> + if (!blk_bs(blk)) { >> + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); >> + return NULL; >> + } >> + >> return blk_bs(blk); >> } >> } >> > > Do we have a consensus on The One True And Proper Way to report > errors? I know Markus usually campaigns against error_set() in favor > of error_setg(). The wiki says to use error_setg(). However, QERR_DEVICE_HAS_NO_MEDIUM is at least a generic error (ERROR_CLASS_GENERIC_ERROR) and I somehow like using a macro for such commonplace errors more. I don't know the real problem with error_set() with macro, though. I can only imagine that if we use macros, people may start to rely on the human-readable error string whereas they should not. If I would not use a macro here, I would've written my own error message which might have differed from QERR_DEVICE_HAS_NO_MEDIUM and thus increased diversity. But I don't feel like that's enough of a reason... I'm most probably just missing something about the evilness of error_set() with macros. Max