[Qemu-devel] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

[Qemu-devel] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

[Alberto Garcia]

The QUORUM_REPORT_BAD event has fields to report the sector in which
the error was detected and the number of affected sectors starting
from that one. This is important for read and write errors, but not
for flush errors.

For flush errors the current code reports the total size of the disk
image. That is however not useful information in this case. Moreover,
the bdrv_getlength() call can fail, and there's no good way of
handling that failure.

Since we're reporting useless information and we cannot even guarantee
to do it in a consistent way, this patch changes the code to report 0
instead in all cases.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/quorum.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 55ba916655..d04da4f430 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -785,8 +785,7 @@ 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) {
-            quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0,
-                              bdrv_getlength(s->children[i]->bs),
+            quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0, 0,
                               s->children[i]->bs->node_name, result);
             result_value.l = result;
             quorum_count_vote(&error_votes, &result_value, i);
-- 
2.11.0

Re: [Qemu-devel] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

On 08/07/2017 07:36 AM, Alberto Garcia wrote:
> The QUORUM_REPORT_BAD event has fields to report the sector in which
> the error was detected and the number of affected sectors starting
> from that one. This is important for read and write errors, but not
> for flush errors.
> 
> For flush errors the current code reports the total size of the disk
> image. That is however not useful information in this case. Moreover,
> the bdrv_getlength() call can fail, and there's no good way of
> handling that failure.
> 
> Since we're reporting useless information and we cannot even guarantee
> to do it in a consistent way, this patch changes the code to report 0
> instead in all cases.
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/quorum.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

[Kevin Wolf]

Am 07.08.2017 um 14:36 hat Alberto Garcia geschrieben:
> The QUORUM_REPORT_BAD event has fields to report the sector in which
> the error was detected and the number of affected sectors starting
> from that one. This is important for read and write errors, but not
> for flush errors.
> 
> For flush errors the current code reports the total size of the disk
> image. That is however not useful information in this case. Moreover,
> the bdrv_getlength() call can fail, and there's no good way of
> handling that failure.
> 
> Since we're reporting useless information and we cannot even guarantee
> to do it in a consistent way, this patch changes the code to report 0
> instead in all cases.
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Thanks, applied to the block branch.

Kevin