On Fri 04 Aug 2017 02:48:03 PM CEST, Markus Armbruster wrote:
> Have a look at quorum_co_flush():
>
> quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0,
> bdrv_getlength(s->children[i]->bs),
> s->children[i]->bs->node_name, result);
>
> bdrv_getlength() can fail. Does it do the right thing then?
If it fails then it returns -errno, but then quorum_report_bad() turns
into uint64_t and assumes it's valid.
Since that number is then rounded up to the next multiple of
BDRV_SECTOR_SIZE in order to calculate end_sector, I think that what
happens in practice is that the user sees a QUORUM_REPORT_BAD event with
sectors-count = 0 (in most cases) or with a very high value in
sectors-count (if errno > BDRV_SECTOR_SIZE).
The result of bdrv_getlength() is only used to report the number of
affected sectors in the QUORUM_REPORT_BAD event, so there are no other
consequences.
Anyway I think it's a good idea not to make assumptions, detect the
error and pass 0 instead.
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -785,8 +785,9 @@ static coroutine_fn int
quorum_co_flush(BlockDriverState *bs)
for (i = 0; i < s->num_children; i++) {
result = bdrv_co_flush(s->children[i]->bs);
if (result) {
+ int64_t length = bdrv_getlength(s->children[i]->bs);
quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0,
- bdrv_getlength(s->children[i]->bs),
+ length > 0 ? length : 0,
s->children[i]->bs->node_name, result);
result_value.l = result;
quorum_count_vote(&error_votes, &result_value, i);
Berto
