block: Create proper size file for disk mirror

Submitted by Vishvananda Ishaya on Jan. 24, 2013, 6 p.m.

Details

Message ID 1359050440-52511-1-git-send-email-vishvananda@gmail.com
State New
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

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