Am 18.06.2019 um 09:38 hat Vladimir Sementsov-Ogievskiy geschrieben: > 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..
I think I would only do the flag because the wrapper is so trivial, but this is a matter of taste. The kind of thing that is decided by whoever writes the patch. Kevin