* [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based
@ 2017-04-18 1:33 Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters() Eric Blake
` (32 more replies)
0 siblings, 33 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow
There are patches floating around to add NBD_CMD_BLOCK_STATUS,
but NBD wants to report status on byte granularity (even if the
reporting will probably be naturally aligned to sectors or even
much higher levels). I've therefore started the task of
converting our block status code to report at a byte granularity
rather than sectors.
This is part three of that conversion: dirty-bitmap. Earlier parts
have been (mostly) reviewed, for bdrv_is_allocated and dirty-bitmaps.
Perhaps I could have split this in two; patches 1-10 vs. 11-31 make
a nice division of labor.
Available as a tag at:
git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-status-v1
It requires the following (v1 of bdrv_is_allocated, v1 of dirty-bitmap,
v9 of blkdebug, and Max's block-next tree):
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01995.html
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01723.html
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01298.html
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02163.html
The diffstat shows no net change in total lines - but I know that the
new code has more lines of comments than the old ;)
I still haven't felt like tackling the task of rewriting migration/block.c
to use bytes (instead of sectors) everywhere - that might give another
net win in lines of code and legibility, but I also know it would
conflict with some of the refactoring work that Juan is currently
posting for review.
Eric Blake (31):
block: Drop unused bdrv_round_sectors_to_clusters()
block: Make bdrv_round_to_clusters() signature more useful
qcow2: Switch is_zero_sectors() to byte-based
block: Switch bdrv_make_zero() to byte-based
qemu-img: Switch get_block_status() to byte-based
block: Convert bdrv_get_block_status() to bytes
block: Switch bdrv_co_get_block_status() to byte-based
block: Switch BdrvCoGetBlockStatusData to byte-based
block: Switch bdrv_co_get_block_status_above() to byte-based
block: Convert bdrv_get_block_status_above() to bytes
block: Add .bdrv_co_block_status() callback
commit: Switch to .bdrv_co_block_status()
file-posix: Switch to .bdrv_co_block_status()
gluster: Switch to .bdrv_co_block_status()
iscsi: Switch cluster_sectors to byte-based
iscsi: Switch iscsi_allocmap_update() to byte-based
iscsi: Switch to .bdrv_co_block_status()
mirror: Switch to .bdrv_co_block_status()
null: Switch to .bdrv_co_block_status()
parallels: Switch to .bdrv_co_block_status()
qcow: Switch to .bdrv_co_block_status()
qcow2: Switch to .bdrv_co_block_status()
qed: Switch to .bdrv_co_block_status()
raw: Switch to .bdrv_co_block_status()
sheepdog: Switch to .bdrv_co_block_status()
vdi: Avoid bitrot of debugging code
vdi: Switch to .bdrv_co_block_status()
vmdk: Switch to .bdrv_co_block_status()
vpc: Switch to .bdrv_co_block_status()
vvfat: Switch to .bdrv_co_block_status()
block: Drop unused .bdrv_co_get_block_status()
include/block/block.h | 33 +++----
include/block/block_int.h | 13 ++-
block/commit.c | 10 +-
block/file-posix.c | 47 +++++----
block/gluster.c | 47 +++++----
block/io.c | 243 ++++++++++++++++++++++------------------------
block/iscsi.c | 144 ++++++++++++++-------------
block/mirror.c | 33 +++----
block/null.c | 21 ++--
block/parallels.c | 15 +--
block/qcow.c | 22 +++--
block/qcow2-cluster.c | 2 +-
block/qcow2.c | 51 +++++-----
block/qed.c | 22 ++---
block/raw-format.c | 16 +--
block/sheepdog.c | 23 +++--
block/vdi.c | 37 ++++---
block/vmdk.c | 24 ++---
block/vpc.c | 31 +++---
block/vvfat.c | 12 +--
qemu-img.c | 70 ++++++-------
block/trace-events | 2 +-
22 files changed, 459 insertions(+), 459 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-26 21:41 ` John Snow
2017-04-18 1:33 ` [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful Eric Blake
` (31 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
Now that the last user [mirror_iteration()] has converted to using
bytes, we no longer need a function to round sectors to clusters.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block.h | 4 ----
block/io.c | 21 ---------------------
2 files changed, 25 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 740cb86..86ad511 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -467,10 +467,6 @@ const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
int bdrv_get_flags(BlockDriverState *bs);
int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
-void bdrv_round_sectors_to_clusters(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- int64_t *cluster_sector_num,
- int *cluster_nb_sectors);
void bdrv_round_to_clusters(BlockDriverState *bs,
int64_t offset, unsigned int bytes,
int64_t *cluster_offset,
diff --git a/block/io.c b/block/io.c
index d22d35f..d61a906 100644
--- a/block/io.c
+++ b/block/io.c
@@ -419,27 +419,6 @@ static void mark_request_serialising(BdrvTrackedRequest *req, uint64_t align)
}
/**
- * Round a region to cluster boundaries (sector-based)
- */
-void bdrv_round_sectors_to_clusters(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- int64_t *cluster_sector_num,
- int *cluster_nb_sectors)
-{
- BlockDriverInfo bdi;
-
- if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
- *cluster_sector_num = sector_num;
- *cluster_nb_sectors = nb_sectors;
- } else {
- int64_t c = bdi.cluster_size / BDRV_SECTOR_SIZE;
- *cluster_sector_num = QEMU_ALIGN_DOWN(sector_num, c);
- *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
- nb_sectors, c);
- }
-}
-
-/**
* Round a region to cluster boundaries
*/
void bdrv_round_to_clusters(BlockDriverState *bs,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters() Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-26 21:41 ` John Snow
2017-04-18 1:33 ` [Qemu-devel] [PATCH 03/31] qcow2: Switch is_zero_sectors() to byte-based Eric Blake
` (30 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz, Jeff Cody
In the process of converting sector-based interfaces to bytes,
I'm finding it easier to represent a byte count as a 64-bit
integer at the block layer (even if we are internally capped
by SIZE_MAX or even INT_MAX for individual transactions, it's
still nicer to not have to worry about truncation/overflow
issues on as many variables). Update the signature of
bdrv_round_to_clusters() to uniformly use uint64_t, matching
the signature already chosen for bdrv_is_allocated, and
adjust clients according to the required fallout.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block.h | 4 ++--
block/io.c | 7 ++++---
block/mirror.c | 9 ++++-----
block/trace-events | 2 +-
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 86ad511..eed1330 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -468,9 +468,9 @@ int bdrv_get_flags(BlockDriverState *bs);
int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
void bdrv_round_to_clusters(BlockDriverState *bs,
- int64_t offset, unsigned int bytes,
+ int64_t offset, int64_t bytes,
int64_t *cluster_offset,
- unsigned int *cluster_bytes);
+ int64_t *cluster_bytes);
const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
void bdrv_get_backing_filename(BlockDriverState *bs,
diff --git a/block/io.c b/block/io.c
index d61a906..07165dc 100644
--- a/block/io.c
+++ b/block/io.c
@@ -422,9 +422,9 @@ static void mark_request_serialising(BdrvTrackedRequest *req, uint64_t align)
* Round a region to cluster boundaries
*/
void bdrv_round_to_clusters(BlockDriverState *bs,
- int64_t offset, unsigned int bytes,
+ int64_t offset, int64_t bytes,
int64_t *cluster_offset,
- unsigned int *cluster_bytes)
+ int64_t *cluster_bytes)
{
BlockDriverInfo bdi;
@@ -920,7 +920,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
struct iovec iov;
QEMUIOVector bounce_qiov;
int64_t cluster_offset;
- unsigned int cluster_bytes;
+ int64_t cluster_bytes;
size_t skip_bytes;
int ret;
@@ -941,6 +941,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
cluster_offset, cluster_bytes);
+ assert(cluster_bytes < SIZE_MAX);
iov.iov_len = cluster_bytes;
iov.iov_base = bounce_buffer = qemu_try_blockalign(bs, iov.iov_len);
if (bounce_buffer == NULL) {
diff --git a/block/mirror.c b/block/mirror.c
index 846e392..2510793 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -177,7 +177,7 @@ static void mirror_read_complete(void *opaque, int ret)
/* Clip bytes relative to offset to not exceed end-of-file */
static inline void mirror_clip_bytes(MirrorBlockJob *s,
int64_t offset,
- unsigned int *bytes)
+ int64_t *bytes)
{
*bytes = MIN(*bytes, s->bdev_length - offset);
}
@@ -190,10 +190,9 @@ static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset,
bool need_cow;
int ret = 0;
int64_t align_offset = *offset;
- unsigned int align_bytes = *bytes;
+ int64_t align_bytes = *bytes;
int max_bytes = s->granularity * s->max_iov;
- assert(*bytes < INT_MAX);
need_cow = !test_bit(*offset / s->granularity, s->cow_bitmap);
need_cow |= !test_bit((*offset + *bytes - 1) / s->granularity,
s->cow_bitmap);
@@ -384,7 +383,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
while (nb_chunks > 0 && offset < s->bdev_length) {
int64_t ret;
int io_sectors;
- unsigned int io_bytes;
+ int64_t io_bytes;
int64_t io_bytes_acct;
BlockDriverState *file;
enum MirrorMethod {
@@ -410,7 +409,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
io_bytes = s->granularity;
} else if (ret >= 0 && !(ret & BDRV_BLOCK_DATA)) {
int64_t target_offset;
- unsigned int target_bytes;
+ int64_t target_bytes;
bdrv_round_to_clusters(blk_bs(s->target), offset, io_bytes,
&target_offset, &target_bytes);
if (target_offset == offset &&
diff --git a/block/trace-events b/block/trace-events
index 04f6463..2301a50 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -15,7 +15,7 @@ bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs
bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags %#x"
-bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, unsigned int cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %u"
+bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, unsigned int cluster_bytes) "bs %p offset %"PRId64" bytes %"PRId64" cluster_offset %"PRId64" cluster_bytes %u"
# block/stream.c
stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 03/31] qcow2: Switch is_zero_sectors() to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters() Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 04/31] block: Switch bdrv_make_zero() " Eric Blake
` (29 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Convert another internal
function (no semantic change), and rename it to is_zero_above() in
the process.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 4d34610..fe4ccf6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2442,23 +2442,30 @@ finish:
}
-static bool is_zero_sectors(BlockDriverState *bs, int64_t start,
- uint32_t count)
+static bool is_zero_above(BlockDriverState *bs, int64_t offset, int64_t bytes)
{
int nr;
BlockDriverState *file;
int64_t res;
+ int64_t start;
- if (start + count > bs->total_sectors) {
- count = bs->total_sectors - start;
+ /* Widen to sector boundaries, then clamp to image length, before
+ * checking status of underlying sectors */
+ start = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
+ bytes = QEMU_ALIGN_UP(offset + bytes, BDRV_SECTOR_SIZE) - start;
+
+ if (start + bytes > bs->total_sectors * BDRV_SECTOR_SIZE) {
+ bytes = bs->total_sectors * BDRV_SECTOR_SIZE - offset;
}
- if (!count) {
+ if (!bytes) {
return true;
}
- res = bdrv_get_block_status_above(bs, NULL, start, count,
+ res = bdrv_get_block_status_above(bs, NULL, start >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS,
&nr, &file);
- return res >= 0 && (res & BDRV_BLOCK_ZERO) && nr == count;
+ return res >= 0 && (res & BDRV_BLOCK_ZERO) &&
+ nr * BDRV_SECTOR_SIZE == bytes;
}
static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
@@ -2476,24 +2483,21 @@ static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
}
if (head || tail) {
- int64_t cl_start = (offset - head) >> BDRV_SECTOR_BITS;
uint64_t off;
unsigned int nr;
assert(head + count <= s->cluster_size);
/* check whether remainder of cluster already reads as zero */
- if (!(is_zero_sectors(bs, cl_start,
- DIV_ROUND_UP(head, BDRV_SECTOR_SIZE)) &&
- is_zero_sectors(bs, (offset + count) >> BDRV_SECTOR_BITS,
- DIV_ROUND_UP(-tail & (s->cluster_size - 1),
- BDRV_SECTOR_SIZE)))) {
+ if (!(is_zero_above(bs, offset - head, head) &&
+ is_zero_above(bs, offset + count,
+ tail ? s->cluster_size - tail : 0))) {
return -ENOTSUP;
}
qemu_co_mutex_lock(&s->lock);
/* We can have new write after previous check */
- offset = cl_start << BDRV_SECTOR_BITS;
+ offset = QEMU_ALIGN_DOWN(offset, s->cluster_size);
count = s->cluster_size;
nr = s->cluster_size;
ret = qcow2_get_cluster_offset(bs, offset, &nr, &off);
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 04/31] block: Switch bdrv_make_zero() to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (2 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 03/31] qcow2: Switch is_zero_sectors() to byte-based Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 05/31] qemu-img: Switch get_block_status() " Eric Blake
` (28 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Change the internal
loop iteration of zeroing a device to track by bytes instead of
sectors (although we are still guaranteed that we iterate by steps
that are sector-aligned).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/io.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/block/io.c b/block/io.c
index 07165dc..1f8ae81 100644
--- a/block/io.c
+++ b/block/io.c
@@ -666,39 +666,39 @@ int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
*/
int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
{
- int64_t target_sectors, ret, nb_sectors, sector_num = 0;
+ int64_t target_size, ret, bytes, offset = 0;
BlockDriverState *bs = child->bs;
BlockDriverState *file;
- int n;
+ int n; /* sectors */
- target_sectors = bdrv_nb_sectors(bs);
- if (target_sectors < 0) {
- return target_sectors;
+ target_size = bdrv_getlength(bs);
+ if (target_size < 0) {
+ return target_size;
}
for (;;) {
- nb_sectors = MIN(target_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS);
- if (nb_sectors <= 0) {
+ bytes = MIN(target_size - offset, BDRV_REQUEST_MAX_BYTES);
+ if (bytes <= 0) {
return 0;
}
- ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n, &file);
+ ret = bdrv_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS, &n, &file);
if (ret < 0) {
- error_report("error getting block status at sector %" PRId64 ": %s",
- sector_num, strerror(-ret));
+ error_report("error getting block status at offset %" PRId64 ": %s",
+ offset, strerror(-ret));
return ret;
}
if (ret & BDRV_BLOCK_ZERO) {
- sector_num += n;
+ offset += n * BDRV_SECTOR_BITS;
continue;
}
- ret = bdrv_pwrite_zeroes(child, sector_num << BDRV_SECTOR_BITS,
- n << BDRV_SECTOR_BITS, flags);
+ ret = bdrv_pwrite_zeroes(child, offset, n * BDRV_SECTOR_SIZE, flags);
if (ret < 0) {
- error_report("error writing zeroes at sector %" PRId64 ": %s",
- sector_num, strerror(-ret));
+ error_report("error writing zeroes at offset %" PRId64 ": %s",
+ offset, strerror(-ret));
return ret;
}
- sector_num += n;
+ offset += n * BDRV_SECTOR_SIZE;
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 05/31] qemu-img: Switch get_block_status() to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (3 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 04/31] block: Switch bdrv_make_zero() " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 06/31] block: Convert bdrv_get_block_status() to bytes Eric Blake
` (27 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Continue by converting
an internal function (no semantic change), and simplifying its
caller accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-img.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index d96b4d6..8cb5165 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2662,14 +2662,16 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e,
}
}
-static int get_block_status(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, MapEntry *e)
+static int get_block_status(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, MapEntry *e)
{
int64_t ret;
int depth;
BlockDriverState *file;
bool has_offset;
+ int nb_sectors = bytes >> BDRV_SECTOR_BITS;
+ assert(bytes < INT_MAX);
/* As an optimization, we could cache the current range of unallocated
* clusters in each file of the chain, and avoid querying the same
* range repeatedly.
@@ -2677,8 +2679,8 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
depth = 0;
for (;;) {
- ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &nb_sectors,
- &file);
+ ret = bdrv_get_block_status(bs, offset >> BDRV_SECTOR_BITS, nb_sectors,
+ &nb_sectors, &file);
if (ret < 0) {
return ret;
}
@@ -2698,7 +2700,7 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID);
*e = (MapEntry) {
- .start = sector_num * BDRV_SECTOR_SIZE,
+ .start = offset,
.length = nb_sectors * BDRV_SECTOR_SIZE,
.data = !!(ret & BDRV_BLOCK_DATA),
.zero = !!(ret & BDRV_BLOCK_ZERO),
@@ -2823,16 +2825,12 @@ static int img_map(int argc, char **argv)
length = blk_getlength(blk);
while (curr.start + curr.length < length) {
- int64_t nsectors_left;
- int64_t sector_num;
- int n;
-
- sector_num = (curr.start + curr.length) >> BDRV_SECTOR_BITS;
+ int64_t offset = curr.start + curr.length;
+ int64_t n;
/* Probe up to 1 GiB at a time. */
- nsectors_left = DIV_ROUND_UP(length, BDRV_SECTOR_SIZE) - sector_num;
- n = MIN(1 << (30 - BDRV_SECTOR_BITS), nsectors_left);
- ret = get_block_status(bs, sector_num, n, &next);
+ n = QEMU_ALIGN_DOWN(MIN(1 << 30, length - offset), BDRV_SECTOR_SIZE);
+ ret = get_block_status(bs, offset, n, &next);
if (ret < 0) {
error_report("Could not read file metadata: %s", strerror(-ret));
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 06/31] block: Convert bdrv_get_block_status() to bytes
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (4 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 05/31] qemu-img: Switch get_block_status() " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 21:34 ` [Qemu-devel] [PATCH v1.5 06.5/31] fixup! " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 07/31] block: Switch bdrv_co_get_block_status() to byte-based Eric Blake
` (26 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. In the common case, allocation is unlikely to ever use
values that are not naturally sector-aligned, but it is possible
that byte-based values will let us be more precise about allocation
at the end of an unaligned file that can do byte-based access.
Changing the name of the function from bdrv_get_block_status() to
bdrv_block_status() ensures that the compiler enforces that all
callers are updated. For now, the io.c layer still assert()s that
all callers are sector-aligned, but that can be relaxed when a later
patch implements byte-based block status in the drivers.
Note that we have an inherent limitation in the BDRV_BLOCK_* return
values: BDRV_BLOCK_OFFSET_VALID can only return the start of a
sector, even if we later relax the interface to query for the status
starting at an intermediate byte; document the obvious interpretation
that valid offsets are always sector-relative.
Therefore, for the most part this patch is just the addition of scaling
at the callers followed by inverse scaling at bdrv_block_status(). But
some code, particularly bdrv_is_allocated(), gets a lot simpler because
it no longer has to mess with sectors; also, it is now possible to pass
NULL if the caller does not care how much of the image is allocated
beyond the initial offset.
For ease of review, bdrv_get_block_status_above() will be tackled
separately.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block.h | 15 +++++++++------
block/io.c | 51 ++++++++++++++++++++++++++-------------------------
block/qcow2-cluster.c | 2 +-
qemu-img.c | 19 ++++++++++---------
4 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index eed1330..b9e7281 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -121,10 +121,10 @@ typedef struct HDGeometry {
/*
* Allocation status flags
- * BDRV_BLOCK_DATA: data is read from a file returned by bdrv_get_block_status.
+ * BDRV_BLOCK_DATA: data is read from a file returned by bdrv_block_status.
* BDRV_BLOCK_ZERO: sectors read as zero
* BDRV_BLOCK_OFFSET_VALID: sector stored as raw data in a file returned by
- * bdrv_get_block_status.
+ * bdrv_block_status.
* BDRV_BLOCK_ALLOCATED: the content of the block is determined by this
* layer (as opposed to the backing file)
* BDRV_BLOCK_RAW: used internally to indicate that the request
@@ -132,7 +132,10 @@ typedef struct HDGeometry {
* should look in bs->file directly.
*
* If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in
- * bs->file where sector data can be read from as raw data.
+ * bs->file that begins the sector containing the address in question,
+ * where the sector can be read from as raw data with individual bytes
+ * at the same sector-relative locations (and thus, this bit cannot be
+ * set for mappings which are not equivalent modulo 512).
*
* DATA == 0 && ZERO == 0 means that data is read from backing_hd if present.
*
@@ -414,9 +417,9 @@ int bdrv_has_zero_init_1(BlockDriverState *bs);
int bdrv_has_zero_init(BlockDriverState *bs);
bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs);
bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
-int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file);
+int64_t bdrv_block_status(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, int64_t *pnum,
+ BlockDriverState **file);
int64_t bdrv_get_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
int64_t sector_num,
diff --git a/block/io.c b/block/io.c
index 1f8ae81..5cdb1f0 100644
--- a/block/io.c
+++ b/block/io.c
@@ -669,7 +669,6 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
int64_t target_size, ret, bytes, offset = 0;
BlockDriverState *bs = child->bs;
BlockDriverState *file;
- int n; /* sectors */
target_size = bdrv_getlength(bs);
if (target_size < 0) {
@@ -681,24 +680,23 @@ int bdrv_make_zero(BdrvChild *child, BdrvRequestFlags flags)
if (bytes <= 0) {
return 0;
}
- ret = bdrv_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
- bytes >> BDRV_SECTOR_BITS, &n, &file);
+ ret = bdrv_block_status(bs, offset, bytes, &bytes, &file);
if (ret < 0) {
error_report("error getting block status at offset %" PRId64 ": %s",
offset, strerror(-ret));
return ret;
}
if (ret & BDRV_BLOCK_ZERO) {
- offset += n * BDRV_SECTOR_BITS;
+ offset += bytes;
continue;
}
- ret = bdrv_pwrite_zeroes(child, offset, n * BDRV_SECTOR_SIZE, flags);
+ ret = bdrv_pwrite_zeroes(child, offset, bytes, flags);
if (ret < 0) {
error_report("error writing zeroes at offset %" PRId64 ": %s",
offset, strerror(-ret));
return ret;
}
- offset += n * BDRV_SECTOR_SIZE;
+ offset += bytes;
}
}
@@ -1754,9 +1752,13 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
}
if (ret & BDRV_BLOCK_RAW) {
+ int64_t bytes = *pnum * BDRV_SECTOR_SIZE;
+
assert(ret & BDRV_BLOCK_OFFSET_VALID);
- ret = bdrv_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
- *pnum, pnum, file);
+ ret = bdrv_block_status(*file, ret & BDRV_BLOCK_OFFSET_MASK,
+ bytes, &bytes, file);
+ assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
+ *pnum = bytes >> BDRV_SECTOR_BITS;
goto out;
}
@@ -1874,35 +1876,34 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
return data.ret;
}
-int64_t bdrv_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+int64_t bdrv_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *pnum,
+ BlockDriverState **file)
{
- return bdrv_get_block_status_above(bs, backing_bs(bs),
- sector_num, nb_sectors, pnum, file);
+ int64_t ret;
+ int n;
+
+ assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
+ assert(bytes <= BDRV_REQUEST_MAX_BYTES);
+ ret = bdrv_get_block_status_above(bs, backing_bs(bs),
+ offset >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS, &n, file);
+ if (pnum) {
+ *pnum = n * BDRV_SECTOR_SIZE;
+ }
+ return ret;
}
int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
int64_t bytes, int64_t *pnum)
{
BlockDriverState *file;
- int64_t sector_num = offset >> BDRV_SECTOR_BITS;
- int nb_sectors = bytes >> BDRV_SECTOR_BITS;
int64_t ret;
- int psectors;
- assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
- assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE) &&
- bytes < INT_MAX * BDRV_SECTOR_SIZE);
- ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &psectors,
- &file);
+ ret = bdrv_block_status(bs, offset, bytes, pnum, &file);
if (ret < 0) {
return ret;
}
- if (pnum) {
- *pnum = psectors * BDRV_SECTOR_SIZE;
- }
return !!(ret & BDRV_BLOCK_ALLOCATED);
}
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index bc59cb8..e5feb89 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1465,7 +1465,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
* cluster is already marked as zero, or if it's unallocated and we
* don't have a backing file.
*
- * TODO We might want to use bdrv_get_block_status(bs) here, but we're
+ * TODO We might want to use bdrv_block_status(bs) here, but we're
* holding s->lock, so that doesn't work today.
*
* If full_discard is true, the sector should not read back as zeroes,
diff --git a/qemu-img.c b/qemu-img.c
index 8cb5165..7ed77d5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1558,12 +1558,16 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
if (s->sector_next_status <= sector_num) {
BlockDriverState *file;
- ret = bdrv_get_block_status(blk_bs(s->src[src_cur]),
- sector_num - src_cur_offset,
- n, &n, &file);
+ int64_t count = n * BDRV_SECTOR_SIZE;
+
+ ret = bdrv_block_status(blk_bs(s->src[src_cur]),
+ (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE,
+ count, &count, &file);
if (ret < 0) {
return ret;
}
+ assert(QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE));
+ n = count >> BDRV_SECTOR_BITS;
if (ret & BDRV_BLOCK_ZERO) {
s->status = BLK_ZERO;
@@ -2669,9 +2673,7 @@ static int get_block_status(BlockDriverState *bs, int64_t offset,
int depth;
BlockDriverState *file;
bool has_offset;
- int nb_sectors = bytes >> BDRV_SECTOR_BITS;
- assert(bytes < INT_MAX);
/* As an optimization, we could cache the current range of unallocated
* clusters in each file of the chain, and avoid querying the same
* range repeatedly.
@@ -2679,12 +2681,11 @@ static int get_block_status(BlockDriverState *bs, int64_t offset,
depth = 0;
for (;;) {
- ret = bdrv_get_block_status(bs, offset >> BDRV_SECTOR_BITS, nb_sectors,
- &nb_sectors, &file);
+ ret = bdrv_block_status(bs, offset, bytes, &bytes, &file);
if (ret < 0) {
return ret;
}
- assert(nb_sectors);
+ assert(bytes);
if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
break;
}
@@ -2701,7 +2702,7 @@ static int get_block_status(BlockDriverState *bs, int64_t offset,
*e = (MapEntry) {
.start = offset,
- .length = nb_sectors * BDRV_SECTOR_SIZE,
+ .length = bytes,
.data = !!(ret & BDRV_BLOCK_DATA),
.zero = !!(ret & BDRV_BLOCK_ZERO),
.offset = ret & BDRV_BLOCK_OFFSET_MASK,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 07/31] block: Switch bdrv_co_get_block_status() to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (5 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 06/31] block: Convert bdrv_get_block_status() to bytes Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 08/31] block: Switch BdrvCoGetBlockStatusData " Eric Blake
` (25 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Convert another internal
function (no semantic change), and as with its public counterpart,
rename to bdrv_co_block_status() to make the compiler enforce that
we catch all uses. For now, we assert that callers still pass
aligned data, but ultimately, this will be the function where we
hand off to a byte-based driver callback, and will eventually need
to add logic to ensure we round calls according to the driver's
request_alignment then touch up the result handed back to the
caller, if we start permitting a caller to pass unaligned offsets.
While at it, adjust the function to accept NULL for pnum or file,
while still guaranteeing the driver callback has a non-NULL pointer
to write into.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/io.c | 88 +++++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/block/io.c b/block/io.c
index 5cdb1f0..e804bdd 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1696,69 +1696,83 @@ typedef struct BdrvCoGetBlockStatusData {
* Drivers not implementing the functionality are assumed to not support
* backing files, hence all their sectors are reported as allocated.
*
- * If 'sector_num' is beyond the end of the disk image the return value is 0
+ * If 'offset' is beyond the end of the disk image the return value is 0
* and 'pnum' is set to 0.
*
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
- * allocated/unallocated state.
+ * 'pnum' is set to the number of bytes (including and immediately following
+ * the specified offset) that are known to be in the same
+ * allocated/unallocated state. It may be NULL.
*
- * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
+ * 'bytes' is the max value 'pnum' should be set to. If bytes goes
* beyond the end of the disk image it will be clamped.
*
- * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, 'file'
- * points to the BDS which the sector range is allocated in.
+ * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, and
+ * 'file' is not NULL, then *file points to the BDS which owns the allocated
+ * sector that contains offset.
*/
-static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes,
+ int64_t *pnum,
+ BlockDriverState **file)
{
- int64_t total_sectors;
- int64_t n;
+ int64_t total_size;
+ int64_t n; /* bytes */
int64_t ret, ret2;
+ int count; /* sectors */
+ BlockDriverState *tmp_file;
- total_sectors = bdrv_nb_sectors(bs);
- if (total_sectors < 0) {
- return total_sectors;
+ total_size = bdrv_getlength(bs);
+ if (total_size < 0) {
+ return total_size;
}
- if (sector_num >= total_sectors) {
+ if (!pnum) {
+ pnum = &n;
+ }
+ if (offset >= total_size) {
*pnum = 0;
return 0;
}
- n = total_sectors - sector_num;
- if (n < nb_sectors) {
- nb_sectors = n;
+ n = total_size - offset;
+ if (n < bytes) {
+ bytes = n;
}
if (!bs->drv->bdrv_co_get_block_status) {
- *pnum = nb_sectors;
+ *pnum = bytes;
ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
if (bs->drv->protocol_name) {
- ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE);
+ ret |= BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
}
return ret;
}
+ if (!file) {
+ file = &tmp_file;
+ }
*file = NULL;
bdrv_inc_in_flight(bs);
- ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum,
+ /* TODO: Rather than require aligned offsets, we could instead
+ * round to the driver's request_alignment here, then touch up
+ * count afterwards back to the caller's expectations. But first
+ * we want to switch the driver callback to likewise be
+ * byte-based. */
+ assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
+ ret = bs->drv->bdrv_co_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS, &count,
file);
if (ret < 0) {
*pnum = 0;
goto out;
}
+ *pnum = count * BDRV_SECTOR_SIZE;
if (ret & BDRV_BLOCK_RAW) {
- int64_t bytes = *pnum * BDRV_SECTOR_SIZE;
-
assert(ret & BDRV_BLOCK_OFFSET_VALID);
ret = bdrv_block_status(*file, ret & BDRV_BLOCK_OFFSET_MASK,
- bytes, &bytes, file);
- assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
- *pnum = bytes >> BDRV_SECTOR_BITS;
+ *pnum, pnum, file);
+ assert(QEMU_IS_ALIGNED(*pnum, BDRV_SECTOR_SIZE));
goto out;
}
@@ -1769,8 +1783,8 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
ret |= BDRV_BLOCK_ZERO;
} else if (bs->backing) {
BlockDriverState *bs2 = bs->backing->bs;
- int64_t nb_sectors2 = bdrv_nb_sectors(bs2);
- if (nb_sectors2 >= 0 && sector_num >= nb_sectors2) {
+ int64_t size2 = bdrv_getlength(bs2);
+ if (size2 >= 0 && offset >= size2) {
ret |= BDRV_BLOCK_ZERO;
}
}
@@ -1779,11 +1793,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
if (*file && *file != bs &&
(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
(ret & BDRV_BLOCK_OFFSET_VALID)) {
- BlockDriverState *file2;
- int file_pnum;
+ int64_t file_pnum;
- ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
- *pnum, &file_pnum, &file2);
+ ret2 = bdrv_co_block_status(*file, ret & BDRV_BLOCK_OFFSET_MASK,
+ *pnum, &file_pnum, NULL);
if (ret2 >= 0) {
/* Ignore errors. This is just providing extra information, it
* is useful but not necessary.
@@ -1818,7 +1831,12 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
assert(bs != base);
for (p = bs; p != base; p = backing_bs(p)) {
- ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file);
+ int64_t count;
+
+ ret = bdrv_co_block_status(p, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE, &count,
+ file);
+ *pnum = count >> BDRV_SECTOR_BITS;
if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
break;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 08/31] block: Switch BdrvCoGetBlockStatusData to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (6 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 07/31] block: Switch bdrv_co_get_block_status() to byte-based Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 09/31] block: Switch bdrv_co_get_block_status_above() " Eric Blake
` (24 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Convert another internal
type (no semantic change), and rename it to match the corresponding
public function rename.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/io.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/block/io.c b/block/io.c
index e804bdd..f7ece8d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1680,16 +1680,16 @@ int bdrv_flush_all(void)
}
-typedef struct BdrvCoGetBlockStatusData {
+typedef struct BdrvCoBlockStatusData {
BlockDriverState *bs;
BlockDriverState *base;
BlockDriverState **file;
- int64_t sector_num;
- int nb_sectors;
- int *pnum;
+ int64_t offset;
+ int64_t bytes;
+ int64_t *pnum;
int64_t ret;
bool done;
-} BdrvCoGetBlockStatusData;
+} BdrvCoBlockStatusData;
/*
* Returns the allocation status of the specified sectors.
@@ -1850,13 +1850,15 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
/* Coroutine wrapper for bdrv_get_block_status_above() */
static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque)
{
- BdrvCoGetBlockStatusData *data = opaque;
+ BdrvCoBlockStatusData *data = opaque;
+ int n;
data->ret = bdrv_co_get_block_status_above(data->bs, data->base,
- data->sector_num,
- data->nb_sectors,
- data->pnum,
+ data->offset >> BDRV_SECTOR_BITS,
+ data->bytes >> BDRV_SECTOR_BITS,
+ &n,
data->file);
+ *data->pnum = n * BDRV_SECTOR_SIZE;
data->done = true;
}
@@ -1872,13 +1874,14 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
BlockDriverState **file)
{
Coroutine *co;
- BdrvCoGetBlockStatusData data = {
+ int64_t n;
+ BdrvCoBlockStatusData data = {
.bs = bs,
.base = base,
.file = file,
- .sector_num = sector_num,
- .nb_sectors = nb_sectors,
- .pnum = pnum,
+ .offset = sector_num * BDRV_SECTOR_SIZE,
+ .bytes = nb_sectors * BDRV_SECTOR_SIZE,
+ .pnum = &n,
.done = false,
};
@@ -1891,6 +1894,7 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
bdrv_coroutine_enter(bs, co);
BDRV_POLL_WHILE(bs, !data.done);
}
+ *pnum = n >> BDRV_SECTOR_BITS;
return data.ret;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 09/31] block: Switch bdrv_co_get_block_status_above() to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (7 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 08/31] block: Switch BdrvCoGetBlockStatusData " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 10/31] block: Convert bdrv_get_block_status_above() to bytes Eric Blake
` (23 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Convert another internal
type (no semantic change), and rename it to match the corresponding
public function rename.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/io.c | 42 ++++++++++++++++--------------------------
1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/block/io.c b/block/io.c
index f7ece8d..10bc011 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1819,11 +1819,11 @@ out:
return ret;
}
-static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
+static int64_t coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
- int64_t sector_num,
- int nb_sectors,
- int *pnum,
+ int64_t offset,
+ int64_t bytes,
+ int64_t *pnum,
BlockDriverState **file)
{
BlockDriverState *p;
@@ -1831,41 +1831,32 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
assert(bs != base);
for (p = bs; p != base; p = backing_bs(p)) {
- int64_t count;
-
- ret = bdrv_co_block_status(p, sector_num * BDRV_SECTOR_SIZE,
- nb_sectors * BDRV_SECTOR_SIZE, &count,
- file);
- *pnum = count >> BDRV_SECTOR_BITS;
+ ret = bdrv_co_block_status(p, offset, bytes, pnum, file);
if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
break;
}
- /* [sector_num, pnum] unallocated on this layer, which could be only
- * the first part of [sector_num, nb_sectors]. */
- nb_sectors = MIN(nb_sectors, *pnum);
+ /* [offset, pnum] unallocated on this layer, which could be only
+ * the first part of [offset, bytes]. */
+ bytes = MIN(bytes, *pnum);
}
return ret;
}
/* Coroutine wrapper for bdrv_get_block_status_above() */
-static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque)
+static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
{
BdrvCoBlockStatusData *data = opaque;
- int n;
- data->ret = bdrv_co_get_block_status_above(data->bs, data->base,
- data->offset >> BDRV_SECTOR_BITS,
- data->bytes >> BDRV_SECTOR_BITS,
- &n,
- data->file);
- *data->pnum = n * BDRV_SECTOR_SIZE;
+ data->ret = bdrv_co_block_status_above(data->bs, data->base,
+ data->offset, data->bytes,
+ data->pnum, data->file);
data->done = true;
}
/*
- * Synchronous wrapper around bdrv_co_get_block_status_above().
+ * Synchronous wrapper around bdrv_co_block_status_above().
*
- * See bdrv_co_get_block_status_above() for details.
+ * See bdrv_co_block_status_above() for details.
*/
int64_t bdrv_get_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
@@ -1887,10 +1878,9 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
- bdrv_get_block_status_above_co_entry(&data);
+ bdrv_block_status_above_co_entry(&data);
} else {
- co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry,
- &data);
+ co = qemu_coroutine_create(bdrv_block_status_above_co_entry, &data);
bdrv_coroutine_enter(bs, co);
BDRV_POLL_WHILE(bs, !data.done);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 10/31] block: Convert bdrv_get_block_status_above() to bytes
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (8 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 09/31] block: Switch bdrv_co_get_block_status_above() " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 11/31] block: Add .bdrv_co_block_status() callback Eric Blake
` (22 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz, Jeff Cody
We are gradually moving away from sector-based interfaces, towards
byte-based. In the common case, allocation is unlikely to ever use
values that are not naturally sector-aligned, but it is possible
that byte-based values will let us be more precise about allocation
at the end of an unaligned file that can do byte-based access.
Changing the name of the function from bdrv_get_block_status_above()
to bdrv_block_status_above() ensures that the compiler enforces that
all callers are updated. For now, the io.c layer still assert()s
that all callers are sector-aligned, but that can be relaxed when a
later patch implements byte-based block status in the drivers.
For the most part this patch is just the addition of scaling at the
callers followed by inverse scaling at bdrv_block_status(). But some
code, particularly bdrv_block_status(), gets a lot simpler because
it no longer has to mess with sectors; also, it is now possible to pass
NULL if the caller does not care how much of the image is allocated
beyond the initial offset, or which BDS in the chain owns the sector.
For ease of review, bdrv_get_block_status() was tackled separately.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block.h | 10 +++++-----
block/io.c | 36 +++++++++++-------------------------
block/mirror.c | 14 +++++---------
block/qcow2.c | 10 +++-------
qemu-img.c | 37 +++++++++++++++++++++----------------
5 files changed, 45 insertions(+), 62 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index b9e7281..8f2b8a2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -420,11 +420,11 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
int64_t bdrv_block_status(BlockDriverState *bs, int64_t offset,
int64_t bytes, int64_t *pnum,
BlockDriverState **file);
-int64_t bdrv_get_block_status_above(BlockDriverState *bs,
- BlockDriverState *base,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file);
+int64_t bdrv_block_status_above(BlockDriverState *bs,
+ BlockDriverState *base,
+ int64_t offset,
+ int64_t bytes, int64_t *pnum,
+ BlockDriverState **file);
int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum);
int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
diff --git a/block/io.c b/block/io.c
index 10bc011..1b101cf 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1842,7 +1842,7 @@ static int64_t coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
return ret;
}
-/* Coroutine wrapper for bdrv_get_block_status_above() */
+/* Coroutine wrapper for bdrv_block_status_above() */
static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
{
BdrvCoBlockStatusData *data = opaque;
@@ -1858,21 +1858,19 @@ static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
*
* See bdrv_co_block_status_above() for details.
*/
-int64_t bdrv_get_block_status_above(BlockDriverState *bs,
- BlockDriverState *base,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+int64_t bdrv_block_status_above(BlockDriverState *bs,
+ BlockDriverState *base,
+ int64_t offset, int64_t bytes, int64_t *pnum,
+ BlockDriverState **file)
{
Coroutine *co;
- int64_t n;
BdrvCoBlockStatusData data = {
.bs = bs,
.base = base,
.file = file,
- .offset = sector_num * BDRV_SECTOR_SIZE,
- .bytes = nb_sectors * BDRV_SECTOR_SIZE,
- .pnum = &n,
+ .offset = offset,
+ .bytes = bytes,
+ .pnum = pnum,
.done = false,
};
@@ -1884,7 +1882,6 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
bdrv_coroutine_enter(bs, co);
BDRV_POLL_WHILE(bs, !data.done);
}
- *pnum = n >> BDRV_SECTOR_BITS;
return data.ret;
}
@@ -1892,27 +1889,16 @@ int64_t bdrv_block_status(BlockDriverState *bs,
int64_t offset, int64_t bytes, int64_t *pnum,
BlockDriverState **file)
{
- int64_t ret;
- int n;
-
- assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
- assert(bytes <= BDRV_REQUEST_MAX_BYTES);
- ret = bdrv_get_block_status_above(bs, backing_bs(bs),
- offset >> BDRV_SECTOR_BITS,
- bytes >> BDRV_SECTOR_BITS, &n, file);
- if (pnum) {
- *pnum = n * BDRV_SECTOR_SIZE;
- }
- return ret;
+ return bdrv_block_status_above(bs, backing_bs(bs),
+ offset, bytes, pnum, file);
}
int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
int64_t bytes, int64_t *pnum)
{
- BlockDriverState *file;
int64_t ret;
- ret = bdrv_block_status(bs, offset, bytes, pnum, &file);
+ ret = bdrv_block_status(bs, offset, bytes, pnum, NULL);
if (ret < 0) {
return ret;
}
diff --git a/block/mirror.c b/block/mirror.c
index 2510793..750be1f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -328,7 +328,6 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
uint64_t delay_ns = 0;
/* At least the first dirty chunk is mirrored in one iteration. */
int nb_chunks = 1;
- int sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target));
int max_io_bytes = MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES);
@@ -374,7 +373,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
}
/* Clear dirty bits before querying the block status, because
- * calling bdrv_get_block_status_above could yield - if some blocks are
+ * calling bdrv_block_status_above could yield - if some blocks are
* marked dirty in this window, we need to know.
*/
bdrv_reset_dirty_bitmap(s->dirty_bitmap, offset,
@@ -382,10 +381,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
bitmap_set(s->in_flight_bitmap, offset / s->granularity, nb_chunks);
while (nb_chunks > 0 && offset < s->bdev_length) {
int64_t ret;
- int io_sectors;
int64_t io_bytes;
int64_t io_bytes_acct;
- BlockDriverState *file;
enum MirrorMethod {
MIRROR_METHOD_COPY,
MIRROR_METHOD_ZERO,
@@ -393,11 +390,10 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
} mirror_method = MIRROR_METHOD_COPY;
assert(!(offset % s->granularity));
- ret = bdrv_get_block_status_above(source, NULL,
- offset >> BDRV_SECTOR_BITS,
- nb_chunks * sectors_per_chunk,
- &io_sectors, &file);
- io_bytes = io_sectors * BDRV_SECTOR_SIZE;
+ ret = bdrv_block_status_above(source, NULL,
+ offset,
+ nb_chunks * s->granularity,
+ &io_bytes, NULL);
if (ret < 0) {
io_bytes = MIN(nb_chunks * s->granularity, max_io_bytes);
} else if (ret & BDRV_BLOCK_DATA) {
diff --git a/block/qcow2.c b/block/qcow2.c
index fe4ccf6..4272cca 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2444,8 +2444,7 @@ finish:
static bool is_zero_above(BlockDriverState *bs, int64_t offset, int64_t bytes)
{
- int nr;
- BlockDriverState *file;
+ int64_t nr;
int64_t res;
int64_t start;
@@ -2461,11 +2460,8 @@ static bool is_zero_above(BlockDriverState *bs, int64_t offset, int64_t bytes)
if (!bytes) {
return true;
}
- res = bdrv_get_block_status_above(bs, NULL, start >> BDRV_SECTOR_BITS,
- bytes >> BDRV_SECTOR_BITS,
- &nr, &file);
- return res >= 0 && (res & BDRV_BLOCK_ZERO) &&
- nr * BDRV_SECTOR_SIZE == bytes;
+ res = bdrv_block_status_above(bs, NULL, start, bytes, &nr, NULL);
+ return res >= 0 && (res & BDRV_BLOCK_ZERO) && nr == bytes;
}
static coroutine_fn int qcow2_co_pwrite_zeroes(BlockDriverState *bs,
diff --git a/qemu-img.c b/qemu-img.c
index 7ed77d5..2fbd956 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1194,7 +1194,7 @@ static int img_compare(int argc, char **argv)
BlockDriverState *bs1, *bs2;
int64_t total_sectors1, total_sectors2;
uint8_t *buf1 = NULL, *buf2 = NULL;
- int pnum1, pnum2;
+ int64_t pnum1, pnum2;
int allocated1, allocated2;
int ret = 0; /* return value - 0 Ident, 1 Different, >1 Error */
bool progress = false, quiet = false, strict = false;
@@ -1336,15 +1336,16 @@ static int img_compare(int argc, char **argv)
for (;;) {
int64_t status1, status2;
- BlockDriverState *file;
nb_sectors = sectors_to_process(total_sectors, sector_num);
if (nb_sectors <= 0) {
break;
}
- status1 = bdrv_get_block_status_above(bs1, NULL, sector_num,
- total_sectors1 - sector_num,
- &pnum1, &file);
+ status1 = bdrv_block_status_above(bs1, NULL,
+ sector_num * BDRV_SECTOR_SIZE,
+ (total_sectors1 - sector_num) *
+ BDRV_SECTOR_SIZE,
+ &pnum1, NULL);
if (status1 < 0) {
ret = 3;
error_report("Sector allocation test failed for %s", filename1);
@@ -1352,9 +1353,11 @@ static int img_compare(int argc, char **argv)
}
allocated1 = status1 & BDRV_BLOCK_ALLOCATED;
- status2 = bdrv_get_block_status_above(bs2, NULL, sector_num,
- total_sectors2 - sector_num,
- &pnum2, &file);
+ status2 = bdrv_block_status_above(bs2, NULL,
+ sector_num * BDRV_SECTOR_SIZE,
+ (total_sectors2 - sector_num) *
+ BDRV_SECTOR_SIZE,
+ &pnum2, NULL);
if (status2 < 0) {
ret = 3;
error_report("Sector allocation test failed for %s", filename2);
@@ -1362,10 +1365,10 @@ static int img_compare(int argc, char **argv)
}
allocated2 = status2 & BDRV_BLOCK_ALLOCATED;
if (pnum1) {
- nb_sectors = MIN(nb_sectors, pnum1);
+ nb_sectors = MIN(nb_sectors, pnum1 >> BDRV_SECTOR_BITS);
}
if (pnum2) {
- nb_sectors = MIN(nb_sectors, pnum2);
+ nb_sectors = MIN(nb_sectors, pnum2 >> BDRV_SECTOR_BITS);
}
if (strict) {
@@ -1379,7 +1382,7 @@ static int img_compare(int argc, char **argv)
}
}
if ((status1 & BDRV_BLOCK_ZERO) && (status2 & BDRV_BLOCK_ZERO)) {
- nb_sectors = MIN(pnum1, pnum2);
+ nb_sectors = MIN(pnum1, pnum2) >> BDRV_SECTOR_BITS;
} else if (allocated1 == allocated2) {
if (allocated1) {
ret = blk_pread(blk1, sector_num << BDRV_SECTOR_BITS, buf1,
@@ -1557,12 +1560,11 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
n = MIN(s->total_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS);
if (s->sector_next_status <= sector_num) {
- BlockDriverState *file;
int64_t count = n * BDRV_SECTOR_SIZE;
ret = bdrv_block_status(blk_bs(s->src[src_cur]),
(sector_num - src_cur_offset) * BDRV_SECTOR_SIZE,
- count, &count, &file);
+ count, &count, NULL);
if (ret < 0) {
return ret;
}
@@ -1579,9 +1581,12 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
/* Check block status of the backing file chain to avoid
* needlessly reading zeroes and limiting the iteration to the
* buffer size */
- ret = bdrv_get_block_status_above(blk_bs(s->src[src_cur]), NULL,
- sector_num - src_cur_offset,
- n, &n, &file);
+ ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
+ (sector_num - src_cur_offset) *
+ BDRV_SECTOR_SIZE,
+ n * BDRV_SECTOR_SIZE, &count, NULL);
+ assert(QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE));
+ n = count >> BDRV_SECTOR_BITS;
if (ret < 0) {
return ret;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 11/31] block: Add .bdrv_co_block_status() callback
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (9 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 10/31] block: Convert bdrv_get_block_status_above() to bytes Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 12/31] commit: Switch to .bdrv_co_block_status() Eric Blake
` (21 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Now that the block layer exposes byte-based allocation,
it's time to tackle the drivers. Add a new callback that operates
on as small as byte boundaries, and update the block layer to ensure
that the callback is only used with inputs aligned to the device's
request_alignment. Subsequent patches will then update individual
drivers, and then finally remove .bdrv_co_get_block_status().
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block_int.h | 12 ++++++++++++
block/io.c | 47 +++++++++++++++++++++++++++++++++++------------
2 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index bc3a28a..8f20bc3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -163,11 +163,23 @@ struct BlockDriver {
*/
int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs,
int64_t offset, int count, BdrvRequestFlags flags);
+
int coroutine_fn (*bdrv_co_pdiscard)(BlockDriverState *bs,
int64_t offset, int count);
+
+ /*
+ * Building block for bdrv_block_status[_above]. The block layer
+ * guarantees input aligned to request_alignment, as well as
+ * non-NULL pnum and file; and the result only has to worry about
+ * BDRV_BLOCK_DATA, _ZERO, _OFFSET_VALID, and _RAW, and only
+ * according to the current BDS.
+ */
int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum,
BlockDriverState **file);
+ int64_t coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bd,
+ int64_t offset, int64_t bytes, int64_t *pnum,
+ BlockDriverState **file);
/*
* Invalidate any cached meta-data.
diff --git a/block/io.c b/block/io.c
index 1b101cf..361eeb8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1718,7 +1718,6 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
int64_t total_size;
int64_t n; /* bytes */
int64_t ret, ret2;
- int count; /* sectors */
BlockDriverState *tmp_file;
total_size = bdrv_getlength(bs);
@@ -1739,7 +1738,7 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
bytes = n;
}
- if (!bs->drv->bdrv_co_get_block_status) {
+ if (!bs->drv->bdrv_co_get_block_status && !bs->drv->bdrv_co_block_status) {
*pnum = bytes;
ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
if (bs->drv->protocol_name) {
@@ -1753,20 +1752,44 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
}
*file = NULL;
bdrv_inc_in_flight(bs);
- /* TODO: Rather than require aligned offsets, we could instead
- * round to the driver's request_alignment here, then touch up
- * count afterwards back to the caller's expectations. But first
- * we want to switch the driver callback to likewise be
- * byte-based. */
- assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
- ret = bs->drv->bdrv_co_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
- bytes >> BDRV_SECTOR_BITS, &count,
- file);
+ if (bs->drv->bdrv_co_get_block_status) {
+ int count; /* sectors */
+
+ assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
+ ret = bs->drv->bdrv_co_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS,
+ &count, file);
+ *pnum = count * BDRV_SECTOR_SIZE;
+ } else {
+ /* Round out to request_alignment boundaries */
+ int64_t aligned_offset, aligned_bytes;
+
+ aligned_offset = QEMU_ALIGN_DOWN(offset, bs->bl.request_alignment);
+ aligned_bytes = ROUND_UP(offset + bytes,
+ bs->bl.request_alignment) - aligned_offset;
+ ret = bs->drv->bdrv_co_block_status(bs, aligned_offset, aligned_bytes,
+ &n, file);
+ /* Clamp pnum and ret to original request */
+ if (aligned_offset != offset && ret >= 0) {
+ int sectors = DIV_ROUND_UP(offset, BDRV_SECTOR_SIZE) -
+ DIV_ROUND_UP(aligned_offset, BDRV_SECTOR_SIZE);
+
+ assert(n >= offset - aligned_offset);
+ n -= offset - aligned_offset;
+ if (sectors) {
+ ret += sectors * BDRV_SECTOR_SIZE;
+ }
+ }
+ if (ret >= 0 && n > bytes) {
+ assert(aligned_bytes != bytes);
+ n = bytes;
+ }
+ *pnum = n;
+ }
if (ret < 0) {
*pnum = 0;
goto out;
}
- *pnum = count * BDRV_SECTOR_SIZE;
if (ret & BDRV_BLOCK_RAW) {
assert(ret & BDRV_BLOCK_OFFSET_VALID);
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 12/31] commit: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (10 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 11/31] block: Add .bdrv_co_block_status() callback Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 13/31] file-posix: " Eric Blake
` (20 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Jeff Cody, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the commit driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/commit.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index 989de7d..1cc7a00 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -228,14 +228,14 @@ static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs,
return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
}
-static int64_t coroutine_fn bdrv_commit_top_get_block_status(
- BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
+static int64_t coroutine_fn bdrv_commit_top_block_status(
+ BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum,
BlockDriverState **file)
{
- *pnum = nb_sectors;
+ *pnum = bytes;
*file = bs->backing->bs;
return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
- (sector_num << BDRV_SECTOR_BITS);
+ (offset & BDRV_BLOCK_OFFSET_MASK);
}
static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts)
@@ -263,7 +263,7 @@ static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c,
static BlockDriver bdrv_commit_top = {
.format_name = "commit_top",
.bdrv_co_preadv = bdrv_commit_top_preadv,
- .bdrv_co_get_block_status = bdrv_commit_top_get_block_status,
+ .bdrv_co_block_status = bdrv_commit_top_block_status,
.bdrv_refresh_filename = bdrv_commit_top_refresh_filename,
.bdrv_close = bdrv_commit_top_close,
.bdrv_child_perm = bdrv_commit_top_child_perm,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 13/31] file-posix: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (11 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 12/31] commit: Switch to .bdrv_co_block_status() Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 20:56 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 14/31] gluster: " Eric Blake
` (19 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the file protocol driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/file-posix.c | 47 +++++++++++++++++++++++------------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 1941fb6..690bd45 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1832,22 +1832,22 @@ static int find_allocation(BlockDriverState *bs, off_t start,
/*
* Returns the allocation status of the specified sectors.
*
- * If 'sector_num' is beyond the end of the disk image the return value is 0
+ * If 'offset' is beyond the end of the disk image the return value is 0
* and 'pnum' is set to 0.
*
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
+ * 'pnum' is set to the number of bytes (including and immediately following
+ * the specified offset) that are known to be in the same
* allocated/unallocated state.
*
- * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
+ * 'bytes' is the max value 'pnum' should be set to. If bytes goes
* beyond the end of the disk image it will be clamped.
*/
-static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+static int64_t coroutine_fn raw_co_block_status(BlockDriverState *bs,
+ int64_t offset,
+ int64_t bytes, int64_t *pnum,
+ BlockDriverState **file)
{
- off_t start, data = 0, hole = 0;
+ off_t data = 0, hole = 0;
int64_t total_size;
int ret;
@@ -1856,39 +1856,38 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
return ret;
}
- start = sector_num * BDRV_SECTOR_SIZE;
total_size = bdrv_getlength(bs);
if (total_size < 0) {
return total_size;
- } else if (start >= total_size) {
+ } else if (offset >= total_size) {
*pnum = 0;
return 0;
- } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
- nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
+ } else if (offset + bytes > total_size) {
+ bytes = total_size - offset;
}
- ret = find_allocation(bs, start, &data, &hole);
+ ret = find_allocation(bs, offset, &data, &hole);
if (ret == -ENXIO) {
/* Trailing hole */
- *pnum = nb_sectors;
+ *pnum = bytes;
ret = BDRV_BLOCK_ZERO;
} else if (ret < 0) {
/* No info available, so pretend there are no holes */
- *pnum = nb_sectors;
+ *pnum = bytes;
ret = BDRV_BLOCK_DATA;
- } else if (data == start) {
- /* On a data extent, compute sectors to the end of the extent,
+ } else if (data == offset) {
+ /* On a data extent, compute bytes to the end of the extent,
* possibly including a partial sector at EOF. */
- *pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
+ *pnum = MIN(bytes, hole - offset);
ret = BDRV_BLOCK_DATA;
} else {
- /* On a hole, compute sectors to the beginning of the next extent. */
- assert(hole == start);
- *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
+ /* On a hole, compute bytes to the beginning of the next extent. */
+ assert(hole == offset);
+ *pnum = MIN(bytes, data - offset);
ret = BDRV_BLOCK_ZERO;
}
*file = bs;
- return ret | BDRV_BLOCK_OFFSET_VALID | start;
+ return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
}
static coroutine_fn BlockAIOCB *raw_aio_pdiscard(BlockDriverState *bs,
@@ -1963,7 +1962,7 @@ BlockDriver bdrv_file = {
.bdrv_close = raw_close,
.bdrv_create = raw_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_get_block_status = raw_co_get_block_status,
+ .bdrv_co_block_status = raw_co_block_status,
.bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
.bdrv_co_preadv = raw_co_preadv,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 14/31] gluster: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (12 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 13/31] file-posix: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-19 13:26 ` [Qemu-devel] [Qemu-block] " Niels de Vos
2017-04-19 14:06 ` [Qemu-devel] " Prasanna Kalever
2017-04-18 1:33 ` [Qemu-devel] [PATCH 15/31] iscsi: Switch cluster_sectors to byte-based Eric Blake
` (18 subsequent siblings)
32 siblings, 2 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Jeff Cody, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the gluster driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/gluster.c | 47 +++++++++++++++++++++++------------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 1d4e2f7..3f252c6 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1332,24 +1332,24 @@ exit:
/*
* Returns the allocation status of the specified sectors.
*
- * If 'sector_num' is beyond the end of the disk image the return value is 0
+ * If 'offset' is beyond the end of the disk image the return value is 0
* and 'pnum' is set to 0.
*
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
+ * 'pnum' is set to the number of bytes (including and immediately following
+ * the specified offset) that are known to be in the same
* allocated/unallocated state.
*
- * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
+ * 'bytes' is the max value 'pnum' should be set to. If bytes goes
* beyond the end of the disk image it will be clamped.
*
- * (Based on raw_co_get_block_status() from file-posix.c.)
+ * (Based on raw_co_block_status() from file-posix.c.)
*/
-static int64_t coroutine_fn qemu_gluster_co_get_block_status(
- BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
+static int64_t coroutine_fn qemu_gluster_co_block_status(
+ BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum,
BlockDriverState **file)
{
BDRVGlusterState *s = bs->opaque;
- off_t start, data = 0, hole = 0;
+ off_t data = 0, hole = 0;
int64_t total_size;
int ret = -EINVAL;
@@ -1357,41 +1357,40 @@ static int64_t coroutine_fn qemu_gluster_co_get_block_status(
return ret;
}
- start = sector_num * BDRV_SECTOR_SIZE;
total_size = bdrv_getlength(bs);
if (total_size < 0) {
return total_size;
- } else if (start >= total_size) {
+ } else if (offset >= total_size) {
*pnum = 0;
return 0;
- } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
- nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
+ } else if (offset + bytes > total_size) {
+ bytes = total_size - offset;
}
- ret = find_allocation(bs, start, &data, &hole);
+ ret = find_allocation(bs, offset, &data, &hole);
if (ret == -ENXIO) {
/* Trailing hole */
- *pnum = nb_sectors;
+ *pnum = bytes;
ret = BDRV_BLOCK_ZERO;
} else if (ret < 0) {
/* No info available, so pretend there are no holes */
- *pnum = nb_sectors;
+ *pnum = bytes;
ret = BDRV_BLOCK_DATA;
- } else if (data == start) {
+ } else if (data == offset) {
/* On a data extent, compute sectors to the end of the extent,
* possibly including a partial sector at EOF. */
- *pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
+ *pnum = MIN(bytes, hole - offset);
ret = BDRV_BLOCK_DATA;
} else {
/* On a hole, compute sectors to the beginning of the next extent. */
- assert(hole == start);
- *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
+ assert(hole == offset);
+ *pnum = MIN(bytes, data - offset);
ret = BDRV_BLOCK_ZERO;
}
*file = bs;
- return ret | BDRV_BLOCK_OFFSET_VALID | start;
+ return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
}
@@ -1419,7 +1418,7 @@ static BlockDriver bdrv_gluster = {
#ifdef CONFIG_GLUSTERFS_ZEROFILL
.bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
#endif
- .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
+ .bdrv_co_block_status = qemu_gluster_co_block_status,
.create_opts = &qemu_gluster_create_opts,
};
@@ -1447,7 +1446,7 @@ static BlockDriver bdrv_gluster_tcp = {
#ifdef CONFIG_GLUSTERFS_ZEROFILL
.bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
#endif
- .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
+ .bdrv_co_block_status = qemu_gluster_co_block_status,
.create_opts = &qemu_gluster_create_opts,
};
@@ -1475,7 +1474,7 @@ static BlockDriver bdrv_gluster_unix = {
#ifdef CONFIG_GLUSTERFS_ZEROFILL
.bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
#endif
- .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
+ .bdrv_co_block_status = qemu_gluster_co_block_status,
.create_opts = &qemu_gluster_create_opts,
};
@@ -1509,7 +1508,7 @@ static BlockDriver bdrv_gluster_rdma = {
#ifdef CONFIG_GLUSTERFS_ZEROFILL
.bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
#endif
- .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
+ .bdrv_co_block_status = qemu_gluster_co_block_status,
.create_opts = &qemu_gluster_create_opts,
};
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 15/31] iscsi: Switch cluster_sectors to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (13 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 14/31] gluster: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 16/31] iscsi: Switch iscsi_allocmap_update() " Eric Blake
` (17 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Ronnie Sahlberg, Paolo Bonzini,
Peter Lieven, Kevin Wolf, Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Convert all uses of
the cluster size in sectors, along with adding assertions that we
are not dividing by zero.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/iscsi.c | 56 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 21 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 5daa201..9648a45 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -79,7 +79,7 @@ typedef struct IscsiLun {
unsigned long *allocmap;
unsigned long *allocmap_valid;
long allocmap_size;
- int cluster_sectors;
+ int cluster_size;
bool use_16_for_rw;
bool write_protected;
bool lbpme;
@@ -460,9 +460,10 @@ static int iscsi_allocmap_init(IscsiLun *iscsilun, int open_flags)
{
iscsi_allocmap_free(iscsilun);
+ assert(iscsilun->cluster_size);
iscsilun->allocmap_size =
- DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, iscsilun),
- iscsilun->cluster_sectors);
+ DIV_ROUND_UP(iscsilun->num_blocks * iscsilun->block_size,
+ iscsilun->cluster_size);
iscsilun->allocmap = bitmap_try_new(iscsilun->allocmap_size);
if (!iscsilun->allocmap) {
@@ -470,7 +471,7 @@ static int iscsi_allocmap_init(IscsiLun *iscsilun, int open_flags)
}
if (open_flags & BDRV_O_NOCACHE) {
- /* in case that cache.direct = on all allocmap entries are
+ /* when cache.direct = on all allocmap entries are
* treated as invalid to force a relookup of the block
* status on every read request */
return 0;
@@ -491,17 +492,19 @@ iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num,
int nb_sectors, bool allocated, bool valid)
{
int64_t cl_num_expanded, nb_cls_expanded, cl_num_shrunk, nb_cls_shrunk;
+ int cluster_sectors = iscsilun->cluster_size >> BDRV_SECTOR_BITS;
if (iscsilun->allocmap == NULL) {
return;
}
/* expand to entirely contain all affected clusters */
- cl_num_expanded = sector_num / iscsilun->cluster_sectors;
+ assert(cluster_sectors);
+ cl_num_expanded = sector_num / cluster_sectors;
nb_cls_expanded = DIV_ROUND_UP(sector_num + nb_sectors,
- iscsilun->cluster_sectors) - cl_num_expanded;
+ cluster_sectors) - cl_num_expanded;
/* shrink to touch only completely contained clusters */
- cl_num_shrunk = DIV_ROUND_UP(sector_num, iscsilun->cluster_sectors);
- nb_cls_shrunk = (sector_num + nb_sectors) / iscsilun->cluster_sectors
+ cl_num_shrunk = DIV_ROUND_UP(sector_num, cluster_sectors);
+ nb_cls_shrunk = (sector_num + nb_sectors) / cluster_sectors
- cl_num_shrunk;
if (allocated) {
bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded);
@@ -565,9 +568,12 @@ iscsi_allocmap_is_allocated(IscsiLun *iscsilun, int64_t sector_num,
if (iscsilun->allocmap == NULL) {
return true;
}
- size = DIV_ROUND_UP(sector_num + nb_sectors, iscsilun->cluster_sectors);
+ assert(iscsilun->cluster_size);
+ size = DIV_ROUND_UP(sector_num + nb_sectors,
+ iscsilun->cluster_size >> BDRV_SECTOR_BITS);
return !(find_next_bit(iscsilun->allocmap, size,
- sector_num / iscsilun->cluster_sectors) == size);
+ sector_num * BDRV_SECTOR_SIZE /
+ iscsilun->cluster_size) == size);
}
static inline bool iscsi_allocmap_is_valid(IscsiLun *iscsilun,
@@ -577,9 +583,12 @@ static inline bool iscsi_allocmap_is_valid(IscsiLun *iscsilun,
if (iscsilun->allocmap_valid == NULL) {
return false;
}
- size = DIV_ROUND_UP(sector_num + nb_sectors, iscsilun->cluster_sectors);
+ assert(iscsilun->cluster_size);
+ size = DIV_ROUND_UP(sector_num + nb_sectors,
+ iscsilun->cluster_size >> BDRV_SECTOR_BITS);
return (find_next_zero_bit(iscsilun->allocmap_valid, size,
- sector_num / iscsilun->cluster_sectors) == size);
+ sector_num * BDRV_SECTOR_SIZE /
+ iscsilun->cluster_size) == size);
}
static int coroutine_fn
@@ -814,16 +823,21 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
BlockDriverState *file;
/* check the block status from the beginning of the cluster
* containing the start sector */
- int64_t ret = iscsi_co_get_block_status(bs,
- sector_num - sector_num % iscsilun->cluster_sectors,
- BDRV_REQUEST_MAX_SECTORS, &pnum, &file);
+ int cluster_sectors = iscsilun->cluster_size >> BDRV_SECTOR_BITS;
+ int head;
+ int64_t ret;
+
+ assert(cluster_sectors);
+ head = sector_num % cluster_sectors;
+ ret = iscsi_co_get_block_status(bs, sector_num - head,
+ BDRV_REQUEST_MAX_SECTORS, &pnum,
+ &file);
if (ret < 0) {
return ret;
}
/* if the whole request falls into an unallocated area we can avoid
- * to read and directly return zeroes instead */
- if (ret & BDRV_BLOCK_ZERO &&
- pnum >= nb_sectors + sector_num % iscsilun->cluster_sectors) {
+ * reading and directly return zeroes instead */
+ if (ret & BDRV_BLOCK_ZERO && pnum >= nb_sectors + head) {
qemu_iovec_memset(iov, 0, 0x00, iov->size);
return 0;
}
@@ -1944,8 +1958,8 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
* reasonable size */
if (iscsilun->bl.opt_unmap_gran * iscsilun->block_size >= 4 * 1024 &&
iscsilun->bl.opt_unmap_gran * iscsilun->block_size <= 16 * 1024 * 1024) {
- iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran *
- iscsilun->block_size) >> BDRV_SECTOR_BITS;
+ iscsilun->cluster_size = iscsilun->bl.opt_unmap_gran *
+ iscsilun->block_size;
if (iscsilun->lbprz) {
ret = iscsi_allocmap_init(iscsilun, bs->open_flags);
}
@@ -2143,7 +2157,7 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
IscsiLun *iscsilun = bs->opaque;
bdi->unallocated_blocks_are_zero = iscsilun->lbprz;
bdi->can_write_zeroes_with_unmap = iscsilun->lbprz && iscsilun->lbp.lbpws;
- bdi->cluster_size = iscsilun->cluster_sectors * BDRV_SECTOR_SIZE;
+ bdi->cluster_size = iscsilun->cluster_size;
return 0;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 16/31] iscsi: Switch iscsi_allocmap_update() to byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (14 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 15/31] iscsi: Switch cluster_sectors to byte-based Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 17/31] iscsi: Switch to .bdrv_co_block_status() Eric Blake
` (16 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Ronnie Sahlberg, Paolo Bonzini,
Peter Lieven, Kevin Wolf, Max Reitz
We are gradually converting to byte-based interfaces, as they are
easier to reason about than sector-based. Convert all uses of
the allocmap (no semantic change). Callers that already had bytes
available are simpler, and callers that now scale to bytes will be
easier to switch to byte-based in the future.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/iscsi.c | 90 +++++++++++++++++++++++++++++------------------------------
1 file changed, 44 insertions(+), 46 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 9648a45..e51fdfb 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -488,24 +488,22 @@ static int iscsi_allocmap_init(IscsiLun *iscsilun, int open_flags)
}
static void
-iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num,
- int nb_sectors, bool allocated, bool valid)
+iscsi_allocmap_update(IscsiLun *iscsilun, int64_t offset,
+ int64_t bytes, bool allocated, bool valid)
{
int64_t cl_num_expanded, nb_cls_expanded, cl_num_shrunk, nb_cls_shrunk;
- int cluster_sectors = iscsilun->cluster_size >> BDRV_SECTOR_BITS;
if (iscsilun->allocmap == NULL) {
return;
}
/* expand to entirely contain all affected clusters */
- assert(cluster_sectors);
- cl_num_expanded = sector_num / cluster_sectors;
- nb_cls_expanded = DIV_ROUND_UP(sector_num + nb_sectors,
- cluster_sectors) - cl_num_expanded;
+ assert(iscsilun->cluster_size);
+ cl_num_expanded = offset / iscsilun->cluster_size;
+ nb_cls_expanded = DIV_ROUND_UP(offset + bytes, iscsilun->cluster_size)
+ - cl_num_expanded;
/* shrink to touch only completely contained clusters */
- cl_num_shrunk = DIV_ROUND_UP(sector_num, cluster_sectors);
- nb_cls_shrunk = (sector_num + nb_sectors) / cluster_sectors
- - cl_num_shrunk;
+ cl_num_shrunk = DIV_ROUND_UP(offset, iscsilun->cluster_size);
+ nb_cls_shrunk = (offset + bytes) / iscsilun->cluster_size - cl_num_shrunk;
if (allocated) {
bitmap_set(iscsilun->allocmap, cl_num_expanded, nb_cls_expanded);
} else {
@@ -528,26 +526,26 @@ iscsi_allocmap_update(IscsiLun *iscsilun, int64_t sector_num,
}
static void
-iscsi_allocmap_set_allocated(IscsiLun *iscsilun, int64_t sector_num,
- int nb_sectors)
+iscsi_allocmap_set_allocated(IscsiLun *iscsilun, int64_t offset,
+ int64_t bytes)
{
- iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, true, true);
+ iscsi_allocmap_update(iscsilun, offset, bytes, true, true);
}
static void
-iscsi_allocmap_set_unallocated(IscsiLun *iscsilun, int64_t sector_num,
- int nb_sectors)
+iscsi_allocmap_set_unallocated(IscsiLun *iscsilun, int64_t offset,
+ int64_t bytes)
{
/* Note: if cache.direct=on the fifth argument to iscsi_allocmap_update
* is ignored, so this will in effect be an iscsi_allocmap_set_invalid.
*/
- iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, false, true);
+ iscsi_allocmap_update(iscsilun, offset, bytes, false, true);
}
-static void iscsi_allocmap_set_invalid(IscsiLun *iscsilun, int64_t sector_num,
- int nb_sectors)
+static void iscsi_allocmap_set_invalid(IscsiLun *iscsilun, int64_t offset,
+ int64_t bytes)
{
- iscsi_allocmap_update(iscsilun, sector_num, nb_sectors, false, false);
+ iscsi_allocmap_update(iscsilun, offset, bytes, false, false);
}
static void iscsi_allocmap_invalidate(IscsiLun *iscsilun)
@@ -561,34 +559,30 @@ static void iscsi_allocmap_invalidate(IscsiLun *iscsilun)
}
static inline bool
-iscsi_allocmap_is_allocated(IscsiLun *iscsilun, int64_t sector_num,
- int nb_sectors)
+iscsi_allocmap_is_allocated(IscsiLun *iscsilun, int64_t offset,
+ int64_t bytes)
{
unsigned long size;
if (iscsilun->allocmap == NULL) {
return true;
}
assert(iscsilun->cluster_size);
- size = DIV_ROUND_UP(sector_num + nb_sectors,
- iscsilun->cluster_size >> BDRV_SECTOR_BITS);
+ size = DIV_ROUND_UP(offset + bytes, iscsilun->cluster_size);
return !(find_next_bit(iscsilun->allocmap, size,
- sector_num * BDRV_SECTOR_SIZE /
- iscsilun->cluster_size) == size);
+ offset / iscsilun->cluster_size) == size);
}
static inline bool iscsi_allocmap_is_valid(IscsiLun *iscsilun,
- int64_t sector_num, int nb_sectors)
+ int64_t offset, int64_t bytes)
{
unsigned long size;
if (iscsilun->allocmap_valid == NULL) {
return false;
}
assert(iscsilun->cluster_size);
- size = DIV_ROUND_UP(sector_num + nb_sectors,
- iscsilun->cluster_size >> BDRV_SECTOR_BITS);
+ size = DIV_ROUND_UP(offset + bytes, iscsilun->cluster_size);
return (find_next_zero_bit(iscsilun->allocmap_valid, size,
- sector_num * BDRV_SECTOR_SIZE /
- iscsilun->cluster_size) == size);
+ offset / iscsilun->cluster_size) == size);
}
static int coroutine_fn
@@ -670,12 +664,14 @@ retry:
}
if (iTask.status != SCSI_STATUS_GOOD) {
- iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors);
+ iscsi_allocmap_set_invalid(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE);
r = iTask.err_code;
goto out_unlock;
}
- iscsi_allocmap_set_allocated(iscsilun, sector_num, nb_sectors);
+ iscsi_allocmap_set_allocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE);
out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
@@ -770,9 +766,11 @@ retry:
}
if (ret & BDRV_BLOCK_ZERO) {
- iscsi_allocmap_set_unallocated(iscsilun, sector_num, *pnum);
+ iscsi_allocmap_set_unallocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ *pnum * BDRV_SECTOR_SIZE);
} else {
- iscsi_allocmap_set_allocated(iscsilun, sector_num, *pnum);
+ iscsi_allocmap_set_allocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ *pnum * BDRV_SECTOR_SIZE);
}
if (*pnum > nb_sectors) {
@@ -810,15 +808,19 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
/* if cache.direct is off and we have a valid entry in our allocation map
* we can skip checking the block status and directly return zeroes if
* the request falls within an unallocated area */
- if (iscsi_allocmap_is_valid(iscsilun, sector_num, nb_sectors) &&
- !iscsi_allocmap_is_allocated(iscsilun, sector_num, nb_sectors)) {
+ if (iscsi_allocmap_is_valid(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE) &&
+ !iscsi_allocmap_is_allocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE)) {
qemu_iovec_memset(iov, 0, 0x00, iov->size);
return 0;
}
if (nb_sectors >= ISCSI_CHECKALLOC_THRES &&
- !iscsi_allocmap_is_valid(iscsilun, sector_num, nb_sectors) &&
- !iscsi_allocmap_is_allocated(iscsilun, sector_num, nb_sectors)) {
+ !iscsi_allocmap_is_valid(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE) &&
+ !iscsi_allocmap_is_allocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE)) {
int pnum;
BlockDriverState *file;
/* check the block status from the beginning of the cluster
@@ -1187,8 +1189,7 @@ retry:
goto out_unlock;
}
- iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
- count >> BDRV_SECTOR_BITS);
+ iscsi_allocmap_set_invalid(iscsilun, offset, count);
out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
@@ -1286,18 +1287,15 @@ retry:
}
if (iTask.status != SCSI_STATUS_GOOD) {
- iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
- count >> BDRV_SECTOR_BITS);
+ iscsi_allocmap_set_invalid(iscsilun, offset, count);
r = iTask.err_code;
goto out_unlock;
}
if (flags & BDRV_REQ_MAY_UNMAP) {
- iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
- count >> BDRV_SECTOR_BITS);
+ iscsi_allocmap_set_invalid(iscsilun, offset, count);
} else {
- iscsi_allocmap_set_allocated(iscsilun, offset >> BDRV_SECTOR_BITS,
- count >> BDRV_SECTOR_BITS);
+ iscsi_allocmap_set_allocated(iscsilun, offset, count);
}
out_unlock:
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 17/31] iscsi: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (15 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 16/31] iscsi: Switch iscsi_allocmap_update() " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 18/31] mirror: " Eric Blake
` (15 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Ronnie Sahlberg, Paolo Bonzini,
Peter Lieven, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the iscsi driver accordingly. In this case,
it is handy to teach iscsi_co_block_status() to handle a NULL file
parameter, even though the block layer passes a non-NULL value,
because we also call the function directly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/iscsi.c | 52 ++++++++++++++++++++++++----------------------------
1 file changed, 24 insertions(+), 28 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index e51fdfb..f7c8a32 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -680,9 +680,9 @@ out_unlock:
-static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
+static int64_t coroutine_fn iscsi_co_block_status(BlockDriverState *bs,
+ int64_t offset,
+ int64_t bytes, int64_t *pnum,
BlockDriverState **file)
{
IscsiLun *iscsilun = bs->opaque;
@@ -693,15 +693,15 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
iscsi_co_init_iscsitask(iscsilun, &iTask);
- if (!is_sector_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+ if (!is_byte_request_lun_aligned(offset, bytes, iscsilun)) {
ret = -EINVAL;
goto out;
}
/* default to all sectors allocated */
ret = BDRV_BLOCK_DATA;
- ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
- *pnum = nb_sectors;
+ ret |= (offset & BDRV_BLOCK_OFFSET_MASK) | BDRV_BLOCK_OFFSET_VALID;
+ *pnum = bytes;
/* LUN does not support logical block provisioning */
if (!iscsilun->lbpme) {
@@ -711,7 +711,7 @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
qemu_mutex_lock(&iscsilun->mutex);
retry:
if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
- sector_qemu2lun(sector_num, iscsilun),
+ offset / iscsilun->block_size,
8 + 16, iscsi_co_generic_cb,
&iTask) == NULL) {
ret = -ENOMEM;
@@ -750,12 +750,12 @@ retry:
lbasd = &lbas->descriptors[0];
- if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
+ if (offset / iscsilun->block_size != lbasd->lba) {
ret = -EIO;
goto out_unlock;
}
- *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
+ *pnum = lbasd->num_blocks * iscsilun->block_size;
if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
@@ -766,15 +766,13 @@ retry:
}
if (ret & BDRV_BLOCK_ZERO) {
- iscsi_allocmap_set_unallocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
- *pnum * BDRV_SECTOR_SIZE);
+ iscsi_allocmap_set_unallocated(iscsilun, offset, *pnum);
} else {
- iscsi_allocmap_set_allocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
- *pnum * BDRV_SECTOR_SIZE);
+ iscsi_allocmap_set_allocated(iscsilun, offset, *pnum);
}
- if (*pnum > nb_sectors) {
- *pnum = nb_sectors;
+ if (*pnum > bytes) {
+ *pnum = bytes;
}
out_unlock:
qemu_mutex_unlock(&iscsilun->mutex);
@@ -782,7 +780,7 @@ out:
if (iTask.task != NULL) {
scsi_free_scsi_task(iTask.task);
}
- if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) {
+ if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID && file) {
*file = bs;
}
return ret;
@@ -821,25 +819,23 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
nb_sectors * BDRV_SECTOR_SIZE) &&
!iscsi_allocmap_is_allocated(iscsilun, sector_num * BDRV_SECTOR_SIZE,
nb_sectors * BDRV_SECTOR_SIZE)) {
- int pnum;
- BlockDriverState *file;
+ int64_t pnum;
/* check the block status from the beginning of the cluster
* containing the start sector */
- int cluster_sectors = iscsilun->cluster_size >> BDRV_SECTOR_BITS;
- int head;
+ int64_t head;
int64_t ret;
- assert(cluster_sectors);
- head = sector_num % cluster_sectors;
- ret = iscsi_co_get_block_status(bs, sector_num - head,
- BDRV_REQUEST_MAX_SECTORS, &pnum,
- &file);
+ assert(iscsilun->cluster_size);
+ head = (sector_num * BDRV_SECTOR_SIZE) % iscsilun->cluster_size;
+ ret = iscsi_co_block_status(bs, sector_num * BDRV_SECTOR_SIZE - head,
+ BDRV_REQUEST_MAX_BYTES, &pnum, NULL);
if (ret < 0) {
return ret;
}
/* if the whole request falls into an unallocated area we can avoid
* reading and directly return zeroes instead */
- if (ret & BDRV_BLOCK_ZERO && pnum >= nb_sectors + head) {
+ if (ret & BDRV_BLOCK_ZERO &&
+ pnum >= nb_sectors * BDRV_SECTOR_SIZE + head) {
qemu_iovec_memset(iov, 0, 0x00, iov->size);
return 0;
}
@@ -2198,7 +2194,7 @@ static BlockDriver bdrv_iscsi = {
.bdrv_truncate = iscsi_truncate,
.bdrv_refresh_limits = iscsi_refresh_limits,
- .bdrv_co_get_block_status = iscsi_co_get_block_status,
+ .bdrv_co_block_status = iscsi_co_block_status,
.bdrv_co_pdiscard = iscsi_co_pdiscard,
.bdrv_co_pwrite_zeroes = iscsi_co_pwrite_zeroes,
.bdrv_co_readv = iscsi_co_readv,
@@ -2233,7 +2229,7 @@ static BlockDriver bdrv_iser = {
.bdrv_truncate = iscsi_truncate,
.bdrv_refresh_limits = iscsi_refresh_limits,
- .bdrv_co_get_block_status = iscsi_co_get_block_status,
+ .bdrv_co_block_status = iscsi_co_block_status,
.bdrv_co_pdiscard = iscsi_co_pdiscard,
.bdrv_co_pwrite_zeroes = iscsi_co_pwrite_zeroes,
.bdrv_co_readv = iscsi_co_readv,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 18/31] mirror: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (16 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 17/31] iscsi: Switch to .bdrv_co_block_status() Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 19/31] null: " Eric Blake
` (14 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Jeff Cody, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the mirror driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/mirror.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 750be1f..ebd0adf 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1027,14 +1027,14 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
return bdrv_co_flush(bs->backing->bs);
}
-static int64_t coroutine_fn bdrv_mirror_top_get_block_status(
- BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
+static int64_t coroutine_fn bdrv_mirror_top_block_status(
+ BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum,
BlockDriverState **file)
{
- *pnum = nb_sectors;
+ *pnum = bytes;
*file = bs->backing->bs;
return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
- (sector_num << BDRV_SECTOR_BITS);
+ (offset & BDRV_BLOCK_OFFSET_MASK);
}
static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
@@ -1083,7 +1083,7 @@ static BlockDriver bdrv_mirror_top = {
.bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes,
.bdrv_co_pdiscard = bdrv_mirror_top_pdiscard,
.bdrv_co_flush = bdrv_mirror_top_flush,
- .bdrv_co_get_block_status = bdrv_mirror_top_get_block_status,
+ .bdrv_co_block_status = bdrv_mirror_top_block_status,
.bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
.bdrv_close = bdrv_mirror_top_close,
.bdrv_child_perm = bdrv_mirror_top_child_perm,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 19/31] null: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (17 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 18/31] mirror: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 20/31] parallels: " Eric Blake
` (13 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Fam Zheng, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the null driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/null.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/block/null.c b/block/null.c
index b300390..2dc2dd7 100644
--- a/block/null.c
+++ b/block/null.c
@@ -204,22 +204,21 @@ static int null_reopen_prepare(BDRVReopenState *reopen_state,
return 0;
}
-static int64_t coroutine_fn null_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+static int64_t coroutine_fn null_co_block_status(BlockDriverState *bs,
+ int64_t offset,
+ int64_t bytes, int64_t *pnum,
+ BlockDriverState **file)
{
BDRVNullState *s = bs->opaque;
- off_t start = sector_num * BDRV_SECTOR_SIZE;
+ int64_t ret = BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
- *pnum = nb_sectors;
+ *pnum = bytes;
*file = bs;
if (s->read_zeroes) {
- return BDRV_BLOCK_OFFSET_VALID | start | BDRV_BLOCK_ZERO;
- } else {
- return BDRV_BLOCK_OFFSET_VALID | start;
+ ret |= BDRV_BLOCK_ZERO;
}
+ return ret;
}
static void null_refresh_filename(BlockDriverState *bs, QDict *opts)
@@ -250,7 +249,7 @@ static BlockDriver bdrv_null_co = {
.bdrv_co_flush_to_disk = null_co_flush,
.bdrv_reopen_prepare = null_reopen_prepare,
- .bdrv_co_get_block_status = null_co_get_block_status,
+ .bdrv_co_block_status = null_co_block_status,
.bdrv_refresh_filename = null_refresh_filename,
};
@@ -269,7 +268,7 @@ static BlockDriver bdrv_null_aio = {
.bdrv_aio_flush = null_aio_flush,
.bdrv_reopen_prepare = null_reopen_prepare,
- .bdrv_co_get_block_status = null_co_get_block_status,
+ .bdrv_co_block_status = null_co_block_status,
.bdrv_refresh_filename = null_refresh_filename,
};
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 20/31] parallels: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (18 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 19/31] null: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 10:28 ` Denis V. Lunev
2017-04-18 1:33 ` [Qemu-devel] [PATCH 21/31] qcow: " Eric Blake
` (12 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Denis V. Lunev,
Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the parallels driver accordingly. Note that
the internal function block_status() is still sector-based, because
it is still in use by other sector-based functions; but that's okay
because request_alignment is 512 as a result of those functions.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/parallels.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 8be46a7..2bc1918 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -274,22 +274,25 @@ static coroutine_fn int parallels_co_flush_to_os(BlockDriverState *bs)
}
-static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn parallels_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *pnum, BlockDriverState **file)
{
BDRVParallelsState *s = bs->opaque;
- int64_t offset;
+ int count;
+ assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
qemu_co_mutex_lock(&s->lock);
- offset = block_status(s, sector_num, nb_sectors, pnum);
+ offset = block_status(s, offset >> BDRV_SECTOR_BITS,
+ bytes >> BDRV_SECTOR_BITS, &count);
qemu_co_mutex_unlock(&s->lock);
if (offset < 0) {
return 0;
}
+ *pnum = count * BDRV_SECTOR_SIZE;
*file = bs->file->bs;
- return (offset << BDRV_SECTOR_BITS) |
+ return (offset & BDRV_BLOCK_OFFSET_MASK) |
BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
}
@@ -775,7 +778,7 @@ static BlockDriver bdrv_parallels = {
.bdrv_open = parallels_open,
.bdrv_close = parallels_close,
.bdrv_child_perm = bdrv_format_default_perms,
- .bdrv_co_get_block_status = parallels_co_get_block_status,
+ .bdrv_co_block_status = parallels_co_block_status,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_co_flush_to_os = parallels_co_flush_to_os,
.bdrv_co_readv = parallels_co_readv,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 21/31] qcow: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (19 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 20/31] parallels: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 22/31] qcow2: " Eric Blake
` (11 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the qcow driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/qcow.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 5d147b9..d7dfa08 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -515,20 +515,22 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
return cluster_offset;
}
-static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn qcow_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *pnum, BlockDriverState **file)
{
BDRVQcowState *s = bs->opaque;
- int index_in_cluster, n;
+ int index_in_cluster;
+ int64_t n;
uint64_t cluster_offset;
qemu_co_mutex_lock(&s->lock);
- cluster_offset = get_cluster_offset(bs, sector_num << 9, 0, 0, 0, 0);
+ cluster_offset = get_cluster_offset(bs, offset, 0, 0, 0, 0);
qemu_co_mutex_unlock(&s->lock);
- index_in_cluster = sector_num & (s->cluster_sectors - 1);
- n = s->cluster_sectors - index_in_cluster;
- if (n > nb_sectors)
- n = nb_sectors;
+ index_in_cluster = offset & (s->cluster_size - 1);
+ n = s->cluster_size - index_in_cluster;
+ if (n > bytes) {
+ n = bytes;
+ }
*pnum = n;
if (!cluster_offset) {
return 0;
@@ -536,7 +538,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->cipher) {
return BDRV_BLOCK_DATA;
}
- cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+ cluster_offset |= (index_in_cluster & BDRV_BLOCK_OFFSET_MASK);
*file = bs->file->bs;
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | cluster_offset;
}
@@ -1061,7 +1063,7 @@ static BlockDriver bdrv_qcow = {
.bdrv_co_readv = qcow_co_readv,
.bdrv_co_writev = qcow_co_writev,
- .bdrv_co_get_block_status = qcow_co_get_block_status,
+ .bdrv_co_block_status = qcow_co_block_status,
.bdrv_set_key = qcow_set_key,
.bdrv_make_empty = qcow_make_empty,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 22/31] qcow2: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (20 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 21/31] qcow: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 23/31] qed: " Eric Blake
` (10 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the qcow2 driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/qcow2.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 4272cca..0de7210 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1358,8 +1358,8 @@ static void qcow2_join_options(QDict *options, QDict *old_options)
}
}
-static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t count, int64_t *pnum, BlockDriverState **file)
{
BDRVQcow2State *s = bs->opaque;
uint64_t cluster_offset;
@@ -1367,21 +1367,20 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
unsigned int bytes;
int64_t status = 0;
- bytes = MIN(INT_MAX, nb_sectors * BDRV_SECTOR_SIZE);
+ bytes = MIN(INT_MAX, count);
qemu_co_mutex_lock(&s->lock);
- ret = qcow2_get_cluster_offset(bs, sector_num << 9, &bytes,
- &cluster_offset);
+ ret = qcow2_get_cluster_offset(bs, offset, &bytes, &cluster_offset);
qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {
return ret;
}
- *pnum = bytes >> BDRV_SECTOR_BITS;
+ *pnum = bytes;
if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED &&
!s->cipher) {
- index_in_cluster = sector_num & (s->cluster_sectors - 1);
- cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+ index_in_cluster = offset & (s->cluster_size - 1);
+ cluster_offset |= (index_in_cluster & BDRV_BLOCK_OFFSET_MASK);
*file = bs->file->bs;
status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset;
}
@@ -3429,7 +3428,7 @@ BlockDriver bdrv_qcow2 = {
.bdrv_child_perm = bdrv_format_default_perms,
.bdrv_create = qcow2_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_get_block_status = qcow2_co_get_block_status,
+ .bdrv_co_block_status = qcow2_co_block_status,
.bdrv_set_key = qcow2_set_key,
.bdrv_co_preadv = qcow2_co_preadv,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 23/31] qed: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (21 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 22/31] qcow2: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 24/31] raw: " Eric Blake
` (9 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the qed driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/qed.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/block/qed.c b/block/qed.c
index fd76817..336dae4 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -729,7 +729,7 @@ typedef struct {
Coroutine *co;
uint64_t pos;
int64_t status;
- int *pnum;
+ int64_t *pnum;
BlockDriverState **file;
} QEDIsAllocatedCB;
@@ -737,10 +737,10 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
{
QEDIsAllocatedCB *cb = opaque;
BDRVQEDState *s = cb->bs->opaque;
- *cb->pnum = len / BDRV_SECTOR_SIZE;
+ *cb->pnum = len;
switch (ret) {
case QED_CLUSTER_FOUND:
- offset |= qed_offset_into_cluster(s, cb->pos);
+ offset |= qed_offset_into_cluster(s, cb->pos) & BDRV_BLOCK_OFFSET_VALID;
cb->status = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
*cb->file = cb->bs->file->bs;
break;
@@ -762,23 +762,23 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
}
}
-static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+static int64_t coroutine_fn bdrv_qed_co_block_status(BlockDriverState *bs,
+ int64_t offset,
+ int64_t bytes,
+ int64_t *pnum,
+ BlockDriverState **file)
{
BDRVQEDState *s = bs->opaque;
- size_t len = (size_t)nb_sectors * BDRV_SECTOR_SIZE;
QEDIsAllocatedCB cb = {
.bs = bs,
- .pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE,
+ .pos = offset,
.status = BDRV_BLOCK_OFFSET_MASK,
.pnum = pnum,
.file = file,
};
QEDRequest request = { .l2_table = NULL };
- qed_find_cluster(s, &request, cb.pos, len, qed_is_allocated_cb, &cb);
+ qed_find_cluster(s, &request, cb.pos, bytes, qed_is_allocated_cb, &cb);
/* Now sleep if the callback wasn't invoked immediately */
while (cb.status == BDRV_BLOCK_OFFSET_MASK) {
@@ -1710,7 +1710,7 @@ static BlockDriver bdrv_qed = {
.bdrv_child_perm = bdrv_format_default_perms,
.bdrv_create = bdrv_qed_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
+ .bdrv_co_block_status = bdrv_qed_co_block_status,
.bdrv_aio_readv = bdrv_qed_aio_readv,
.bdrv_aio_writev = bdrv_qed_aio_writev,
.bdrv_co_pwrite_zeroes = bdrv_qed_co_pwrite_zeroes,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 24/31] raw: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (22 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 23/31] qed: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 25/31] sheepdog: " Eric Blake
` (8 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the raw driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/raw-format.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/block/raw-format.c b/block/raw-format.c
index 36e6503..746beed 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -250,17 +250,17 @@ fail:
return ret;
}
-static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum,
- BlockDriverState **file)
+static int64_t coroutine_fn raw_co_block_status(BlockDriverState *bs,
+ int64_t offset,
+ int64_t bytes, int64_t *pnum,
+ BlockDriverState **file)
{
BDRVRawState *s = bs->opaque;
- *pnum = nb_sectors;
+ *pnum = bytes;
*file = bs->file->bs;
- sector_num += s->offset / BDRV_SECTOR_SIZE;
+ offset += s->offset;
return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
- (sector_num << BDRV_SECTOR_BITS);
+ (offset & BDRV_BLOCK_OFFSET_MASK);
}
static int coroutine_fn raw_co_pwrite_zeroes(BlockDriverState *bs,
@@ -475,7 +475,7 @@ BlockDriver bdrv_raw = {
.bdrv_co_pwritev = &raw_co_pwritev,
.bdrv_co_pwrite_zeroes = &raw_co_pwrite_zeroes,
.bdrv_co_pdiscard = &raw_co_pdiscard,
- .bdrv_co_get_block_status = &raw_co_get_block_status,
+ .bdrv_co_block_status = &raw_co_block_status,
.bdrv_truncate = &raw_truncate,
.bdrv_getlength = &raw_getlength,
.has_variable_length = true,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 25/31] sheepdog: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (23 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 24/31] raw: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code Eric Blake
` (7 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Hitoshi Mitake, Liu Yuan, Jeff Cody,
Kevin Wolf, Max Reitz, open list:Sheepdog
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the sheepdog driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/sheepdog.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1ccb81b..be7cc39 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2971,18 +2971,17 @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
}
static coroutine_fn int64_t
-sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
- int *pnum, BlockDriverState **file)
+sd_co_block_status(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ int64_t *pnum, BlockDriverState **file)
{
BDRVSheepdogState *s = bs->opaque;
SheepdogInode *inode = &s->inode;
uint32_t object_size = (UINT32_C(1) << inode->block_size_shift);
- uint64_t offset = sector_num * BDRV_SECTOR_SIZE;
unsigned long start = offset / object_size,
- end = DIV_ROUND_UP((sector_num + nb_sectors) *
- BDRV_SECTOR_SIZE, object_size);
+ end = DIV_ROUND_UP(offset + bytes, object_size);
unsigned long idx;
- int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
+ int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
+ (offset & BDRV_BLOCK_OFFSET_MASK);
for (idx = start; idx < end; idx++) {
if (inode->data_vdi_id[idx] == 0) {
@@ -2999,9 +2998,9 @@ sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
}
}
- *pnum = (idx - start) * object_size / BDRV_SECTOR_SIZE;
- if (*pnum > nb_sectors) {
- *pnum = nb_sectors;
+ *pnum = (idx - start) * object_size;
+ if (*pnum > bytes) {
+ *pnum = bytes;
}
if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) {
*file = bs;
@@ -3079,7 +3078,7 @@ static BlockDriver bdrv_sheepdog = {
.bdrv_co_writev = sd_co_writev,
.bdrv_co_flush_to_disk = sd_co_flush_to_disk,
.bdrv_co_pdiscard = sd_co_pdiscard,
- .bdrv_co_get_block_status = sd_co_get_block_status,
+ .bdrv_co_block_status = sd_co_block_status,
.bdrv_snapshot_create = sd_snapshot_create,
.bdrv_snapshot_goto = sd_snapshot_goto,
@@ -3115,7 +3114,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
.bdrv_co_writev = sd_co_writev,
.bdrv_co_flush_to_disk = sd_co_flush_to_disk,
.bdrv_co_pdiscard = sd_co_pdiscard,
- .bdrv_co_get_block_status = sd_co_get_block_status,
+ .bdrv_co_block_status = sd_co_block_status,
.bdrv_snapshot_create = sd_snapshot_create,
.bdrv_snapshot_goto = sd_snapshot_goto,
@@ -3151,7 +3150,7 @@ static BlockDriver bdrv_sheepdog_unix = {
.bdrv_co_writev = sd_co_writev,
.bdrv_co_flush_to_disk = sd_co_flush_to_disk,
.bdrv_co_pdiscard = sd_co_pdiscard,
- .bdrv_co_get_block_status = sd_co_get_block_status,
+ .bdrv_co_block_status = sd_co_block_status,
.bdrv_snapshot_create = sd_snapshot_create,
.bdrv_snapshot_goto = sd_snapshot_goto,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (24 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 25/31] sheepdog: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 5:13 ` Stefan Weil
2017-04-18 1:33 ` [Qemu-devel] [PATCH 27/31] vdi: Switch to .bdrv_co_block_status() Eric Blake
` (6 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Stefan Weil, Kevin Wolf, Max Reitz
Rework the debug define so that we always get -Wformat checking,
even when debugging is disabled.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/vdi.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index d12d9cd..a70b969 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -86,12 +86,18 @@
#define DEFAULT_CLUSTER_SIZE (1 * MiB)
#if defined(CONFIG_VDI_DEBUG)
-#define logout(fmt, ...) \
- fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__)
+#define VDI_DEBUG 1
#else
-#define logout(fmt, ...) ((void)0)
+#define VDI_DEBUG 0
#endif
+#define logout(fmt, ...) \
+ do { \
+ if (VDI_DEBUG) { \
+ fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__); \
+ } \
+ } while (0)
+
/* Image signature. */
#define VDI_SIGNATURE 0xbeda107f
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 27/31] vdi: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (25 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 28/31] vmdk: " Eric Blake
` (5 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Stefan Weil, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the vdi driver accordingly. Note that the
TODO is already covered (the block layer guarantees bounds of its
requests), and that we can remove the now-unused s->block_sectors.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/vdi.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/block/vdi.c b/block/vdi.c
index a70b969..390e2f1 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -171,8 +171,6 @@ typedef struct {
uint32_t *bmap;
/* Size of block (bytes). */
uint32_t block_size;
- /* Size of block (sectors). */
- uint32_t block_sectors;
/* First sector of block map. */
uint32_t bmap_sector;
/* VDI header (converted to host endianness). */
@@ -462,7 +460,6 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
bs->total_sectors = header.disk_size / SECTOR_SIZE;
s->block_size = header.block_size;
- s->block_sectors = header.block_size / SECTOR_SIZE;
s->bmap_sector = header.offset_bmap / SECTOR_SIZE;
s->header = header;
@@ -508,23 +505,17 @@ static int vdi_reopen_prepare(BDRVReopenState *state,
return 0;
}
-static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn vdi_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *pnum, BlockDriverState **file)
{
- /* TODO: Check for too large sector_num (in bdrv_is_allocated or here). */
BDRVVdiState *s = (BDRVVdiState *)bs->opaque;
- size_t bmap_index = sector_num / s->block_sectors;
- size_t sector_in_block = sector_num % s->block_sectors;
- int n_sectors = s->block_sectors - sector_in_block;
+ size_t bmap_index = offset / s->block_size;
+ size_t index_in_block = offset % s->block_size;
uint32_t bmap_entry = le32_to_cpu(s->bmap[bmap_index]);
- uint64_t offset;
int result;
- logout("%p, %" PRId64 ", %d, %p\n", bs, sector_num, nb_sectors, pnum);
- if (n_sectors > nb_sectors) {
- n_sectors = nb_sectors;
- }
- *pnum = n_sectors;
+ logout("%p, %" PRId64 ", %" PRId64 ", %p\n", bs, offset, bytes, pnum);
+ *pnum = MIN(s->block_size, bytes);
result = VDI_IS_ALLOCATED(bmap_entry);
if (!result) {
return 0;
@@ -532,7 +523,7 @@ static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
offset = s->header.offset_data +
(uint64_t)bmap_entry * s->block_size +
- sector_in_block * SECTOR_SIZE;
+ (index_in_block & BDRV_BLOCK_OFFSET_MASK);
*file = bs->file->bs;
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
}
@@ -901,7 +892,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_child_perm = bdrv_format_default_perms,
.bdrv_create = vdi_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_get_block_status = vdi_co_get_block_status,
+ .bdrv_co_block_status = vdi_co_block_status,
.bdrv_make_empty = vdi_make_empty,
.bdrv_co_preadv = vdi_co_preadv,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 28/31] vmdk: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (26 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 27/31] vdi: Switch to .bdrv_co_block_status() Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 29/31] vpc: " Eric Blake
` (4 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Fam Zheng, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the vmdk driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/vmdk.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index c61b9cc..c85ce96 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1286,25 +1286,24 @@ static inline uint64_t vmdk_find_index_in_cluster(VmdkExtent *extent,
return offset / BDRV_SECTOR_SIZE;
}
-static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn vmdk_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *pnum, BlockDriverState **file)
{
BDRVVmdkState *s = bs->opaque;
int64_t index_in_cluster, n, ret;
- uint64_t offset;
+ uint64_t cluster_offset;
VmdkExtent *extent;
- extent = find_extent(s, sector_num, NULL);
+ extent = find_extent(s, offset >> BDRV_SECTOR_BITS, NULL);
if (!extent) {
return 0;
}
qemu_co_mutex_lock(&s->lock);
- ret = get_cluster_offset(bs, extent, NULL,
- sector_num * 512, false, &offset,
+ ret = get_cluster_offset(bs, extent, NULL, offset, false, &cluster_offset,
0, 0);
qemu_co_mutex_unlock(&s->lock);
- index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
+ index_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
switch (ret) {
case VMDK_ERROR:
ret = -EIO;
@@ -1319,18 +1318,15 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
ret = BDRV_BLOCK_DATA;
if (!extent->compressed) {
ret |= BDRV_BLOCK_OFFSET_VALID;
- ret |= (offset + (index_in_cluster << BDRV_SECTOR_BITS))
+ ret |= (cluster_offset + index_in_cluster)
& BDRV_BLOCK_OFFSET_MASK;
}
*file = extent->file->bs;
break;
}
- n = extent->cluster_sectors - index_in_cluster;
- if (n > nb_sectors) {
- n = nb_sectors;
- }
- *pnum = n;
+ n = extent->cluster_sectors * BDRV_SECTOR_SIZE - index_in_cluster;
+ *pnum = MIN(n, bytes);
return ret;
}
@@ -2362,7 +2358,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_close = vmdk_close,
.bdrv_create = vmdk_create,
.bdrv_co_flush_to_disk = vmdk_co_flush,
- .bdrv_co_get_block_status = vmdk_co_get_block_status,
+ .bdrv_co_block_status = vmdk_co_block_status,
.bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
.bdrv_has_zero_init = vmdk_has_zero_init,
.bdrv_get_specific_info = vmdk_get_specific_info,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 29/31] vpc: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (27 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 28/31] vmdk: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-07-13 12:55 ` Kevin Wolf
2017-04-18 1:33 ` [Qemu-devel] [PATCH 30/31] vvfat: " Eric Blake
` (3 subsequent siblings)
32 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the vpc driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/vpc.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index ecfee77..3cd56e7 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -689,46 +689,45 @@ fail:
return ret;
}
-static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
+static int64_t coroutine_fn vpc_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *pnum, BlockDriverState **file)
{
BDRVVPCState *s = bs->opaque;
VHDFooter *footer = (VHDFooter*) s->footer_buf;
- int64_t start, offset;
+ int64_t start, image_offset;
bool allocated;
int n;
if (be32_to_cpu(footer->type) == VHD_FIXED) {
- *pnum = nb_sectors;
+ *pnum = bytes;
*file = bs->file->bs;
return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
- (sector_num << BDRV_SECTOR_BITS);
+ (offset & BDRV_BLOCK_OFFSET_MASK);
}
- offset = get_sector_offset(bs, sector_num, 0);
- start = offset;
- allocated = (offset != -1);
+ image_offset = get_image_offset(bs, offset, 0);
+ start = image_offset & BDRV_BLOCK_OFFSET_MASK;
+ allocated = (image_offset != -1);
*pnum = 0;
do {
/* All sectors in a block are contiguous (without using the bitmap) */
- n = ROUND_UP(sector_num + 1, s->block_size / BDRV_SECTOR_SIZE)
- - sector_num;
- n = MIN(n, nb_sectors);
+ n = ROUND_UP(offset + 1, s->block_size) - offset;
+ n = MIN(n, bytes);
*pnum += n;
- sector_num += n;
- nb_sectors -= n;
+ offset += n;
+ bytes -= n;
/* *pnum can't be greater than one block for allocated
* sectors since there is always a bitmap in between. */
if (allocated) {
*file = bs->file->bs;
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
}
- if (nb_sectors == 0) {
+ if (bytes == 0) {
break;
}
- offset = get_sector_offset(bs, sector_num, 0);
+ image_offset = get_sector_offset(bs, offset, 0);
} while (offset == -1);
return 0;
@@ -1074,7 +1073,7 @@ static BlockDriver bdrv_vpc = {
.bdrv_co_preadv = vpc_co_preadv,
.bdrv_co_pwritev = vpc_co_pwritev,
- .bdrv_co_get_block_status = vpc_co_get_block_status,
+ .bdrv_co_block_status = vpc_co_block_status,
.bdrv_get_info = vpc_get_info,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 30/31] vvfat: Switch to .bdrv_co_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (28 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 29/31] vpc: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 31/31] block: Drop unused .bdrv_co_get_block_status() Eric Blake
` (2 subsequent siblings)
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Update the vvfat driver accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/vvfat.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index bef2056..825fe72 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2961,13 +2961,13 @@ vvfat_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
return ret;
}
-static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file)
+static int64_t coroutine_fn vvfat_co_block_status(BlockDriverState *bs,
+ int64_t offset, int64_t bytes, int64_t *n, BlockDriverState **file)
{
BDRVVVFATState* s = bs->opaque;
- *n = s->sector_count - sector_num;
- if (*n > nb_sectors) {
- *n = nb_sectors;
+ *n = s->sector_count * BDRV_SECTOR_SIZE - offset;
+ if (*n > bytes) {
+ *n = bytes;
} else if (*n < 0) {
return 0;
}
@@ -3124,7 +3124,7 @@ static BlockDriver bdrv_vvfat = {
.bdrv_co_preadv = vvfat_co_preadv,
.bdrv_co_pwritev = vvfat_co_pwritev,
- .bdrv_co_get_block_status = vvfat_co_get_block_status,
+ .bdrv_co_block_status = vvfat_co_block_status,
};
static void bdrv_vvfat_init(void)
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH 31/31] block: Drop unused .bdrv_co_get_block_status()
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (29 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 30/31] vvfat: " Eric Blake
@ 2017-04-18 1:33 ` Eric Blake
2017-04-18 1:37 ` [Qemu-devel] [Qemu-block] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
2017-04-18 20:23 ` [Qemu-devel] " Eric Blake
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:33 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Fam Zheng, Kevin Wolf,
Max Reitz
We are gradually moving away from sector-based interfaces, towards
byte-based. Now that all drivers have been updated to provide the
byte-based .bdrv_co_block_status(), we can delete the sector-based
interface.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/block/block_int.h | 3 ---
block/io.c | 59 ++++++++++++++++++++---------------------------
2 files changed, 25 insertions(+), 37 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8f20bc3..25197d7 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -174,9 +174,6 @@ struct BlockDriver {
* BDRV_BLOCK_DATA, _ZERO, _OFFSET_VALID, and _RAW, and only
* according to the current BDS.
*/
- int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors, int *pnum,
- BlockDriverState **file);
int64_t coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bd,
int64_t offset, int64_t bytes, int64_t *pnum,
BlockDriverState **file);
diff --git a/block/io.c b/block/io.c
index 361eeb8..0488d08 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1719,6 +1719,7 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
int64_t n; /* bytes */
int64_t ret, ret2;
BlockDriverState *tmp_file;
+ int64_t aligned_offset, aligned_bytes;
total_size = bdrv_getlength(bs);
if (total_size < 0) {
@@ -1738,7 +1739,7 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
bytes = n;
}
- if (!bs->drv->bdrv_co_get_block_status && !bs->drv->bdrv_co_block_status) {
+ if (!bs->drv->bdrv_co_block_status) {
*pnum = bytes;
ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
if (bs->drv->protocol_name) {
@@ -1752,45 +1753,35 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
}
*file = NULL;
bdrv_inc_in_flight(bs);
- if (bs->drv->bdrv_co_get_block_status) {
- int count; /* sectors */
- assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
- ret = bs->drv->bdrv_co_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
- bytes >> BDRV_SECTOR_BITS,
- &count, file);
- *pnum = count * BDRV_SECTOR_SIZE;
- } else {
- /* Round out to request_alignment boundaries */
- int64_t aligned_offset, aligned_bytes;
-
- aligned_offset = QEMU_ALIGN_DOWN(offset, bs->bl.request_alignment);
- aligned_bytes = ROUND_UP(offset + bytes,
- bs->bl.request_alignment) - aligned_offset;
- ret = bs->drv->bdrv_co_block_status(bs, aligned_offset, aligned_bytes,
- &n, file);
- /* Clamp pnum and ret to original request */
- if (aligned_offset != offset && ret >= 0) {
- int sectors = DIV_ROUND_UP(offset, BDRV_SECTOR_SIZE) -
- DIV_ROUND_UP(aligned_offset, BDRV_SECTOR_SIZE);
-
- assert(n >= offset - aligned_offset);
- n -= offset - aligned_offset;
- if (sectors) {
- ret += sectors * BDRV_SECTOR_SIZE;
- }
- }
- if (ret >= 0 && n > bytes) {
- assert(aligned_bytes != bytes);
- n = bytes;
- }
- *pnum = n;
- }
+ /* Round out to request_alignment boundaries */
+ aligned_offset = QEMU_ALIGN_DOWN(offset, bs->bl.request_alignment);
+ aligned_bytes = ROUND_UP(offset + bytes,
+ bs->bl.request_alignment) - aligned_offset;
+ ret = bs->drv->bdrv_co_block_status(bs, aligned_offset, aligned_bytes,
+ &n, file);
if (ret < 0) {
*pnum = 0;
goto out;
}
+ /* Clamp pnum and ret to original request */
+ if (aligned_offset != offset && ret >= 0) {
+ int sectors = DIV_ROUND_UP(offset, BDRV_SECTOR_SIZE) -
+ DIV_ROUND_UP(aligned_offset, BDRV_SECTOR_SIZE);
+
+ assert(n >= offset - aligned_offset);
+ n -= offset - aligned_offset;
+ if (sectors) {
+ ret += sectors * BDRV_SECTOR_SIZE;
+ }
+ }
+ if (n > bytes) {
+ assert(aligned_bytes != bytes);
+ n = bytes;
+ }
+ *pnum = n;
+
if (ret & BDRV_BLOCK_RAW) {
assert(ret & BDRV_BLOCK_OFFSET_VALID);
ret = bdrv_block_status(*file, ret & BDRV_BLOCK_OFFSET_MASK,
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 00/31] make bdrv_get_block_status byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (30 preceding siblings ...)
2017-04-18 1:33 ` [Qemu-devel] [PATCH 31/31] block: Drop unused .bdrv_co_get_block_status() Eric Blake
@ 2017-04-18 1:37 ` Eric Blake
2017-04-18 20:23 ` [Qemu-devel] " Eric Blake
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 1:37 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 701 bytes --]
On 04/17/2017 08:33 PM, Eric Blake wrote:
> There are patches floating around to add NBD_CMD_BLOCK_STATUS,
> but NBD wants to report status on byte granularity (even if the
> reporting will probably be naturally aligned to sectors or even
> much higher levels). I've therefore started the task of
> converting our block status code to report at a byte granularity
> rather than sectors.
Apologies for botching Kevin's address in the mail; if you reply, you'll
want to edit the to list if you don't want a bounce message about an
undeliverable address ;(
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code
2017-04-18 1:33 ` [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code Eric Blake
@ 2017-04-18 5:13 ` Stefan Weil
2017-05-13 20:35 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 48+ messages in thread
From: Stefan Weil @ 2017-04-18 5:13 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: qemu-block, kwolf, jsnow, Kevin Wolf, Max Reitz
Am 18.04.2017 um 03:33 schrieb Eric Blake:
> Rework the debug define so that we always get -Wformat checking,
> even when debugging is disabled.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
Reviewed-by: Stefan Weil <sw@weilnetz.de>
> block/vdi.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/block/vdi.c b/block/vdi.c
> index d12d9cd..a70b969 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -86,12 +86,18 @@
> #define DEFAULT_CLUSTER_SIZE (1 * MiB)
>
> #if defined(CONFIG_VDI_DEBUG)
> -#define logout(fmt, ...) \
> - fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__)
> +#define VDI_DEBUG 1
> #else
> -#define logout(fmt, ...) ((void)0)
> +#define VDI_DEBUG 0
> #endif
>
> +#define logout(fmt, ...) \
> + do { \
> + if (VDI_DEBUG) { \
> + fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__); \
> + } \
> + } while (0)
> +
> /* Image signature. */
> #define VDI_SIGNATURE 0xbeda107f
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 20/31] parallels: Switch to .bdrv_co_block_status()
2017-04-18 1:33 ` [Qemu-devel] [PATCH 20/31] parallels: " Eric Blake
@ 2017-04-18 10:28 ` Denis V. Lunev
0 siblings, 0 replies; 48+ messages in thread
From: Denis V. Lunev @ 2017-04-18 10:28 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: qemu-block, kwolf, jsnow, Stefan Hajnoczi, Kevin Wolf, Max Reitz
On 04/18/2017 04:33 AM, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the parallels driver accordingly. Note that
> the internal function block_status() is still sector-based, because
> it is still in use by other sector-based functions; but that's okay
> because request_alignment is 512 as a result of those functions.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
> ---
> block/parallels.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 8be46a7..2bc1918 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -274,22 +274,25 @@ static coroutine_fn int parallels_co_flush_to_os(BlockDriverState *bs)
> }
>
>
> -static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
> +static int64_t coroutine_fn parallels_co_block_status(BlockDriverState *bs,
> + int64_t offset, int64_t bytes, int64_t *pnum, BlockDriverState **file)
> {
> BDRVParallelsState *s = bs->opaque;
> - int64_t offset;
> + int count;
>
> + assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
> qemu_co_mutex_lock(&s->lock);
> - offset = block_status(s, sector_num, nb_sectors, pnum);
> + offset = block_status(s, offset >> BDRV_SECTOR_BITS,
> + bytes >> BDRV_SECTOR_BITS, &count);
> qemu_co_mutex_unlock(&s->lock);
>
> if (offset < 0) {
> return 0;
> }
>
> + *pnum = count * BDRV_SECTOR_SIZE;
> *file = bs->file->bs;
> - return (offset << BDRV_SECTOR_BITS) |
> + return (offset & BDRV_BLOCK_OFFSET_MASK) |
> BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
> }
>
> @@ -775,7 +778,7 @@ static BlockDriver bdrv_parallels = {
> .bdrv_open = parallels_open,
> .bdrv_close = parallels_close,
> .bdrv_child_perm = bdrv_format_default_perms,
> - .bdrv_co_get_block_status = parallels_co_get_block_status,
> + .bdrv_co_block_status = parallels_co_block_status,
> .bdrv_has_zero_init = bdrv_has_zero_init_1,
> .bdrv_co_flush_to_os = parallels_co_flush_to_os,
> .bdrv_co_readv = parallels_co_readv,
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
` (31 preceding siblings ...)
2017-04-18 1:37 ` [Qemu-devel] [Qemu-block] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
@ 2017-04-18 20:23 ` Eric Blake
32 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 20:23 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jsnow, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
On 04/17/2017 08:33 PM, Eric Blake wrote:
> There are patches floating around to add NBD_CMD_BLOCK_STATUS,
> but NBD wants to report status on byte granularity (even if the
> reporting will probably be naturally aligned to sectors or even
> much higher levels). I've therefore started the task of
> converting our block status code to report at a byte granularity
> rather than sectors.
>
> This is part three of that conversion: dirty-bitmap. Earlier parts
Correction: this is part three: bdrv_get_block_status
> have been (mostly) reviewed, for bdrv_is_allocated and dirty-bitmaps.
>
> Perhaps I could have split this in two; patches 1-10 vs. 11-31 make
> a nice division of labor.
>
> Available as a tag at:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-status-v1
>
> It requires the following (v1 of bdrv_is_allocated, v1 of dirty-bitmap,
> v9 of blkdebug, and Max's block-next tree):
> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01995.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01723.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg01298.html
> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02163.html
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 13/31] file-posix: Switch to .bdrv_co_block_status()
2017-04-18 1:33 ` [Qemu-devel] [PATCH 13/31] file-posix: " Eric Blake
@ 2017-04-18 20:56 ` Eric Blake
0 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 20:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, jsnow, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 3125 bytes --]
On 04/17/2017 08:33 PM, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the file protocol driver accordingly.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> block/file-posix.c | 47 +++++++++++++++++++++++------------------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
This patch makes qemu-iotests 109 fail, due to an assertion in 6/31 that
the nested call to bdrv_block_status() [thanks to BDRV_BLOCK_RAW] is
still sector-aligned. The culprit:
> +static int64_t coroutine_fn raw_co_block_status(BlockDriverState *bs,
> + int64_t offset,
> + int64_t bytes, int64_t *pnum,
> + BlockDriverState **file)
> {
> - off_t start, data = 0, hole = 0;
> + off_t data = 0, hole = 0;
> int64_t total_size;
> int ret;
>
> @@ -1856,39 +1856,38 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
> return ret;
> }
>
> - start = sector_num * BDRV_SECTOR_SIZE;
> total_size = bdrv_getlength(bs);
total_size is always sector-aligned (bdrv_getlength() purposefully
widens the input file size, 560 bytes in the case of an empty qcow image
in test 109, out to a sector boundary)...
> if (total_size < 0) {
> return total_size;
> - } else if (start >= total_size) {
> + } else if (offset >= total_size) {
> *pnum = 0;
> return 0;
> - } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
> - nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
> + } else if (offset + bytes > total_size) {
> + bytes = total_size - offset;
> }
>
> - ret = find_allocation(bs, start, &data, &hole);
> + ret = find_allocation(bs, offset, &data, &hole);
...while find_allocation still obeys byte-granularity limits of the
underlying file system, and therefore reports a hole starting at offset
560 (the only time a hole starts at a non-sector boundary); pre-patch
that didn't matter (because we rounded before returning sectors through
*pnum), but post-patch, returning 560 instead of 1024 triggers problems.
I'm still debating whether the best fix is to tweak the file-posix
limits to just round up to sector boundaries at EOF, or to make the
block layer itself special-case EOF in case other protocols also have
byte-granularity support. Any advice you have while reviewing is
appreciated, but it does mean I'll need a v2 of the series one way or
another.
And I didn't spot it during my earlier testing because I was skipping
109 as failing prior to my patches even started testing it - but now
that Jeff and Fam have both posted patches for two separate problems in
test 109 (the need to munge "len", and failure to do proper cleanup), I
no longer have that excuse.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* [Qemu-devel] [PATCH v1.5 06.5/31] fixup! block: Convert bdrv_get_block_status() to bytes
2017-04-18 1:33 ` [Qemu-devel] [PATCH 06/31] block: Convert bdrv_get_block_status() to bytes Eric Blake
@ 2017-04-18 21:34 ` Eric Blake
0 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-04-18 21:34 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, qemu-block, Stefan Hajnoczi, Max Reitz
[adjust a paragraph of the original commit message]
...
Note that we have an inherent limitation in the BDRV_BLOCK_* return
values: BDRV_BLOCK_OFFSET_VALID can only return the start of a
sector, even if we later relax the interface to query for the status
starting at an intermediate byte; document the obvious interpretation
that valid offsets are always sector-relative. Furthermore, note that
many callers are expecting sector-aligned answers, especially since
bdrv_getlength() is sector-aligned even when it exceeds the actual
file size; so we intentionally lie and state that any partial sector
at the end of a file has the same status for the entire sector,
rather than a literal interpretation of a hole starting at a
mid-sector location due to end-of-file.
...
Signed-off-by: Eric Blake <eblake@redhat.com>
---
This was sufficient to avoid failure of qemu-iotests 109 after patch
13/31 adjusts file-posix.c to clamp EOF numbers to mid-sector locations.
block/io.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/io.c b/block/io.c
index 361eeb8..bee6c71 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1769,6 +1769,14 @@ static int64_t coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
bs->bl.request_alignment) - aligned_offset;
ret = bs->drv->bdrv_co_block_status(bs, aligned_offset, aligned_bytes,
&n, file);
+
+ /* total_size is always sector-aligned, by sometimes exceeding actual
+ * file size. Expand n if it lands mid-sector due to end-of-file. */
+ if (ret >= 0 && QEMU_ALIGN_UP(n + aligned_offset,
+ BDRV_SECTOR_SIZE) == total_size) {
+ n = total_size - aligned_offset;
+ }
+
/* Clamp pnum and ret to original request */
if (aligned_offset != offset && ret >= 0) {
int sectors = DIV_ROUND_UP(offset, BDRV_SECTOR_SIZE) -
--
2.9.3
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 14/31] gluster: Switch to .bdrv_co_block_status()
2017-04-18 1:33 ` [Qemu-devel] [PATCH 14/31] gluster: " Eric Blake
@ 2017-04-19 13:26 ` Niels de Vos
2017-04-19 14:06 ` [Qemu-devel] " Prasanna Kalever
1 sibling, 0 replies; 48+ messages in thread
From: Niels de Vos @ 2017-04-19 13:26 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Kevin Wolf, qemu-block, Max Reitz
On Mon, Apr 17, 2017 at 08:33:39PM -0500, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the gluster driver accordingly.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> block/gluster.c | 47 +++++++++++++++++++++++------------------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
Looks good to me, thanks.
Reviewed-by: Niels de Vos <ndevos@redhat.com>
> diff --git a/block/gluster.c b/block/gluster.c
> index 1d4e2f7..3f252c6 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -1332,24 +1332,24 @@ exit:
> /*
> * Returns the allocation status of the specified sectors.
> *
> - * If 'sector_num' is beyond the end of the disk image the return value is 0
> + * If 'offset' is beyond the end of the disk image the return value is 0
> * and 'pnum' is set to 0.
> *
> - * 'pnum' is set to the number of sectors (including and immediately following
> - * the specified sector) that are known to be in the same
> + * 'pnum' is set to the number of bytes (including and immediately following
> + * the specified offset) that are known to be in the same
> * allocated/unallocated state.
> *
> - * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
> + * 'bytes' is the max value 'pnum' should be set to. If bytes goes
> * beyond the end of the disk image it will be clamped.
> *
> - * (Based on raw_co_get_block_status() from file-posix.c.)
> + * (Based on raw_co_block_status() from file-posix.c.)
> */
> -static int64_t coroutine_fn qemu_gluster_co_get_block_status(
> - BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
> +static int64_t coroutine_fn qemu_gluster_co_block_status(
> + BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum,
> BlockDriverState **file)
> {
> BDRVGlusterState *s = bs->opaque;
> - off_t start, data = 0, hole = 0;
> + off_t data = 0, hole = 0;
> int64_t total_size;
> int ret = -EINVAL;
>
> @@ -1357,41 +1357,40 @@ static int64_t coroutine_fn qemu_gluster_co_get_block_status(
> return ret;
> }
>
> - start = sector_num * BDRV_SECTOR_SIZE;
> total_size = bdrv_getlength(bs);
> if (total_size < 0) {
> return total_size;
> - } else if (start >= total_size) {
> + } else if (offset >= total_size) {
> *pnum = 0;
> return 0;
> - } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
> - nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
> + } else if (offset + bytes > total_size) {
> + bytes = total_size - offset;
> }
>
> - ret = find_allocation(bs, start, &data, &hole);
> + ret = find_allocation(bs, offset, &data, &hole);
> if (ret == -ENXIO) {
> /* Trailing hole */
> - *pnum = nb_sectors;
> + *pnum = bytes;
> ret = BDRV_BLOCK_ZERO;
> } else if (ret < 0) {
> /* No info available, so pretend there are no holes */
> - *pnum = nb_sectors;
> + *pnum = bytes;
> ret = BDRV_BLOCK_DATA;
> - } else if (data == start) {
> + } else if (data == offset) {
> /* On a data extent, compute sectors to the end of the extent,
> * possibly including a partial sector at EOF. */
> - *pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
> + *pnum = MIN(bytes, hole - offset);
> ret = BDRV_BLOCK_DATA;
> } else {
> /* On a hole, compute sectors to the beginning of the next extent. */
> - assert(hole == start);
> - *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
> + assert(hole == offset);
> + *pnum = MIN(bytes, data - offset);
> ret = BDRV_BLOCK_ZERO;
> }
>
> *file = bs;
>
> - return ret | BDRV_BLOCK_OFFSET_VALID | start;
> + return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
> }
>
>
> @@ -1419,7 +1418,7 @@ static BlockDriver bdrv_gluster = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> @@ -1447,7 +1446,7 @@ static BlockDriver bdrv_gluster_tcp = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> @@ -1475,7 +1474,7 @@ static BlockDriver bdrv_gluster_unix = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> @@ -1509,7 +1508,7 @@ static BlockDriver bdrv_gluster_rdma = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 14/31] gluster: Switch to .bdrv_co_block_status()
2017-04-18 1:33 ` [Qemu-devel] [PATCH 14/31] gluster: " Eric Blake
2017-04-19 13:26 ` [Qemu-devel] [Qemu-block] " Niels de Vos
@ 2017-04-19 14:06 ` Prasanna Kalever
2017-04-19 14:08 ` Prasanna Kalever
1 sibling, 1 reply; 48+ messages in thread
From: Prasanna Kalever @ 2017-04-19 14:06 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, Kevin Wolf, qemu-block, Jeff Cody, Max Reitz, kwolf, jsnow
On Tue, Apr 18, 2017 at 7:03 AM, Eric Blake <eblake@redhat.com> wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the gluster driver accordingly.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> block/gluster.c | 47 +++++++++++++++++++++++------------------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 1d4e2f7..3f252c6 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -1332,24 +1332,24 @@ exit:
> /*
> * Returns the allocation status of the specified sectors.
> *
> - * If 'sector_num' is beyond the end of the disk image the return value is 0
> + * If 'offset' is beyond the end of the disk image the return value is 0
> * and 'pnum' is set to 0.
> *
> - * 'pnum' is set to the number of sectors (including and immediately following
> - * the specified sector) that are known to be in the same
> + * 'pnum' is set to the number of bytes (including and immediately following
> + * the specified offset) that are known to be in the same
> * allocated/unallocated state.
> *
> - * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
> + * 'bytes' is the max value 'pnum' should be set to. If bytes goes
> * beyond the end of the disk image it will be clamped.
> *
> - * (Based on raw_co_get_block_status() from file-posix.c.)
> + * (Based on raw_co_block_status() from file-posix.c.)
> */
> -static int64_t coroutine_fn qemu_gluster_co_get_block_status(
> - BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
> +static int64_t coroutine_fn qemu_gluster_co_block_status(
> + BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum,
> BlockDriverState **file)
> {
> BDRVGlusterState *s = bs->opaque;
> - off_t start, data = 0, hole = 0;
> + off_t data = 0, hole = 0;
> int64_t total_size;
> int ret = -EINVAL;
>
> @@ -1357,41 +1357,40 @@ static int64_t coroutine_fn qemu_gluster_co_get_block_status(
> return ret;
> }
>
> - start = sector_num * BDRV_SECTOR_SIZE;
> total_size = bdrv_getlength(bs);
> if (total_size < 0) {
> return total_size;
> - } else if (start >= total_size) {
> + } else if (offset >= total_size) {
> *pnum = 0;
> return 0;
> - } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
> - nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
> + } else if (offset + bytes > total_size) {
> + bytes = total_size - offset;
> }
>
> - ret = find_allocation(bs, start, &data, &hole);
> + ret = find_allocation(bs, offset, &data, &hole);
> if (ret == -ENXIO) {
> /* Trailing hole */
> - *pnum = nb_sectors;
> + *pnum = bytes;
> ret = BDRV_BLOCK_ZERO;
> } else if (ret < 0) {
> /* No info available, so pretend there are no holes */
> - *pnum = nb_sectors;
> + *pnum = bytes;
> ret = BDRV_BLOCK_DATA;
> - } else if (data == start) {
> + } else if (data == offset) {
> /* On a data extent, compute sectors to the end of the extent,
s/sectors/byes/
> * possibly including a partial sector at EOF. */
> - *pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
> + *pnum = MIN(bytes, hole - offset);
> ret = BDRV_BLOCK_DATA;
> } else {
> /* On a hole, compute sectors to the beginning of the next extent. */
s/sectors/byes/
--
prasanna
> - assert(hole == start);
> - *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
> + assert(hole == offset);
> + *pnum = MIN(bytes, data - offset);
> ret = BDRV_BLOCK_ZERO;
> }
>
> *file = bs;
>
> - return ret | BDRV_BLOCK_OFFSET_VALID | start;
> + return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
> }
>
>
> @@ -1419,7 +1418,7 @@ static BlockDriver bdrv_gluster = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> @@ -1447,7 +1446,7 @@ static BlockDriver bdrv_gluster_tcp = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> @@ -1475,7 +1474,7 @@ static BlockDriver bdrv_gluster_unix = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> @@ -1509,7 +1508,7 @@ static BlockDriver bdrv_gluster_rdma = {
> #ifdef CONFIG_GLUSTERFS_ZEROFILL
> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
> #endif
> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
> + .bdrv_co_block_status = qemu_gluster_co_block_status,
> .create_opts = &qemu_gluster_create_opts,
> };
>
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 14/31] gluster: Switch to .bdrv_co_block_status()
2017-04-19 14:06 ` [Qemu-devel] " Prasanna Kalever
@ 2017-04-19 14:08 ` Prasanna Kalever
0 siblings, 0 replies; 48+ messages in thread
From: Prasanna Kalever @ 2017-04-19 14:08 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, Jeff Cody, Max Reitz, kwolf, jsnow
oops!
That's 'bytes'.
On Wed, Apr 19, 2017 at 7:36 PM, Prasanna Kalever <pkalever@redhat.com> wrote:
> On Tue, Apr 18, 2017 at 7:03 AM, Eric Blake <eblake@redhat.com> wrote:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based. Update the gluster driver accordingly.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>> block/gluster.c | 47 +++++++++++++++++++++++------------------------
>> 1 file changed, 23 insertions(+), 24 deletions(-)
>>
>> diff --git a/block/gluster.c b/block/gluster.c
>> index 1d4e2f7..3f252c6 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -1332,24 +1332,24 @@ exit:
>> /*
>> * Returns the allocation status of the specified sectors.
>> *
>> - * If 'sector_num' is beyond the end of the disk image the return value is 0
>> + * If 'offset' is beyond the end of the disk image the return value is 0
>> * and 'pnum' is set to 0.
>> *
>> - * 'pnum' is set to the number of sectors (including and immediately following
>> - * the specified sector) that are known to be in the same
>> + * 'pnum' is set to the number of bytes (including and immediately following
>> + * the specified offset) that are known to be in the same
>> * allocated/unallocated state.
>> *
>> - * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
>> + * 'bytes' is the max value 'pnum' should be set to. If bytes goes
>> * beyond the end of the disk image it will be clamped.
>> *
>> - * (Based on raw_co_get_block_status() from file-posix.c.)
>> + * (Based on raw_co_block_status() from file-posix.c.)
>> */
>> -static int64_t coroutine_fn qemu_gluster_co_get_block_status(
>> - BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum,
>> +static int64_t coroutine_fn qemu_gluster_co_block_status(
>> + BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum,
>> BlockDriverState **file)
>> {
>> BDRVGlusterState *s = bs->opaque;
>> - off_t start, data = 0, hole = 0;
>> + off_t data = 0, hole = 0;
>> int64_t total_size;
>> int ret = -EINVAL;
>>
>> @@ -1357,41 +1357,40 @@ static int64_t coroutine_fn qemu_gluster_co_get_block_status(
>> return ret;
>> }
>>
>> - start = sector_num * BDRV_SECTOR_SIZE;
>> total_size = bdrv_getlength(bs);
>> if (total_size < 0) {
>> return total_size;
>> - } else if (start >= total_size) {
>> + } else if (offset >= total_size) {
>> *pnum = 0;
>> return 0;
>> - } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
>> - nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
>> + } else if (offset + bytes > total_size) {
>> + bytes = total_size - offset;
>> }
>>
>> - ret = find_allocation(bs, start, &data, &hole);
>> + ret = find_allocation(bs, offset, &data, &hole);
>> if (ret == -ENXIO) {
>> /* Trailing hole */
>> - *pnum = nb_sectors;
>> + *pnum = bytes;
>> ret = BDRV_BLOCK_ZERO;
>> } else if (ret < 0) {
>> /* No info available, so pretend there are no holes */
>> - *pnum = nb_sectors;
>> + *pnum = bytes;
>> ret = BDRV_BLOCK_DATA;
>> - } else if (data == start) {
>> + } else if (data == offset) {
>> /* On a data extent, compute sectors to the end of the extent,
>
> s/sectors/byes/
>
>> * possibly including a partial sector at EOF. */
>> - *pnum = MIN(nb_sectors, DIV_ROUND_UP(hole - start, BDRV_SECTOR_SIZE));
>> + *pnum = MIN(bytes, hole - offset);
>> ret = BDRV_BLOCK_DATA;
>> } else {
>> /* On a hole, compute sectors to the beginning of the next extent. */
>
> s/sectors/byes/
>
>
> --
> prasanna
>
>> - assert(hole == start);
>> - *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
>> + assert(hole == offset);
>> + *pnum = MIN(bytes, data - offset);
>> ret = BDRV_BLOCK_ZERO;
>> }
>>
>> *file = bs;
>>
>> - return ret | BDRV_BLOCK_OFFSET_VALID | start;
>> + return ret | BDRV_BLOCK_OFFSET_VALID | (offset & BDRV_BLOCK_OFFSET_MASK);
>> }
>>
>>
>> @@ -1419,7 +1418,7 @@ static BlockDriver bdrv_gluster = {
>> #ifdef CONFIG_GLUSTERFS_ZEROFILL
>> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
>> #endif
>> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
>> + .bdrv_co_block_status = qemu_gluster_co_block_status,
>> .create_opts = &qemu_gluster_create_opts,
>> };
>>
>> @@ -1447,7 +1446,7 @@ static BlockDriver bdrv_gluster_tcp = {
>> #ifdef CONFIG_GLUSTERFS_ZEROFILL
>> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
>> #endif
>> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
>> + .bdrv_co_block_status = qemu_gluster_co_block_status,
>> .create_opts = &qemu_gluster_create_opts,
>> };
>>
>> @@ -1475,7 +1474,7 @@ static BlockDriver bdrv_gluster_unix = {
>> #ifdef CONFIG_GLUSTERFS_ZEROFILL
>> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
>> #endif
>> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
>> + .bdrv_co_block_status = qemu_gluster_co_block_status,
>> .create_opts = &qemu_gluster_create_opts,
>> };
>>
>> @@ -1509,7 +1508,7 @@ static BlockDriver bdrv_gluster_rdma = {
>> #ifdef CONFIG_GLUSTERFS_ZEROFILL
>> .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes,
>> #endif
>> - .bdrv_co_get_block_status = qemu_gluster_co_get_block_status,
>> + .bdrv_co_block_status = qemu_gluster_co_block_status,
>> .create_opts = &qemu_gluster_create_opts,
>> };
>>
>> --
>> 2.9.3
>>
>>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful
2017-04-18 1:33 ` [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful Eric Blake
@ 2017-04-26 21:41 ` John Snow
2017-04-26 21:47 ` Eric Blake
0 siblings, 1 reply; 48+ messages in thread
From: John Snow @ 2017-04-26 21:41 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Fam Zheng, qemu-block, Jeff Cody, Max Reitz,
Stefan Hajnoczi, kwolf
On 04/17/2017 09:33 PM, Eric Blake wrote:
> In the process of converting sector-based interfaces to bytes,
> I'm finding it easier to represent a byte count as a 64-bit
> integer at the block layer (even if we are internally capped
> by SIZE_MAX or even INT_MAX for individual transactions, it's
> still nicer to not have to worry about truncation/overflow
> issues on as many variables). Update the signature of
> bdrv_round_to_clusters() to uniformly use uint64_t, matching
^^^^^^^^
While we're here, since you went with int64_t in the end, what steered
you away from uint64_t, or was that just a thinko?
(AFAICT: off_t is usually something like int64_t, so your choice makes
sense to me, generally.)
--js
> the signature already chosen for bdrv_is_allocated, and
> adjust clients according to the required fallout.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> include/block/block.h | 4 ++--
> block/io.c | 7 ++++---
> block/mirror.c | 9 ++++-----
> block/trace-events | 2 +-
> 4 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 86ad511..eed1330 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -468,9 +468,9 @@ int bdrv_get_flags(BlockDriverState *bs);
> int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
> ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
> void bdrv_round_to_clusters(BlockDriverState *bs,
> - int64_t offset, unsigned int bytes,
> + int64_t offset, int64_t bytes,
> int64_t *cluster_offset,
> - unsigned int *cluster_bytes);
> + int64_t *cluster_bytes);
>
> const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
> void bdrv_get_backing_filename(BlockDriverState *bs,
> diff --git a/block/io.c b/block/io.c
> index d61a906..07165dc 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -422,9 +422,9 @@ static void mark_request_serialising(BdrvTrackedRequest *req, uint64_t align)
> * Round a region to cluster boundaries
> */
> void bdrv_round_to_clusters(BlockDriverState *bs,
> - int64_t offset, unsigned int bytes,
> + int64_t offset, int64_t bytes,
> int64_t *cluster_offset,
> - unsigned int *cluster_bytes)
> + int64_t *cluster_bytes)
> {
> BlockDriverInfo bdi;
>
> @@ -920,7 +920,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
> struct iovec iov;
> QEMUIOVector bounce_qiov;
> int64_t cluster_offset;
> - unsigned int cluster_bytes;
> + int64_t cluster_bytes;
> size_t skip_bytes;
> int ret;
>
> @@ -941,6 +941,7 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
> trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
> cluster_offset, cluster_bytes);
>
> + assert(cluster_bytes < SIZE_MAX);
> iov.iov_len = cluster_bytes;
> iov.iov_base = bounce_buffer = qemu_try_blockalign(bs, iov.iov_len);
> if (bounce_buffer == NULL) {
> diff --git a/block/mirror.c b/block/mirror.c
> index 846e392..2510793 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -177,7 +177,7 @@ static void mirror_read_complete(void *opaque, int ret)
> /* Clip bytes relative to offset to not exceed end-of-file */
> static inline void mirror_clip_bytes(MirrorBlockJob *s,
> int64_t offset,
> - unsigned int *bytes)
> + int64_t *bytes)
> {
> *bytes = MIN(*bytes, s->bdev_length - offset);
> }
> @@ -190,10 +190,9 @@ static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset,
> bool need_cow;
> int ret = 0;
> int64_t align_offset = *offset;
> - unsigned int align_bytes = *bytes;
> + int64_t align_bytes = *bytes;
> int max_bytes = s->granularity * s->max_iov;
>
> - assert(*bytes < INT_MAX);
> need_cow = !test_bit(*offset / s->granularity, s->cow_bitmap);
> need_cow |= !test_bit((*offset + *bytes - 1) / s->granularity,
> s->cow_bitmap);
> @@ -384,7 +383,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> while (nb_chunks > 0 && offset < s->bdev_length) {
> int64_t ret;
> int io_sectors;
> - unsigned int io_bytes;
> + int64_t io_bytes;
> int64_t io_bytes_acct;
> BlockDriverState *file;
> enum MirrorMethod {
> @@ -410,7 +409,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> io_bytes = s->granularity;
> } else if (ret >= 0 && !(ret & BDRV_BLOCK_DATA)) {
> int64_t target_offset;
> - unsigned int target_bytes;
> + int64_t target_bytes;
> bdrv_round_to_clusters(blk_bs(s->target), offset, io_bytes,
> &target_offset, &target_bytes);
> if (target_offset == offset &&
> diff --git a/block/trace-events b/block/trace-events
> index 04f6463..2301a50 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -15,7 +15,7 @@ bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs
> bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
> bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
> bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags %#x"
> -bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, unsigned int cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %u"
> +bdrv_co_do_copy_on_readv(void *bs, int64_t offset, int64_t bytes, int64_t cluster_offset, unsigned int cluster_bytes) "bs %p offset %"PRId64" bytes %"PRId64" cluster_offset %"PRId64" cluster_bytes %u"
>
> # block/stream.c
> stream_one_iteration(void *s, int64_t offset, uint64_t bytes, int is_allocated) "s %p offset %" PRId64 " bytes %" PRIu64 " is_allocated %d"
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters()
2017-04-18 1:33 ` [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters() Eric Blake
@ 2017-04-26 21:41 ` John Snow
0 siblings, 0 replies; 48+ messages in thread
From: John Snow @ 2017-04-26 21:41 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Fam Zheng, qemu-block, Max Reitz, Stefan Hajnoczi, kwolf
On 04/17/2017 09:33 PM, Eric Blake wrote:
> Now that the last user [mirror_iteration()] has converted to using
> bytes, we no longer need a function to round sectors to clusters.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
> ---
> include/block/block.h | 4 ----
> block/io.c | 21 ---------------------
> 2 files changed, 25 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 740cb86..86ad511 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -467,10 +467,6 @@ const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
> int bdrv_get_flags(BlockDriverState *bs);
> int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
> ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
> -void bdrv_round_sectors_to_clusters(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors,
> - int64_t *cluster_sector_num,
> - int *cluster_nb_sectors);
> void bdrv_round_to_clusters(BlockDriverState *bs,
> int64_t offset, unsigned int bytes,
> int64_t *cluster_offset,
> diff --git a/block/io.c b/block/io.c
> index d22d35f..d61a906 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -419,27 +419,6 @@ static void mark_request_serialising(BdrvTrackedRequest *req, uint64_t align)
> }
>
> /**
> - * Round a region to cluster boundaries (sector-based)
> - */
> -void bdrv_round_sectors_to_clusters(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors,
> - int64_t *cluster_sector_num,
> - int *cluster_nb_sectors)
> -{
> - BlockDriverInfo bdi;
> -
> - if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) {
> - *cluster_sector_num = sector_num;
> - *cluster_nb_sectors = nb_sectors;
> - } else {
> - int64_t c = bdi.cluster_size / BDRV_SECTOR_SIZE;
> - *cluster_sector_num = QEMU_ALIGN_DOWN(sector_num, c);
> - *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
> - nb_sectors, c);
> - }
> -}
> -
> -/**
> * Round a region to cluster boundaries
> */
> void bdrv_round_to_clusters(BlockDriverState *bs,
>
--
—js
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful
2017-04-26 21:41 ` John Snow
@ 2017-04-26 21:47 ` Eric Blake
2017-04-26 21:54 ` John Snow
0 siblings, 1 reply; 48+ messages in thread
From: Eric Blake @ 2017-04-26 21:47 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: Kevin Wolf, Fam Zheng, qemu-block, Jeff Cody, Max Reitz,
Stefan Hajnoczi, kwolf
[-- Attachment #1: Type: text/plain, Size: 1568 bytes --]
On 04/26/2017 04:41 PM, John Snow wrote:
>
>
> On 04/17/2017 09:33 PM, Eric Blake wrote:
>> In the process of converting sector-based interfaces to bytes,
>> I'm finding it easier to represent a byte count as a 64-bit
>> integer at the block layer (even if we are internally capped
>> by SIZE_MAX or even INT_MAX for individual transactions, it's
>> still nicer to not have to worry about truncation/overflow
>> issues on as many variables). Update the signature of
>> bdrv_round_to_clusters() to uniformly use uint64_t, matching
> ^^^^^^^^
>
> While we're here, since you went with int64_t in the end, what steered
> you away from uint64_t, or was that just a thinko?
Later patches were made easier with signed (the compiler complained when
I mixed signed and unsigned pointers).
>
> (AFAICT: off_t is usually something like int64_t, so your choice makes
> sense to me, generally.)
Indeed, and that's something I should update my commit message to mention.
>
> --js
>
>> the signature already chosen for bdrv_is_allocated, and
>> adjust clients according to the required fallout.
If you want me to try and use uint64_t *pnum instead of int64_t *pnum
throughout both my series 1 (the changes to bdrv_is_allocated) and this
one, it will take more effort. I'll do it if there's a reason, but I'd
rather not if the signed version is good enough.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful
2017-04-26 21:47 ` Eric Blake
@ 2017-04-26 21:54 ` John Snow
0 siblings, 0 replies; 48+ messages in thread
From: John Snow @ 2017-04-26 21:54 UTC (permalink / raw)
To: Eric Blake, qemu-devel
Cc: Kevin Wolf, Fam Zheng, qemu-block, Jeff Cody, Max Reitz,
Stefan Hajnoczi, kwolf
[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]
On 04/26/2017 05:47 PM, Eric Blake wrote:
> On 04/26/2017 04:41 PM, John Snow wrote:
>>
>>
>> On 04/17/2017 09:33 PM, Eric Blake wrote:
>>> In the process of converting sector-based interfaces to bytes,
>>> I'm finding it easier to represent a byte count as a 64-bit
>>> integer at the block layer (even if we are internally capped
>>> by SIZE_MAX or even INT_MAX for individual transactions, it's
>>> still nicer to not have to worry about truncation/overflow
>>> issues on as many variables). Update the signature of
>>> bdrv_round_to_clusters() to uniformly use uint64_t, matching
>> ^^^^^^^^
>>
>> While we're here, since you went with int64_t in the end, what steered
>> you away from uint64_t, or was that just a thinko?
>
> Later patches were made easier with signed (the compiler complained when
> I mixed signed and unsigned pointers).
>
>>
>> (AFAICT: off_t is usually something like int64_t, so your choice makes
>> sense to me, generally.)
>
> Indeed, and that's something I should update my commit message to mention.
>
>>
>> --js
>>
>>> the signature already chosen for bdrv_is_allocated, and
>>> adjust clients according to the required fallout.
>
> If you want me to try and use uint64_t *pnum instead of int64_t *pnum
> throughout both my series 1 (the changes to bdrv_is_allocated) and this
> one, it will take more effort. I'll do it if there's a reason, but I'd
> rather not if the signed version is good enough.
>
No, I didn't mean to imply you should, I was just pointing out the
commit message typo. int64_t is likely the correct choice for a number
of reasons, at least being able to return -1 from functions returning a
byte offset being the chief reason.
--js
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 843 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code
2017-04-18 5:13 ` Stefan Weil
@ 2017-05-13 20:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 48+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-13 20:35 UTC (permalink / raw)
To: Stefan Weil, Eric Blake, qemu-devel
Cc: Kevin Wolf, kwolf, jsnow, qemu-block, Max Reitz
On 04/18/2017 02:13 AM, Stefan Weil wrote:
> Am 18.04.2017 um 03:33 schrieb Eric Blake:
>> Rework the debug define so that we always get -Wformat checking,
>> even when debugging is disabled.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>
> Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> block/vdi.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/vdi.c b/block/vdi.c
>> index d12d9cd..a70b969 100644
>> --- a/block/vdi.c
>> +++ b/block/vdi.c
>> @@ -86,12 +86,18 @@
>> #define DEFAULT_CLUSTER_SIZE (1 * MiB)
>>
>> #if defined(CONFIG_VDI_DEBUG)
>> -#define logout(fmt, ...) \
>> - fprintf(stderr, "vdi\t%-24s" fmt, __func__,
>> ##__VA_ARGS__)
>> +#define VDI_DEBUG 1
>> #else
>> -#define logout(fmt, ...) ((void)0)
>> +#define VDI_DEBUG 0
>> #endif
>>
>> +#define logout(fmt, ...) \
>> + do
>> { \
>> + if (VDI_DEBUG)
>> { \
>> + fprintf(stderr, "vdi\t%-24s" fmt, __func__,
>> ##__VA_ARGS__); \
>> +
>> } \
>> + } while (0)
>> +
>> /* Image signature. */
>> #define VDI_SIGNATURE 0xbeda107f
>>
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 29/31] vpc: Switch to .bdrv_co_block_status()
2017-04-18 1:33 ` [Qemu-devel] [PATCH 29/31] vpc: " Eric Blake
@ 2017-07-13 12:55 ` Kevin Wolf
2017-07-13 13:52 ` Eric Blake
0 siblings, 1 reply; 48+ messages in thread
From: Kevin Wolf @ 2017-07-13 12:55 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, qemu-block, kwolf, jsnow, Max Reitz
Am 18.04.2017 um 03:33 hat Eric Blake geschrieben:
> We are gradually moving away from sector-based interfaces, towards
> byte-based. Update the vpc driver accordingly.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
I know this is an old series, but I think you split it for later
versions and there hasn't been a respin of this final part of the series
yet.
When I just told Peter that get_sector_offset() would go away with your
patches, I decided to check whether this was actually true, and found...
> /* *pnum can't be greater than one block for allocated
> * sectors since there is always a bitmap in between. */
> if (allocated) {
> *file = bs->file->bs;
> return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
> }
> - if (nb_sectors == 0) {
> + if (bytes == 0) {
> break;
> }
> - offset = get_sector_offset(bs, sector_num, 0);
> + image_offset = get_sector_offset(bs, offset, 0);
> } while (offset == -1);
...this bug. I think you want to use get_image_offset() now.
This should also be the last caller of get_sector_offset(), so the
function should go away in this commit.
Kevin
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [Qemu-devel] [PATCH 29/31] vpc: Switch to .bdrv_co_block_status()
2017-07-13 12:55 ` Kevin Wolf
@ 2017-07-13 13:52 ` Eric Blake
0 siblings, 0 replies; 48+ messages in thread
From: Eric Blake @ 2017-07-13 13:52 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, kwolf, jsnow, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]
On 07/13/2017 07:55 AM, Kevin Wolf wrote:
> Am 18.04.2017 um 03:33 hat Eric Blake geschrieben:
>> We are gradually moving away from sector-based interfaces, towards
>> byte-based. Update the vpc driver accordingly.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> I know this is an old series, but I think you split it for later
> versions and there hasn't been a respin of this final part of the series
> yet.
Yep, the respin should be coming up later today.
>
> When I just told Peter that get_sector_offset() would go away with your
> patches, I decided to check whether this was actually true, and found...
>
>> /* *pnum can't be greater than one block for allocated
>> * sectors since there is always a bitmap in between. */
>> if (allocated) {
>> *file = bs->file->bs;
>> return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
>> }
>> - if (nb_sectors == 0) {
>> + if (bytes == 0) {
>> break;
>> }
>> - offset = get_sector_offset(bs, sector_num, 0);
>> + image_offset = get_sector_offset(bs, offset, 0);
>> } while (offset == -1);
>
> ...this bug. I think you want to use get_image_offset() now.
>
> This should also be the last caller of get_sector_offset(), so the
> function should go away in this commit.
Will do, and thanks for catching it!
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2017-07-13 13:52 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 1:33 [Qemu-devel] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 01/31] block: Drop unused bdrv_round_sectors_to_clusters() Eric Blake
2017-04-26 21:41 ` John Snow
2017-04-18 1:33 ` [Qemu-devel] [PATCH 02/31] block: Make bdrv_round_to_clusters() signature more useful Eric Blake
2017-04-26 21:41 ` John Snow
2017-04-26 21:47 ` Eric Blake
2017-04-26 21:54 ` John Snow
2017-04-18 1:33 ` [Qemu-devel] [PATCH 03/31] qcow2: Switch is_zero_sectors() to byte-based Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 04/31] block: Switch bdrv_make_zero() " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 05/31] qemu-img: Switch get_block_status() " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 06/31] block: Convert bdrv_get_block_status() to bytes Eric Blake
2017-04-18 21:34 ` [Qemu-devel] [PATCH v1.5 06.5/31] fixup! " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 07/31] block: Switch bdrv_co_get_block_status() to byte-based Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 08/31] block: Switch BdrvCoGetBlockStatusData " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 09/31] block: Switch bdrv_co_get_block_status_above() " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 10/31] block: Convert bdrv_get_block_status_above() to bytes Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 11/31] block: Add .bdrv_co_block_status() callback Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 12/31] commit: Switch to .bdrv_co_block_status() Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 13/31] file-posix: " Eric Blake
2017-04-18 20:56 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 14/31] gluster: " Eric Blake
2017-04-19 13:26 ` [Qemu-devel] [Qemu-block] " Niels de Vos
2017-04-19 14:06 ` [Qemu-devel] " Prasanna Kalever
2017-04-19 14:08 ` Prasanna Kalever
2017-04-18 1:33 ` [Qemu-devel] [PATCH 15/31] iscsi: Switch cluster_sectors to byte-based Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 16/31] iscsi: Switch iscsi_allocmap_update() " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 17/31] iscsi: Switch to .bdrv_co_block_status() Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 18/31] mirror: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 19/31] null: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 20/31] parallels: " Eric Blake
2017-04-18 10:28 ` Denis V. Lunev
2017-04-18 1:33 ` [Qemu-devel] [PATCH 21/31] qcow: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 22/31] qcow2: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 23/31] qed: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 24/31] raw: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 25/31] sheepdog: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 26/31] vdi: Avoid bitrot of debugging code Eric Blake
2017-04-18 5:13 ` Stefan Weil
2017-05-13 20:35 ` Philippe Mathieu-Daudé
2017-04-18 1:33 ` [Qemu-devel] [PATCH 27/31] vdi: Switch to .bdrv_co_block_status() Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 28/31] vmdk: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 29/31] vpc: " Eric Blake
2017-07-13 12:55 ` Kevin Wolf
2017-07-13 13:52 ` Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 30/31] vvfat: " Eric Blake
2017-04-18 1:33 ` [Qemu-devel] [PATCH 31/31] block: Drop unused .bdrv_co_get_block_status() Eric Blake
2017-04-18 1:37 ` [Qemu-devel] [Qemu-block] [PATCH 00/31] make bdrv_get_block_status byte-based Eric Blake
2017-04-18 20:23 ` [Qemu-devel] " Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.