From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40524) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X448g-0001lw-Rx for qemu-devel@nongnu.org; Mon, 07 Jul 2014 04:19:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X448b-0000nA-BN for qemu-devel@nongnu.org; Mon, 07 Jul 2014 04:19:06 -0400 Received: from mail-wg0-x22d.google.com ([2a00:1450:400c:c00::22d]:38692) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X448a-0000n4-W9 for qemu-devel@nongnu.org; Mon, 07 Jul 2014 04:19:01 -0400 Received: by mail-wg0-f45.google.com with SMTP id z12so2131329wgg.28 for ; Mon, 07 Jul 2014 01:19:00 -0700 (PDT) Date: Mon, 7 Jul 2014 10:18:57 +0200 From: Stefan Hajnoczi Message-ID: <20140707081857.GC7963@stefanha-thinkpad.redhat.com> References: <1403781805-17171-1-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="2/5bycvrmDh4d1IB" Content-Disposition: inline In-Reply-To: <1403781805-17171-1-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, mreitz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, benoit@irqsave.net --2/5bycvrmDh4d1IB Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 26, 2014 at 01:23:15PM +0200, Markus Armbruster wrote: > Issues addressed in this series: >=20 > * BlockDriver method bdrv_getlength() generally returns -errno, but > some implementations return -1 instead. Fix them [PATCH 1]. >=20 > * Frequent conversions between sectors and bytes complicate the code > needlessly. Clean up some [PATCH 2-7]. >=20 > * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but > some places appear to be confused about that, and align the result > up or down. Don't [PATCH 8]. >=20 > * bdrv_get_geometry() hides errors. Don't use it in places where > errors should be detected [PATCH 9+10]. >=20 > Issues not addressed: >=20 > * We want to move away from counting in arbitrary units of 512 bytes > we call "sector", even though it's not really related to either > guest or host sector size. My patches mostly move sideways: >=20 > - Sector-based bdrv_get_geometry() gets partly replaced by new > bdrv_nb_sectors(), still sector-based. >=20 > - Some sector-based places get converted from bdrv_getlength() to > bdrv_nb_sectors(). At least, this de-duplicates the conversion > from bytes to sectors. >=20 > - Two places get converted from bdrv_get_geometry() to > bdrv_getlength(). Two baby steps forward. >=20 > * There are quite a few literals left in the code where > BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be > used instead. >=20 > * Error handling is missing in places, but it's not always obvious > whether errors can actually happen, and if yes, how to handle them. >=20 > * Several calls of bdrv_get_geometry() remain in hw/. I wanted to > replace them all, but ran out of steam. >=20 > v5: > * Straightforward rebase, only 10/10 conflicted > v4: > * Trivially rebased > * Set ret on error paths in img_compare() and img_rebase() in PATCH 10 > [Beno=EEt] > v3: > * Trivially rebased > * Correct silly g_new() vs. g_new0() mistake in PATCH 09 [Max] > v2: > * Trivially rebased > * Correct silly bdrv_getlength() vs. bdrv_nb_sectors() mistake in > PATCH 03 > * Split PATCH 03 into 03-07 [Kevin] > * Conversion of bs_sectors to array in PATCH 05 had a subscript off by > one, fix [Kevin] > * Split PATCH 05 into 09-10 [Kevin] >=20 > Markus Armbruster (10): > raw-posix: Fix raw_getlength() to always return -errno on error > block: New bdrv_nb_sectors() > block: Use bdrv_nb_sectors() in bdrv_make_zero() > block: Use bdrv_nb_sectors() in bdrv_aligned_preadv() > block: Use bdrv_nb_sectors() in bdrv_co_get_block_status() > block: Use bdrv_nb_sectors() in img_convert() > block: Use bdrv_nb_sectors() where sectors, not bytes are wanted > block: Drop superfluous aligning of bdrv_getlength()'s value > qemu-img: Make img_convert() get image size just once per image > block: Avoid bdrv_get_geometry() where errors should be detected >=20 > block-migration.c | 9 +++-- > block.c | 81 +++++++++++++++++++++++------------------- > block/qapi.c | 14 +++++--- > block/qcow2.c | 3 +- > block/raw-posix.c | 28 +++++++++++---- > block/vmdk.c | 5 ++- > include/block/block.h | 1 + > qemu-img.c | 98 +++++++++++++++++++++++++++++++++++----------= ------ > 8 files changed, 152 insertions(+), 87 deletions(-) Thanks, applied Patches 2-10 to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan --2/5bycvrmDh4d1IB Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJTulfxAAoJEJykq7OBq3PI7nQIAJHGl9k9GdYtKInkrxQ1rFu8 DnBhezdjIcF7gkOvppBKLJslweFNwuNs0vTDwxwKT12Fbrp3AXR8N/NPm71oAg55 cCvyc0zZPN0dfsLbOTc7XNd33ZNmuZ70jzsBR9A+ZlKha0zmjETBTBSOofh5VZQZ n+C1XQm//L+AnRIvda/yKdsbWZvbzbhoVZ6Lio+2vutiTnf2xSia4JoqmxLsCvjF nI7brlN2rt7yv6sfp97DM2bg5Rdkp7MUXJ9GyA6CoYdEl34PR/Vvfkn3wcnj4i2G YzKSFBZSEHJTi86VUlvC7XSsXy92SeBFhRb/SYQHSAeUpVY9mbJcagoT1DEQSz0= =Fgf1 -----END PGP SIGNATURE----- --2/5bycvrmDh4d1IB--