* [Qemu-devel] [PATCH v2 1/4] qed: mark image clean after repair succeeds
2012-08-09 12:05 [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Stefan Hajnoczi
@ 2012-08-09 12:05 ` Stefan Hajnoczi
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 2/4] qcow2: " Stefan Hajnoczi
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-08-09 12:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
The dirty bit is cleared after image repair succeeds in qed_open().
Move this into qed_check() so that all callers benefit from this
behavior when fix=true.
This is necessary so qemu-img check can call .bdrv_check() and mark the
image clean.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
block/qed-check.c | 26 ++++++++++++++++++++++++++
block/qed.c | 9 +--------
block/qed.h | 5 +++++
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/block/qed-check.c b/block/qed-check.c
index 5edf607..b473dcd 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -194,6 +194,28 @@ static void qed_check_for_leaks(QEDCheck *check)
}
}
+/**
+ * Mark an image clean once it passes check or has been repaired
+ */
+static void qed_check_mark_clean(BDRVQEDState *s, BdrvCheckResult *result)
+{
+ /* Skip if there were unfixable corruptions or I/O errors */
+ if (result->corruptions > 0 || result->check_errors > 0) {
+ return;
+ }
+
+ /* Skip if image is already marked clean */
+ if (!(s->header.features & QED_F_NEED_CHECK)) {
+ return;
+ }
+
+ /* Ensure fixes reach storage before clearing check bit */
+ bdrv_flush(s->bs);
+
+ s->header.features &= ~QED_F_NEED_CHECK;
+ qed_write_header_sync(s);
+}
+
int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
{
QEDCheck check = {
@@ -215,6 +237,10 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
if (ret == 0) {
/* Only check for leaks if entire image was scanned successfully */
qed_check_for_leaks(&check);
+
+ if (fix) {
+ qed_check_mark_clean(s, result);
+ }
}
g_free(check.used_clusters);
diff --git a/block/qed.c b/block/qed.c
index 5f3eefa..226545d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -89,7 +89,7 @@ static void qed_header_cpu_to_le(const QEDHeader *cpu, QEDHeader *le)
le->backing_filename_size = cpu_to_le32(cpu->backing_filename_size);
}
-static int qed_write_header_sync(BDRVQEDState *s)
+int qed_write_header_sync(BDRVQEDState *s)
{
QEDHeader le;
int ret;
@@ -491,13 +491,6 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
if (ret) {
goto out;
}
- if (!result.corruptions && !result.check_errors) {
- /* Ensure fixes reach storage before clearing check bit */
- bdrv_flush(s->bs);
-
- s->header.features &= ~QED_F_NEED_CHECK;
- qed_write_header_sync(s);
- }
}
}
diff --git a/block/qed.h b/block/qed.h
index c716772..a063bf7 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -211,6 +211,11 @@ void *gencb_alloc(size_t len, BlockDriverCompletionFunc *cb, void *opaque);
void gencb_complete(void *opaque, int ret);
/**
+ * Header functions
+ */
+int qed_write_header_sync(BDRVQEDState *s);
+
+/**
* L2 cache functions
*/
void qed_init_l2_cache(L2TableCache *l2_cache);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 2/4] qcow2: mark image clean after repair succeeds
2012-08-09 12:05 [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Stefan Hajnoczi
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 1/4] qed: mark image clean after repair succeeds Stefan Hajnoczi
@ 2012-08-09 12:05 ` Stefan Hajnoczi
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 3/4] block: add BLOCK_O_CHECK for qemu-img check Stefan Hajnoczi
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-08-09 12:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
The dirty bit is cleared after image repair succeeds in qcow2_open().
Move this into qcow2_check() so that all callers benefit from this
behavior when fix mode is enabled.
This is necessary so qemu-img check can call .bdrv_check() and mark the
image clean.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
block/qcow2.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index fd5e214..5896fd6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -270,6 +270,20 @@ static int qcow2_mark_clean(BlockDriverState *bs)
return 0;
}
+static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
+ BdrvCheckMode fix)
+{
+ int ret = qcow2_check_refcounts(bs, result, fix);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (fix && result->check_errors == 0 && result->corruptions == 0) {
+ return qcow2_mark_clean(bs);
+ }
+ return ret;
+}
+
static int qcow2_open(BlockDriverState *bs, int flags)
{
BDRVQcowState *s = bs->opaque;
@@ -474,12 +488,7 @@ static int qcow2_open(BlockDriverState *bs, int flags)
!bs->read_only) {
BdrvCheckResult result = {0};
- ret = qcow2_check_refcounts(bs, &result, BDRV_FIX_ERRORS);
- if (ret < 0) {
- goto fail;
- }
-
- ret = qcow2_mark_clean(bs);
+ ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
if (ret < 0) {
goto fail;
}
@@ -1568,13 +1577,6 @@ static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
return 0;
}
-
-static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
- BdrvCheckMode fix)
-{
- return qcow2_check_refcounts(bs, result, fix);
-}
-
#if 0
static void dump_refcounts(BlockDriverState *bs)
{
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 3/4] block: add BLOCK_O_CHECK for qemu-img check
2012-08-09 12:05 [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Stefan Hajnoczi
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 1/4] qed: mark image clean after repair succeeds Stefan Hajnoczi
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 2/4] qcow2: " Stefan Hajnoczi
@ 2012-08-09 12:05 ` Stefan Hajnoczi
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: skip 039 with ./check -nocache Stefan Hajnoczi
2012-08-09 15:02 ` [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Kevin Wolf
4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-08-09 12:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
Image formats with a dirty bit, like qed and qcow2, repair dirty image
files upon open with BDRV_O_RDWR. Performing automatic repair when
qemu-img check runs is not ideal because the bdrv_open() call repairs
the image before the actual bdrv_check() call from qemu-img.c.
Fix this "double repair" since it leads to confusing output from
qemu-img check. Tell the block driver that this image is being opened
just for bdrv_check(). This skips automatic repair and qemu-img.c can
invoke it manually with bdrv_check().
Update the golden output for qemu-iotests 039 to reflect the new
qemu-img check output.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
block.h | 1 +
block/qcow2.c | 4 ++--
block/qed.c | 2 +-
qemu-img.c | 2 +-
tests/qemu-iotests/039.out | 6 ++++++
5 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/block.h b/block.h
index 650d872..2e2be11 100644
--- a/block.h
+++ b/block.h
@@ -79,6 +79,7 @@ typedef struct BlockDevOps {
#define BDRV_O_NO_FLUSH 0x0200 /* disable flushing on this disk */
#define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
#define BDRV_O_INCOMING 0x0800 /* consistency hint for incoming migration */
+#define BDRV_O_CHECK 0x1000 /* open solely for consistency check */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
diff --git a/block/qcow2.c b/block/qcow2.c
index 5896fd6..8f183f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -484,8 +484,8 @@ static int qcow2_open(BlockDriverState *bs, int flags)
qemu_co_mutex_init(&s->lock);
/* Repair image if dirty */
- if ((s->incompatible_features & QCOW2_INCOMPAT_DIRTY) &&
- !bs->read_only) {
+ if (!(flags & BDRV_O_CHECK) && !bs->read_only &&
+ (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) {
BdrvCheckResult result = {0};
ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
diff --git a/block/qed.c b/block/qed.c
index 226545d..a02dbfd 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -477,7 +477,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
}
/* If image was not closed cleanly, check consistency */
- if (s->header.features & QED_F_NEED_CHECK) {
+ if (!(flags & BDRV_O_CHECK) && (s->header.features & QED_F_NEED_CHECK)) {
/* Read-only images cannot be fixed. There is no risk of corruption
* since write operations are not possible. Therefore, allow
* potentially inconsistent images to be opened read-only. This can
diff --git a/qemu-img.c b/qemu-img.c
index 94a31ad..b41e670 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -379,7 +379,7 @@ static int img_check(int argc, char **argv)
BlockDriverState *bs;
BdrvCheckResult result;
int fix = 0;
- int flags = BDRV_O_FLAGS;
+ int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
fmt = NULL;
for(;;) {
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 155a05e..cb510d6 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -26,6 +26,12 @@ incompatible_features 0x1
== Repairing the image file must succeed ==
ERROR OFLAG_COPIED: offset=8000000000050000 refcount=0
Repairing cluster 5 refcount=0 reference=1
+The following inconsistencies were found and repaired:
+
+ 0 leaked clusters
+ 1 corruptions
+
+Double checking the fixed image now...
No errors were found on the image.
incompatible_features 0x0
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 4/4] qemu-iotests: skip 039 with ./check -nocache
2012-08-09 12:05 [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Stefan Hajnoczi
` (2 preceding siblings ...)
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 3/4] block: add BLOCK_O_CHECK for qemu-img check Stefan Hajnoczi
@ 2012-08-09 12:05 ` Stefan Hajnoczi
2012-08-09 15:02 ` [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Kevin Wolf
4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2012-08-09 12:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
When the qemu-io --nocache option is used the 039 test case cannot abort
QEMU at a point where the image is dirty. Skip the test case.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
tests/qemu-iotests/039 | 1 +
tests/qemu-iotests/common.rc | 14 ++++++++++++++
2 files changed, 15 insertions(+)
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index a749fcf..c5ae806 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -44,6 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto generic
_supported_os Linux
+_unsupported_qemu_io_options --nocache
size=128M
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 7782808..c5129f4 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -297,6 +297,20 @@ _supported_os()
_notrun "not suitable for this OS: $HOSTOS"
}
+_unsupported_qemu_io_options()
+{
+ for bad_opt
+ do
+ for opt in $QEMU_IO_OPTIONS
+ do
+ if [ "$bad_opt" = "$opt" ]
+ then
+ _notrun "not suitable for qemu-io option: $bad_opt"
+ fi
+ done
+ done
+}
+
# this test requires that a specified command (executable) exists
#
_require_command()
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts
2012-08-09 12:05 [Qemu-devel] [PATCH v2 0/4] qcow2: clean-ups around lazy refcounts Stefan Hajnoczi
` (3 preceding siblings ...)
2012-08-09 12:05 ` [Qemu-devel] [PATCH v2 4/4] qemu-iotests: skip 039 with ./check -nocache Stefan Hajnoczi
@ 2012-08-09 15:02 ` Kevin Wolf
4 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2012-08-09 15:02 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
Am 09.08.2012 14:05, schrieb Stefan Hajnoczi:
> The qcow2 lazy refcount feature was merged with two pending clean-ups suggested
> by Kevin:
>
> 1. Ensure that qemu-iotests/check -nocache 039 does not fail.
>
> 2. Fix qemu-img check output when a dirty image file is opened. It currently
> omits the error summary because automatic repair cleans the image during
> bdrv_open() before qemu-img.c can call bdrv_check().
>
> v2:
> * Fail when qcow2 check finds corruptions [Kevin]
>
> Stefan Hajnoczi (4):
> qed: mark image clean after repair succeeds
> qcow2: mark image clean after repair succeeds
> block: add BLOCK_O_CHECK for qemu-img check
> qemu-iotests: skip 039 with ./check -nocache
>
> block.h | 1 +
> block/qcow2.c | 32 +++++++++++++++++---------------
> block/qed-check.c | 26 ++++++++++++++++++++++++++
> block/qed.c | 11 ++---------
> block/qed.h | 5 +++++
> qemu-img.c | 2 +-
> tests/qemu-iotests/039 | 1 +
> tests/qemu-iotests/039.out | 6 ++++++
> tests/qemu-iotests/common.rc | 14 ++++++++++++++
> 9 files changed, 73 insertions(+), 25 deletions(-)
Thanks, applied all to the block branch.
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread