* [Qemu-devel] [PATCH v5 01/21] cow: make reads go at a decent speed
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 02/21] cow: make writes go at a less indecent speed Paolo Bonzini
` (20 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Do not do two reads for each sector; load each sector of the bitmap
and use bitmap operations to process it.
Writes are still dog slow!
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/cow.c | 54 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 1cc2e89..fa3d41f 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -126,18 +126,31 @@ static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
return 0;
}
-static inline int is_bit_set(BlockDriverState *bs, int64_t bitnum)
+#define BITS_PER_BITMAP_SECTOR (512 * 8)
+
+/* Cannot use bitmap.c on big-endian machines. */
+static int cow_test_bit(int64_t bitnum, const uint8_t *bitmap)
{
- uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
- uint8_t bitmap;
- int ret;
+ return (bitmap[bitnum / 8] & (1 << (bitnum & 7))) != 0;
+}
- ret = bdrv_pread(bs->file, offset, &bitmap, sizeof(bitmap));
- if (ret < 0) {
- return ret;
+static int cow_find_streak(const uint8_t *bitmap, int value, int start, int nb_sectors)
+{
+ int streak_value = value ? 0xFF : 0;
+ int last = MIN(start + nb_sectors, BITS_PER_BITMAP_SECTOR);
+ int bitnum = start;
+ while (bitnum < last) {
+ if ((bitnum & 7) == 0 && bitmap[bitnum / 8] == streak_value) {
+ bitnum += 8;
+ continue;
+ }
+ if (cow_test_bit(bitnum, bitmap) == value) {
+ bitnum++;
+ continue;
+ }
+ break;
}
-
- return !!(bitmap & (1 << (bitnum % 8)));
+ return MIN(bitnum, last) - start;
}
/* Return true if first block has been changed (ie. current version is
@@ -146,23 +159,20 @@ static inline int is_bit_set(BlockDriverState *bs, int64_t bitnum)
static int coroutine_fn cow_co_is_allocated(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *num_same)
{
+ int64_t bitnum = sector_num + sizeof(struct cow_header_v2) * 8;
+ uint64_t offset = (bitnum / 8) & -BDRV_SECTOR_SIZE;
+ uint8_t bitmap[BDRV_SECTOR_SIZE];
+ int ret;
int changed;
- if (nb_sectors == 0) {
- *num_same = nb_sectors;
- return 0;
- }
-
- changed = is_bit_set(bs, sector_num);
- if (changed < 0) {
- return 0; /* XXX: how to return I/O errors? */
- }
-
- for (*num_same = 1; *num_same < nb_sectors; (*num_same)++) {
- if (is_bit_set(bs, sector_num + *num_same) != changed)
- break;
+ ret = bdrv_pread(bs->file, offset, &bitmap, sizeof(bitmap));
+ if (ret < 0) {
+ return ret;
}
+ bitnum &= BITS_PER_BITMAP_SECTOR - 1;
+ changed = cow_test_bit(bitnum, bitmap);
+ *num_same = cow_find_streak(bitmap, changed, bitnum, nb_sectors);
return changed;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 02/21] cow: make writes go at a less indecent speed
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 01/21] cow: make reads go at a decent speed Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 03/21] cow: do not call bdrv_co_is_allocated Paolo Bonzini
` (19 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Only sync once per write, rather than once per sector.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/cow.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index fa3d41f..9ae2d6a 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -106,7 +106,7 @@ static int cow_open(BlockDriverState *bs, QDict *options, int flags)
* XXX(hch): right now these functions are extremely inefficient.
* We should just read the whole bitmap we'll need in one go instead.
*/
-static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
+static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum, bool *first)
{
uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
uint8_t bitmap;
@@ -117,9 +117,21 @@ static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
return ret;
}
+ if (bitmap & (1 << (bitnum % 8))) {
+ return 0;
+ }
+
+ if (*first) {
+ ret = bdrv_flush(bs->file);
+ if (ret < 0) {
+ return ret;
+ }
+ *first = false;
+ }
+
bitmap |= (1 << (bitnum % 8));
- ret = bdrv_pwrite_sync(bs->file, offset, &bitmap, sizeof(bitmap));
+ ret = bdrv_pwrite(bs->file, offset, &bitmap, sizeof(bitmap));
if (ret < 0) {
return ret;
}
@@ -181,9 +193,10 @@ static int cow_update_bitmap(BlockDriverState *bs, int64_t sector_num,
{
int error = 0;
int i;
+ bool first = true;
for (i = 0; i < nb_sectors; i++) {
- error = cow_set_bit(bs, sector_num + i);
+ error = cow_set_bit(bs, sector_num + i, &first);
if (error) {
break;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 03/21] cow: do not call bdrv_co_is_allocated
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 01/21] cow: make reads go at a decent speed Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 02/21] cow: make writes go at a less indecent speed Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 04/21] block: keep bs->total_sectors up to date even for growable block devices Paolo Bonzini
` (18 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
As we change bdrv_is_allocated to gather more information from bs and
bs->file, it will become a bit slower. It is still appropriate for online
jobs, but not for reads/writes. Call the internal function instead.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/cow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/cow.c b/block/cow.c
index 9ae2d6a..55bac50 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -212,7 +212,7 @@ static int coroutine_fn cow_read(BlockDriverState *bs, int64_t sector_num,
int ret, n;
while (nb_sectors > 0) {
- if (bdrv_co_is_allocated(bs, sector_num, nb_sectors, &n)) {
+ if (cow_co_is_allocated(bs, sector_num, nb_sectors, &n)) {
ret = bdrv_pread(bs->file,
s->cow_sectors_offset + sector_num * 512,
buf, n * 512);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 04/21] block: keep bs->total_sectors up to date even for growable block devices
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (2 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 03/21] cow: do not call bdrv_co_is_allocated Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 05/21] block: make bdrv_co_is_allocated static Paolo Bonzini
` (17 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
If a BlockDriverState is growable, after every write we need to
check if bs->total_sectors might have changed. With this change,
bdrv_getlength does not need anymore a system call.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 26639e8..d5800fc 100644
--- a/block.c
+++ b/block.c
@@ -2711,6 +2711,9 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
if (bs->wr_highest_sector < sector_num + nb_sectors - 1) {
bs->wr_highest_sector = sector_num + nb_sectors - 1;
}
+ if (bs->growable && ret >= 0) {
+ bs->total_sectors = MAX(bs->total_sectors, sector_num + nb_sectors);
+ }
tracked_request_end(&req);
@@ -2785,7 +2788,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
if (!drv)
return -ENOMEDIUM;
- if (bs->growable || bdrv_dev_has_removable_media(bs)) {
+ if (bdrv_dev_has_removable_media(bs)) {
if (drv->bdrv_getlength) {
return drv->bdrv_getlength(bs);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 05/21] block: make bdrv_co_is_allocated static
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (3 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 04/21] block: keep bs->total_sectors up to date even for growable block devices Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 06/21] block: do not use ->total_sectors in bdrv_co_is_allocated Paolo Bonzini
` (16 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
bdrv_is_allocated can detect coroutine context and go through a fast
path, similar to other block layer functions.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 24 +++++++++++++++---------
block/backup.c | 4 ++--
block/raw_bsd.c | 2 +-
block/stream.c | 4 ++--
include/block/block.h | 2 --
5 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/block.c b/block.c
index d5800fc..0e95b47 100644
--- a/block.c
+++ b/block.c
@@ -2559,7 +2559,7 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
if (flags & BDRV_REQ_COPY_ON_READ) {
int pnum;
- ret = bdrv_co_is_allocated(bs, sector_num, nb_sectors, &pnum);
+ ret = bdrv_is_allocated(bs, sector_num, nb_sectors, &pnum);
if (ret < 0) {
goto out;
}
@@ -3041,8 +3041,9 @@ typedef struct BdrvCoIsAllocatedData {
* 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
* beyond the end of the disk image it will be clamped.
*/
-int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, int *pnum)
+static int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum)
{
int64_t n;
@@ -3092,10 +3093,15 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
.done = false,
};
- co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
- qemu_coroutine_enter(co, &data);
- while (!data.done) {
- qemu_aio_wait();
+ if (qemu_in_coroutine()) {
+ /* Fast-path if already in coroutine context */
+ bdrv_is_allocated_co_entry(&data);
+ } else {
+ co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
+ qemu_coroutine_enter(co, &data);
+ while (!data.done) {
+ qemu_aio_wait();
+ }
}
return data.ret;
}
@@ -3123,8 +3129,8 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
intermediate = top;
while (intermediate && intermediate != base) {
int pnum_inter;
- ret = bdrv_co_is_allocated(intermediate, sector_num, nb_sectors,
- &pnum_inter);
+ ret = bdrv_is_allocated(intermediate, sector_num, nb_sectors,
+ &pnum_inter);
if (ret < 0) {
return ret;
} else if (ret) {
diff --git a/block/backup.c b/block/backup.c
index 23c7264..481758d 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -289,14 +289,14 @@ static void coroutine_fn backup_run(void *opaque)
* backing file. */
for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;) {
- /* bdrv_co_is_allocated() only returns true/false based
+ /* bdrv_is_allocated() only returns true/false based
* on the first set of sectors it comes across that
* are are all in the same state.
* For that reason we must verify each sector in the
* backup cluster length. We end up copying more than
* needed but at some point that is always the case. */
alloced =
- bdrv_co_is_allocated(bs,
+ bdrv_is_allocated(bs,
start * BACKUP_SECTORS_PER_CLUSTER + i,
BACKUP_SECTORS_PER_CLUSTER - i, &n);
i += n;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index ab2b0fd..926712e 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -62,7 +62,7 @@ static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
int64_t sector_num, int nb_sectors,
int *pnum)
{
- return bdrv_co_is_allocated(bs->file, sector_num, nb_sectors, pnum);
+ return bdrv_is_allocated(bs->file, sector_num, nb_sectors, pnum);
}
static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs,
diff --git a/block/stream.c b/block/stream.c
index 9982125..cc131e8 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -119,8 +119,8 @@ wait:
break;
}
- ret = bdrv_co_is_allocated(bs, sector_num,
- STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
+ ret = bdrv_is_allocated(bs, sector_num,
+ STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
if (ret == 1) {
/* Allocated in the top, no need to copy. */
copy = false;
diff --git a/include/block/block.h b/include/block/block.h
index e6b391c..33f7a9d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -181,8 +181,6 @@ int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
*/
int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
int nb_sectors);
-int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, int *pnum);
int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
BlockDriverState *base,
int64_t sector_num,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 06/21] block: do not use ->total_sectors in bdrv_co_is_allocated
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (4 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 05/21] block: make bdrv_co_is_allocated static Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 07/21] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction Paolo Bonzini
` (15 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
This is more robust when the device has removable media.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 0e95b47..410d5dd 100644
--- a/block.c
+++ b/block.c
@@ -3045,9 +3045,15 @@ static int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum)
{
+ int64_t length;
int64_t n;
- if (sector_num >= bs->total_sectors) {
+ length = bdrv_getlength(bs);
+ if (length < 0) {
+ return length;
+ }
+
+ if (sector_num >= (length >> BDRV_SECTOR_BITS)) {
*pnum = 0;
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 07/21] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (5 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 06/21] block: do not use ->total_sectors in bdrv_co_is_allocated Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 08/21] block: expect errors from bdrv_co_is_allocated Paolo Bonzini
` (14 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Now that bdrv_is_allocated detects coroutine context, the two can
use the same code.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 46 ++++------------------------------------------
block/commit.c | 6 +++---
block/mirror.c | 4 ++--
block/stream.c | 4 ++--
include/block/block.h | 4 ----
5 files changed, 11 insertions(+), 53 deletions(-)
diff --git a/block.c b/block.c
index 410d5dd..1c8899a 100644
--- a/block.c
+++ b/block.c
@@ -3124,10 +3124,10 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
* allocated/unallocated state.
*
*/
-int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
- BlockDriverState *base,
- int64_t sector_num,
- int nb_sectors, int *pnum)
+int bdrv_is_allocated_above(BlockDriverState *top,
+ BlockDriverState *base,
+ int64_t sector_num,
+ int nb_sectors, int *pnum)
{
BlockDriverState *intermediate;
int ret, n = nb_sectors;
@@ -3163,44 +3163,6 @@ int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
return 0;
}
-/* Coroutine wrapper for bdrv_is_allocated_above() */
-static void coroutine_fn bdrv_is_allocated_above_co_entry(void *opaque)
-{
- BdrvCoIsAllocatedData *data = opaque;
- BlockDriverState *top = data->bs;
- BlockDriverState *base = data->base;
-
- data->ret = bdrv_co_is_allocated_above(top, base, data->sector_num,
- data->nb_sectors, data->pnum);
- data->done = true;
-}
-
-/*
- * Synchronous wrapper around bdrv_co_is_allocated_above().
- *
- * See bdrv_co_is_allocated_above() for details.
- */
-int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
- int64_t sector_num, int nb_sectors, int *pnum)
-{
- Coroutine *co;
- BdrvCoIsAllocatedData data = {
- .bs = top,
- .base = base,
- .sector_num = sector_num,
- .nb_sectors = nb_sectors,
- .pnum = pnum,
- .done = false,
- };
-
- co = qemu_coroutine_create(bdrv_is_allocated_above_co_entry);
- qemu_coroutine_enter(co, &data);
- while (!data.done) {
- qemu_aio_wait();
- }
- return data.ret;
-}
-
const char *bdrv_get_encrypted_filename(BlockDriverState *bs)
{
if (bs->backing_hd && bs->backing_hd->encrypted)
diff --git a/block/commit.c b/block/commit.c
index 51a1ab3..ac4b7cc 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -108,9 +108,9 @@ wait:
break;
}
/* Copy if allocated above the base */
- ret = bdrv_co_is_allocated_above(top, base, sector_num,
- COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
- &n);
+ ret = bdrv_is_allocated_above(top, base, sector_num,
+ COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE,
+ &n);
copy = (ret == 1);
trace_commit_one_iteration(s, sector_num, n, ret);
if (copy) {
diff --git a/block/mirror.c b/block/mirror.c
index 86de458..139938a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -338,8 +338,8 @@ static void coroutine_fn mirror_run(void *opaque)
base = s->mode == MIRROR_SYNC_MODE_FULL ? NULL : bs->backing_hd;
for (sector_num = 0; sector_num < end; ) {
int64_t next = (sector_num | (sectors_per_chunk - 1)) + 1;
- ret = bdrv_co_is_allocated_above(bs, base,
- sector_num, next - sector_num, &n);
+ ret = bdrv_is_allocated_above(bs, base,
+ sector_num, next - sector_num, &n);
if (ret < 0) {
goto immediate_exit;
diff --git a/block/stream.c b/block/stream.c
index cc131e8..9a63eaf 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -127,8 +127,8 @@ wait:
} else {
/* Copy if allocated in the intermediate images. Limit to the
* known-unallocated area [sector_num, sector_num+n). */
- ret = bdrv_co_is_allocated_above(bs->backing_hd, base,
- sector_num, n, &n);
+ ret = bdrv_is_allocated_above(bs->backing_hd, base,
+ sector_num, n, &n);
/* Finish early if end of backing file has been reached */
if (ret == 0 && n == 0) {
diff --git a/include/block/block.h b/include/block/block.h
index 33f7a9d..4a284b2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -181,10 +181,6 @@ int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
*/
int coroutine_fn bdrv_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
int nb_sectors);
-int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
- BlockDriverState *base,
- int64_t sector_num,
- int nb_sectors, int *pnum);
BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
const char *backing_file);
int bdrv_get_backing_file_depth(BlockDriverState *bs);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 08/21] block: expect errors from bdrv_co_is_allocated
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (6 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 07/21] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 09/21] qemu-img: always probe the input image for allocated sectors Paolo Bonzini
` (13 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, stefanha
Some bdrv_is_allocated callers do not expect errors, but the fallback
in qcow2.c might make other callers trip on assertion failures or
infinite loops.
Fix the callers to always look for errors.
Cc: qemu-stable@nongnu.org
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 7 +++++--
block/cow.c | 6 +++++-
block/qcow2.c | 4 +---
block/stream.c | 2 +-
qemu-img.c | 16 ++++++++++++++--
qemu-io-cmds.c | 4 ++++
6 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index 1c8899a..a3f7577 100644
--- a/block.c
+++ b/block.c
@@ -1829,8 +1829,11 @@ int bdrv_commit(BlockDriverState *bs)
buf = g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
for (sector = 0; sector < total_sectors; sector += n) {
- if (bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n)) {
-
+ ret = bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n);
+ if (ret < 0) {
+ goto ro_cleanup;
+ }
+ if (ret) {
if (bdrv_read(bs, sector, buf, n) != 0) {
ret = -EIO;
goto ro_cleanup;
diff --git a/block/cow.c b/block/cow.c
index 55bac50..61656fa 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -212,7 +212,11 @@ static int coroutine_fn cow_read(BlockDriverState *bs, int64_t sector_num,
int ret, n;
while (nb_sectors > 0) {
- if (cow_co_is_allocated(bs, sector_num, nb_sectors, &n)) {
+ ret = cow_co_is_allocated(bs, sector_num, nb_sectors, &n);
+ if (ret < 0) {
+ return ret;
+ }
+ if (ret) {
ret = bdrv_pread(bs->file,
s->cow_sectors_offset + sector_num * 512,
buf, n * 512);
diff --git a/block/qcow2.c b/block/qcow2.c
index 4bc679a..6b06a8e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -696,13 +696,11 @@ static int coroutine_fn qcow2_co_is_allocated(BlockDriverState *bs,
int ret;
*pnum = nb_sectors;
- /* FIXME We can get errors here, but the bdrv_co_is_allocated interface
- * can't pass them on today */
qemu_co_mutex_lock(&s->lock);
ret = qcow2_get_cluster_offset(bs, sector_num << 9, pnum, &cluster_offset);
qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {
- *pnum = 0;
+ return ret;
}
return (cluster_offset != 0) || (ret == QCOW2_CLUSTER_ZERO);
diff --git a/block/stream.c b/block/stream.c
index 9a63eaf..b0cbf34 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -124,7 +124,7 @@ wait:
if (ret == 1) {
/* Allocated in the top, no need to copy. */
copy = false;
- } else {
+ } else if (ret >= 0) {
/* Copy if allocated in the intermediate images. Limit to the
* known-unallocated area [sector_num, sector_num+n). */
ret = bdrv_is_allocated_above(bs->backing_hd, base,
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..b01998b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1485,8 +1485,15 @@ static int img_convert(int argc, char **argv)
are present in both the output's and input's base images (no
need to copy them). */
if (out_baseimg) {
- if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
- n, &n1)) {
+ ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
+ n, &n1);
+ if (ret < 0) {
+ error_report("error while reading metadata for sector "
+ "%" PRId64 ": %s",
+ sector_num - bs_offset, strerror(-ret));
+ goto out;
+ }
+ if (!ret) {
sector_num += n1;
continue;
}
@@ -2076,6 +2083,11 @@ static int img_rebase(int argc, char **argv)
/* If the cluster is allocated, we don't need to take action */
ret = bdrv_is_allocated(bs, sector, n, &n);
+ if (ret < 0) {
+ error_report("error while reading image metadata: %s",
+ strerror(-ret));
+ goto out;
+ }
if (ret) {
continue;
}
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index f91b6c4..8565d49 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1830,6 +1830,10 @@ static int alloc_f(BlockDriverState *bs, int argc, char **argv)
sector_num = offset >> 9;
while (remaining) {
ret = bdrv_is_allocated(bs, sector_num, remaining, &num);
+ if (ret < 0) {
+ printf("is_allocated failed: %s\n", strerror(-ret));
+ return 0;
+ }
sector_num += num;
remaining -= num;
if (ret) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 09/21] qemu-img: always probe the input image for allocated sectors
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (7 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 08/21] block: expect errors from bdrv_co_is_allocated Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 10/21] block: make bdrv_has_zero_init return false for copy-on-write-images Paolo Bonzini
` (12 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
qemu-img convert can assume "that sectors which are unallocated in the
input image are present in both the output's and input's base images".
However it is only doing this if the output image returns true for
bdrv_has_zero_init(). Testing bdrv_has_zero_init() does not make much
sense if the output image is copy-on-write, because a copy-on-write
image is never initialized to zero (it is initialized to the content
of the backing file).
There is nothing here that makes has_zero_init images special. The
input and output must be equal for the operation to make sense, and
that's it.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-img.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index b01998b..837f8bc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1479,28 +1479,26 @@ static int img_convert(int argc, char **argv)
n = bs_offset + bs_sectors - sector_num;
}
- if (has_zero_init) {
- /* If the output image is being created as a copy on write image,
- assume that sectors which are unallocated in the input image
- are present in both the output's and input's base images (no
- need to copy them). */
- if (out_baseimg) {
- ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
- n, &n1);
- if (ret < 0) {
- error_report("error while reading metadata for sector "
- "%" PRId64 ": %s",
- sector_num - bs_offset, strerror(-ret));
- goto out;
- }
- if (!ret) {
- sector_num += n1;
- continue;
- }
- /* The next 'n1' sectors are allocated in the input image. Copy
- only those as they may be followed by unallocated sectors. */
- n = n1;
+ /* If the output image is being created as a copy on write image,
+ assume that sectors which are unallocated in the input image
+ are present in both the output's and input's base images (no
+ need to copy them). */
+ if (out_baseimg) {
+ ret = bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
+ n, &n1);
+ if (ret < 0) {
+ error_report("error while reading metadata for sector "
+ "%" PRId64 ": %s",
+ sector_num - bs_offset, strerror(-ret));
+ goto out;
+ }
+ if (!ret) {
+ sector_num += n1;
+ continue;
}
+ /* The next 'n1' sectors are allocated in the input image. Copy
+ only those as they may be followed by unallocated sectors. */
+ n = n1;
} else {
n1 = n;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 10/21] block: make bdrv_has_zero_init return false for copy-on-write-images
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (8 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 09/21] qemu-img: always probe the input image for allocated sectors Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 11/21] block: introduce bdrv_get_block_status API Paolo Bonzini
` (11 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
This helps implementing is_allocated on top of get_block_status.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 5 +++++
qemu-img.c | 9 +--------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index a3f7577..e18ff71 100644
--- a/block.c
+++ b/block.c
@@ -3011,6 +3011,11 @@ int bdrv_has_zero_init(BlockDriverState *bs)
{
assert(bs->drv);
+ /* If BS is a copy on write image, it is initialized to
+ the contents of the base image, which may not be zeroes. */
+ if (bs->backing_hd) {
+ return 0;
+ }
if (bs->drv->bdrv_has_zero_init) {
return bs->drv->bdrv_has_zero_init(bs);
}
diff --git a/qemu-img.c b/qemu-img.c
index 837f8bc..d50e0fc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1514,14 +1514,7 @@ static int img_convert(int argc, char **argv)
should add a specific call to have the info to go faster */
buf1 = buf;
while (n > 0) {
- /* If the output image is being created as a copy on write image,
- copy all sectors even the ones containing only NUL bytes,
- because they may differ from the sectors in the base image.
-
- If the output is to a host device, we also write out
- sectors that are entirely 0, since whatever data was
- already there is garbage, not 0s. */
- if (!has_zero_init || out_baseimg ||
+ if (!has_zero_init ||
is_allocated_sectors_min(buf1, n, &n1, min_sparse)) {
ret = bdrv_write(out_bs, sector_num, buf1, n1);
if (ret < 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 11/21] block: introduce bdrv_get_block_status API
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (9 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 10/21] block: make bdrv_has_zero_init return false for copy-on-write-images Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value Paolo Bonzini
` (10 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
For now, bdrv_get_block_status is just another name for bdrv_is_allocated.
The next patches will add more flags.
This also touches all block drivers with a mostly mechanical rename. The
sole exception is cow; because it calls cow_co_is_allocated from the read
code, we keep that function and make cow_co_get_block_status a wrapper.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 46 ++++++++++++++++++++++++++--------------------
block/cow.c | 8 +++++++-
block/qcow.c | 4 ++--
block/qcow2.c | 4 ++--
block/qed.c | 4 ++--
block/raw-posix.c | 4 ++--
block/raw_bsd.c | 10 +++++-----
block/sheepdog.c | 12 ++++++------
block/vdi.c | 4 ++--
block/vmdk.c | 4 ++--
block/vvfat.c | 4 ++--
include/block/block.h | 2 ++
include/block/block_int.h | 2 +-
13 files changed, 61 insertions(+), 47 deletions(-)
diff --git a/block.c b/block.c
index e18ff71..47a5d63 100644
--- a/block.c
+++ b/block.c
@@ -3024,15 +3024,15 @@ int bdrv_has_zero_init(BlockDriverState *bs)
return 0;
}
-typedef struct BdrvCoIsAllocatedData {
+typedef struct BdrvCoGetBlockStatusData {
BlockDriverState *bs;
BlockDriverState *base;
int64_t sector_num;
int nb_sectors;
int *pnum;
- int ret;
+ int64_t ret;
bool done;
-} BdrvCoIsAllocatedData;
+} BdrvCoGetBlockStatusData;
/*
* Returns true iff the specified sector is present in the disk image. Drivers
@@ -3049,9 +3049,9 @@ typedef struct BdrvCoIsAllocatedData {
* 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
* beyond the end of the disk image it will be clamped.
*/
-static int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs,
- int64_t sector_num,
- int nb_sectors, int *pnum)
+static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum)
{
int64_t length;
int64_t n;
@@ -3071,35 +3071,35 @@ static int coroutine_fn bdrv_co_is_allocated(BlockDriverState *bs,
nb_sectors = n;
}
- if (!bs->drv->bdrv_co_is_allocated) {
+ if (!bs->drv->bdrv_co_get_block_status) {
*pnum = nb_sectors;
return 1;
}
- return bs->drv->bdrv_co_is_allocated(bs, sector_num, nb_sectors, pnum);
+ return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
}
-/* Coroutine wrapper for bdrv_is_allocated() */
-static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
+/* Coroutine wrapper for bdrv_get_block_status() */
+static void coroutine_fn bdrv_get_block_status_co_entry(void *opaque)
{
- BdrvCoIsAllocatedData *data = opaque;
+ BdrvCoGetBlockStatusData *data = opaque;
BlockDriverState *bs = data->bs;
- data->ret = bdrv_co_is_allocated(bs, data->sector_num, data->nb_sectors,
- data->pnum);
+ data->ret = bdrv_co_get_block_status(bs, data->sector_num, data->nb_sectors,
+ data->pnum);
data->done = true;
}
/*
- * Synchronous wrapper around bdrv_co_is_allocated().
+ * Synchronous wrapper around bdrv_co_get_block_status().
*
- * See bdrv_co_is_allocated() for details.
+ * See bdrv_co_get_block_status() for details.
*/
-int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
- int *pnum)
+int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, int *pnum)
{
Coroutine *co;
- BdrvCoIsAllocatedData data = {
+ BdrvCoGetBlockStatusData data = {
.bs = bs,
.sector_num = sector_num,
.nb_sectors = nb_sectors,
@@ -3109,9 +3109,9 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
- bdrv_is_allocated_co_entry(&data);
+ bdrv_get_block_status_co_entry(&data);
} else {
- co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
+ co = qemu_coroutine_create(bdrv_get_block_status_co_entry);
qemu_coroutine_enter(co, &data);
while (!data.done) {
qemu_aio_wait();
@@ -3120,6 +3120,12 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
return data.ret;
}
+int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, int *pnum)
+{
+ return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
+}
+
/*
* Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP]
*
diff --git a/block/cow.c b/block/cow.c
index 61656fa..98b5cb3 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -188,6 +188,12 @@ static int coroutine_fn cow_co_is_allocated(BlockDriverState *bs,
return changed;
}
+static int64_t coroutine_fn cow_co_get_block_status(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors, int *num_same)
+{
+ return cow_co_is_allocated(bs, sector_num, nb_sectors, num_same);
+}
+
static int cow_update_bitmap(BlockDriverState *bs, int64_t sector_num,
int nb_sectors)
{
@@ -371,7 +377,7 @@ static BlockDriver bdrv_cow = {
.bdrv_read = cow_co_read,
.bdrv_write = cow_co_write,
- .bdrv_co_is_allocated = cow_co_is_allocated,
+ .bdrv_co_get_block_status = cow_co_get_block_status,
.create_options = cow_create_options,
};
diff --git a/block/qcow.c b/block/qcow.c
index 5239bd6..acd1aeb 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -395,7 +395,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
return cluster_offset;
}
-static int coroutine_fn qcow_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum)
{
BDRVQcowState *s = bs->opaque;
@@ -896,7 +896,7 @@ static BlockDriver bdrv_qcow = {
.bdrv_co_readv = qcow_co_readv,
.bdrv_co_writev = qcow_co_writev,
- .bdrv_co_is_allocated = qcow_co_is_allocated,
+ .bdrv_co_get_block_status = qcow_co_get_block_status,
.bdrv_set_key = qcow_set_key,
.bdrv_make_empty = qcow_make_empty,
diff --git a/block/qcow2.c b/block/qcow2.c
index 6b06a8e..4802e32 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -688,7 +688,7 @@ static int qcow2_reopen_prepare(BDRVReopenState *state,
return 0;
}
-static int coroutine_fn qcow2_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum)
{
BDRVQcowState *s = bs->opaque;
@@ -1866,7 +1866,7 @@ static BlockDriver bdrv_qcow2 = {
.bdrv_reopen_prepare = qcow2_reopen_prepare,
.bdrv_create = qcow2_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_is_allocated = qcow2_co_is_allocated,
+ .bdrv_co_get_block_status = qcow2_co_get_block_status,
.bdrv_set_key = qcow2_set_key,
.bdrv_make_empty = qcow2_make_empty,
diff --git a/block/qed.c b/block/qed.c
index cc904c4..14e1ede 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -667,7 +667,7 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
}
}
-static int coroutine_fn bdrv_qed_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum)
{
@@ -1575,7 +1575,7 @@ static BlockDriver bdrv_qed = {
.bdrv_reopen_prepare = bdrv_qed_reopen_prepare,
.bdrv_create = bdrv_qed_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_is_allocated = bdrv_qed_co_is_allocated,
+ .bdrv_co_get_block_status = bdrv_qed_co_get_block_status,
.bdrv_make_empty = bdrv_qed_make_empty,
.bdrv_aio_readv = bdrv_qed_aio_readv,
.bdrv_aio_writev = bdrv_qed_aio_writev,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index ba721d3..dbc65b0 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1084,7 +1084,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
* 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
* beyond the end of the disk image it will be clamped.
*/
-static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum)
{
@@ -1200,7 +1200,7 @@ static BlockDriver bdrv_file = {
.bdrv_close = raw_close,
.bdrv_create = raw_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_is_allocated = raw_co_is_allocated,
+ .bdrv_co_get_block_status = raw_co_get_block_status,
.bdrv_aio_readv = raw_aio_readv,
.bdrv_aio_writev = raw_aio_writev,
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 926712e..a9060ca 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -58,11 +58,11 @@ static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
}
-static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- int *pnum)
+static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum)
{
- return bdrv_is_allocated(bs->file, sector_num, nb_sectors, pnum);
+ return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
}
static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs,
@@ -164,7 +164,7 @@ static BlockDriver bdrv_raw = {
.bdrv_co_writev = &raw_co_writev,
.bdrv_co_write_zeroes = &raw_co_write_zeroes,
.bdrv_co_discard = &raw_co_discard,
- .bdrv_co_is_allocated = &raw_co_is_allocated,
+ .bdrv_co_get_block_status = &raw_co_get_block_status,
.bdrv_truncate = &raw_truncate,
.bdrv_getlength = &raw_getlength,
.bdrv_get_info = &raw_get_info,
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1ad4d07..ffc07c2 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2270,9 +2270,9 @@ static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num,
return acb->ret;
}
-static coroutine_fn int
-sd_co_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
- int *pnum)
+static coroutine_fn int64_t
+sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+ int *pnum)
{
BDRVSheepdogState *s = bs->opaque;
SheepdogInode *inode = &s->inode;
@@ -2338,7 +2338,7 @@ static BlockDriver bdrv_sheepdog = {
.bdrv_co_writev = sd_co_writev,
.bdrv_co_flush_to_disk = sd_co_flush_to_disk,
.bdrv_co_discard = sd_co_discard,
- .bdrv_co_is_allocated = sd_co_is_allocated,
+ .bdrv_co_get_block_status = sd_co_get_block_status,
.bdrv_snapshot_create = sd_snapshot_create,
.bdrv_snapshot_goto = sd_snapshot_goto,
@@ -2366,7 +2366,7 @@ static BlockDriver bdrv_sheepdog_tcp = {
.bdrv_co_writev = sd_co_writev,
.bdrv_co_flush_to_disk = sd_co_flush_to_disk,
.bdrv_co_discard = sd_co_discard,
- .bdrv_co_is_allocated = sd_co_is_allocated,
+ .bdrv_co_get_block_status = sd_co_get_block_status,
.bdrv_snapshot_create = sd_snapshot_create,
.bdrv_snapshot_goto = sd_snapshot_goto,
@@ -2394,7 +2394,7 @@ static BlockDriver bdrv_sheepdog_unix = {
.bdrv_co_writev = sd_co_writev,
.bdrv_co_flush_to_disk = sd_co_flush_to_disk,
.bdrv_co_discard = sd_co_discard,
- .bdrv_co_is_allocated = sd_co_is_allocated,
+ .bdrv_co_get_block_status = sd_co_get_block_status,
.bdrv_snapshot_create = sd_snapshot_create,
.bdrv_snapshot_goto = sd_snapshot_goto,
diff --git a/block/vdi.c b/block/vdi.c
index 8a91525..7ab2567 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -470,7 +470,7 @@ static int vdi_reopen_prepare(BDRVReopenState *state,
return 0;
}
-static int coroutine_fn vdi_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum)
{
/* TODO: Check for too large sector_num (in bdrv_is_allocated or here). */
@@ -780,7 +780,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_reopen_prepare = vdi_reopen_prepare,
.bdrv_create = vdi_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
- .bdrv_co_is_allocated = vdi_co_is_allocated,
+ .bdrv_co_get_block_status = vdi_co_get_block_status,
.bdrv_make_empty = vdi_make_empty,
.bdrv_read = vdi_co_read,
diff --git a/block/vmdk.c b/block/vmdk.c
index 63b489d..d4516fd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1042,7 +1042,7 @@ static VmdkExtent *find_extent(BDRVVmdkState *s,
return NULL;
}
-static int coroutine_fn vmdk_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum)
{
BDRVVmdkState *s = bs->opaque;
@@ -1837,7 +1837,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_close = vmdk_close,
.bdrv_create = vmdk_create,
.bdrv_co_flush_to_disk = vmdk_co_flush,
- .bdrv_co_is_allocated = vmdk_co_is_allocated,
+ .bdrv_co_get_block_status = vmdk_co_get_block_status,
.bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
.bdrv_has_zero_init = vmdk_has_zero_init,
diff --git a/block/vvfat.c b/block/vvfat.c
index cd3b8ed..4c30620 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2874,7 +2874,7 @@ static coroutine_fn int vvfat_co_write(BlockDriverState *bs, int64_t sector_num,
return ret;
}
-static int coroutine_fn vvfat_co_is_allocated(BlockDriverState *bs,
+static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int* n)
{
BDRVVVFATState* s = bs->opaque;
@@ -2984,7 +2984,7 @@ static BlockDriver bdrv_vvfat = {
.bdrv_read = vvfat_co_read,
.bdrv_write = vvfat_co_write,
- .bdrv_co_is_allocated = vvfat_co_is_allocated,
+ .bdrv_co_get_block_status = vvfat_co_get_block_status,
};
static void bdrv_vvfat_init(void)
diff --git a/include/block/block.h b/include/block/block.h
index 4a284b2..c73e679 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -271,6 +271,8 @@ int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors);
int bdrv_has_zero_init_1(BlockDriverState *bs);
int bdrv_has_zero_init(BlockDriverState *bs);
+int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, int *pnum);
int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
int *pnum);
int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8012e25..2604bc4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -135,7 +135,7 @@ struct BlockDriver {
int64_t sector_num, int nb_sectors);
int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors);
- int coroutine_fn (*bdrv_co_is_allocated)(BlockDriverState *bs,
+ int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum);
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (10 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 11/21] block: introduce bdrv_get_block_status API Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2014-05-05 14:34 ` Kevin Wolf
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 13/21] block: return get_block_status data and flags for formats Paolo Bonzini
` (9 subsequent siblings)
21 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Define the return value of get_block_status. Bits 0, 1, 2 and 9-62
are valid; bit 63 (the sign bit) is reserved for errors. Bits 3-8
are left for future extensions.
The return code is compatible with the old is_allocated API: if a driver
only returns 0 or 1 (aka BDRV_BLOCK_DATA) like is_allocated used to,
clients of is_allocated will not have any change in behavior. Still,
we will return more precise information in the next patches and the
new definition of bdrv_is_allocated is already prepared for this.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 10 ++++++++--
include/block/block.h | 26 ++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 47a5d63..0eb5e43 100644
--- a/block.c
+++ b/block.c
@@ -3073,7 +3073,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
if (!bs->drv->bdrv_co_get_block_status) {
*pnum = nb_sectors;
- return 1;
+ return BDRV_BLOCK_DATA;
}
return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
@@ -3123,7 +3123,13 @@ int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, int *pnum)
{
- return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
+ int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
+ if (ret < 0) {
+ return ret;
+ }
+ return
+ (ret & BDRV_BLOCK_DATA) ||
+ ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
}
/*
diff --git a/include/block/block.h b/include/block/block.h
index c73e679..578941f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -81,6 +81,32 @@ typedef struct BlockDevOps {
#define BDRV_SECTOR_SIZE (1ULL << BDRV_SECTOR_BITS)
#define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1)
+/* BDRV_BLOCK_DATA: data is read from bs->file or another file
+ * BDRV_BLOCK_ZERO: sectors read as zero
+ * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
+ *
+ * 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.
+ *
+ * DATA == 0 && ZERO == 0 means that data is read from backing_hd if present.
+ *
+ * DATA ZERO OFFSET_VALID
+ * t t t sectors read as zero, bs->file is zero at offset
+ * t f t sectors read as valid from bs->file at offset
+ * f t t sectors preallocated, read as zero, bs->file not
+ * necessarily zero at offset
+ * f f t sectors preallocated but read from backing_hd,
+ * bs->file contains garbage at offset
+ * t t f sectors preallocated, read as zero, unknown offset
+ * t f f sectors read from unknown file or offset
+ * f t f not allocated or unknown offset, read as zero
+ * f f f not allocated or unknown offset, read from backing_hd
+ */
+#define BDRV_BLOCK_DATA 1
+#define BDRV_BLOCK_ZERO 2
+#define BDRV_BLOCK_OFFSET_VALID 4
+#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
+
typedef enum {
BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
} BlockErrorAction;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value Paolo Bonzini
@ 2014-05-05 14:34 ` Kevin Wolf
2014-05-05 14:58 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Kevin Wolf @ 2014-05-05 14:34 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha
Am 04.09.2013 um 19:00 hat Paolo Bonzini geschrieben:
> Define the return value of get_block_status. Bits 0, 1, 2 and 9-62
> are valid; bit 63 (the sign bit) is reserved for errors. Bits 3-8
> are left for future extensions.
>
> The return code is compatible with the old is_allocated API: if a driver
> only returns 0 or 1 (aka BDRV_BLOCK_DATA) like is_allocated used to,
> clients of is_allocated will not have any change in behavior. Still,
> we will return more precise information in the next patches and the
> new definition of bdrv_is_allocated is already prepared for this.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> block.c | 10 ++++++++--
> include/block/block.h | 26 ++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index 47a5d63..0eb5e43 100644
> --- a/block.c
> +++ b/block.c
> @@ -3073,7 +3073,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>
> if (!bs->drv->bdrv_co_get_block_status) {
> *pnum = nb_sectors;
> - return 1;
> + return BDRV_BLOCK_DATA;
> }
>
> return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
> @@ -3123,7 +3123,13 @@ int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
> int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
> int nb_sectors, int *pnum)
> {
> - return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
> + int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
> + if (ret < 0) {
> + return ret;
> + }
> + return
> + (ret & BDRV_BLOCK_DATA) ||
> + ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
> }
I'm afraid that the assumptions eventually became too clever:
$ ./qemu-img create -f qcow2 /tmp/backing.qcow2 1G
Formatting '/tmp/backing.qcow2', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 lazy_refcounts=off
$ ./qemu-img create -f qcow2 -b /tmp/backing.qcow2 /tmp/overlay.qcow2 2G
Formatting '/tmp/overlay.qcow2', fmt=qcow2 size=2147483648 backing_file='/tmp/backing.qcow2' encryption=off cluster_size=65536 lazy_refcounts=off
$ ./qemu-io -c 'alloc 1G 64k' /tmp/overlay.qcow2
65536/65536 sectors allocated at offset 1 GiB
BDRV_BLOCK_ZERO is set for everything past the end of the backing file
(which kind of makes sense), and bdrv_has_zero_init() returns false for
anything that has a backing file (which makes sense, too), so now we're
reporting all sectors past the end of the backing file as allocated
(which is bad).
I think we're missing the information whether the BDRV_BLOCK_ZERO flag
actually comes from bs itself or from the backing chain. Do we need
another flag or does someone have a better idea?
Kevin
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2014-05-05 14:34 ` Kevin Wolf
@ 2014-05-05 14:58 ` Paolo Bonzini
2014-05-06 11:34 ` Kevin Wolf
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2014-05-05 14:58 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, stefanha
Il 05/05/2014 16:34, Kevin Wolf ha scritto:
> I think we're missing the information whether the BDRV_BLOCK_ZERO flag
> actually comes from bs itself or from the backing chain. Do we need
> another flag or does someone have a better idea?
Not sure if it is a better idea :) but we can duplicate the logic of
bdrv_get_co_block_status in bdrv_is_allocated.
The observation here is that bdrv_has_zero_init should have been
changed to bdrv_unallocated_blocks_are_zero in commit c3d8688
(block/get_block_status: fix BDRV_BLOCK_ZERO for unallocated blocks,
2013-10-24); the logic in the commit message applies just as well to
bdrv_is_allocated.
Of course, instead of cut-and-paste we can wrap all this into a
bdrv_unallocated_block_is_zero(bs, sector_num) function like this:
if (bdrv_unallocated_blocks_are_zero(bs)) {
return true;
}
if (bs->backing_hd) {
BlockDriverState *bs2 = bs->backing_hd;
int64_t length2 = bdrv_getlength(bs2);
if (length2 >= 0 && sector_num >= (length2 >> BDRV_SECTOR_BITS)) {
return true;
}
}
return false;
The function can then be used in bdrv_get_block_status to add a
BDRV_BLOCK_ZERO, and in bdrv_is_allocated instead of bdrv_has_zero_init.
(Bonus points for renaming bdrv_is_allocated to something like
bdrv_has_content, and similarly for bdrv_is_allocated_above).
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2014-05-05 14:58 ` Paolo Bonzini
@ 2014-05-06 11:34 ` Kevin Wolf
2014-05-06 12:31 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Kevin Wolf @ 2014-05-06 11:34 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha
Am 05.05.2014 um 16:58 hat Paolo Bonzini geschrieben:
> Il 05/05/2014 16:34, Kevin Wolf ha scritto:
> > I think we're missing the information whether the BDRV_BLOCK_ZERO flag
> > actually comes from bs itself or from the backing chain. Do we need
> > another flag or does someone have a better idea?
>
> Not sure if it is a better idea :) but we can duplicate the logic of
> bdrv_get_co_block_status in bdrv_is_allocated.
>
> The observation here is that bdrv_has_zero_init should have been
> changed to bdrv_unallocated_blocks_are_zero in commit c3d8688
> (block/get_block_status: fix BDRV_BLOCK_ZERO for unallocated blocks,
> 2013-10-24); the logic in the commit message applies just as well to
> bdrv_is_allocated.
But the result still wouldn't be right. We have an implication that you
can't simply reverse:
If this block is unallocated and unallocated blocks read as zero,
then this block reads as zero.
You're trying to use something like "If the block reads as zero and
unallocated blocks read as zero, it must be unallocated", which quite
obviously doesn't work.
Kevin
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2014-05-06 11:34 ` Kevin Wolf
@ 2014-05-06 12:31 ` Paolo Bonzini
2014-05-06 13:08 ` Kevin Wolf
0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2014-05-06 12:31 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, stefanha
Il 06/05/2014 13:34, Kevin Wolf ha scritto:
> But the result still wouldn't be right. We have an implication that you
> can't simply reverse:
>
> If this block is unallocated and unallocated blocks read as zero,
> then this block reads as zero.
>
> You're trying to use something like "If the block reads as zero and
> unallocated blocks read as zero, it must be unallocated", which quite
> obviously doesn't work.
This is not what the condition says in is_allocated. The right one is
(excluding the backing_hd case, which is handled with a function like
the one I sketched in the previous message):
If the block reads as zero and not all unallocated blocks read as
zero, the sector will be read from BS rather than the backing file,
even if it is not allocated.
This is true, because bdrv_get_block_status does not ask the backing
file whether it is zero in the backing file. Thus, reading from the
backing file does not guarantee to read zeroes.
<pedantic>
Since you wrote "something like", I guess it's not the difference
between what you wrote and what I wrote is not too relevant. But here
is the pedantic explanation of it:
- is_allocated does not answer the question "is the block allocated",
but rather "will the sector be read from BS or from a backing file?" (or
from an imaginary all-zeroes backing file if there is none). This is
why it is better to rename is_allocated before it brings more confusion
(and the brain damage I got from this confusion is also the reason why
BDRV_BLOCK_DATA is not called BDRV_BLOCK_ALLOCATED).
- this is the right hand side of a "||", so the outcome is not "it must
be unallocated", but "it must be allocated". Or better, given the
previous point, "the sector will be read from BS"
- the bdrv_has_zero_init (which is wrong, but can be fixed) is negated,
so it is not "unallocated blocks are zero" but rather "if the block
reads as zero and not all unallocated blocks read as zero".
</pedantic>
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2014-05-06 12:31 ` Paolo Bonzini
@ 2014-05-06 13:08 ` Kevin Wolf
2014-05-06 13:20 ` Paolo Bonzini
0 siblings, 1 reply; 29+ messages in thread
From: Kevin Wolf @ 2014-05-06 13:08 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha
Am 06.05.2014 um 14:31 hat Paolo Bonzini geschrieben:
> Il 06/05/2014 13:34, Kevin Wolf ha scritto:
> >But the result still wouldn't be right. We have an implication that you
> >can't simply reverse:
> >
> > If this block is unallocated and unallocated blocks read as zero,
> > then this block reads as zero.
> >
> >You're trying to use something like "If the block reads as zero and
> >unallocated blocks read as zero, it must be unallocated", which quite
> >obviously doesn't work.
>
> This is not what the condition says in is_allocated. The right one
> is (excluding the backing_hd case, which is handled with a function
> like the one I sketched in the previous message):
>
> If the block reads as zero and not all unallocated blocks read as
> zero, the sector will be read from BS rather than the backing file,
> even if it is not allocated.
>
> This is true, because bdrv_get_block_status does not ask the backing
> file whether it is zero in the backing file. Thus, reading from the
> backing file does not guarantee to read zeroes.
Aha, this paragraph is the key. :-)
I assumed that bdrv_get_block_status() does ask the backing file, but
apparently it doesn't. The current meaning of BDRV_BLOCK_ZERO is
therefore not simply "sectors read as zero", but "sectors read as zero
and this is determined by bs itself (mostly, except for the backing file
size)".
Is it a good idea to rely on this or would it be reasonable for
bdrv_get_block_status() to look at the backing file for returning ZERO
sometime in the future? I think that could be useful information in some
cases.
Kevin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value
2014-05-06 13:08 ` Kevin Wolf
@ 2014-05-06 13:20 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2014-05-06 13:20 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, stefanha
Il 06/05/2014 15:08, Kevin Wolf ha scritto:
>> This is true, because bdrv_get_block_status does not ask the backing
>> file whether it is zero in the backing file. Thus, reading from the
>> backing file does not guarantee to read zeroes.
>
> Aha, this paragraph is the key. :-)
>
> I assumed that bdrv_get_block_status() does ask the backing file, but
> apparently it doesn't. The current meaning of BDRV_BLOCK_ZERO is
> therefore not simply "sectors read as zero", but "sectors read as zero
> and this is determined by bs itself (mostly, except for the backing file
> size)".
Exactly.
> Is it a good idea to rely on this or would it be reasonable for
> bdrv_get_block_status() to look at the backing file for returning ZERO
> sometime in the future? I think that could be useful information in some
> cases.
It is reasonable, for example SCSI emulation could use it. However, it
would be a different API (like bdrv_is_allocated_above). It would be
very similar to get_block_status() in qemu-img.c.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v5 13/21] block: return get_block_status data and flags for formats
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (11 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 12/21] block: define get_block_status return value Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 14/21] block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO Paolo Bonzini
` (8 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/cow.c | 8 +++++++-
block/qcow.c | 9 ++++++++-
block/qcow2.c | 16 ++++++++++++++--
block/qed.c | 35 ++++++++++++++++++++++++++++-------
block/sheepdog.c | 2 +-
block/vdi.c | 13 ++++++++++++-
block/vmdk.c | 19 ++++++++++++++++++-
block/vvfat.c | 11 ++++++-----
8 files changed, 94 insertions(+), 19 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 98b5cb3..33d1ade 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -191,7 +191,13 @@ static int coroutine_fn cow_co_is_allocated(BlockDriverState *bs,
static int64_t coroutine_fn cow_co_get_block_status(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *num_same)
{
- return cow_co_is_allocated(bs, sector_num, nb_sectors, num_same);
+ BDRVCowState *s = bs->opaque;
+ int ret = cow_co_is_allocated(bs, sector_num, nb_sectors, num_same);
+ int64_t offset = s->cow_sectors_offset + (sector_num << BDRV_SECTOR_BITS);
+ if (ret < 0) {
+ return ret;
+ }
+ return (ret ? BDRV_BLOCK_DATA : 0) | offset | BDRV_BLOCK_OFFSET_VALID;
}
static int cow_update_bitmap(BlockDriverState *bs, int64_t sector_num,
diff --git a/block/qcow.c b/block/qcow.c
index acd1aeb..1a65822 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -410,7 +410,14 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
if (n > nb_sectors)
n = nb_sectors;
*pnum = n;
- return (cluster_offset != 0);
+ if (!cluster_offset) {
+ return 0;
+ }
+ if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypt_method) {
+ return BDRV_BLOCK_DATA;
+ }
+ cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+ return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | cluster_offset;
}
static int decompress_buffer(uint8_t *out_buf, int out_buf_size,
diff --git a/block/qcow2.c b/block/qcow2.c
index 4802e32..07c050d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -693,7 +693,8 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
{
BDRVQcowState *s = bs->opaque;
uint64_t cluster_offset;
- int ret;
+ int index_in_cluster, ret;
+ int64_t status = 0;
*pnum = nb_sectors;
qemu_co_mutex_lock(&s->lock);
@@ -703,7 +704,18 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
return ret;
}
- return (cluster_offset != 0) || (ret == QCOW2_CLUSTER_ZERO);
+ if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED &&
+ !s->crypt_method) {
+ index_in_cluster = sector_num & (s->cluster_sectors - 1);
+ cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+ status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset;
+ }
+ if (ret == QCOW2_CLUSTER_ZERO) {
+ status |= BDRV_BLOCK_ZERO;
+ } else if (ret != QCOW2_CLUSTER_UNALLOCATED) {
+ status |= BDRV_BLOCK_DATA;
+ }
+ return status;
}
/* handle reading after the end of the backing file */
diff --git a/block/qed.c b/block/qed.c
index 14e1ede..487eabb 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -652,16 +652,36 @@ static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options)
}
typedef struct {
+ BlockDriverState *bs;
Coroutine *co;
- int is_allocated;
+ uint64_t pos;
+ int64_t status;
int *pnum;
} QEDIsAllocatedCB;
static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t len)
{
QEDIsAllocatedCB *cb = opaque;
+ BDRVQEDState *s = cb->bs->opaque;
*cb->pnum = len / BDRV_SECTOR_SIZE;
- cb->is_allocated = (ret == QED_CLUSTER_FOUND || ret == QED_CLUSTER_ZERO);
+ switch (ret) {
+ case QED_CLUSTER_FOUND:
+ offset |= qed_offset_into_cluster(s, cb->pos);
+ cb->status = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
+ break;
+ case QED_CLUSTER_ZERO:
+ cb->status = BDRV_BLOCK_ZERO;
+ break;
+ case QED_CLUSTER_L2:
+ case QED_CLUSTER_L1:
+ cb->status = 0;
+ break;
+ default:
+ assert(ret < 0);
+ cb->status = ret;
+ break;
+ }
+
if (cb->co) {
qemu_coroutine_enter(cb->co, NULL);
}
@@ -672,25 +692,26 @@ static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
int nb_sectors, int *pnum)
{
BDRVQEDState *s = bs->opaque;
- uint64_t pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE;
size_t len = (size_t)nb_sectors * BDRV_SECTOR_SIZE;
QEDIsAllocatedCB cb = {
- .is_allocated = -1,
+ .bs = bs,
+ .pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE,
+ .status = BDRV_BLOCK_OFFSET_MASK,
.pnum = pnum,
};
QEDRequest request = { .l2_table = NULL };
- qed_find_cluster(s, &request, pos, len, qed_is_allocated_cb, &cb);
+ qed_find_cluster(s, &request, cb.pos, len, qed_is_allocated_cb, &cb);
/* Now sleep if the callback wasn't invoked immediately */
- while (cb.is_allocated == -1) {
+ while (cb.status == BDRV_BLOCK_OFFSET_MASK) {
cb.co = qemu_coroutine_self();
qemu_coroutine_yield();
}
qed_unref_l2_cache_entry(request.l2_table);
- return cb.is_allocated;
+ return cb.status;
}
static int bdrv_qed_make_empty(BlockDriverState *bs)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index ffc07c2..8506900 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2280,7 +2280,7 @@ sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
end = DIV_ROUND_UP((sector_num + nb_sectors) *
BDRV_SECTOR_SIZE, SD_DATA_OBJ_SIZE);
unsigned long idx;
- int ret = 1;
+ int64_t ret = BDRV_BLOCK_DATA;
for (idx = start; idx < end; idx++) {
if (inode->data_vdi_id[idx] == 0) {
diff --git a/block/vdi.c b/block/vdi.c
index 7ab2567..1bf7dc5 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -479,12 +479,23 @@ static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
size_t sector_in_block = sector_num % s->block_sectors;
int n_sectors = s->block_sectors - sector_in_block;
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;
- return VDI_IS_ALLOCATED(bmap_entry);
+ result = VDI_IS_ALLOCATED(bmap_entry);
+ if (!result) {
+ return 0;
+ }
+
+ offset = s->header.offset_data +
+ (uint64_t)bmap_entry * s->block_size +
+ sector_in_block * SECTOR_SIZE;
+ return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
}
static int vdi_co_read(BlockDriverState *bs,
diff --git a/block/vmdk.c b/block/vmdk.c
index d4516fd..3b661b9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1059,7 +1059,24 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
sector_num * 512, 0, &offset);
qemu_co_mutex_unlock(&s->lock);
- ret = (ret == VMDK_OK || ret == VMDK_ZEROED);
+ switch (ret) {
+ case VMDK_ERROR:
+ ret = -EIO;
+ break;
+ case VMDK_UNALLOC:
+ ret = 0;
+ break;
+ case VMDK_ZEROED:
+ ret = BDRV_BLOCK_ZERO;
+ break;
+ case VMDK_OK:
+ ret = BDRV_BLOCK_DATA;
+ if (extent->file == bs->file) {
+ ret |= BDRV_BLOCK_OFFSET_VALID | offset;
+ }
+
+ break;
+ }
index_in_cluster = sector_num % extent->cluster_sectors;
n = extent->cluster_sectors - index_in_cluster;
diff --git a/block/vvfat.c b/block/vvfat.c
index 4c30620..dd0efca 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2879,11 +2879,12 @@ static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
{
BDRVVVFATState* s = bs->opaque;
*n = s->sector_count - sector_num;
- if (*n > nb_sectors)
- *n = nb_sectors;
- else if (*n < 0)
- return 0;
- return 1;
+ if (*n > nb_sectors) {
+ *n = nb_sectors;
+ } else if (*n < 0) {
+ return 0;
+ }
+ return BDRV_BLOCK_DATA;
}
static int write_target_commit(BlockDriverState *bs, int64_t sector_num,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 14/21] block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (12 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 13/21] block: return get_block_status data and flags for formats Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 15/21] block: return BDRV_BLOCK_ZERO past end of backing file Paolo Bonzini
` (7 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Alternatively, this could use a "discard zeroes data" flag returned
by bdrv_get_info.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 0eb5e43..b6d380f 100644
--- a/block.c
+++ b/block.c
@@ -3055,6 +3055,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
{
int64_t length;
int64_t n;
+ int64_t ret;
length = bdrv_getlength(bs);
if (length < 0) {
@@ -3076,7 +3077,15 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
return BDRV_BLOCK_DATA;
}
- return bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
+ ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (!(ret & BDRV_BLOCK_DATA) && bdrv_has_zero_init(bs)) {
+ ret |= BDRV_BLOCK_ZERO;
+ }
+ return ret;
}
/* Coroutine wrapper for bdrv_get_block_status() */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 15/21] block: return BDRV_BLOCK_ZERO past end of backing file
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (13 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 14/21] block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 16/21] qemu-img: add a "map" subcommand Paolo Bonzini
` (6 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
If the sectors are unallocated and we are past the end of the
backing file, they will read as zero.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index b6d380f..6df387c 100644
--- a/block.c
+++ b/block.c
@@ -3082,8 +3082,16 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
return ret;
}
- if (!(ret & BDRV_BLOCK_DATA) && bdrv_has_zero_init(bs)) {
- ret |= BDRV_BLOCK_ZERO;
+ if (!(ret & BDRV_BLOCK_DATA)) {
+ if (bdrv_has_zero_init(bs)) {
+ ret |= BDRV_BLOCK_ZERO;
+ } else {
+ BlockDriverState *bs2 = bs->backing_hd;
+ int64_t length2 = bdrv_getlength(bs2);
+ if (length2 >= 0 && sector_num >= (length2 >> BDRV_SECTOR_BITS)) {
+ ret |= BDRV_BLOCK_ZERO;
+ }
+ }
}
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 16/21] qemu-img: add a "map" subcommand
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (14 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 15/21] block: return BDRV_BLOCK_ZERO past end of backing file Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 17/21] docs, qapi: document qemu-img map Paolo Bonzini
` (5 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
This command dumps the metadata of an entire chain, in either tabular or JSON
format.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-img-cmds.hx | 6 ++
qemu-img.c | 192 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 198 insertions(+)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 4ca7e95..c97a1f4 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -45,6 +45,12 @@ STEXI
@item info [-f @var{fmt}] [--output=@var{ofmt}] [--backing-chain] @var{filename}
ETEXI
+DEF("map", img_map,
+ "map [-f fmt] [--output=ofmt] filename")
+STEXI
+@item map [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+ETEXI
+
DEF("snapshot", img_snapshot,
"snapshot [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
STEXI
diff --git a/qemu-img.c b/qemu-img.c
index d50e0fc..9e35190 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1778,6 +1778,198 @@ static int img_info(int argc, char **argv)
return 0;
}
+
+typedef struct MapEntry {
+ int flags;
+ int depth;
+ int64_t start;
+ int64_t length;
+ int64_t offset;
+ BlockDriverState *bs;
+} MapEntry;
+
+static void dump_map_entry(OutputFormat output_format, MapEntry *e,
+ MapEntry *next)
+{
+ switch (output_format) {
+ case OFORMAT_HUMAN:
+ if ((e->flags & BDRV_BLOCK_DATA) &&
+ !(e->flags & BDRV_BLOCK_OFFSET_VALID)) {
+ error_report("File contains external, encrypted or compressed clusters.");
+ exit(1);
+ }
+ if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) {
+ printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n",
+ e->start, e->length, e->offset, e->bs->filename);
+ }
+ /* This format ignores the distinction between 0, ZERO and ZERO|DATA.
+ * Modify the flags here to allow more coalescing.
+ */
+ if (next &&
+ (next->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) != BDRV_BLOCK_DATA) {
+ next->flags &= ~BDRV_BLOCK_DATA;
+ next->flags |= BDRV_BLOCK_ZERO;
+ }
+ break;
+ case OFORMAT_JSON:
+ printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": %d,"
+ " \"zero\": %s, \"data\": %s",
+ (e->start == 0 ? "[" : ",\n"),
+ e->start, e->length, e->depth,
+ (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false",
+ (e->flags & BDRV_BLOCK_DATA) ? "true" : "false");
+ if (e->flags & BDRV_BLOCK_OFFSET_VALID) {
+ printf(", 'offset': %"PRId64"", e->offset);
+ }
+ putchar('}');
+
+ if (!next) {
+ printf("]\n");
+ }
+ break;
+ }
+}
+
+static int get_block_status(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, MapEntry *e)
+{
+ int64_t ret;
+ int depth;
+
+ /* 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.
+ */
+
+ depth = 0;
+ for (;;) {
+ ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &nb_sectors);
+ if (ret < 0) {
+ return ret;
+ }
+ assert(nb_sectors);
+ if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
+ break;
+ }
+ bs = bs->backing_hd;
+ if (bs == NULL) {
+ ret = 0;
+ break;
+ }
+
+ depth++;
+ }
+
+ e->start = sector_num * BDRV_SECTOR_SIZE;
+ e->length = nb_sectors * BDRV_SECTOR_SIZE;
+ e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK;
+ e->offset = ret & BDRV_BLOCK_OFFSET_MASK;
+ e->depth = depth;
+ e->bs = bs;
+ return 0;
+}
+
+static int img_map(int argc, char **argv)
+{
+ int c;
+ OutputFormat output_format = OFORMAT_HUMAN;
+ BlockDriverState *bs;
+ const char *filename, *fmt, *output;
+ int64_t length;
+ MapEntry curr = { .length = 0 }, next;
+ int ret = 0;
+
+ fmt = NULL;
+ output = NULL;
+ for (;;) {
+ int option_index = 0;
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"format", required_argument, 0, 'f'},
+ {"output", required_argument, 0, OPTION_OUTPUT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "f:h",
+ long_options, &option_index);
+ if (c == -1) {
+ break;
+ }
+ switch (c) {
+ case '?':
+ case 'h':
+ help();
+ break;
+ case 'f':
+ fmt = optarg;
+ break;
+ case OPTION_OUTPUT:
+ output = optarg;
+ break;
+ }
+ }
+ if (optind >= argc) {
+ help();
+ }
+ filename = argv[optind++];
+
+ if (output && !strcmp(output, "json")) {
+ output_format = OFORMAT_JSON;
+ } else if (output && !strcmp(output, "human")) {
+ output_format = OFORMAT_HUMAN;
+ } else if (output) {
+ error_report("--output must be used with human or json as argument.");
+ return 1;
+ }
+
+ bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS, true, false);
+ if (!bs) {
+ return 1;
+ }
+
+ if (output_format == OFORMAT_HUMAN) {
+ printf("%-16s%-16s%-16s%s\n", "Offset", "Length", "Mapped to", "File");
+ }
+
+ length = bdrv_getlength(bs);
+ 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;
+
+ /* 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);
+
+ if (ret < 0) {
+ error_report("Could not read file metadata: %s", strerror(-ret));
+ goto out;
+ }
+
+ if (curr.length != 0 && curr.flags == next.flags &&
+ curr.depth == next.depth &&
+ ((curr.flags & BDRV_BLOCK_OFFSET_VALID) == 0 ||
+ curr.offset + curr.length == next.offset)) {
+ curr.length += next.length;
+ continue;
+ }
+
+ if (curr.length > 0) {
+ dump_map_entry(output_format, &curr, &next);
+ }
+ curr = next;
+ }
+
+ dump_map_entry(output_format, &curr, NULL);
+
+out:
+ bdrv_close(bs);
+ bdrv_delete(bs);
+ return ret < 0;
+}
+
#define SNAPSHOT_LIST 1
#define SNAPSHOT_CREATE 2
#define SNAPSHOT_APPLY 3
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 17/21] docs, qapi: document qemu-img map
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (15 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 16/21] qemu-img: add a "map" subcommand Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 18/21] raw-posix: return get_block_status data and flags Paolo Bonzini
` (4 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Eric Blake also requested including the output in qapi-schema.json,
so that it is published through the introspection mechanism.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qapi-schema.json | 29 +++++++++++++++++++++++++++++
qemu-img.texi | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index a51f7d2..6955898 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -813,6 +813,35 @@
{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] }
##
+# @BlockDeviceMapEntry:
+#
+# Entry in the metadata map of the device (returned by "qemu-img map")
+#
+# @start: Offset in the image of the first byte described by this entry
+# (in bytes)
+#
+# @length: Length of the range described by this entry (in bytes)
+#
+# @depth: Number of layers (0 = top image, 1 = top image's backing file, etc.)
+# before reaching one for which the range is allocated. The value is
+# in the range 0 to the depth of the image chain - 1.
+#
+# @zero: the sectors in this range read as zeros
+#
+# @data: reading the image will actually read data from a file (in particular,
+# if @offset is present this means that the sectors are not simply
+# preallocated, but contain actual data in raw format)
+#
+# @offset: if present, the image file stores the data for this range in
+# raw format at the given offset.
+#
+# Since 1.7
+##
+{ 'type': 'BlockDeviceMapEntry',
+ 'data': { 'start': 'int', 'length': 'int', 'depth': 'int', 'zero': 'bool',
+ 'data': 'bool', '*offset': 'int' } }
+
+##
# @BlockDirtyInfo:
#
# Block dirty bitmap information.
diff --git a/qemu-img.texi b/qemu-img.texi
index 69f1bda..8364fa1 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -213,6 +213,61 @@ To enumerate information about each disk image in the above chain, starting from
qemu-img info --backing-chain snap2.qcow2
@end example
+@item map [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
+
+Dump the metadata of image @var{filename} and its backing file chain.
+In particular, this commands dumps the allocation state of every sector
+of @var{filename}, together with the topmost file that allocates it in
+the backing file chain.
+
+Two option formats are possible. The default format (@code{human})
+only dumps known-nonzero areas of the file. Known-zero parts of the
+file are omitted altogether, and likewise for parts that are not allocated
+throughout the chain. @command{qemu-img} output will identify a file
+from where the data can be read, and the offset in the file. Each line
+will include four fields, the first three of which are hexadecimal
+numbers. For example the first line of:
+@example
+Offset Length Mapped to File
+0 0x20000 0x50000 /tmp/overlay.qcow2
+0x100000 0x10000 0x95380000 /tmp/backing.qcow2
+@end example
+@noindent
+means that 0x20000 (131072) bytes starting at offset 0 in the image are
+available in /tmp/overlay.qcow2 (opened in @code{raw} format) starting
+at offset 0x50000 (327680). Data that is compressed, encrypted, or
+otherwise not available in raw format will cause an error if @code{human}
+format is in use. Note that file names can include newlines, thus it is
+not safe to parse this output format in scripts.
+
+The alternative format @code{json} will return an array of dictionaries
+in JSON format. It will include similar information in
+the @code{start}, @code{length}, @code{offset} fields;
+it will also include other more specific information:
+@itemize @minus
+@item
+whether the sectors contain actual data or not (boolean field @code{data};
+if false, the sectors are either unallocated or stored as optimized
+all-zero clusters);
+
+@item
+whether the data is known to read as zero (boolean field @code{zero});
+
+@item
+in order to make the output shorter, the target file is expressed as
+a @code{depth}; for example, a depth of 2 refers to the backing file
+of the backing file of @var{filename}.
+@end itemize
+
+In JSON format, the @code{offset} field is optional; it is absent in
+cases where @code{human} format would omit the entry or exit with an error.
+If @code{data} is false and the @code{offset} field is present, the
+corresponding sectors in the file are not yet in use, but they are
+preallocated.
+
+For more information, consult @file{include/block/block.h} in QEMU's
+source code.
+
@item snapshot [-l | -a @var{snapshot} | -c @var{snapshot} | -d @var{snapshot} ] @var{filename}
List, apply, create or delete snapshots in image @var{filename}.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 18/21] raw-posix: return get_block_status data and flags
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (16 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 17/21] docs, qapi: document qemu-img map Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 19/21] raw-posix: report unwritten extents as zero Paolo Bonzini
` (3 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/raw-posix.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index dbc65b0..d011cfd 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1089,7 +1089,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
int nb_sectors, int *pnum)
{
off_t start, data, hole;
- int ret;
+ int64_t ret;
ret = fd_open(bs);
if (ret < 0) {
@@ -1097,6 +1097,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
}
start = sector_num * BDRV_SECTOR_SIZE;
+ ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
#ifdef CONFIG_FIEMAP
@@ -1114,7 +1115,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
if (ioctl(s->fd, FS_IOC_FIEMAP, &f) == -1) {
/* Assume everything is allocated. */
*pnum = nb_sectors;
- return 1;
+ return ret;
}
if (f.fm.fm_mapped_extents == 0) {
@@ -1141,7 +1142,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
/* Most likely EINVAL. Assume everything is allocated. */
*pnum = nb_sectors;
- return 1;
+ return ret;
}
if (hole > start) {
@@ -1154,19 +1155,21 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
}
}
#else
- *pnum = nb_sectors;
- return 1;
+ data = 0;
+ hole = start + nb_sectors * BDRV_SECTOR_SIZE;
#endif
if (data <= start) {
/* On a data extent, compute sectors to the end of the extent. */
*pnum = MIN(nb_sectors, (hole - start) / BDRV_SECTOR_SIZE);
- return 1;
} else {
/* On a hole, compute sectors to the beginning of the next extent. */
*pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
- return 0;
+ ret &= ~BDRV_BLOCK_DATA;
+ ret |= BDRV_BLOCK_ZERO;
}
+
+ return ret;
}
static coroutine_fn BlockDriverAIOCB *raw_aio_discard(BlockDriverState *bs,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 19/21] raw-posix: report unwritten extents as zero
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (17 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 18/21] raw-posix: return get_block_status data and flags Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 20/21] block: add default get_block_status implementation for protocols Paolo Bonzini
` (2 subsequent siblings)
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
These are created for example with XFS_IOC_ZERO_RANGE.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/raw-posix.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index d011cfd..1b41ea3 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1128,6 +1128,9 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
} else {
data = f.fe.fe_logical;
hole = f.fe.fe_logical + f.fe.fe_length;
+ if (f.fe.fe_flags & FIEMAP_EXTENT_UNWRITTEN) {
+ ret |= BDRV_BLOCK_ZERO;
+ }
}
#elif defined SEEK_HOLE && defined SEEK_DATA
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 20/21] block: add default get_block_status implementation for protocols
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (18 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 19/21] raw-posix: report unwritten extents as zero Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 21/21] block: look for zero blocks in bs->file Paolo Bonzini
2013-09-05 13:55 ` [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Stefan Hajnoczi
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Protocols return raw data, so you can assume the offsets to pass
through unchanged.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 6df387c..18cdfe6 100644
--- a/block.c
+++ b/block.c
@@ -3074,7 +3074,11 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
if (!bs->drv->bdrv_co_get_block_status) {
*pnum = nb_sectors;
- return BDRV_BLOCK_DATA;
+ ret = BDRV_BLOCK_DATA;
+ if (bs->drv->protocol_name) {
+ ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE);
+ }
+ return ret;
}
ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v5 21/21] block: look for zero blocks in bs->file
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (19 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 20/21] block: add default get_block_status implementation for protocols Paolo Bonzini
@ 2013-09-04 17:00 ` Paolo Bonzini
2013-09-05 13:55 ` [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Stefan Hajnoczi
21 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2013-09-04 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 18cdfe6..6e6f974 100644
--- a/block.c
+++ b/block.c
@@ -3055,7 +3055,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
{
int64_t length;
int64_t n;
- int64_t ret;
+ int64_t ret, ret2;
length = bdrv_getlength(bs);
if (length < 0) {
@@ -3097,6 +3097,20 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
}
}
}
+
+ if (bs->file &&
+ (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
+ (ret & BDRV_BLOCK_OFFSET_VALID)) {
+ ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
+ *pnum, pnum);
+ if (ret2 >= 0) {
+ /* Ignore errors. This is just providing extra information, it
+ * is useful but not necessary.
+ */
+ ret |= (ret2 & BDRV_BLOCK_ZERO);
+ }
+ }
+
return ret;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata
2013-09-04 17:00 [Qemu-devel] [PATCH v5 00/21] Add qemu-img subcommand to dump file metadata Paolo Bonzini
` (20 preceding siblings ...)
2013-09-04 17:00 ` [Qemu-devel] [PATCH v5 21/21] block: look for zero blocks in bs->file Paolo Bonzini
@ 2013-09-05 13:55 ` Stefan Hajnoczi
21 siblings, 0 replies; 29+ messages in thread
From: Stefan Hajnoczi @ 2013-09-05 13:55 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha
On Wed, Sep 04, 2013 at 07:00:17PM +0200, Paolo Bonzini wrote:
> v4->v5: no change except for context and correcting the "Since: 1.6"
> in qapi-schema.json
>
> Paolo Bonzini (21):
> cow: make reads go at a decent speed
> cow: make writes go at a less indecent speed
> cow: do not call bdrv_co_is_allocated
> block: keep bs->total_sectors up to date even for growable block
> devices
> block: make bdrv_co_is_allocated static
> block: do not use ->total_sectors in bdrv_co_is_allocated
> block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above
> distinction
> block: expect errors from bdrv_co_is_allocated
> qemu-img: always probe the input image for allocated sectors
> block: make bdrv_has_zero_init return false for copy-on-write-images
> block: introduce bdrv_get_block_status API
> block: define get_block_status return value
> block: return get_block_status data and flags for formats
> block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO
> block: return BDRV_BLOCK_ZERO past end of backing file
> qemu-img: add a "map" subcommand
> docs, qapi: document qemu-img map
> raw-posix: return get_block_status data and flags
> raw-posix: report unwritten extents as zero
> block: add default get_block_status implementation for protocols
> block: look for zero blocks in bs->file
>
> block.c | 174 +++++++++++++++++++--------------
> block/backup.c | 4 +-
> block/commit.c | 6 +-
> block/cow.c | 91 +++++++++++++-----
> block/mirror.c | 4 +-
> block/qcow.c | 13 ++-
> block/qcow2.c | 24 +++--
> block/qed.c | 39 ++++++--
> block/raw-posix.c | 24 +++--
> block/raw_bsd.c | 10 +-
> block/sheepdog.c | 14 +--
> block/stream.c | 10 +-
> block/vdi.c | 17 +++-
> block/vmdk.c | 23 ++++-
> block/vvfat.c | 15 +--
> include/block/block.h | 34 +++++--
> include/block/block_int.h | 2 +-
> qapi-schema.json | 29 ++++++
> qemu-img-cmds.hx | 6 ++
> qemu-img.c | 239 +++++++++++++++++++++++++++++++++++++++++-----
> qemu-img.texi | 55 +++++++++++
> qemu-io-cmds.c | 4 +
> 22 files changed, 646 insertions(+), 191 deletions(-)
>
> --
> 1.8.3.1
>
>
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread