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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1868 bytes --]

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]