|
From: | Max Reitz |
Subject: | Re: [Qemu-devel] [PATCH RESEND 17/50] block: Respect empty BB in bdrv_lookup_bs() |
Date: | Wed, 28 Jan 2015 13:35:44 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
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 <address@hidden> --- 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
[Prev in Thread] | Current Thread | [Next in Thread] |