Patchwork block: Create proper size file for disk mirror

login
register
mail settings
Submitter Vishvananda Ishaya
Date Jan. 24, 2013, 6 p.m.
Message ID <1359050440-52511-1-git-send-email-vishvananda@gmail.com>
Download mbox | patch
Permalink /patch/215461/
State New
Headers show

Comments

Vishvananda Ishaya - Jan. 24, 2013, 6 p.m.
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(-)
Kevin Wolf - Jan. 25, 2013, 9:18 a.m.
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
Stefan Hajnoczi - Jan. 25, 2013, 9:31 a.m.
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>
Vishvananda Ishaya - Jan. 25, 2013, 6:10 p.m.
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
Eric Blake - Jan. 25, 2013, 6:13 p.m.
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.
Kevin Wolf - Jan. 25, 2013, 6:28 p.m.
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

Patch

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();