diff mbox

[01/13] vvfat: fix qemu-img map and qemu-img convert

Message ID 20170515203114.9477-2-hpoussin@reactos.org (mailing list archive)
State New, archived
Headers show

Commit Message

Hervé Poussineau May 15, 2017, 8:31 p.m. UTC
- bs->total_sectors is the number of sectors of the whole disk
- s->sector_count is the number of sectors of the FAT partition

This fixes the following assert in qemu-img map:
qemu-img.c:2641: get_block_status: Assertion `nb_sectors' failed.

This also fixes an infinite loop in qemu-img convert.

Fixes: 4480e0f924a42e1db8b8cfcac4d0634dd1bb27a0
Fixes: https://bugs.launchpad.net/qemu/+bug/1599539
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 block/vvfat.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Eric Blake May 15, 2017, 8:42 p.m. UTC | #1
On 05/15/2017 03:31 PM, Hervé Poussineau wrote:
> - bs->total_sectors is the number of sectors of the whole disk
> - s->sector_count is the number of sectors of the FAT partition
> 
> This fixes the following assert in qemu-img map:
> qemu-img.c:2641: get_block_status: Assertion `nb_sectors' failed.
> 
> This also fixes an infinite loop in qemu-img convert.
> 
> Fixes: 4480e0f924a42e1db8b8cfcac4d0634dd1bb27a0

Wow - broken since 1.2? Not many vvfat users, are there.

Hervé, you might want to work out with Kevin whether to take
co-maintainership over vvfat, in addition to your other areas.

> Fixes: https://bugs.launchpad.net/qemu/+bug/1599539
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

CC: qemu-stable@nongnu.org

> ---
>  block/vvfat.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

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

(I will rebase my work for changing block_status into byte-based on top
of this)
Kevin Wolf May 16, 2017, 1:17 p.m. UTC | #2
Am 15.05.2017 um 22:42 hat Eric Blake geschrieben:
> On 05/15/2017 03:31 PM, Hervé Poussineau wrote:
> > - bs->total_sectors is the number of sectors of the whole disk
> > - s->sector_count is the number of sectors of the FAT partition

I wonder if we should rename s->sector_count into something like
s->part_sectors that makes this difference clearer.

> > This fixes the following assert in qemu-img map:
> > qemu-img.c:2641: get_block_status: Assertion `nb_sectors' failed.
> > 
> > This also fixes an infinite loop in qemu-img convert.
> > 
> > Fixes: 4480e0f924a42e1db8b8cfcac4d0634dd1bb27a0
> 
> Wow - broken since 1.2? Not many vvfat users, are there.

Probably vvfat and bdrv_get_block_status() aren't used much together.
When I use vvfat, it's simple read/write from a guest usually and no
complicated operations like block jobs that would look at the block
status.

> Hervé, you might want to work out with Kevin whether to take
> co-maintainership over vvfat, in addition to your other areas.

I don't mind either way. If Hervé is planning to spend more on time on
vvfat, it could make sense.

Kevin
diff mbox

Patch

diff --git a/block/vvfat.c b/block/vvfat.c
index af5153d27d..dfa2a242e1 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2958,8 +2958,7 @@  vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
 	int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file)
 {
-    BDRVVVFATState* s = bs->opaque;
-    *n = s->sector_count - sector_num;
+    *n = bs->total_sectors - sector_num;
     if (*n > nb_sectors) {
         *n = nb_sectors;
     } else if (*n < 0) {