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 <mre...@redhat.com>
---
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
