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
