Re: [Qemu-devel] [Qemu-block] [PATCH] block: document semanatics of bdrv_co_preadv|pwritev

[Daniel P. Berrange]

On Tue, Aug 08, 2017 at 10:39:29AM +0800, Fam Zheng wrote:
> On Fri, 08/04 16:49, Daniel P. Berrange wrote:
> > This is odd.  In the bdrv_aligned_readv() it looks very much like
> > we'll reference qiov->niov, if bytes != 0, so if qiov was NULL we
> > would crash.
> 
> It doesn't make sense if read doesn't have an iov, where should the data be
> placed? :)
> 
> > 
> > In bdrv_aligned_writev(), qiov->niov is also refernced if bytes != 0,
> > *unless*  flags contains BDRV_REQ_ZERO_WRITE, in which case we'll
> > invoke bdrv_co_do_pwrite_zeroes() instead.
> 
> This is intended. Zero-write doesn't need qiov, hence the BDRV_REQ_ZERO_WRITE
> branch. Otherwise, we can assert qiov != NULL.
> 
> > 
> > So unless I'm missing something, bdrv_co_preadv|writev cannot be
> > called with a NULL  qiov, and bdrv_aligned_writev|readv might
> > need their assertions tightened up.
> 
> bdrv_co_pwritev _is_ called with a NULL qiov from blk_aio_pwrite_zeroes. Your
> other reasonings are right.

That will have BDRV_REQ_ZERO_WRITE flag set though, so we don't end up
calling the bdrv_co_pwritev() callback function registered by the block
driver.

> So for write we cannot remove the bytes parameter.

We can't remove it from the bdrv_co_pwritev() function, but we can remove
it from bdrv_co_pwritev block driver callback AFAICT.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|