qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, benoit@irqsave.net, stefanha@redhat.com,
	mreitz@redhat.com
Subject: [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength()
Date: Thu, 26 Jun 2014 13:23:15 +0200	[thread overview]
Message-ID: <1403781805-17171-1-git-send-email-armbru@redhat.com> (raw)

Issues addressed in this series:

* BlockDriver method bdrv_getlength() generally returns -errno, but
  some implementations return -1 instead.  Fix them [PATCH 1].

* Frequent conversions between sectors and bytes complicate the code
  needlessly.  Clean up some [PATCH 2-7].

* 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].

* bdrv_get_geometry() hides errors.  Don't use it in places where
  errors should be detected [PATCH 9+10].

Issues not addressed:

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

  - Sector-based bdrv_get_geometry() gets partly replaced by new
    bdrv_nb_sectors(), still sector-based.

  - Some sector-based places get converted from bdrv_getlength() to
    bdrv_nb_sectors().  At least, this de-duplicates the conversion
    from bytes to sectors.

  - Two places get converted from bdrv_get_geometry() to
    bdrv_getlength().  Two baby steps forward.

* 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.

* Error handling is missing in places, but it's not always obvious
  whether errors can actually happen, and if yes, how to handle them.

* Several calls of bdrv_get_geometry() remain in hw/.  I wanted to
  replace them all, but ran out of steam.

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ît]
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]

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

 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(-)

-- 
1.9.3

             reply	other threads:[~2014-06-26 11:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-26 11:23 Markus Armbruster [this message]
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 01/10] raw-posix: Fix raw_getlength() to always return -errno on error Markus Armbruster
2014-07-07  7:55   ` Stefan Hajnoczi
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 02/10] block: New bdrv_nb_sectors() Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 03/10] block: Use bdrv_nb_sectors() in bdrv_make_zero() Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 04/10] block: Use bdrv_nb_sectors() in bdrv_aligned_preadv() Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 05/10] block: Use bdrv_nb_sectors() in bdrv_co_get_block_status() Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 06/10] block: Use bdrv_nb_sectors() in img_convert() Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 07/10] block: Use bdrv_nb_sectors() where sectors, not bytes are wanted Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 08/10] block: Drop superfluous aligning of bdrv_getlength()'s value Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 09/10] qemu-img: Make img_convert() get image size just once per image Markus Armbruster
2014-06-26 11:23 ` [Qemu-devel] [PATCH v5 10/10] block: Avoid bdrv_get_geometry() where errors should be detected Markus Armbruster
2014-07-04 13:08 ` [Qemu-devel] [PATCH v5 00/10] Clean up around bdrv_getlength() Markus Armbruster
2014-07-07  8:18 ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1403781805-17171-1-git-send-email-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=benoit@irqsave.net \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).