* [Qemu-devel] [PATCHv2 1/2] qemu-img: find the highest offset in use during check
@ 2012-12-10 17:01 Federico Simoncelli
2012-12-10 17:01 ` [Qemu-devel] [PATCH 2/2] qemu-img: add json output option to the check command Federico Simoncelli
0 siblings, 1 reply; 3+ messages in thread
From: Federico Simoncelli @ 2012-12-10 17:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Federico Simoncelli
This patch adds the support for reporting the highest offset in use by
an image. This is particularly useful after a conversion (or a rebase)
where the destination is a block device in order to find the actual
amount of space in use.
Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
---
block.h | 1 +
block/qcow2-refcount.c | 10 ++++++++--
qemu-img.c | 4 ++++
tests/qemu-iotests/026 | 6 +++---
tests/qemu-iotests/036 | 3 ++-
tests/qemu-iotests/039 | 2 +-
tests/qemu-iotests/common.rc | 5 +++--
7 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/block.h b/block.h
index 722c620..de42e8c 100644
--- a/block.h
+++ b/block.h
@@ -213,6 +213,7 @@ typedef struct BdrvCheckResult {
int check_errors;
int corruptions_fixed;
int leaks_fixed;
+ int64_t highest_offset;
BlockFragInfo bfi;
} BdrvCheckResult;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 96224d1..017439d 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1116,7 +1116,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix)
{
BDRVQcowState *s = bs->opaque;
- int64_t size, i;
+ int64_t size, i, highest_cluster;
int nb_clusters, refcount1, refcount2;
QCowSnapshot *sn;
uint16_t *refcount_table;
@@ -1154,7 +1154,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
s->refcount_table_offset,
s->refcount_table_size * sizeof(uint64_t));
- for(i = 0; i < s->refcount_table_size; i++) {
+ for(i = 0, highest_cluster = 0; i < s->refcount_table_size; i++) {
uint64_t offset, cluster;
offset = s->refcount_table[i];
cluster = offset >> s->cluster_bits;
@@ -1197,6 +1197,11 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
}
refcount2 = refcount_table[i];
+
+ if (refcount1 > 0 || refcount2 > 0) {
+ highest_cluster = i;
+ }
+
if (refcount1 != refcount2) {
/* Check if we're allowed to fix the mismatch */
@@ -1231,6 +1236,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
}
}
+ res->highest_offset = (highest_cluster + 1) * s->cluster_size;
ret = 0;
fail:
diff --git a/qemu-img.c b/qemu-img.c
index e29e01b..45c1ec1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -470,6 +470,10 @@ static int img_check(int argc, char **argv)
result.bfi.fragmented_clusters * 100.0 / result.bfi.allocated_clusters);
}
+ if (result.highest_offset > 0) {
+ printf("Highest offset in use: %" PRId64 "\n", result.highest_offset);
+ }
+
bdrv_delete(bs);
if (ret < 0 || result.check_errors) {
diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index 1602ccd..107a3ff 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -102,7 +102,7 @@ if [ "$event" == "l2_load" ]; then
$QEMU_IO -c "read $vmstate 0 128k " $BLKDBG_TEST_IMG | _filter_qemu_io
fi
-$QEMU_IMG check $TEST_IMG 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
done
done
@@ -147,7 +147,7 @@ echo
echo "Event: $event; errno: $errno; imm: $imm; once: $once; write $vmstate"
$QEMU_IO -c "write $vmstate 0 64M" $BLKDBG_TEST_IMG | _filter_qemu_io
-$QEMU_IMG check $TEST_IMG 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
done
done
@@ -186,7 +186,7 @@ echo
echo "Event: $event; errno: $errno; imm: $imm; once: $once"
$QEMU_IO -c "write -b 0 64k" $BLKDBG_TEST_IMG | _filter_qemu_io
-$QEMU_IMG check $TEST_IMG 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
done
done
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index 329533e..4dbfc57 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -59,7 +59,8 @@ _make_test_img 64M
echo
echo === Repair image ===
echo
-$QEMU_IMG check -r all $TEST_IMG
+_check_test_img -r all
+
./qcow2.py $TEST_IMG dump-header
# success, all done
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index c5ae806..ae35175 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -86,7 +86,7 @@ $QEMU_IO -r -c "read -P 0x5a 0 512" $TEST_IMG | _filter_qemu_io
echo
echo "== Repairing the image file must succeed =="
-$QEMU_IMG check -r all $TEST_IMG
+_check_test_img -r all
# The dirty bit must not be set
./qcow2.py $TEST_IMG dump-header | grep incompatible_features
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index aef5f52..22c0186 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -161,9 +161,10 @@ _cleanup_test_img()
_check_test_img()
{
- $QEMU_IMG check -f $IMGFMT $TEST_IMG 2>&1 | \
+ $QEMU_IMG check "$@" -f $IMGFMT $TEST_IMG 2>&1 | \
grep -v "fragmented$" | \
- sed -e 's/qemu-img\: This image format does not support checks/No errors were found on the image./'
+ sed -e 's/qemu-img\: This image format does not support checks/No errors were found on the image./' | \
+ sed -e '/Highest offset in use: [0-9]\+/d'
}
_img_info()
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH 2/2] qemu-img: add json output option to the check command
2012-12-10 17:01 [Qemu-devel] [PATCHv2 1/2] qemu-img: find the highest offset in use during check Federico Simoncelli
@ 2012-12-10 17:01 ` Federico Simoncelli
2012-12-13 12:38 ` Kevin Wolf
0 siblings, 1 reply; 3+ messages in thread
From: Federico Simoncelli @ 2012-12-10 17:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Federico Simoncelli
This option --output=[human|json] make qemu-img check output an human
or JSON representation at the choice of the user.
Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
---
qapi-schema.json | 38 +++++++++
qemu-img-cmds.hx | 4 +-
qemu-img.c | 246 +++++++++++++++++++++++++++++++++++++++---------------
qemu-img.texi | 5 +-
4 files changed, 221 insertions(+), 72 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..8877285 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -245,6 +245,44 @@
'*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
##
+# @ImageCheck:
+#
+# Information about a QEMU image file check
+#
+# @filename: name of the image file checked
+#
+# @format: format of the image file checked
+#
+# @check-errors: number of unexpected errors occurred during check
+#
+# @highest-offset: #optional highest offset (in bytes) in use by the image
+#
+# @corruptions: #optional number of corruptions found during the check
+#
+# @leaks: #optional number of leaks found during the check
+#
+# @corruptions-fixed: #optional number of corruptions fixed during the check
+#
+# @leaks-fixed: #optional number of leaks fixed during the check
+#
+# @total-clusters: #optional total number of clusters
+#
+# @allocated-clusters: #optional total number of allocated clusters
+#
+# @fragmented-clusters: #optional total number of fragmented clusters
+#
+# Since: 1.4
+#
+##
+
+{ 'type': 'ImageCheck',
+ 'data': {'filename': 'str', 'format': 'str', 'check-errors': 'int',
+ '*highest-offset': 'int', '*corruptions': 'int', '*leaks': 'int',
+ '*corruptions-fixed': 'int', '*leaks-fixed': 'int',
+ '*total-clusters': 'int', '*allocated-clusters': 'int',
+ '*fragmented-clusters': 'int' } }
+
+##
# @StatusInfo:
#
# Information about VCPU run state
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index a181363..259fc14 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,9 +10,9 @@ STEXI
ETEXI
DEF("check", img_check,
- "check [-f fmt] [-r [leaks | all]] filename")
+ "check [-f fmt] [--output=ofmt] [-r [leaks | all]] filename")
STEXI
-@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
+@item check [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] @var{filename}
ETEXI
DEF("create", img_create,
diff --git a/qemu-img.c b/qemu-img.c
index 45c1ec1..18ba5c2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -42,6 +42,16 @@ typedef struct img_cmd_t {
int (*handler)(int argc, char **argv);
} img_cmd_t;
+enum {
+ OPTION_OUTPUT = 256,
+ OPTION_BACKING_CHAIN = 257,
+};
+
+typedef enum OutputFormat {
+ OFORMAT_JSON,
+ OFORMAT_HUMAN,
+} OutputFormat;
+
/* Default to cache=writeback as data integrity is not important for qemu-tcg. */
#define BDRV_O_FLAGS BDRV_O_CACHE_WB
#define BDRV_DEFAULT_CACHE "writeback"
@@ -370,6 +380,122 @@ out:
return 0;
}
+static void dump_json_image_check(ImageCheck *check)
+{
+ Error *errp = NULL;
+ QString *str;
+ QmpOutputVisitor *ov = qmp_output_visitor_new();
+ QObject *obj;
+ visit_type_ImageCheck(qmp_output_get_visitor(ov),
+ &check, NULL, &errp);
+ obj = qmp_output_get_qobject(ov);
+ str = qobject_to_json_pretty(obj);
+ assert(str != NULL);
+ printf("%s\n", qstring_get_str(str));
+ qobject_decref(obj);
+ qmp_output_visitor_cleanup(ov);
+ QDECREF(str);
+}
+
+static void dump_human_image_check(ImageCheck *check)
+{
+ if (check->corruptions_fixed || check->leaks_fixed) {
+ printf("The following inconsistencies were found and repaired:\n\n"
+ " %" PRId64 " leaked clusters\n"
+ " %" PRId64 " corruptions\n\n"
+ "Double checking the fixed image now...\n",
+ check->leaks_fixed,
+ check->corruptions_fixed);
+ }
+
+ if (!(check->corruptions || check->leaks || check->check_errors)) {
+ printf("No errors were found on the image.\n");
+ } else {
+ if (check->corruptions) {
+ printf("\n%" PRId64 " errors were found on the image.\n"
+ "Data may be corrupted, or further writes to the image "
+ "may corrupt it.\n",
+ check->corruptions);
+ }
+
+ if (check->leaks) {
+ printf("\n%" PRId64 " leaked clusters were found on the image.\n"
+ "This means waste of disk space, but no harm to data.\n",
+ check->leaks);
+ }
+
+ if (check->check_errors) {
+ printf("\n%" PRId64 " internal errors have occurred during the check.\n",
+ check->check_errors);
+ }
+ }
+
+ if (check->total_clusters != 0 && check->allocated_clusters != 0) {
+ printf("%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
+ check->allocated_clusters, check->total_clusters,
+ check->allocated_clusters * 100.0 / check->total_clusters,
+ check->fragmented_clusters * 100.0 / check->allocated_clusters);
+ }
+
+ if (check->highest_offset) {
+ printf("Highest offset in use: %" PRId64 "\n", check->highest_offset);
+ }
+}
+
+static int collect_image_check(BlockDriverState *bs,
+ ImageCheck *check,
+ const char *filename,
+ const char *fmt,
+ int fix)
+{
+ int ret;
+ BdrvCheckResult result;
+
+ ret = bdrv_check(bs, &result, fix);
+
+ if (ret < 0) {
+ return ret;
+ }
+
+ check->filename = g_strdup(filename);
+ check->format = g_strdup(bdrv_get_format_name(bs));
+ check->check_errors = result.check_errors;
+
+ check->corruptions = result.corruptions;
+ check->has_corruptions = result.corruptions != 0;
+ check->leaks = result.leaks;
+ check->has_leaks = result.leaks != 0;
+ check->corruptions_fixed = result.corruptions_fixed;
+ check->has_corruptions_fixed = result.corruptions != 0;
+ check->leaks_fixed = result.leaks_fixed;
+ check->has_leaks_fixed = result.leaks != 0;
+ check->highest_offset = result.highest_offset;
+ check->has_highest_offset = result.highest_offset != 0;
+
+ check->total_clusters = result.bfi.total_clusters;
+ check->has_total_clusters = result.bfi.total_clusters != 0;
+ check->allocated_clusters = result.bfi.allocated_clusters;
+ check->has_allocated_clusters = result.bfi.allocated_clusters != 0;
+ check->fragmented_clusters = result.bfi.fragmented_clusters;
+ check->has_fragmented_clusters = result.bfi.fragmented_clusters != 0;
+
+ /* double checking the fixed image */
+ if (result.corruptions_fixed || result.leaks_fixed) {
+ ret = bdrv_check(bs, &result, 0);
+
+ if (ret < 0) {
+ return ret;
+ }
+
+ check->corruptions = result.corruptions;
+ check->has_corruptions = result.corruptions != 0;
+ check->leaks = result.leaks;
+ check->has_leaks = result.leaks != 0;
+ }
+
+ return 0;
+}
+
/*
* Checks an image for consistency. Exit codes:
*
@@ -381,15 +507,26 @@ out:
static int img_check(int argc, char **argv)
{
int c, ret;
- const char *filename, *fmt;
+ OutputFormat output_format = OFORMAT_HUMAN;
+ const char *filename, *fmt, *output;
BlockDriverState *bs;
- BdrvCheckResult result;
int fix = 0;
int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
+ ImageCheck *check;
fmt = NULL;
+ output = NULL;
for(;;) {
- c = getopt(argc, argv, "f:hr:");
+ int option_index = 0;
+ static const struct option long_options[] = {
+ {"help", no_argument, 0, 'h'},
+ {"format", required_argument, 0, 'f'},
+ {"repair", no_argument, 0, 'r'},
+ {"output", required_argument, 0, OPTION_OUTPUT},
+ {0, 0, 0, 0}
+ };
+ c = getopt_long(argc, argv, "f:hr:",
+ long_options, &option_index);
if (c == -1) {
break;
}
@@ -412,84 +549,67 @@ static int img_check(int argc, char **argv)
help();
}
break;
+ case OPTION_OUTPUT:
+ output = optarg;
+ break;
}
}
+
if (optind >= argc) {
help();
}
+
filename = argv[optind++];
- bs = bdrv_new_open(filename, fmt, flags, true);
- if (!bs) {
+ 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;
}
- ret = bdrv_check(bs, &result, fix);
- if (ret == -ENOTSUP) {
- error_report("This image format does not support checks");
- bdrv_delete(bs);
+ bs = bdrv_new_open(filename, fmt, flags, true);
+ if (!bs) {
return 1;
}
- if (result.corruptions_fixed || result.leaks_fixed) {
- printf("The following inconsistencies were found and repaired:\n\n"
- " %d leaked clusters\n"
- " %d corruptions\n\n"
- "Double checking the fixed image now...\n",
- result.leaks_fixed,
- result.corruptions_fixed);
- ret = bdrv_check(bs, &result, 0);
- }
+ check = g_new0(ImageCheck, 1);
+ ret = collect_image_check(bs, check, filename, fmt, fix);
- if (!(result.corruptions || result.leaks || result.check_errors)) {
- printf("No errors were found on the image.\n");
- } else {
- if (result.corruptions) {
- printf("\n%d errors were found on the image.\n"
- "Data may be corrupted, or further writes to the image "
- "may corrupt it.\n",
- result.corruptions);
- }
-
- if (result.leaks) {
- printf("\n%d leaked clusters were found on the image.\n"
- "This means waste of disk space, but no harm to data.\n",
- result.leaks);
- }
+ if (ret == -ENOTSUP) {
+ ret = 1;
+ goto fail;
+ }
- if (result.check_errors) {
- printf("\n%d internal errors have occurred during the check.\n",
- result.check_errors);
- }
+ switch (output_format) {
+ case OFORMAT_HUMAN:
+ dump_human_image_check(check);
+ break;
+ case OFORMAT_JSON:
+ dump_json_image_check(check);
+ break;
}
- if (result.bfi.total_clusters != 0 && result.bfi.allocated_clusters != 0) {
- printf("%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
- result.bfi.allocated_clusters, result.bfi.total_clusters,
- result.bfi.allocated_clusters * 100.0 / result.bfi.total_clusters,
- result.bfi.fragmented_clusters * 100.0 / result.bfi.allocated_clusters);
+ if (ret || check->check_errors) {
+ ret = 1;
+ goto fail;
}
- if (result.highest_offset > 0) {
- printf("Highest offset in use: %" PRId64 "\n", result.highest_offset);
+ if (check->corruptions) {
+ ret = 2;
+ } else if (check->leaks) {
+ ret = 3;
+ } else {
+ ret = 0;
}
+fail:
+ qapi_free_ImageCheck(check);
bdrv_delete(bs);
- if (ret < 0 || result.check_errors) {
- printf("\nAn error has occurred during the check: %s\n"
- "The check is not complete and may have missed error.\n",
- strerror(-ret));
- return 1;
- }
-
- if (result.corruptions) {
- return 2;
- } else if (result.leaks) {
- return 3;
- } else {
- return 0;
- }
+ return ret;
}
static int img_commit(int argc, char **argv)
@@ -1391,16 +1511,6 @@ err:
return NULL;
}
-enum {
- OPTION_OUTPUT = 256,
- OPTION_BACKING_CHAIN = 257,
-};
-
-typedef enum OutputFormat {
- OFORMAT_JSON,
- OFORMAT_HUMAN,
-} OutputFormat;
-
static int img_info(int argc, char **argv)
{
int c;
diff --git a/qemu-img.texi b/qemu-img.texi
index 00fca8d..1a6c9e3 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -84,9 +84,10 @@ lists all snapshots in the given image
Command description:
@table @option
-@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
+@item check [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] @var{filename}
-Perform a consistency check on the disk image @var{filename}.
+Perform a consistency check on the disk image @var{filename}. The command can
+output in the format @var{ofmt} which is either @code{human} or @code{json}.
If @code{-r} is specified, qemu-img tries to repair any inconsistencies found
during the check. @code{-r leaks} repairs only cluster leaks, whereas
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] qemu-img: add json output option to the check command
2012-12-10 17:01 ` [Qemu-devel] [PATCH 2/2] qemu-img: add json output option to the check command Federico Simoncelli
@ 2012-12-13 12:38 ` Kevin Wolf
0 siblings, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2012-12-13 12:38 UTC (permalink / raw)
To: Federico Simoncelli; +Cc: qemu-devel
Am 10.12.2012 18:01, schrieb Federico Simoncelli:
> This option --output=[human|json] make qemu-img check output an human
> or JSON representation at the choice of the user.
>
> Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
> ---
> qapi-schema.json | 38 +++++++++
> qemu-img-cmds.hx | 4 +-
> qemu-img.c | 246 +++++++++++++++++++++++++++++++++++++++---------------
> qemu-img.texi | 5 +-
> 4 files changed, 221 insertions(+), 72 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5dfa052..8877285 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -245,6 +245,44 @@
> '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } }
>
> ##
> +# @ImageCheck:
> +#
> +# Information about a QEMU image file check
> +#
> +# @filename: name of the image file checked
> +#
> +# @format: format of the image file checked
> +#
> +# @check-errors: number of unexpected errors occurred during check
> +#
> +# @highest-offset: #optional highest offset (in bytes) in use by the image
How about adding something like "This field is present if the driver for
the image format supports it" in order to explain the #optional?
> +#
> +# @corruptions: #optional number of corruptions found during the check
> +#
> +# @leaks: #optional number of leaks found during the check
> +#
> +# @corruptions-fixed: #optional number of corruptions fixed during the check
> +#
> +# @leaks-fixed: #optional number of leaks fixed during the check
These four could be clarified by adding "if any"
> +#
> +# @total-clusters: #optional total number of clusters
> +#
> +# @allocated-clusters: #optional total number of allocated clusters
> +#
> +# @fragmented-clusters: #optional total number of fragmented clusters
And here #optional is about the image format again
> +#
> +# Since: 1.4
> +#
> +##
> +
> +{ 'type': 'ImageCheck',
> + 'data': {'filename': 'str', 'format': 'str', 'check-errors': 'int',
> + '*highest-offset': 'int', '*corruptions': 'int', '*leaks': 'int',
> + '*corruptions-fixed': 'int', '*leaks-fixed': 'int',
> + '*total-clusters': 'int', '*allocated-clusters': 'int',
> + '*fragmented-clusters': 'int' } }
> +
> +##
> # @StatusInfo:
> #
> # Information about VCPU run state
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index a181363..259fc14 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -10,9 +10,9 @@ STEXI
> ETEXI
>
> DEF("check", img_check,
> - "check [-f fmt] [-r [leaks | all]] filename")
> + "check [-f fmt] [--output=ofmt] [-r [leaks | all]] filename")
> STEXI
> -@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
> +@item check [-f @var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] @var{filename}
> ETEXI
>
> DEF("create", img_create,
> diff --git a/qemu-img.c b/qemu-img.c
> index 45c1ec1..18ba5c2 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -42,6 +42,16 @@ typedef struct img_cmd_t {
> int (*handler)(int argc, char **argv);
> } img_cmd_t;
>
> +enum {
> + OPTION_OUTPUT = 256,
> + OPTION_BACKING_CHAIN = 257,
> +};
> +
> +typedef enum OutputFormat {
> + OFORMAT_JSON,
> + OFORMAT_HUMAN,
> +} OutputFormat;
> +
> /* Default to cache=writeback as data integrity is not important for qemu-tcg. */
> #define BDRV_O_FLAGS BDRV_O_CACHE_WB
> #define BDRV_DEFAULT_CACHE "writeback"
> @@ -370,6 +380,122 @@ out:
> return 0;
> }
>
> +static void dump_json_image_check(ImageCheck *check)
> +{
> + Error *errp = NULL;
> + QString *str;
> + QmpOutputVisitor *ov = qmp_output_visitor_new();
> + QObject *obj;
> + visit_type_ImageCheck(qmp_output_get_visitor(ov),
> + &check, NULL, &errp);
> + obj = qmp_output_get_qobject(ov);
> + str = qobject_to_json_pretty(obj);
> + assert(str != NULL);
> + printf("%s\n", qstring_get_str(str));
> + qobject_decref(obj);
> + qmp_output_visitor_cleanup(ov);
> + QDECREF(str);
> +}
> +
> +static void dump_human_image_check(ImageCheck *check)
> +{
> + if (check->corruptions_fixed || check->leaks_fixed) {
> + printf("The following inconsistencies were found and repaired:\n\n"
> + " %" PRId64 " leaked clusters\n"
> + " %" PRId64 " corruptions\n\n"
> + "Double checking the fixed image now...\n",
> + check->leaks_fixed,
> + check->corruptions_fixed);
> + }
This message is now printed in the wrong place. It should be after the
first run, but before the second one. I guess we'll have to move it to
collect_image_check for this, and check explicitly for human mode there.
> +
> + if (!(check->corruptions || check->leaks || check->check_errors)) {
> + printf("No errors were found on the image.\n");
> + } else {
> + if (check->corruptions) {
> + printf("\n%" PRId64 " errors were found on the image.\n"
> + "Data may be corrupted, or further writes to the image "
> + "may corrupt it.\n",
> + check->corruptions);
> + }
> +
> + if (check->leaks) {
> + printf("\n%" PRId64 " leaked clusters were found on the image.\n"
> + "This means waste of disk space, but no harm to data.\n",
> + check->leaks);
> + }
> +
> + if (check->check_errors) {
> + printf("\n%" PRId64 " internal errors have occurred during the check.\n",
> + check->check_errors);
> + }
> + }
> +
> + if (check->total_clusters != 0 && check->allocated_clusters != 0) {
> + printf("%" PRId64 "/%" PRId64 "= %0.2f%% allocated, %0.2f%% fragmented\n",
> + check->allocated_clusters, check->total_clusters,
> + check->allocated_clusters * 100.0 / check->total_clusters,
> + check->fragmented_clusters * 100.0 / check->allocated_clusters);
> + }
> +
> + if (check->highest_offset) {
> + printf("Highest offset in use: %" PRId64 "\n", check->highest_offset);
> + }
> +}
> +
> +static int collect_image_check(BlockDriverState *bs,
> + ImageCheck *check,
> + const char *filename,
> + const char *fmt,
> + int fix)
> +{
> + int ret;
> + BdrvCheckResult result;
> +
> + ret = bdrv_check(bs, &result, fix);
> +
> + if (ret < 0) {
> + return ret;
> + }
> +
> + check->filename = g_strdup(filename);
> + check->format = g_strdup(bdrv_get_format_name(bs));
> + check->check_errors = result.check_errors;
> +
> + check->corruptions = result.corruptions;
> + check->has_corruptions = result.corruptions != 0;
> + check->leaks = result.leaks;
> + check->has_leaks = result.leaks != 0;
> + check->corruptions_fixed = result.corruptions_fixed;
> + check->has_corruptions_fixed = result.corruptions != 0;
> + check->leaks_fixed = result.leaks_fixed;
> + check->has_leaks_fixed = result.leaks != 0;
> + check->highest_offset = result.highest_offset;
> + check->has_highest_offset = result.highest_offset != 0;
> +
> + check->total_clusters = result.bfi.total_clusters;
> + check->has_total_clusters = result.bfi.total_clusters != 0;
> + check->allocated_clusters = result.bfi.allocated_clusters;
> + check->has_allocated_clusters = result.bfi.allocated_clusters != 0;
> + check->fragmented_clusters = result.bfi.fragmented_clusters;
> + check->has_fragmented_clusters = result.bfi.fragmented_clusters != 0;
Do the empty lines have any meaning? I think using them to group related
fields together is a good idea, but I can't see the logic in this
specific grouping.
Also, can you align all = consistently to the same column?
> +
> + /* double checking the fixed image */
> + if (result.corruptions_fixed || result.leaks_fixed) {
> + ret = bdrv_check(bs, &result, 0);
> +
> + if (ret < 0) {
> + return ret;
> + }
> +
> + check->corruptions = result.corruptions;
> + check->has_corruptions = result.corruptions != 0;
> + check->leaks = result.leaks;
> + check->has_leaks = result.leaks != 0;
> + }
I think most fields should get the result from the second run, for
example the number of allocated clusters could change when the image was
repaired.
You can leave filename/format and *_fixed where it is, and just move the
initialisation of the other fields to below this if block. Then you also
don't have to duplicate the lines for corruptions/leaks.
Kevin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-12-13 12:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-10 17:01 [Qemu-devel] [PATCHv2 1/2] qemu-img: find the highest offset in use during check Federico Simoncelli
2012-12-10 17:01 ` [Qemu-devel] [PATCH 2/2] qemu-img: add json output option to the check command Federico Simoncelli
2012-12-13 12:38 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).