Re: [Qemu-devel] [PATCH v3 0/4] qcow2: async handling of fragmented io
15.08.2019 18:39, Vladimir Sementsov-Ogievskiy wrote: > 15.08.2019 17:09, Max Reitz wrote: >> On 15.08.19 14:10, Vladimir Sementsov-Ogievskiy wrote: >>> Hi all! >>> >>> Here is an asynchronous scheme for handling fragmented qcow2 >>> reads and writes. Both qcow2 read and write functions loops through >>> sequential portions of data. The series aim it to parallelize these >>> loops iterations. >>> It improves performance for fragmented qcow2 images, I've tested it >>> as described below. >> >> Looks good to me, but I can’t take it yet because I need to wait for >> Stefan’s branch to be merged, of course. >> >> Speaking of which, why didn’t you add any tests for the *_part() >> methods? I find it a bit unsettling that nothing would have caught the >> bug you had in v2 in patch 3. >> > > Hmm, any test with write to fragmented area should have caught it.. Ok, > I'll think of something. > > And now I see that it's not trivial to make such a test: 1. qcow2 write is broken when we give nonzero qiov_offset to it, but only qcow2_write calls bdrv_co_pwritev_part, so we need to have a test where qcow2 is a file child for qcow2 2. Then, the bug leads to the beginning of the qiov will be written to all parts. But our testing tool qemu-io has only "write -P" command with buffer filled with the same one byte, so we can't catch it -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v3 0/4] qcow2: async handling of fragmented io
15.08.2019 17:09, Max Reitz wrote: > On 15.08.19 14:10, Vladimir Sementsov-Ogievskiy wrote: >> Hi all! >> >> Here is an asynchronous scheme for handling fragmented qcow2 >> reads and writes. Both qcow2 read and write functions loops through >> sequential portions of data. The series aim it to parallelize these >> loops iterations. >> It improves performance for fragmented qcow2 images, I've tested it >> as described below. > > Looks good to me, but I can’t take it yet because I need to wait for > Stefan’s branch to be merged, of course. > > Speaking of which, why didn’t you add any tests for the *_part() > methods? I find it a bit unsettling that nothing would have caught the > bug you had in v2 in patch 3. > Hmm, any test with write to fragmented area should have caught it.. Ok, I'll think of something. -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v3 0/4] qcow2: async handling of fragmented io
15.08.2019 16:21, Max Reitz wrote: > On 15.08.19 14:10, Vladimir Sementsov-Ogievskiy wrote: >> 01: - use coroutine_fn where appropriate !!! > > :-) > Ahahaha, I'll explain: When comparing v2 vs v3 and writing this difference script I noticed that I added coroutine_fn marks only to .h and not to .c. So I added some of "!!!" to not foreget to fix it. -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v3 0/4] qcow2: async handling of fragmented io
On 15.08.19 14:10, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > Here is an asynchronous scheme for handling fragmented qcow2 > reads and writes. Both qcow2 read and write functions loops through > sequential portions of data. The series aim it to parallelize these > loops iterations. > It improves performance for fragmented qcow2 images, I've tested it > as described below. Looks good to me, but I can’t take it yet because I need to wait for Stefan’s branch to be merged, of course. Speaking of which, why didn’t you add any tests for the *_part() methods? I find it a bit unsettling that nothing would have caught the bug you had in v2 in patch 3. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 0/4] qcow2: async handling of fragmented io
On 15.08.19 14:10, Vladimir Sementsov-Ogievskiy wrote: > 01: - use coroutine_fn where appropriate !!! :-) signature.asc Description: OpenPGP digital signature