Re: [Qemu-devel] [PATCH 2/3] qemu-io: Respect early image end for map
The Saturday 16 Aug 2014 à 20:54:17 (+0200), Max Reitz wrote :
> bdrv_is_allocated() may report zero clusters which most probably means
> the image (file) is shorter than expected. Respect this case in order to
> avoid an infinite loop.
>
> Signed-off-by: Max Reitz
> ---
> qemu-io-cmds.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index c503fc6..860b834 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1909,7 +1909,7 @@ static int map_is_allocated(BlockDriverState *bs,
> int64_t sector_num,
>
> num_checked = MIN(nb_sectors, INT_MAX);
> ret = bdrv_is_allocated(bs, sector_num, num_checked, &num);
> -if (ret == firstret) {
> +if (ret == firstret && num) {
> *pnum += num;
> } else {
> break;
> @@ -1936,6 +1936,9 @@ static int map_f(BlockDriverState *bs, int argc, char
> **argv)
> if (ret < 0) {
> error_report("Failed to get allocation status: %s",
> strerror(-ret));
> return 0;
> +} else if (!num) {
> +error_report("Unexpected end of image");
> +return 0;
> }
>
> retstr = ret ? "allocated" : "not allocated";
> --
> 2.0.4
>
>
Reviewed-by: Benoît Canet
Re: [Qemu-devel] [PATCH 2/3] qemu-io: Respect early image end for map
The Saturday 11 Oct 2014 à 11:53:40 (+0200), Max Reitz wrote :
> Am 10.10.2014 um 14:03 schrieb Benoît Canet:
> >>+} else if (!num) {
> >>+error_report("Unexpected end of image");
> >>+return 0;
> >I think this test can miss some case of Unexpected end of image.
> >
> >For example supose that in map_is_allocated the first bdrv_is_allocated
> >actually succeed then *pnum = num. Then the bottom loop has exit on failure
> >and the function return.
> >
> >in map_f &num is map_is_allocated *pnum so map_f's num != 0 and this very
> >test
> >fails to see the unexpected end of image error.
>
> num != 0 because some sectors where successfully queried. In my opinion, we
> should print the information about them we have. Then, the do-while loop is
> repeated; and this time, map_is_allocated() either again returns num > 0
> (for whatever reason, but I'd be fine with it) or, more probably, it'll be
> num == 0 this time. So the error is not missed, it's just printed one
> iteration later.
Ok that make sense.
Best regards
Benoît
>
> Max
>
> >Best regards
> >
> >Benoît
> >
> >> }
> >> retstr = ret ? "allocated" : "not allocated";
> >>--
> >>2.0.4
> >>
> >>
>
>
Re: [Qemu-devel] [PATCH 2/3] qemu-io: Respect early image end for map
Am 10.10.2014 um 14:03 schrieb Benoît Canet:
+} else if (!num) {
+error_report("Unexpected end of image");
+return 0;
I think this test can miss some case of Unexpected end of image.
For example supose that in map_is_allocated the first bdrv_is_allocated
actually succeed then *pnum = num. Then the bottom loop has exit on failure
and the function return.
in map_f &num is map_is_allocated *pnum so map_f's num != 0 and this very test
fails to see the unexpected end of image error.
num != 0 because some sectors where successfully queried. In my opinion,
we should print the information about them we have. Then, the do-while
loop is repeated; and this time, map_is_allocated() either again returns
num > 0 (for whatever reason, but I'd be fine with it) or, more
probably, it'll be num == 0 this time. So the error is not missed, it's
just printed one iteration later.
Max
Best regards
Benoît
}
retstr = ret ? "allocated" : "not allocated";
--
2.0.4
Re: [Qemu-devel] [PATCH 2/3] qemu-io: Respect early image end for map
> +} else if (!num) {
> +error_report("Unexpected end of image");
> +return 0;
I think this test can miss some case of Unexpected end of image.
For example supose that in map_is_allocated the first bdrv_is_allocated
actually succeed then *pnum = num. Then the bottom loop has exit on failure
and the function return.
in map_f &num is map_is_allocated *pnum so map_f's num != 0 and this very test
fails to see the unexpected end of image error.
Best regards
Benoît
> }
>
> retstr = ret ? "allocated" : "not allocated";
> --
> 2.0.4
>
>
Re: [Qemu-devel] [PATCH 2/3] qemu-io: Respect early image end for map
On 08/16/2014 12:54 PM, Max Reitz wrote: > bdrv_is_allocated() may report zero clusters which most probably means > the image (file) is shorter than expected. Respect this case in order to > avoid an infinite loop. > > Signed-off-by: Max Reitz > --- > qemu-io-cmds.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 2/3] qemu-io: Respect early image end for map
bdrv_is_allocated() may report zero clusters which most probably means
the image (file) is shorter than expected. Respect this case in order to
avoid an infinite loop.
Signed-off-by: Max Reitz
---
qemu-io-cmds.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index c503fc6..860b834 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1909,7 +1909,7 @@ static int map_is_allocated(BlockDriverState *bs, int64_t
sector_num,
num_checked = MIN(nb_sectors, INT_MAX);
ret = bdrv_is_allocated(bs, sector_num, num_checked, &num);
-if (ret == firstret) {
+if (ret == firstret && num) {
*pnum += num;
} else {
break;
@@ -1936,6 +1936,9 @@ static int map_f(BlockDriverState *bs, int argc, char
**argv)
if (ret < 0) {
error_report("Failed to get allocation status: %s",
strerror(-ret));
return 0;
+} else if (!num) {
+error_report("Unexpected end of image");
+return 0;
}
retstr = ret ? "allocated" : "not allocated";
--
2.0.4
