On Fri, Jan 25, 2019 at 07:46:01PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Hi all. > > What about such a simple helper for a very often patter around > qemu_iovec_init_external ?
Sounds good, qemu_iovec_init() has 55 references vs qemu_iovec_init_external() with 51. It's worth making qemu_iovec_init_external() nicer to use. > If we like it, I'll update other callers of qemu_iovec_init_external. > > Possible interface change would be > LOCAL_QIOV(lc, buf, len); > instead of > LocalQiov lc = LOCAL_QIOV(lc, buf, len); > > or, may be, someone has a better idea? Bike-shedding territory, but I prefer LocalQiov lc = LOCAL_QIOV(lc, buf, len) because it reveals the type. This makes the code easier to read than just LOCAL_QIOV(lc, buf, len) by itself - the reader is forced to look up the macro definition to figure out what magic happens. > diff --git a/block/io.c b/block/io.c > index bd9d688f8b..c7d7b199c1 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -949,18 +949,13 @@ int bdrv_preadv(BdrvChild *child, int64_t offset, > QEMUIOVector *qiov) > > int bdrv_pread(BdrvChild *child, int64_t offset, void *buf, int bytes) > { > - QEMUIOVector qiov; > - struct iovec iov = { > - .iov_base = (void *)buf, > - .iov_len = bytes, > - }; > + LocalQiov lq = LOCAL_QIOV(lq, buf, bytes); > > if (bytes < 0) { > return -EINVAL; > } > > - qemu_iovec_init_external(&qiov, &iov, 1); > - return bdrv_preadv(child, offset, &qiov); > + return bdrv_preadv(child, offset, &lq.qiov); I think it's unfortunate that LocalQiov is necessary since the caller only needs the qiov. Can we afford to embed the struct iovec into QEMUIOVector? That way callers don't need a separate LocalQiov type: QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); ... return bdrv_preadv(child, offset, &qiov);
signature.asc
Description: PGP signature