subject:"\[Qemu\-devel\] \[PATCH 1\/2\] qemu\-img check\: Distinguish different kinds of errors"

Re: [Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors

2010-07-06 Thread MORITA Kazutaka

At Fri,  2 Jul 2010 19:14:59 +0200,
Kevin Wolf wrote:
 
 People think that their images are corrupted when in fact there are just some
 leaked clusters. Differentiating several error cases should make the messages
 more comprehensible.
 
 Signed-off-by: Kevin Wolf kw...@redhat.com
 ---
  block.c|   10 ++--
  block.h|   10 -
  qemu-img.c |   62 +--
  3 files changed, 63 insertions(+), 19 deletions(-)
 
 diff --git a/block.c b/block.c
 index dd6dd76..b0ceef0 100644
 --- a/block.c
 +++ b/block.c
 @@ -710,15 +710,19 @@ DeviceState *bdrv_get_attached(BlockDriverState *bs)
  /*
   * Run consistency checks on an image
   *
 - * Returns the number of errors or -errno when an internal error occurs
 + * Returns 0 if the check could be completed (it doesn't mean that the image 
 is
 + * free of errors) or -errno when an internal error occured. The results of 
 the
 + * check are stored in res.
   */
 -int bdrv_check(BlockDriverState *bs)
 +int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res)
  {
  if (bs-drv-bdrv_check == NULL) {
  return -ENOTSUP;
  }
  
 -return bs-drv-bdrv_check(bs);
 +memset(res, 0, sizeof(*res));
 +res-corruptions = bs-drv-bdrv_check(bs);
 +return res-corruptions  0 ? res-corruptions : 0;
  }
  
  /* commit COW file into the raw image */
 diff --git a/block.h b/block.h
 index 3d03b3e..c2a7e4c 100644
 --- a/block.h
 +++ b/block.h
 @@ -74,7 +74,6 @@ void bdrv_close(BlockDriverState *bs);
  int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
  void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
  DeviceState *bdrv_get_attached(BlockDriverState *bs);
 -int bdrv_check(BlockDriverState *bs);
  int bdrv_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
  int bdrv_write(BlockDriverState *bs, int64_t sector_num,
 @@ -97,6 +96,15 @@ int bdrv_change_backing_file(BlockDriverState *bs,
  const char *backing_file, const char *backing_fmt);
  void bdrv_register(BlockDriver *bdrv);
  
 +
 +typedef struct BdrvCheckResult {
 +int corruptions;
 +int leaks;
 +int check_errors;
 +} BdrvCheckResult;
 +
 +int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res);
 +
  /* async block I/O */
  typedef struct BlockDriverAIOCB BlockDriverAIOCB;
  typedef void BlockDriverCompletionFunc(void *opaque, int ret);
 diff --git a/qemu-img.c b/qemu-img.c
 index 700af21..1782ac9 100644
 --- a/qemu-img.c
 +++ b/qemu-img.c
 @@ -425,11 +425,20 @@ out:
  return 0;
  }
  
 +/*
 + * Checks an image for consistency. Exit codes:
 + *
 + * 0 - Check completed, image is good
 + * 1 - Check not completed because of internal errors
 + * 2 - Check completed, image is corrupted
 + * 3 - Check completed, image has leaked clusters, but is good otherwise
 + */
  static int img_check(int argc, char **argv)
  {
  int c, ret;
  const char *filename, *fmt;
  BlockDriverState *bs;
 +BdrvCheckResult result;
  
  fmt = NULL;
  for(;;) {
 @@ -453,28 +462,51 @@ static int img_check(int argc, char **argv)
  if (!bs) {
  return 1;
  }
 -ret = bdrv_check(bs);
 -switch(ret) {
 -case 0:
 -printf(No errors were found on the image.\n);
 -break;
 -case -ENOTSUP:
 +ret = bdrv_check(bs, result);
 +
 +if (ret == -ENOTSUP) {
  error(This image format does not support checks);
 -break;
 -default:
 -if (ret  0) {
 -error(An error occurred during the check);
 -} else {
 -printf(%d errors were found on the image.\n, ret);
 +return 1;

Is it okay to call bdrv_delete(bs) before return?  It is necessary for
the sheepdog driver to pass qemu-iotests.

Kazutaka


--- a/qemu-img.c
+++ b/qemu-img.c
@@ -466,6 +466,7 @@ static int img_check(int argc, char **argv)
 
 if (ret == -ENOTSUP) {
 error(This image format does not support checks);
+bdrv_delete(bs);
 return 1;
 }

Re: [Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors

2010-07-06 Thread Kevin Wolf

Am 06.07.2010 12:51, schrieb MORITA Kazutaka:
 At Fri,  2 Jul 2010 19:14:59 +0200,
 Kevin Wolf wrote:

 People think that their images are corrupted when in fact there are just some
 leaked clusters. Differentiating several error cases should make the messages
 more comprehensible.

 Signed-off-by: Kevin Wolf kw...@redhat.com
 ---
  block.c|   10 ++--
  block.h|   10 -
  qemu-img.c |   62 
 +--
  3 files changed, 63 insertions(+), 19 deletions(-)

 @@ -453,28 +462,51 @@ static int img_check(int argc, char **argv)
  if (!bs) {
  return 1;
  }
 -ret = bdrv_check(bs);
 -switch(ret) {
 -case 0:
 -printf(No errors were found on the image.\n);
 -break;
 -case -ENOTSUP:
 +ret = bdrv_check(bs, result);
 +
 +if (ret == -ENOTSUP) {
  error(This image format does not support checks);
 -break;
 -default:
 -if (ret  0) {
 -error(An error occurred during the check);
 -} else {
 -printf(%d errors were found on the image.\n, ret);
 +return 1;
 
 Is it okay to call bdrv_delete(bs) before return?  It is necessary for
 the sheepdog driver to pass qemu-iotests.
 
 Kazutaka

Yes, you're right. Thanks for catching this, I'll send a new version.

Kevin

[Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors

2010-07-02 Thread Kevin Wolf

People think that their images are corrupted when in fact there are just some
leaked clusters. Differentiating several error cases should make the messages
more comprehensible.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block.c|   10 ++--
 block.h|   10 -
 qemu-img.c |   62 +--
 3 files changed, 63 insertions(+), 19 deletions(-)

diff --git a/block.c b/block.c
index dd6dd76..b0ceef0 100644
--- a/block.c
+++ b/block.c
@@ -710,15 +710,19 @@ DeviceState *bdrv_get_attached(BlockDriverState *bs)
 /*
  * Run consistency checks on an image
  *
- * Returns the number of errors or -errno when an internal error occurs
+ * Returns 0 if the check could be completed (it doesn't mean that the image is
+ * free of errors) or -errno when an internal error occured. The results of the
+ * check are stored in res.
  */
-int bdrv_check(BlockDriverState *bs)
+int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res)
 {
 if (bs-drv-bdrv_check == NULL) {
 return -ENOTSUP;
 }
 
-return bs-drv-bdrv_check(bs);
+memset(res, 0, sizeof(*res));
+res-corruptions = bs-drv-bdrv_check(bs);
+return res-corruptions  0 ? res-corruptions : 0;
 }
 
 /* commit COW file into the raw image */
diff --git a/block.h b/block.h
index 3d03b3e..c2a7e4c 100644
--- a/block.h
+++ b/block.h
@@ -74,7 +74,6 @@ void bdrv_close(BlockDriverState *bs);
 int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
 void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
 DeviceState *bdrv_get_attached(BlockDriverState *bs);
-int bdrv_check(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
   uint8_t *buf, int nb_sectors);
 int bdrv_write(BlockDriverState *bs, int64_t sector_num,
@@ -97,6 +96,15 @@ int bdrv_change_backing_file(BlockDriverState *bs,
 const char *backing_file, const char *backing_fmt);
 void bdrv_register(BlockDriver *bdrv);
 
+
+typedef struct BdrvCheckResult {
+int corruptions;
+int leaks;
+int check_errors;
+} BdrvCheckResult;
+
+int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res);
+
 /* async block I/O */
 typedef struct BlockDriverAIOCB BlockDriverAIOCB;
 typedef void BlockDriverCompletionFunc(void *opaque, int ret);
diff --git a/qemu-img.c b/qemu-img.c
index 700af21..1782ac9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -425,11 +425,20 @@ out:
 return 0;
 }
 
+/*
+ * Checks an image for consistency. Exit codes:
+ *
+ * 0 - Check completed, image is good
+ * 1 - Check not completed because of internal errors
+ * 2 - Check completed, image is corrupted
+ * 3 - Check completed, image has leaked clusters, but is good otherwise
+ */
 static int img_check(int argc, char **argv)
 {
 int c, ret;
 const char *filename, *fmt;
 BlockDriverState *bs;
+BdrvCheckResult result;
 
 fmt = NULL;
 for(;;) {
@@ -453,28 +462,51 @@ static int img_check(int argc, char **argv)
 if (!bs) {
 return 1;
 }
-ret = bdrv_check(bs);
-switch(ret) {
-case 0:
-printf(No errors were found on the image.\n);
-break;
-case -ENOTSUP:
+ret = bdrv_check(bs, result);
+
+if (ret == -ENOTSUP) {
 error(This image format does not support checks);
-break;
-default:
-if (ret  0) {
-error(An error occurred during the check);
-} else {
-printf(%d errors were found on the image.\n, ret);
+return 1;
+}
+
+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 (result.check_errors) {
+printf(\n%d internal errors have occurred during the check.\n,
+result.check_errors);
 }
-break;
 }
 
 bdrv_delete(bs);
-if (ret) {
+
+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;
 }
-return 0;
+
+if (result.corruptions) {
+return 2;
+} else if (result.leaks) {
+return 3;
+} else {
+return 0;
+}
 }
 
 static int img_commit(int argc, char **argv)
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors

Re: [Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors

[Qemu-devel] [PATCH 1/2] qemu-img check: Distinguish different kinds of errors

3 matches

Site Navigation

Mail list logo

Footer information