From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59259) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEU96-0008NG-86 for qemu-devel@nongnu.org; Fri, 14 Feb 2014 20:34:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WEU91-00085k-5L for qemu-devel@nongnu.org; Fri, 14 Feb 2014 20:34:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:63907) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WEU90-00085R-Ta for qemu-devel@nongnu.org; Fri, 14 Feb 2014 20:34:15 -0500 Message-ID: <52FEC4AA.8060400@redhat.com> Date: Sat, 15 Feb 2014 02:36:42 +0100 From: Max Reitz MIME-Version: 1.0 References: <1392242799-16364-1-git-send-email-benoit.canet@irqsave.net> <1392242799-16364-5-git-send-email-benoit.canet@irqsave.net> In-Reply-To: <1392242799-16364-5-git-send-email-benoit.canet@irqsave.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V17 04/12] blkverify: Extract qemu_iovec_clone() and qemu_iovec_compare() from blkverify. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , stefanha@redhat.com On 12.02.2014 23:06, Beno=C3=AEt Canet wrote: > From: Beno=C3=AEt Canet > > qemu_iovec_compare() will be used to compare IOs vectors in quorum blkv= erify > mode. The patch extract these functions in order to factorize the code. "extracts" (just a sidenode: If I'm correcting (or thinking I'm=20 correcting) spelling mistakes, I'm just saying "Try to fix it if you're=20 doing a new version anyway", but I won't force you to; but if you don't=20 correct it, prepare for me pointing it out again ;-)) Reviewed-by: Max Reitz > Signed-off-by: Benoit Canet > Reviewed-by: Max Reitz > --- > block/blkverify.c | 108 +----------------------------------------= --------- > include/qemu-common.h | 2 + > util/iov.c | 106 +++++++++++++++++++++++++++++++++++++++++= ++++++++ > 3 files changed, 110 insertions(+), 106 deletions(-) > > diff --git a/block/blkverify.c b/block/blkverify.c > index b57da59..c86ad7a 100644 > --- a/block/blkverify.c > +++ b/block/blkverify.c > @@ -173,110 +173,6 @@ static int64_t blkverify_getlength(BlockDriverSta= te *bs) > return bdrv_getlength(s->test_file); > } > =20 > -/** > - * Check that I/O vector contents are identical > - * > - * @a: I/O vector > - * @b: I/O vector > - * @ret: Offset to first mismatching byte or -1 if match > - */ > -static ssize_t blkverify_iovec_compare(QEMUIOVector *a, QEMUIOVector *= b) > -{ > - int i; > - ssize_t offset =3D 0; > - > - assert(a->niov =3D=3D b->niov); > - for (i =3D 0; i < a->niov; i++) { > - size_t len =3D 0; > - uint8_t *p =3D (uint8_t *)a->iov[i].iov_base; > - uint8_t *q =3D (uint8_t *)b->iov[i].iov_base; > - > - assert(a->iov[i].iov_len =3D=3D b->iov[i].iov_len); > - while (len < a->iov[i].iov_len && *p++ =3D=3D *q++) { > - len++; > - } > - > - offset +=3D len; > - > - if (len !=3D a->iov[i].iov_len) { > - return offset; > - } > - } > - return -1; > -} > - > -typedef struct { > - int src_index; > - struct iovec *src_iov; > - void *dest_base; > -} IOVectorSortElem; > - > -static int sortelem_cmp_src_base(const void *a, const void *b) > -{ > - const IOVectorSortElem *elem_a =3D a; > - const IOVectorSortElem *elem_b =3D b; > - > - /* Don't overflow */ > - if (elem_a->src_iov->iov_base < elem_b->src_iov->iov_base) { > - return -1; > - } else if (elem_a->src_iov->iov_base > elem_b->src_iov->iov_base) = { > - return 1; > - } else { > - return 0; > - } > -} > - > -static int sortelem_cmp_src_index(const void *a, const void *b) > -{ > - const IOVectorSortElem *elem_a =3D a; > - const IOVectorSortElem *elem_b =3D b; > - > - return elem_a->src_index - elem_b->src_index; > -} > - > -/** > - * Copy contents of I/O vector > - * > - * The relative relationships of overlapping iovecs are preserved. Th= is is > - * necessary to ensure identical semantics in the cloned I/O vector. > - */ > -static void blkverify_iovec_clone(QEMUIOVector *dest, const QEMUIOVect= or *src, > - void *buf) > -{ > - IOVectorSortElem sortelems[src->niov]; > - void *last_end; > - int i; > - > - /* Sort by source iovecs by base address */ > - for (i =3D 0; i < src->niov; i++) { > - sortelems[i].src_index =3D i; > - sortelems[i].src_iov =3D &src->iov[i]; > - } > - qsort(sortelems, src->niov, sizeof(sortelems[0]), sortelem_cmp_src= _base); > - > - /* Allocate buffer space taking into account overlapping iovecs */ > - last_end =3D NULL; > - for (i =3D 0; i < src->niov; i++) { > - struct iovec *cur =3D sortelems[i].src_iov; > - ptrdiff_t rewind =3D 0; > - > - /* Detect overlap */ > - if (last_end && last_end > cur->iov_base) { > - rewind =3D last_end - cur->iov_base; > - } > - > - sortelems[i].dest_base =3D buf - rewind; > - buf +=3D cur->iov_len - MIN(rewind, cur->iov_len); > - last_end =3D MAX(cur->iov_base + cur->iov_len, last_end); > - } > - > - /* Sort by source iovec index and build destination iovec */ > - qsort(sortelems, src->niov, sizeof(sortelems[0]), sortelem_cmp_src= _index); > - for (i =3D 0; i < src->niov; i++) { > - qemu_iovec_add(dest, sortelems[i].dest_base, src->iov[i].iov_l= en); > - } > -} > - > static BlkverifyAIOCB *blkverify_aio_get(BlockDriverState *bs, bool i= s_write, > int64_t sector_num, QEMUIOVe= ctor *qiov, > int nb_sectors, > @@ -340,7 +236,7 @@ static void blkverify_aio_cb(void *opaque, int ret) > =20 > static void blkverify_verify_readv(BlkverifyAIOCB *acb) > { > - ssize_t offset =3D blkverify_iovec_compare(acb->qiov, &acb->raw_qi= ov); > + ssize_t offset =3D qemu_iovec_compare(acb->qiov, &acb->raw_qiov); > if (offset !=3D -1) { > blkverify_err(acb, "contents mismatch in sector %" PRId64, > acb->sector_num + (int64_t)(offset / BDRV_SECTO= R_SIZE)); > @@ -358,7 +254,7 @@ static BlockDriverAIOCB *blkverify_aio_readv(BlockD= riverState *bs, > acb->verify =3D blkverify_verify_readv; > acb->buf =3D qemu_blockalign(bs->file, qiov->size); > qemu_iovec_init(&acb->raw_qiov, acb->qiov->niov); > - blkverify_iovec_clone(&acb->raw_qiov, qiov, acb->buf); > + qemu_iovec_clone(&acb->raw_qiov, qiov, acb->buf); > =20 > bdrv_aio_readv(s->test_file, sector_num, qiov, nb_sectors, > blkverify_aio_cb, acb); > diff --git a/include/qemu-common.h b/include/qemu-common.h > index 5054836..d70783e 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -346,6 +346,8 @@ size_t qemu_iovec_from_buf(QEMUIOVector *qiov, size= _t offset, > const void *buf, size_t bytes); > size_t qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, > int fillc, size_t bytes); > +ssize_t qemu_iovec_compare(QEMUIOVector *a, QEMUIOVector *b); > +void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, voi= d *buf); > =20 > bool buffer_is_zero(const void *buf, size_t len); > =20 > diff --git a/util/iov.c b/util/iov.c > index bb46c04..03934da 100644 > --- a/util/iov.c > +++ b/util/iov.c > @@ -378,6 +378,112 @@ size_t qemu_iovec_memset(QEMUIOVector *qiov, size= _t offset, > return iov_memset(qiov->iov, qiov->niov, offset, fillc, bytes); > } > =20 > +/** > + * Check that I/O vector contents are identical > + * > + * The IO vectors must have the same structure (same length of all par= ts). > + * A typical usage is to compare vectors created with qemu_iovec_clone= (). > + * > + * @a: I/O vector > + * @b: I/O vector > + * @ret: Offset to first mismatching byte or -1 if match > + */ > +ssize_t qemu_iovec_compare(QEMUIOVector *a, QEMUIOVector *b) > +{ > + int i; > + ssize_t offset =3D 0; > + > + assert(a->niov =3D=3D b->niov); > + for (i =3D 0; i < a->niov; i++) { > + size_t len =3D 0; > + uint8_t *p =3D (uint8_t *)a->iov[i].iov_base; > + uint8_t *q =3D (uint8_t *)b->iov[i].iov_base; > + > + assert(a->iov[i].iov_len =3D=3D b->iov[i].iov_len); > + while (len < a->iov[i].iov_len && *p++ =3D=3D *q++) { > + len++; > + } > + > + offset +=3D len; > + > + if (len !=3D a->iov[i].iov_len) { > + return offset; > + } > + } > + return -1; > +} > + > +typedef struct { > + int src_index; > + struct iovec *src_iov; > + void *dest_base; > +} IOVectorSortElem; > + > +static int sortelem_cmp_src_base(const void *a, const void *b) > +{ > + const IOVectorSortElem *elem_a =3D a; > + const IOVectorSortElem *elem_b =3D b; > + > + /* Don't overflow */ > + if (elem_a->src_iov->iov_base < elem_b->src_iov->iov_base) { > + return -1; > + } else if (elem_a->src_iov->iov_base > elem_b->src_iov->iov_base) = { > + return 1; > + } else { > + return 0; > + } > +} > + > +static int sortelem_cmp_src_index(const void *a, const void *b) > +{ > + const IOVectorSortElem *elem_a =3D a; > + const IOVectorSortElem *elem_b =3D b; > + > + return elem_a->src_index - elem_b->src_index; > +} > + > +/** > + * Copy contents of I/O vector > + * > + * The relative relationships of overlapping iovecs are preserved. Th= is is > + * necessary to ensure identical semantics in the cloned I/O vector. > + */ > +void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, voi= d *buf) > +{ > + IOVectorSortElem sortelems[src->niov]; > + void *last_end; > + int i; > + > + /* Sort by source iovecs by base address */ > + for (i =3D 0; i < src->niov; i++) { > + sortelems[i].src_index =3D i; > + sortelems[i].src_iov =3D &src->iov[i]; > + } > + qsort(sortelems, src->niov, sizeof(sortelems[0]), sortelem_cmp_src= _base); > + > + /* Allocate buffer space taking into account overlapping iovecs */ > + last_end =3D NULL; > + for (i =3D 0; i < src->niov; i++) { > + struct iovec *cur =3D sortelems[i].src_iov; > + ptrdiff_t rewind =3D 0; > + > + /* Detect overlap */ > + if (last_end && last_end > cur->iov_base) { > + rewind =3D last_end - cur->iov_base; > + } > + > + sortelems[i].dest_base =3D buf - rewind; > + buf +=3D cur->iov_len - MIN(rewind, cur->iov_len); > + last_end =3D MAX(cur->iov_base + cur->iov_len, last_end); > + } > + > + /* Sort by source iovec index and build destination iovec */ > + qsort(sortelems, src->niov, sizeof(sortelems[0]), sortelem_cmp_src= _index); > + for (i =3D 0; i < src->niov; i++) { > + qemu_iovec_add(dest, sortelems[i].dest_base, src->iov[i].iov_l= en); > + } > +} > + > size_t iov_discard_front(struct iovec **iov, unsigned int *iov_cnt, > size_t bytes) > {