qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.1] block/backup: Fix hang for unaligned im


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH for-2.1] block/backup: Fix hang for unaligned image size
Date: Mon, 07 Jul 2014 09:00:17 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0

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 <address@hidden>
> ---
>  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 <address@hidden>

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]