Re: [Qemu-devel] [PATCH 0/3] block: blk_co_pcache

[Vladimir Sementsov-Ogievskiy]

17.06.2019 16:20, Kevin Wolf wrote:
> Am 17.06.2019 um 15:09 hat Eric Blake geschrieben:
>> On 6/17/19 7:09 AM, Kevin Wolf wrote:
>>
>>>>>
>>>>> Hmm, don't you think that blk_co_pcache sends NBD_CMD_CACHE if called on 
>>>>> nbd driver?
>>>>> I didn't implement it. But may be I should..
>>>>>
>>>>> May aim was only to avoid extra allocation and unnecessary reads. But if 
>>>>> we implement
>>>>> full-featured io request, what should it do?
>>>>>
>>>>> On qcow2 with backing it should pull data from backing to top, like in 
>>>>> copy-on-read.
>>>>> And for nbd it will send NBD_CMD_CACHE?
>>>>> These semantics seems different, but why not?
>>>>>
>>>>
>>>> Any opinions here? Should I resend or could we use it as a first step,
>>>> not touching external API but improving things a little bit?
>>>
>>> I'm not opposed to making only a first step now. The interface seems to
>>> make sense to me; the only thing that I don't really like is the naming
>>> both of the operation (blk_co_pcache) and of the flag (BDRV_REQ_CACHE)
>>> because to me, "cache" doesn't mean "read, but ignore the result".
>>>
>>> The operation only results in something being cached if the block graph
>>> is configured to cache things. And indeed, the most importatn use case
>>> for the flag is not populating a cache, but triggering copy-on-read. So
>>> I think calling this operation "cache" is misleading.
>>>
>>> Now, I don't have very good ideas for better names either. I guess
>>> BDRV_REQ_NO_DATA could work, though it's not perfect. (Also, not sure if
>>> a blk_co_preadv_no_read wrapper is even needed when you can just call
>>> blk_co_preadv with the right flag.)
>>>
>>> I'm open for good naming ideas.
>>
>> Would 'prefetch' be a more useful name? The name NBD_CMD_CACHE was
>> chosen in the NBD project, but we are not stuck to that name if we think
>> something better makes more sense.
> 
> Whether 'prefetch' is entirely accurate really depends on the graph
> configuration, too. But at least it gives me the right idea of "read
> something, but don't return it yet", so yes, I think that would work for
> me.
> 

Do you mean BDRV_REQ_PREFETCH + blk_co_pprefetch, or only the flag? Hmm, 
doubled 'p'
in blk_co_pprefetch looks strange, bit it should be here for consistency with 
other
requests..


-- 
Best regards,
Vladimir