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

[Daniel P. Berrange]

On Fri, Aug 04, 2017 at 04:02:10PM +0200, Kevin Wolf wrote:
> Am 04.08.2017 um 12:50 hat Daniel P. Berrange geschrieben:
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >  include/block/block_int.h | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index d4f4ea7584..deb81a58bd 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -147,12 +147,41 @@ struct BlockDriver {
> >  
> >      int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
> >          int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
> > +
> > +    /**
> > +     * @offset: position in bytes to read at
> > +     * @bytes: number of bytes to read
> > +     * @qiov: the buffers to fill with read data
> > +     *
> > +     * @offset and @bytes will be a multiple of 'request_alignment',
> > +     * but the length of individual @qiov elements does not have to
> > +     * be a multiple.
> > +     *
> > +     * @bytes may be less than the total sizeof @iov, and will be
> > +     * no larger than 'max_transfer'.
> 
> Really? We are asserting that they match in bdrv_aligned_preadv():
> 
>     assert(!qiov || bytes == qiov->size);

Hmm, why do we pass @bytes at all then ? If they're always the same,
how about deleting it and just letting everyone read qiov->size
directly.

> Also, s/sizeof @iov/size of @qiov/



> 
> > +     *
> > +     * The buffer in @qiov may point directly to guest memory.
> > +     */
> >      int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
> >          uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
> >      int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
> >          int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
> >      int coroutine_fn (*bdrv_co_writev_flags)(BlockDriverState *bs,
> >          int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
> > +    /**
> > +     * @offset: position in bytes to write at
> > +     * @bytes: number of bytes to write
> > +     * @qiov: the buffers containing data to write
> > +     *
> > +     * @offset and @bytes will be a multiple of 'request_alignment',
> > +     * but the length of individual @qiov elements does not have to
> > +     * be a multiple.
> > +     *
> > +     * @bytes may be less than the total sizeof @iov, and will be
> > +     * no larger than 'max_transfer'.
> 
> The same assertion exists in bdrv_aligned_pwritev() (and the same typo
> in your comment).
> 
> > +     * The buffer in @qiov may point directly to guest memory.
> > +     */
> >      int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
> >          uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
> 
> Kevin

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 :|