Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Am 23.09.2014 um 10:55 hat Peter Lieven geschrieben: > >> - should we only merge requests within the same cluster? > >Does it hurt to merge everything we can? The block driver needs to be > >able to take things apart anyway, the large request could come from > >somewhere else (guest, block job, builtin NBD server, etc.) > > I was just thinking. It was unclear what the original intention was. I hope it's clearer now. Anyway, for qcow2 merging even across clusters does help a bit if those clusters are physically contiguous. > My concern was that merging too much will increase latency for > the specific I/O even if it increases throughput. That's probably a valid concern. Perhaps we should add a set of options to enable or disable certain request manipulations that the block layer can do. This includes request merging, but also alignment adjustments, zero detection (which already has an option) etc. > >> - why is there no multiread_merge? > >Because nobody implemented it. :-) > > Maybe its worth looking at this. Taking the multiwrite_merge stuff as > a basis it shouldn't be too hard. Yes, it should be doable. We need numbers, though, before merging something like this. > >As I said above, writes hurt a lot because of qcow2 cluster allocation. > >Reads are probably losing some performance as well (someone would need > >to check this), but processing them separately isn't quite as painful. > > I will try to sort that out. Great, thanks! Kevin
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 23.09.2014 10:47, Kevin Wolf wrote: Am 08.09.2014 um 16:54 hat Peter Lieven geschrieben: On 08.09.2014 16:42, Paolo Bonzini wrote: Il 08/09/2014 16:35, Peter Lieven ha scritto: Whats your opinion changed the max_xfer_len to 0x regardsless of use_16_for_rw in iSCSI? If you implemented request splitting in the block layer, it would be okay to force max_xfer_len to 0x. Unfortunately, I currently have no time for that. It will include some shuffling with qiovs that has to be properly tested. Regarding iSCSI: In fact currently the limit is 0x for all iSCSI Targets < 2TB. So I thought that its not obvious at all why a > 2TB target can handle bigger requests. To the root cause of this patch multiwrite_merge I still have some thoughts: - why are we merging requests for raw (especially host devices and/or iSCSI?) The original patch from Kevin was to mitigate a QCOW2 performance regression. The problem wasn't in qcow2, though, it just became more apparent there because lots of small requests are deadly to performance during cluster allocation. Installation simply took ages compared to IDE. If you do a real benchmark, you'll probably see (smaller) differences with raw, too. The core problem is virtio-blk getting much smaller requests. IIRC, I got an average of 128k-512k per request for IDE and something as poor as 4k-32k for virtio-blk. If I read this thread correctly, you're saying that this is still the case. I suspect there is something wrong with the guest driver, but somebody would have to dig into that. This seems to be the case. I will check if this disappears with most recent kernels virtio-blk versions. For iSCSI the qiov concats are destroying all the zerocopy efforts we made. If this is true, it just means that the iscsi driver sucks for vectored I/O and needs to be fixed. It works quite well, I misunderstood the iovec_concat routine at first. I thought it was copying payload data around. The overhead for bilding the qiov structure only should be neglectible. - should we only merge requests within the same cluster? Does it hurt to merge everything we can? The block driver needs to be able to take things apart anyway, the large request could come from somewhere else (guest, block job, builtin NBD server, etc.) I was just thinking. It was unclear what the original intention was. My concern was that merging too much will increase latency for the specific I/O even if it increases throughput. - why is there no multiread_merge? Because nobody implemented it. :-) Maybe its worth looking at this. Taking the multiwrite_merge stuff as a basis it shouldn't be too hard. As I said above, writes hurt a lot because of qcow2 cluster allocation. Reads are probably losing some performance as well (someone would need to check this), but processing them separately isn't quite as painful. I will try to sort that out. Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Am 08.09.2014 um 16:54 hat Peter Lieven geschrieben: > On 08.09.2014 16:42, Paolo Bonzini wrote: > >Il 08/09/2014 16:35, Peter Lieven ha scritto: > >>Whats your opinion changed the max_xfer_len to 0x regardsless > >>of use_16_for_rw in iSCSI? > >If you implemented request splitting in the block layer, it would be > >okay to force max_xfer_len to 0x. > > Unfortunately, I currently have no time for that. It will include some > shuffling with > qiovs that has to be properly tested. > > Regarding iSCSI: In fact currently the limit is 0x for all iSCSI Targets > < 2TB. > So I thought that its not obvious at all why a > 2TB target can handle bigger > requests. > > To the root cause of this patch multiwrite_merge I still have some thoughts: > - why are we merging requests for raw (especially host devices and/or iSCSI?) >The original patch from Kevin was to mitigate a QCOW2 performance > regression. The problem wasn't in qcow2, though, it just became more apparent there because lots of small requests are deadly to performance during cluster allocation. Installation simply took ages compared to IDE. If you do a real benchmark, you'll probably see (smaller) differences with raw, too. The core problem is virtio-blk getting much smaller requests. IIRC, I got an average of 128k-512k per request for IDE and something as poor as 4k-32k for virtio-blk. If I read this thread correctly, you're saying that this is still the case. I suspect there is something wrong with the guest driver, but somebody would have to dig into that. >For iSCSI the qiov concats are destroying all the zerocopy efforts we made. If this is true, it just means that the iscsi driver sucks for vectored I/O and needs to be fixed. > - should we only merge requests within the same cluster? Does it hurt to merge everything we can? The block driver needs to be able to take things apart anyway, the large request could come from somewhere else (guest, block job, builtin NBD server, etc.) > - why is there no multiread_merge? Because nobody implemented it. :-) As I said above, writes hurt a lot because of qcow2 cluster allocation. Reads are probably losing some performance as well (someone would need to check this), but processing them separately isn't quite as painful. Kevin
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Am 18.09.2014 um 16:17 schrieb Paolo Bonzini : > Il 18/09/2014 16:16, Peter Lieven ha scritto: >>> As you can see from the multiwrite_merge trace the merging has never been stopped because of the max_transfer_length. The question is, why are the I/O requests not coming in as specified? >>> I think that's because of the filesystem. Try writing directly to a >>> device. >> >> thats what I did... > > Ah, I saw of=test. It was definitely /dev/vda. Any ideas? Maybe something in the virtio drivers? Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 18/09/2014 16:16, Peter Lieven ha scritto: >> >>> As you can see from the multiwrite_merge trace the merging has never >>> been stopped because of >>> the max_transfer_length. The question is, why are the I/O requests >>> not coming in as specified? >> I think that's because of the filesystem. Try writing directly to a >> device. > > thats what I did... Ah, I saw of=test. Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 18.09.2014 16:12, Paolo Bonzini wrote: Il 12/09/2014 13:43, Peter Lieven ha scritto: As you can see from the multiwrite_merge trace the merging has never been stopped because of the max_transfer_length. The question is, why are the I/O requests not coming in as specified? I think that's because of the filesystem. Try writing directly to a device. thats what I did...
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 12/09/2014 13:43, Peter Lieven ha scritto: > > As you can see from the multiwrite_merge trace the merging has never been > stopped because of > the max_transfer_length. The question is, why are the I/O requests not coming > in as specified? I think that's because of the filesystem. Try writing directly to a device. Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Am 08.09.2014 um 18:29 schrieb Paolo Bonzini: > Il 08/09/2014 18:18, Peter Lieven ha scritto: When copying data, gparted will try using very large I/O sizes. Of course if something breaks it will just use a smaller size, but it would make performance worse. I tried now (with local storage, not virtual---but with such large block sizes it's disk bound anyway, one request can take 0.1 seconds to execute) and a 2 MB block size is 20% slower than 16 MB block size on your usual 3.5" rotational SATA disk. >> can you share with what command exactly you ran these tests? >> >> i tried myself and found that without multiwrite_merge i was not able to >> create a request bigger than 0x sectors from inside linux. > On a different machine: > > $ time dd if=/dev/zero of=test bs=16777216 count=30 oflag=direct > real 0m13.497s > user 0m0.001s > sys 0m0.541s > > $ time dd if=/dev/zero of=test2 bs=1048576 count=480 oflag=direct > real 0m15.835s > user 0m0.005s > sys 0m0.770s > > The bigger block size is 17% faster; for disk-to-disk copy: > > $ time dd if=test of=test3 bs=16777216 count=30 iflag=direct oflag=direct > real 0m26.075s > user 0m0.001s > sys 0m0.678s > > $ time dd if=test2 of=test4 bs=1048576 count=480 iflag=direct oflag=direct > real 0m45.210s > user 0m0.005s > sys 0m1.145s > > The bigger block size is 73% faster. I perfectly believe that 16MB blocksize is faster than 2MB. That is true for iSCSI as well. However, I do not see requests of this size coming in via virtio-blk when I ran: dd if=/dev/zero of=test bs=16777216 count=30 oflag=direct. As you can see from the multiwrite_merge trace the merging has never been stopped because of the max_transfer_length. The question is, why are the I/O requests not coming in as specified? multiwrite_merge: num_reqs 15 -> 1 iscsi_co_writev: sector_num 0 nb_sectors 15360 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 17 -> 1 iscsi_co_writev: sector_num 15360 nb_sectors 17408 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 11 -> 1 iscsi_co_writev: sector_num 32768 nb_sectors 11264 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 19 -> 1 iscsi_co_writev: sector_num 44032 nb_sectors 19456 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 2 -> 1 iscsi_co_writev: sector_num 63488 nb_sectors 2048 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 12 -> 1 iscsi_co_writev: sector_num 65536 nb_sectors 12288 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 18 -> 1 iscsi_co_writev: sector_num 77824 nb_sectors 18432 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 2 -> 1 iscsi_co_writev: sector_num 96256 nb_sectors 2048 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 12 -> 1 iscsi_co_writev: sector_num 98304 nb_sectors 12288 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 19 -> 1 iscsi_co_writev: sector_num 110592 nb_sectors 19456 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 1 -> 1 iscsi_co_writev: sector_num 130048 nb_sectors 1024 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 11 -> 1 iscsi_co_writev: sector_num 131072 nb_sectors 11264 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 19 -> 1 iscsi_co_writev: sector_num 142336 nb_sectors 19456 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 2 -> 1 iscsi_co_writev: sector_num 161792 nb_sectors 2048 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 12 -> 1 iscsi_co_writev: sector_num 163840 nb_sectors 12288 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 19 -> 1 iscsi_co_writev: sector_num 176128 nb_sectors 19456 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 1 -> 1 iscsi_co_writev: sector_num 195584 nb_sectors 1024 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 7 -> 1 iscsi_co_writev: sector_num 196608 nb_sectors 7168 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 18 -> 1 iscsi_co_writev: sector_num 203776 nb_sectors 18432 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 7 -> 1 iscsi_co_writev: sector_num 08 nb_sectors 7168 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 11 -> 1 iscsi_co_writev: sector_num 229376 nb_sectors 11264 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 19 -> 1 iscsi_co_writev: sector_num 240640 nb_sectors 19456 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 2 -> 1 iscsi_co_writev: sector_num 260096 nb_sectors 2048 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 11 -> 1 iscsi_co_writev: sector_num 262144 nb_sectors 11264 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 18 -> 1 iscsi_co_writev: sector_num 273408 nb_sectors 18432 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 3 -> 1 iscsi_co_writev: sector_num 291840 nb_sectors 3072 bs->bl.max_transfer_length 65535 multiwrite_merge: num_reqs 12 -> 1 iscsi_co_writev: sector_nu
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 08/09/2014 18:18, Peter Lieven ha scritto: >> > When copying data, gparted will try using very large I/O sizes. Of >> > course if something breaks it will just use a smaller size, but it would >> > make performance worse. >> > >> > I tried now (with local storage, not virtual---but with such large block >> > sizes it's disk bound anyway, one request can take 0.1 seconds to >> > execute) and a 2 MB block size is 20% slower than 16 MB block size on >> > your usual 3.5" rotational SATA disk. >> > > can you share with what command exactly you ran these tests? > > i tried myself and found that without multiwrite_merge i was not able to > create a request bigger than 0x sectors from inside linux. On a different machine: $ time dd if=/dev/zero of=test bs=16777216 count=30 oflag=direct real0m13.497s user0m0.001s sys 0m0.541s $ time dd if=/dev/zero of=test2 bs=1048576 count=480 oflag=direct real0m15.835s user0m0.005s sys 0m0.770s The bigger block size is 17% faster; for disk-to-disk copy: $ time dd if=test of=test3 bs=16777216 count=30 iflag=direct oflag=direct real0m26.075s user0m0.001s sys 0m0.678s $ time dd if=test2 of=test4 bs=1048576 count=480 iflag=direct oflag=direct real0m45.210s user0m0.005s sys 0m1.145s The bigger block size is 73% faster. Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
> Am 08.09.2014 um 17:27 schrieb Paolo Bonzini : > > Il 08/09/2014 17:18, Peter Lieven ha scritto: >>> >>> That's why we have splitting code for discard, and why we would have to >>> add it for read/write too. >> >> Why should a guest generate such big requests. > > When copying data, gparted will try using very large I/O sizes. Of > course if something breaks it will just use a smaller size, but it would > make performance worse. > > I tried now (with local storage, not virtual---but with such large block > sizes it's disk bound anyway, one request can take 0.1 seconds to > execute) and a 2 MB block size is 20% slower than 16 MB block size on > your usual 3.5" rotational SATA disk. > can you share with what command exactly you ran these tests? i tried myself and found that without multiwrite_merge i was not able to create a request bigger than 0x sectors from inside linux. Peter > Paolo > >> Afaik the reported >> limit for e.g. virtio-blk is 1024 sectors (reported through blockdev >> --getmaxsect /dev/vda). >> I think it was only the multiwrite_merge code causing trouble here. > >
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 08/09/2014 17:18, Peter Lieven ha scritto: >> >> That's why we have splitting code for discard, and why we would have to >> add it for read/write too. > > Why should a guest generate such big requests. When copying data, gparted will try using very large I/O sizes. Of course if something breaks it will just use a smaller size, but it would make performance worse. I tried now (with local storage, not virtual---but with such large block sizes it's disk bound anyway, one request can take 0.1 seconds to execute) and a 2 MB block size is 20% slower than 16 MB block size on your usual 3.5" rotational SATA disk. Paolo > Afaik the reported > limit for e.g. virtio-blk is 1024 sectors (reported through blockdev > --getmaxsect /dev/vda). > I think it was only the multiwrite_merge code causing trouble here.
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 08.09.2014 17:15, Paolo Bonzini wrote: Il 08/09/2014 17:13, ronnie sahlberg ha scritto: What I would like to see would also be to report these limitations to the guest itself to prevent it from generating too large I/Os. That's difficult because you don't want a backend change (e.g. from local storage to iSCSI) to change the vital product data in the guest. I hadn't that in mind That's why we have splitting code for discard, and why we would have to add it for read/write too. Why should a guest generate such big requests. Afaik the reported limit for e.g. virtio-blk is 1024 sectors (reported through blockdev --getmaxsect /dev/vda). I think it was only the multiwrite_merge code causing trouble here. Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 08.09.2014 17:13, ronnie sahlberg wrote: On Mon, Sep 8, 2014 at 7:42 AM, Paolo Bonzini wrote: Il 08/09/2014 16:35, Peter Lieven ha scritto: messages. :) So you would not throw an error msg here? No, though a trace could be useful. Is there a howto somewhere how to implement that? Try commit 4ac4458076e1aaf3b01a6361154117df20e22215. Whats your opinion changed the max_xfer_len to 0x regardsless of use_16_for_rw in iSCSI? If you implemented request splitting in the block layer, it would be okay to force max_xfer_len to 0x. I think it should be OK if that is a different series. This only affects the iSCSI transport since it is the only transport so far to record or report a max transfer length. If a guest generates these big requests(i.e. not multiwrite) we already fail them due to the READ10 limitations. We would just fail the request at a different layer in the stack with these patches. What I would like to see would also be to report these limitations to the guest itself to prevent it from generating too large I/Os. I am willing to do that part once the initial max_xfer_len ones are finished. Yes, I also think this approach is straightforward. We should avoid big requests at the beginning and not fix them at the end. I had a look it shouldn't be too hard. There are default values for virtio-scsi HD inquiry emulation which are set if no cmdline values are specified. It should be possible to feed them from bs->bl if set. Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 08/09/2014 17:13, ronnie sahlberg ha scritto: > > What I would like to see would also be to report these limitations to > the guest itself to prevent it from generating too large I/Os. That's difficult because you don't want a backend change (e.g. from local storage to iSCSI) to change the vital product data in the guest. That's why we have splitting code for discard, and why we would have to add it for read/write too. Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On Mon, Sep 8, 2014 at 7:42 AM, Paolo Bonzini wrote: > Il 08/09/2014 16:35, Peter Lieven ha scritto: > > messages. :) So you would not throw an error msg here? >>> No, though a trace could be useful. >> >> Is there a howto somewhere how to implement that? > > Try commit 4ac4458076e1aaf3b01a6361154117df20e22215. > >> Whats your opinion changed the max_xfer_len to 0x regardsless >> of use_16_for_rw in iSCSI? > > If you implemented request splitting in the block layer, it would be > okay to force max_xfer_len to 0x. I think it should be OK if that is a different series. This only affects the iSCSI transport since it is the only transport so far to record or report a max transfer length. If a guest generates these big requests(i.e. not multiwrite) we already fail them due to the READ10 limitations. We would just fail the request at a different layer in the stack with these patches. What I would like to see would also be to report these limitations to the guest itself to prevent it from generating too large I/Os. I am willing to do that part once the initial max_xfer_len ones are finished.
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 08.09.2014 16:42, Paolo Bonzini wrote: Il 08/09/2014 16:35, Peter Lieven ha scritto: messages. :) So you would not throw an error msg here? No, though a trace could be useful. Is there a howto somewhere how to implement that? Try commit 4ac4458076e1aaf3b01a6361154117df20e22215. Thanks for the pointer. Whats your opinion changed the max_xfer_len to 0x regardsless of use_16_for_rw in iSCSI? If you implemented request splitting in the block layer, it would be okay to force max_xfer_len to 0x. Unfortunately, I currently have no time for that. It will include some shuffling with qiovs that has to be properly tested. Regarding iSCSI: In fact currently the limit is 0x for all iSCSI Targets < 2TB. So I thought that its not obvious at all why a > 2TB target can handle bigger requests. To the root cause of this patch multiwrite_merge I still have some thoughts: - why are we merging requests for raw (especially host devices and/or iSCSI?) The original patch from Kevin was to mitigate a QCOW2 performance regression. For iSCSI the qiov concats are destroying all the zerocopy efforts we made. - should we only merge requests within the same cluster? - why is there no multiread_merge? Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 08/09/2014 16:35, Peter Lieven ha scritto: messages. :) >>> So you would not throw an error msg here? >> No, though a trace could be useful. > > Is there a howto somewhere how to implement that? Try commit 4ac4458076e1aaf3b01a6361154117df20e22215. > Whats your opinion changed the max_xfer_len to 0x regardsless > of use_16_for_rw in iSCSI? If you implemented request splitting in the block layer, it would be okay to force max_xfer_len to 0x. Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 08.09.2014 15:58, Paolo Bonzini wrote: Il 08/09/2014 15:56, Peter Lieven ha scritto: Look like you are changing the coroutine version. Some hardware like virtio-blk uses the AIO version of read and writes. What would happen if all the block drivers down the chain are AIO enabled ? The AIO version still goes through bdrv_co_do_readv/writev. However, error_report is not something you can use for guest-accessible error messages, unless you want your logs to fill up with error messages. :) So you would not throw an error msg here? No, though a trace could be useful. Is there a howto somewhere how to implement that? Then I would send a v2 if there are no other complaints. Whats your opinion changed the max_xfer_len to 0x regardsless of use_16_for_rw in iSCSI? Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 08/09/2014 15:56, Peter Lieven ha scritto: > >>> Look like you are changing the coroutine version. >>> >>> Some hardware like virtio-blk uses the AIO version of read and writes. >>> What would happen if all the block drivers down the chain are AIO >>> enabled ? >> The AIO version still goes through bdrv_co_do_readv/writev. >> >> However, error_report is not something you can use for guest-accessible >> error messages, unless you want your logs to fill up with error >> messages. :) > > So you would not throw an error msg here? No, though a trace could be useful. Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
On 08.09.2014 15:49, Paolo Bonzini wrote:
Il 08/09/2014 15:44, Benoît Canet ha scritto:
+if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length)
{
+error_report("read of %d sectors at sector %ld exceeds device max"
+ " transfer length of %d sectors.", nb_sectors, sector_num,
+ bs->bl.max_transfer_length);
+return -EINVAL;
+}
+
return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
}
@@ -3507,6 +3514,13 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
return -EINVAL;
}
+if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length) {
+error_report("write of %d sectors at sector %ld exceeds device max"
+ " transfer length of %d sectors.", nb_sectors, sector_num,
+ bs->bl.max_transfer_length);
+return -EINVAL;
+}
+
return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
}
--
1.7.9.5
Look like you are changing the coroutine version.
Some hardware like virtio-blk uses the AIO version of read and writes.
What would happen if all the block drivers down the chain are AIO enabled ?
The AIO version still goes through bdrv_co_do_readv/writev.
However, error_report is not something you can use for guest-accessible
error messages, unless you want your logs to fill up with error messages. :)
So you would not throw an error msg here?
Peter
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Il 08/09/2014 15:44, Benoît Canet ha scritto:
>> > +if (bs->bl.max_transfer_length && nb_sectors >
>> > bs->bl.max_transfer_length) {
>> > +error_report("read of %d sectors at sector %ld exceeds device max"
>> > + " transfer length of %d sectors.", nb_sectors,
>> > sector_num,
>> > + bs->bl.max_transfer_length);
>> > +return -EINVAL;
>> > +}
>> > +
>> > return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
>> > nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
>> > }
>> > @@ -3507,6 +3514,13 @@ static int coroutine_fn
>> > bdrv_co_do_writev(BlockDriverState *bs,
>> > return -EINVAL;
>> > }
>> >
>> > +if (bs->bl.max_transfer_length && nb_sectors >
>> > bs->bl.max_transfer_length) {
>> > +error_report("write of %d sectors at sector %ld exceeds device
>> > max"
>> > + " transfer length of %d sectors.", nb_sectors,
>> > sector_num,
>> > + bs->bl.max_transfer_length);
>> > +return -EINVAL;
>> > +}
>> > +
>> > return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
>> >nb_sectors << BDRV_SECTOR_BITS, qiov,
>> > flags);
>> > }
>> > --
>> > 1.7.9.5
>> >
>> >
> Look like you are changing the coroutine version.
>
> Some hardware like virtio-blk uses the AIO version of read and writes.
> What would happen if all the block drivers down the chain are AIO enabled ?
The AIO version still goes through bdrv_co_do_readv/writev.
However, error_report is not something you can use for guest-accessible
error messages, unless you want your logs to fill up with error messages. :)
Paolo
Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
The Friday 05 Sep 2014 à 18:51:26 (+0200), Peter Lieven wrote :
> Signed-off-by: Peter Lieven
> ---
> block.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/block.c b/block.c
> index 2c4a5de..fa4c34b 100644
> --- a/block.c
> +++ b/block.c
> @@ -3215,6 +3215,13 @@ static int coroutine_fn
> bdrv_co_do_readv(BlockDriverState *bs,
> return -EINVAL;
> }
>
> +if (bs->bl.max_transfer_length && nb_sectors >
> bs->bl.max_transfer_length) {
> +error_report("read of %d sectors at sector %ld exceeds device max"
> + " transfer length of %d sectors.", nb_sectors,
> sector_num,
> + bs->bl.max_transfer_length);
> +return -EINVAL;
> +}
> +
> return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
> nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
> }
> @@ -3507,6 +3514,13 @@ static int coroutine_fn
> bdrv_co_do_writev(BlockDriverState *bs,
> return -EINVAL;
> }
>
> +if (bs->bl.max_transfer_length && nb_sectors >
> bs->bl.max_transfer_length) {
> +error_report("write of %d sectors at sector %ld exceeds device max"
> + " transfer length of %d sectors.", nb_sectors,
> sector_num,
> + bs->bl.max_transfer_length);
> +return -EINVAL;
> +}
> +
> return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
>nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
> }
> --
> 1.7.9.5
>
>
Look like you are changing the coroutine version.
Some hardware like virtio-blk uses the AIO version of read and writes.
What would happen if all the block drivers down the chain are AIO enabled ?
Best regards
Benoît
[Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests
Signed-off-by: Peter Lieven
---
block.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/block.c b/block.c
index 2c4a5de..fa4c34b 100644
--- a/block.c
+++ b/block.c
@@ -3215,6 +3215,13 @@ static int coroutine_fn
bdrv_co_do_readv(BlockDriverState *bs,
return -EINVAL;
}
+if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length)
{
+error_report("read of %d sectors at sector %ld exceeds device max"
+ " transfer length of %d sectors.", nb_sectors, sector_num,
+ bs->bl.max_transfer_length);
+return -EINVAL;
+}
+
return bdrv_co_do_preadv(bs, sector_num << BDRV_SECTOR_BITS,
nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
}
@@ -3507,6 +3514,13 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
return -EINVAL;
}
+if (bs->bl.max_transfer_length && nb_sectors > bs->bl.max_transfer_length)
{
+error_report("write of %d sectors at sector %ld exceeds device max"
+ " transfer length of %d sectors.", nb_sectors, sector_num,
+ bs->bl.max_transfer_length);
+return -EINVAL;
+}
+
return bdrv_co_do_pwritev(bs, sector_num << BDRV_SECTOR_BITS,
nb_sectors << BDRV_SECTOR_BITS, qiov, flags);
}
--
1.7.9.5
