[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] qapi: Introduce blockdev-group-snapshot-
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] qapi: Introduce blockdev-group-snapshot-sync command |
Date: |
Mon, 27 Feb 2012 14:40:43 +0000 |
On Mon, Feb 27, 2012 at 2:26 PM, Jeff Cody <address@hidden> wrote:
> On 02/27/2012 06:13 AM, Stefan Hajnoczi wrote:
>> On Sun, Feb 26, 2012 at 7:31 PM, Jeff Cody <address@hidden> wrote:
>>
>> Do you have automated tests for this feature?
>>
>
> No, not yet. The testing has been manual.
For image streaming I used the Python unittest framework along with
QMP/qmp.py to create tests. I am going to submit it as a qemu-iotest.
We really need something along the lines of a harness with QMP
support so that these block layer features can be tested. I will CC
you on the email.
>
>>> +/*
>>> + * Add new bs contents at the top of an image chain while the chain is
>>> live,
>>> + * while keeping required fields on the top layer.
>>
>> Please also document the swap behavior. It's pretty important for the
>> caller to realize that once this function returns, their
>> BlockDriverState arguments with have swapped.
>
> Good point. How about this:
>
> /*
> * Add new bs contents at the top of an image chain while the chain is
> * live, while keeping required fields on the top layer.
> *
> * This will modify the BlockDriverState fields, and swap contents
> * between bs_new and bs_top. Both bs_new and bs_top are modified.
Looks good.
>>> + * It is assumed that bs_new already points to an existing image,
>>> + * with the correct backing filename of top->backing_file
>>
>> Not sure what this means. Isn't bs_new going to use bs_top as its
>> backing file? Why "top->backing_file"?
>
> Sorry, that should have been 'bs_top->backing_file'. The image file is
> not created by this function. I added some more explanation, and
> corrected that typo, in the above comment block. Let me know if you
> think it still needs more clarification.
I still don't follow. Old bs_top's image file itself becomes the
backing file, not bs_top->backing_file. Perhaps I'm misinterpreting
because of how swap changes bs_top and bs_new, but I'm reading it from
the perspective of the caller when they pass in bs_top.
The rest looks good.
Stefan
Re: [Qemu-devel] [PATCH v2 1/2] qapi: Introduce blockdev-group-snapshot-sync command, Kevin Wolf, 2012/02/27
[Qemu-devel] [PATCH v2 2/2] QMP: Add qmp command for blockdev-group-snapshot-sync, Jeff Cody, 2012/02/26