Re: [Qemu-devel] [PATCH v3 3/3] iotests: Add test for external image truncation

[Max Reitz <mreitz@redhat.com>]

On 2014-10-23 at 09:46, Kevin Wolf wrote:
> Am 23.10.2014 um 09:26 hat Max Reitz geschrieben:
>> On 2014-10-22 at 18:50, Eric Blake wrote:
>>> On 10/22/2014 09:57 AM, Max Reitz wrote:
>>>> It should not be happening, but it is possible to truncate an image
>>>> outside of qemu while qemu is running (or any of the qemu tools using
>>>> the block layer. raw_co_get_block_status() should not break then.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> ---
>>>>   tests/qemu-iotests/102     | 15 +++++++++++++++
>>>>   tests/qemu-iotests/102.out |  9 +++++++++
>>>>   2 files changed, 24 insertions(+)
>>>>
>>>> diff --git a/tests/qemu-iotests/102 b/tests/qemu-iotests/102
>>>> index 34b363f..027198b 100755
>>>> --- a/tests/qemu-iotests/102
>>>> +++ b/tests/qemu-iotests/102
>>>> @@ -58,6 +58,21 @@ truncate -s $((5 * 64 * 1024)) "$TEST_IMG"
>>>>   $QEMU_IO -c map "$TEST_IMG"
>>>>   $QEMU_IMG map "$TEST_IMG"
>>>> +echo
>>>> +echo '=== Testing map on an image file truncated outside of qemu ==='
>>>> +echo
>>>> +
>>>> +# Same as above, only now we concurrently truncate and map the image
>>>> +_make_test_img $IMG_SIZE
>>>> +$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
>>>> +
>>>> +(sleep 0.2; $QEMU_IO -c map "$TEST_IMG"; $QEMU_IMG map "$TEST_IMG") &
>>> Should you use '&&' instead of ';' between the three operations, to
>>> ensure that you can detect failure of the overall background subshell? [1]
>> No, I don't think so. The output is compared against the test output
>> and I probably want to have both the output of qemu-io -c map and
>> qemu-img map, even if one fails.
>>
>>> Fractional sleep is a GNU extension, and won't work on BSD.  It adds .8
>>> seconds to make this sleep 1, but the extra portability may be worth it.
>> Probably so, yes.
>>
>>> It also makes the test more robust, and less likely to fail a race in a
>>> heavily-loaded tester.  Then again, it is not the first use of
>>> fractional sleep in the testsuite.
>>>
>>> Hmm - does the blkdebug driver allow us to pause qemu operation to
>>> reliably allow an external action callback, and then resume qemu?  That
>>> might be less prone to race failure, as well as reducing the need to
>>> blindly sleep for a fixed amount of time.
>> It does not yet. But when you're asking like this, I'm willing to
>> build a time block driver which pauses one second for every sector
>> you're reading from it.
>>
>> Okay, so without kidding, I think to make this right, we could try
>> to keep qemu-io open, do the truncate, and then continue writing
>> commands to qemu-io. I think I can do that by not using qemu-io but
>> qemu directly and then use the common.qemu functions (along with HMP
>> qemu-io). Of course, this makes testing qemu-img map impossible, but
>> using blkdebug would have done the same, probably. Also, just
>> qemu-io -c map should be enough for this case.
> It should be possible to stop qemu-img at a good enough place with
> blkdebug. The problem would just be how to resume...
>
> I agree that the qemu-img test doesn't add much here anyway.
>
>>>> +truncate -s $((5 * 64 * 1024)) "$TEST_IMG"
>>> truncate is a GNU program, not necessarily available on all platforms;
>>> but this is not the first test using it.
>> Well, if it's not the first test, I'm inclined to leave it. But
>> since I'm going to respin anyway and you're asking so kindly, I'll
>> reach deep into my box of tricks and use qemu-img resize "json:{'driver':'raw','file':{'driver':'file','filename':'$TEST_IMG'}}"
>> $((5 * 64 * 1024)).
> What's wrong with 'qemu-img resize -f raw "$TEST_IMG"'? Also doesn't
> hardcode the protocol.
>
>> Speaking of resize not taking a format, we should probably at some
>> point in time add an input format parameter to all of the qemu-img
>> subcommands (resize and snapshot don't have one yet).
> Well, _my_ resize does have one and has been there since resize was
> introduced in 2010 (commit ae6b0ed6).

Tell that to my qemu-img --help. :-P

Max