[Qemu-trivial] [PATCH] block: Create proper size file for disk mirror

[Qemu-trivial] [PATCH] block: Create proper size file for disk mirror

[Vishvananda Ishaya]

The qmp monitor command to mirror a disk was passing -1 for size
along with the disk's backing file. This size of the resulting disk
is the size of the backing file, which is incorrect if the disk
has been resized. Therefore we should always pass in the size of
the current disk.

Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>
---
Fixes https://bugs.launchpad.net/qemu/+bug/1103903

 blockdev.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9126587..bb63162 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1259,11 +1259,11 @@ void qmp_drive_mirror(const char *device, const char *target,
         return;
     }
 
+    bdrv_get_geometry(bs, &size);
+    size *= 512;
     if (sync == MIRROR_SYNC_MODE_FULL && mode != NEW_IMAGE_MODE_EXISTING) {
         /* create new image w/o backing file */
         assert(format && drv);
-        bdrv_get_geometry(bs, &size);
-        size *= 512;
         bdrv_img_create(target, format,
                         NULL, NULL, NULL, size, flags, &local_err);
     } else {
@@ -1276,7 +1276,7 @@ void qmp_drive_mirror(const char *device, const char *target,
             bdrv_img_create(target, format,
                             source->filename,
                             source->drv->format_name,
-                            NULL, -1, flags, &local_err);
+                            NULL, size, flags, &local_err);
             break;
         default:
             abort();
-- 
1.7.9.5

Re: [Qemu-trivial] [PATCH] block: Create proper size file for disk mirror

[Kevin Wolf]

Am 24.01.2013 19:00, schrieb Vishvananda Ishaya:
> The qmp monitor command to mirror a disk was passing -1 for size
> along with the disk's backing file. This size of the resulting disk
> is the size of the backing file, which is incorrect if the disk
> has been resized. Therefore we should always pass in the size of
> the current disk.
> 
> Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>

Thanks, applied to the block branch.

Can you send a qemu-iotests test case that tests this condition?
(Probably best as an extension to case 041)

Kevin

Re: [Qemu-trivial] [PATCH] block: Create proper size file for disk mirror

[Stefan Hajnoczi]

On Thu, Jan 24, 2013 at 10:00:40AM -0800, Vishvananda Ishaya wrote:
> The qmp monitor command to mirror a disk was passing -1 for size
> along with the disk's backing file. This size of the resulting disk
> is the size of the backing file, which is incorrect if the disk
> has been resized. Therefore we should always pass in the size of
> the current disk.
> 
> Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>
> ---
> Fixes https://bugs.launchpad.net/qemu/+bug/1103903
> 
>  blockdev.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Also CCed Paolo Bonzini who wrote the code.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-trivial] [PATCH] block: Create proper size file for disk mirror

[Vishvananda Ishaya]

On Jan 25, 2013, at 1:18 AM, Kevin Wolf <kwolf@redhat.com> wrote:

> Am 24.01.2013 19:00, schrieb Vishvananda Ishaya:
>> The qmp monitor command to mirror a disk was passing -1 for size
>> along with the disk's backing file. This size of the resulting disk
>> is the size of the backing file, which is incorrect if the disk
>> has been resized. Therefore we should always pass in the size of
>> the current disk.
>> 
>> Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>
> 
> Thanks, applied to the block branch.
> 
> Can you send a qemu-iotests test case that tests this condition?
> (Probably best as an extension to case 041)

I have a test for this. Would you like it as a separate patch or
should i respin the original?

Vish

Re: [Qemu-trivial] [Qemu-devel] [PATCH] block: Create proper size file for disk mirror

[Eric Blake]

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

On 01/25/2013 11:10 AM, Vishvananda Ishaya wrote:
> 
> On Jan 25, 2013, at 1:18 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> 
>> Am 24.01.2013 19:00, schrieb Vishvananda Ishaya:
>>> The qmp monitor command to mirror a disk was passing -1 for size
>>> along with the disk's backing file. This size of the resulting disk
>>> is the size of the backing file, which is incorrect if the disk
>>> has been resized. Therefore we should always pass in the size of
>>> the current disk.
>>>
>>> Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>
>>
>> Thanks, applied to the block branch.
>>
>> Can you send a qemu-iotests test case that tests this condition?
>> (Probably best as an extension to case 041)
> 
> I have a test for this. Would you like it as a separate patch or
> should i respin the original?

Since the original has already been applied to the block branch, you are
best off sending a separate patch for the test addition.

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


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

Re: [Qemu-trivial] [Qemu-devel] [PATCH] block: Create proper size file for disk mirror

[Kevin Wolf]

Am 25.01.2013 19:13, schrieb Eric Blake:
> On 01/25/2013 11:10 AM, Vishvananda Ishaya wrote:
>>
>> On Jan 25, 2013, at 1:18 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>
>>> Am 24.01.2013 19:00, schrieb Vishvananda Ishaya:
>>>> The qmp monitor command to mirror a disk was passing -1 for size
>>>> along with the disk's backing file. This size of the resulting disk
>>>> is the size of the backing file, which is incorrect if the disk
>>>> has been resized. Therefore we should always pass in the size of
>>>> the current disk.
>>>>
>>>> Signed-off-by: Vishvananda Ishaya <vishvananda@gmail.com>
>>>
>>> Thanks, applied to the block branch.
>>>
>>> Can you send a qemu-iotests test case that tests this condition?
>>> (Probably best as an extension to case 041)
>>
>> I have a test for this. Would you like it as a separate patch or
>> should i respin the original?
> 
> Since the original has already been applied to the block branch, you are
> best off sending a separate patch for the test addition.

Yes, please.

Kevin

[Qemu-trivial] [PATCH] block: Adds mirroring tests for resized images

[Vishvananda Ishaya]

This test verifies two mirroring issues are fixed with resized images:

 * sync='top' creates an image that is the proper size
 * sync='full' doesn't cause an assertion failure and crash qemu
---
These are tests for my patches for the following bugs:

https://bugs.launchpad.net/qemu/+bug/1103868
https://bugs.launchpad.net/qemu/+bug/1103903

 tests/qemu-iotests/041     |   48 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/041.out |    4 ++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index c6eb851..e7c004a 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -292,6 +292,54 @@ class TestMirrorNoBacking(ImageMirroringTestCase):
         self.assertTrue(self.compare_images(test_img, target_img),
                         'target image does not match source after mirroring')
 
+class TestMirrorResized(ImageMirroringTestCase):
+    backing_len = 1 * 1024 * 1024 # MB
+    image_len = 2 * 1024 * 1024 # MB
+
+    def setUp(self):
+        self.create_image(backing_img, TestMirrorResized.backing_len)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
+        qemu_img('resize', test_img, '2M')
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(backing_img)
+        try:
+            os.remove(target_img)
+        except OSError:
+            pass
+
+    def test_complete_top(self):
+        self.assert_no_active_mirrors()
+
+        result = self.vm.qmp('drive-mirror', device='drive0', sync='top',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait()
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/inserted/file', target_img)
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
+    def test_complete_full(self):
+        self.assert_no_active_mirrors()
+
+        result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait()
+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/inserted/file', target_img)
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after mirroring')
+
 class TestReadErrors(ImageMirroringTestCase):
     image_len = 2 * 1024 * 1024 # MB
 
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 71009c2..3a89159 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-..................
+....................
 ----------------------------------------------------------------------
-Ran 18 tests
+Ran 20 tests
 
 OK
-- 
1.7.9.5

Re: [Qemu-trivial] [PATCH] block: Adds mirroring tests for resized images

[Paolo Bonzini]

Il 25/01/2013 19:57, Vishvananda Ishaya ha scritto:
> This test verifies two mirroring issues are fixed with resized images:
> 
>  * sync='top' creates an image that is the proper size
>  * sync='full' doesn't cause an assertion failure and crash qemu
> ---
> These are tests for my patches for the following bugs:
> 
> https://bugs.launchpad.net/qemu/+bug/1103868
> https://bugs.launchpad.net/qemu/+bug/1103903
> 
>  tests/qemu-iotests/041     |   48 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/041.out |    4 ++--
>  2 files changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index c6eb851..e7c004a 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -292,6 +292,54 @@ class TestMirrorNoBacking(ImageMirroringTestCase):
>          self.assertTrue(self.compare_images(test_img, target_img),
>                          'target image does not match source after mirroring')
>  
> +class TestMirrorResized(ImageMirroringTestCase):
> +    backing_len = 1 * 1024 * 1024 # MB
> +    image_len = 2 * 1024 * 1024 # MB
> +
> +    def setUp(self):
> +        self.create_image(backing_img, TestMirrorResized.backing_len)
> +        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
> +        qemu_img('resize', test_img, '2M')
> +        self.vm = iotests.VM().add_drive(test_img)
> +        self.vm.launch()
> +
> +    def tearDown(self):
> +        self.vm.shutdown()
> +        os.remove(test_img)
> +        os.remove(backing_img)
> +        try:
> +            os.remove(target_img)
> +        except OSError:
> +            pass
> +
> +    def test_complete_top(self):
> +        self.assert_no_active_mirrors()
> +
> +        result = self.vm.qmp('drive-mirror', device='drive0', sync='top',
> +                             target=target_img)
> +        self.assert_qmp(result, 'return', {})
> +
> +        self.complete_and_wait()
> +        result = self.vm.qmp('query-block')
> +        self.assert_qmp(result, 'return[0]/inserted/file', target_img)
> +        self.vm.shutdown()
> +        self.assertTrue(self.compare_images(test_img, target_img),
> +                        'target image does not match source after mirroring')
> +
> +    def test_complete_full(self):
> +        self.assert_no_active_mirrors()
> +
> +        result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> +                             target=target_img)
> +        self.assert_qmp(result, 'return', {})
> +
> +        self.complete_and_wait()
> +        result = self.vm.qmp('query-block')
> +        self.assert_qmp(result, 'return[0]/inserted/file', target_img)
> +        self.vm.shutdown()
> +        self.assertTrue(self.compare_images(test_img, target_img),
> +                        'target image does not match source after mirroring')
> +
>  class TestReadErrors(ImageMirroringTestCase):
>      image_len = 2 * 1024 * 1024 # MB
>  
> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
> index 71009c2..3a89159 100644
> --- a/tests/qemu-iotests/041.out
> +++ b/tests/qemu-iotests/041.out
> @@ -1,5 +1,5 @@
> -..................
> +....................
>  ----------------------------------------------------------------------
> -Ran 18 tests
> +Ran 20 tests
>  
>  OK
> 

The patch doesn't apply to the block branch, but the conflicts should be
trivial.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-trivial] [PATCH] block: Adds mirroring tests for resized images

[Stefan Hajnoczi]

On Fri, Jan 25, 2013 at 10:57:20AM -0800, Vishvananda Ishaya wrote:
> This test verifies two mirroring issues are fixed with resized images:
> 
>  * sync='top' creates an image that is the proper size
>  * sync='full' doesn't cause an assertion failure and crash qemu
> ---
> These are tests for my patches for the following bugs:
> 
> https://bugs.launchpad.net/qemu/+bug/1103868
> https://bugs.launchpad.net/qemu/+bug/1103903
> 
>  tests/qemu-iotests/041     |   48 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/041.out |    4 ++--
>  2 files changed, 50 insertions(+), 2 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan