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

[Max Reitz <mreitz@redhat.com>]

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.

>> +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)).

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

>> +# To be sure the image has been truncated before $QEMU_IO and $QEMU_IMG run
>> +echo '--- truncated ---'
>> +
>> +sleep 0.3
>> +
>>   # success, all done
> I think you should have a 'wait' here to reap the background child
> process before declaring this test all done.  Particularly if you take
> my advice above[1] about detecting errors.

Will probably not take the advice (unless I failed to understand you 
correctly and you can convince me otherwise), but will use `wait` anyway.

> Looks like a good start, but I'm worried about false positives if you
> can't guarantee some timing between actions.

Yes, you're right. Thanks!

Max