On 12/07/2016 07:55 AM, Kevin Wolf wrote: > Am 02.12.2016 um 20:22 hat Eric Blake geschrieben: >> In order to test the effects of artificial geometry constraints >> on operations like write zero or discard, we first need blkdebug >> to manage these actions. It also allows us to inject errors on >> those operations, just like we can for read/write/flush. >>
>>
>>
>> +static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
>> + int64_t offset, int count,
>> + BdrvRequestFlags flags)
>> +{
>> + BDRVBlkdebugState *s = bs->opaque;
>> + BlkdebugRule *rule = NULL;
>> + uint32_t align = MAX(bs->bl.request_alignment,
>> + bs->bl.pwrite_zeroes_alignment);
>> +
>> + /* Only pass through requests that are larger than requested
>> + * preferred alignment (so that we test the fallback to writes on
>> + * unaligned portions), and check that the block layer never hands
>> + * us anything crossing an alignment boundary. */
>> + if (count < align) {
>> + return -ENOTSUP;
>> + }
This early exit bypasses the checks for error injection - but the block
layer will then fall back to write which will perform the check there.
>> + assert(QEMU_IS_ALIGNED(offset, align));
>> + assert(QEMU_IS_ALIGNED(count, align));
>> + if (bs->bl.max_pwrite_zeroes) {
>> + assert(count <= bs->bl.max_pwrite_zeroes);
>> + }
>> +
>> + QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
>> + if (rule->options.inject.offset == -1) {
>
> We do have offset and bytes parameters in this function, so I guess we
> should check overlaps like in the read/write functions instead of only
> executing the rule if it doesn't specify an offset.
Oh, right. I copied and pasted from flush, when I should have copied
from read.
>> + return bdrv_co_pwrite_zeroes(bs->file, offset, count, flags);
>> +}
>> +
>> +
>
> Why two newlines?
>
Habit; they aren't essential, so I'll trim to 1 for consistency with the
rest of the file.
>> +static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
>> + int64_t offset, int count)
>> +{
>> + BDRVBlkdebugState *s = bs->opaque;
>> + BlkdebugRule *rule = NULL;
>> + uint32_t align = bs->bl.pdiscard_alignment;
>> +
>> + /* Only pass through requests that are larger than requested
>> + * minimum alignment, and ensure that unaligned requests do not
>> + * cross optimum discard boundaries. */
>> + if (count < bs->bl.request_alignment) {
>> + return -ENOTSUP;
>> + }
Here, the early exit is a no-op; we never see the error injection
because there is no fallback path at the block layer. But I guess
that's still okay - when we are discarding the unaligned portion of a
request, there really is no I/O and therefore no way to inject an error
representing failed I/O.
Looks like I get to spin a v4 for the error injection to do specific
range checking.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
