From: Kevin Wolf <kwolf@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH 2/5] block: New bdrv_nb_sectors()
Date: Mon, 12 May 2014 13:11:56 +0200 [thread overview]
Message-ID: <20140512111156.GB4371@noname.redhat.com> (raw)
In-Reply-To: <1399628898-3241-3-git-send-email-armbru@redhat.com>
Am 09.05.2014 um 11:48 hat Markus Armbruster geschrieben:
> A call to retrieve the image size converts between bytes and sectors
> several times:
>
> * BlockDriver method bdrv_getlength() returns bytes.
>
> * refresh_total_sectors() converts to sectors, rounding up, and stores
> in total_sectors.
>
> * bdrv_getlength() converts total_sectors back to bytes (now rounded
> up to a multiple of the sector size).
>
> * Callers wanting sectors rather bytes convert it right back.
> Example: bdrv_get_geometry().
>
> bdrv_nb_sectors() provides a way to omit the last two conversions.
> It's exactly bdrv_getlength() with the conversion to bytes omitted.
> It's functionally like bdrv_get_geometry() without its odd error
> handling.
>
> Reimplement bdrv_getlength() and bdrv_get_geometry() on top of
> bdrv_nb_sectors().
>
> The next patches will convert some users of bdrv_getlength() to
> bdrv_nb_sectors().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Is this really the right direction to move?
Generally I think we should be trying to move away from those arbitrary
units of 512 bytes (that are called sectors for some reason, but aren't
really related to either guest or host sector sizes), and towards
interfaces that use byte granularity.
So I would rather see bs->total_sectors becoming bs->total_bytes than
adding a new sector-based function.
Kevin
next prev parent reply other threads:[~2014-05-12 11:12 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-09 9:48 [Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength() Markus Armbruster
2014-05-09 9:48 ` [Qemu-devel] [PATCH 1/5] raw-posix: Fix raw_getlength() to always return -errno on error Markus Armbruster
2014-05-09 9:48 ` [Qemu-devel] [PATCH 2/5] block: New bdrv_nb_sectors() Markus Armbruster
2014-05-12 11:11 ` Kevin Wolf [this message]
2014-05-12 12:36 ` Markus Armbruster
2014-05-12 12:50 ` Eric Blake
2014-05-09 9:48 ` [Qemu-devel] [PATCH 3/5] block: Use bdrv_nb_sectors() when sectors, not bytes are wanted Markus Armbruster
2014-05-09 16:27 ` Markus Armbruster
2014-05-09 9:48 ` [Qemu-devel] [PATCH 4/5] block: Drop superfluous aligning of bdrv_getlength()'s value Markus Armbruster
2014-05-09 9:48 ` [Qemu-devel] [PATCH 5/5] block: Avoid bdrv_get_geometry() where errors should be detected Markus Armbruster
2014-05-09 12:29 ` [Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength() Stefan Hajnoczi
2014-05-09 15:30 ` 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=20140512111156.GB4371@noname.redhat.com \
--to=kwolf@redhat.com \
--cc=armbru@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).