On 07/07/2014 08:53 AM, Kevin Wolf wrote:
> When doing a block backup of an image with an unaligned size (with
> respect to the BACKUP_CLUSTER_SIZE), qemu would check the allocation
> status of sectors after the end of the image. bdrv_is_allocated()
> returns a result that is valid for 0 sectors in this case, so the backup
> job ran into an endless loop.
> 
> Stop looping when seeing a result valid for 0 sectors, we're at EOF then.
> 
> The test case looks somewhat unrelated at first sight because I
> originally tried to reproduce a different suspected bug that turned out
> to not exist. Still a good test case and it accidentally found this one.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/backup.c             |   2 +-
>  tests/qemu-iotests/028     |  27 ++++-
>  tests/qemu-iotests/028.out | 269 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 296 insertions(+), 2 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 7978ae2..d0b0225 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -307,7 +307,7 @@ static void coroutine_fn backup_run(void *opaque)
>                                  BACKUP_SECTORS_PER_CLUSTER - i, &n);
>                      i += n;
>  
> -                    if (alloced == 1) {
> +                    if (alloced == 1 || n == 0) {
>                          break;
>                      }
>                  }
> diff --git a/tests/qemu-iotests/028 b/tests/qemu-iotests/028
> index a99e4fa..864e9cc 100755
> --- a/tests/qemu-iotests/028
> +++ b/tests/qemu-iotests/028
> @@ -33,7 +33,8 @@ status=1	# failure is the default!
>  
>  _cleanup()
>  {
> -	_cleanup_test_img
> +    rm -f $TEST_IMG.copy

Doesn't this need to be "$TEST_IMG.copy", to allow for spaces in $TEST_IMG?

> +QEMU X.Y.Z monitor - type 'help' for more information
> +(qemu) ddrdridrivdrivedrive_drive_bdrive_badrive_bacdrive_backdrive_backudrive_backupdrive_backup drive_backup ddrive_backup didrive_backup disdrive_backup diskdrive_backup disk drive_backup disk /drive_backup disk /hdrive_backup disk /ho!
>  drive_backup disk /homdrive_backup disk /homedrive_backup disk /home/drive_backup disk /home/kdrive_backup disk /home/kwdrive_backup disk /home/kwodrive_backup disk /home/kwoldrive_backup disk /home/kwolfdrive_backup disk /home/kwolf/drive_backup disk /!
>  home/kwolf/sdrive_backup disk /home/kwolf/sodrive_backup disk /home/kwolf/soudrive_backup disk /home/kwolf/sourdrive_backup disk /home/kwolf/sourcdrive_backup disk /home/kwolf/sourcedrive_backup disk /home/kwolf/source/drive_backup disk /home/kwolf/sour!
>  ce/q!
>  [Ddrive_backup disk /home/kwolf/source/qedrive_backup disk /home/kwolf/source/qemdrive_backup disk /home/kwolf/source/qemudrive_backup disk /home/kwolf/source/qemu/drive_backup disk /home/kwolf/source/qemu/tdrive_backup disk /home/kwolf/source/qemu/te[!
>  Ddrive_backup disk /home/kwolf/source/qemu/tesdrive_backup disk /home/kwolf/source/qemu/testdrive_backup disk /home/kwolf/source/qemu/testsdrive_backup disk /home/kwolf/source/qemu/tests/drive_backup disk /home/kwolf/source/qemu/tests/q[!
>  Ddrive_backup disk /home/kwol!

Is there any way to avoid super-long-lines like this in the testsuite?
But that's a generic complaint, and doesn't affect the correctness of
this patch.

With the quotes added,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org