On 19.11.2016 22:45, Max Reitz wrote: > On 18.11.2016 02:28, Eric Blake wrote: >> On 11/17/2016 05:42 PM, Max Reitz wrote: >>> On 17.11.2016 21:14, Eric Blake wrote: >>>> Use blkdebug's new geometry constraints to emulate setups that >>>> have caused recent regression fixes: write zeroes asserting >>>> when running through a loopback block device with max-transfer >>>> smaller than cluster size, and discard rounding away requests >>>> that were not aligned to power-of-two boundaries. Also, add >>>> coverage that the block layer is honoring max transfer limits. >>>> >>>> For now, a single iotest performs all actions, with the idea >>>> that we can add future blkdebug constraint test cases in the >>>> same file; but it can be split into multiple iotests if we find >>>> reason to run one portion of the test in more setups than what >>>> are possible in the other. >>>> >> >>> >>>> +# Dell Equallogic iSCSI device is unusual with its 15M page size >>>> +echo >>>> +echo "== non-power-of-2 write zeroes ==" >>>> + >>>> +limits=opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M >>>> +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ >>>> + -c "write -z 32M 32M" | _filter_qemu_io >>>> + >>>> +echo >>>> +echo "== non-power-of-2 discard ==" >>>> + >>>> +limits=opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M >>>> +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ >>>> + -c "discard 80000000 30M" | _filter_qemu_io >>> >>> Question: What does this test has to do with iscsi? >>> >>> The first case just tests that we fall back to writing the head and tail >>> as full zeroes when the driver returns -ENOTSUP. >> >> The first one isn't all that interesting, so much as making sure we >> don't regress. I couldn't make it fail, pre-patch. The real test is the >> second one... >> >>> >>> The second test, as far as I can see, just gives some discard request to >>> blkdebug (split up into head, mid and tail), which blkdebug just passes >>> on (because 80000000 is a multiple of 512). qcow2 then discards part of >>> that and drops the head and tail of the request it receives (but head >>> and tail are now calculated based on qcow2's 64k limit). >> >> Thanks to the opt-discard=15M, the blkdebug layer is forcing the block >> layer to break it into head/middle/tail on 15M boundaries, but throwing >> away the head and tail without giving blkdebug a chance, so it only >> zeroed 90-105M. Then, with the block layer fixed to pass the head on >> through anyways, but without patch 2/9, the qcow2 code was seeing that >> the start offset was not cluster-aligned ($TEST_IMG has 1M clusters), >> and with patch 4/9 that was making qcow2 return -ENOTSUP, and still >> ignoring everything up to 90M. It took all of 2, 4, and 5 before the >> discard finally affected the range 77M-90M (since 80000000 is just >> before 77M). > > OK, thank you for the explanation, but again, I don't know how that is > related to the iscsi case.
And now with my last response to patch 5, I do know. So because the "align" option isn't set, the request_alignment defaults to 512. blkdebug will accept requests and pass them on even if they're not aligned to pdiscard_alignment, so that's similar to how iscsi only drops discard requests that are not aligned to request_alignment but passes everything on regardless of whether it's aligned to pdiscard_alignment. I'd be fine with either adjusting the "Dell..." comment to be something entirely iscsi-unrelated, or with a description of the complete picture here. Max
signature.asc
Description: OpenPGP digital signature
