qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3] Improve error handling in do_snapshot_blkdev


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH v3] Improve error handling in do_snapshot_blkdev()
Date: Mon, 07 Mar 2011 10:34:55 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8

On 03/07/2011 09:27 AM, address@hidden wrote:
From: Jes Sorensen<address@hidden>

In case we cannot open the newly created snapshot image, try to fall
back to the original image file and continue running on that, which
should prevent the guest from aborting.

This is a corner case which can happen if the admin by mistake
specifies the snapshot file on a virtual file system which does not
support O_DIRECT. bdrv_create() does not use O_DIRECT, but the
following open in bdrv_open() does and will then fail.

Signed-off-by: Jes Sorensen<address@hidden>
---
  blockdev.c |   29 +++++++++++++++++++++++------
  1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0690cc8..d52eef0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -574,9 +574,10 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
      const char *filename = qdict_get_try_str(qdict, "snapshot_file");
      const char *format = qdict_get_try_str(qdict, "format");
      BlockDriverState *bs;
-    BlockDriver *drv, *proto_drv;
+    BlockDriver *drv, *old_drv, *proto_drv;
      int ret = 0;
      int flags;
+    char old_filename[1024];

      if (!filename) {
          qerror_report(QERR_MISSING_PARAMETER, "snapshot_file");
@@ -591,6 +592,11 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
          goto out;
      }

+    pstrcpy(old_filename, sizeof(old_filename), bs->filename);
+
+    old_drv = bs->drv;
+    flags = bs->open_flags;
+
      if (!format) {
          format = "qcow2";
      }
@@ -610,7 +616,7 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
      }

      ret = bdrv_img_create(filename, format, bs->filename,
-                          bs->drv->format_name, NULL, -1, bs->open_flags);
+                          bs->drv->format_name, NULL, -1, flags);
      if (ret) {
          goto out;
      }
@@ -618,15 +624,26 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
      qemu_aio_flush();
      bdrv_flush(bs);

-    flags = bs->open_flags;
      bdrv_close(bs);
      ret = bdrv_open(bs, filename, flags, drv);
      /*
-     * If reopening the image file we just created fails, we really
-     * are in trouble :(
+     * If reopening the image file we just created fails, fall back
+     * and try to re-open the original image. If that fails too, we
+     * are in serious trouble.
       */
      if (ret != 0) {
-        abort();
+        qerror_report(QERR_OPEN_FILE_FAILED, filename);
+        error_printf("do_snapshot_blkdev(): Unable to open newly created "
+                     "snapshot file: \n");
+        error_printf("%s. Attempting to revert to original image: %s\n",
+                     filename, old_filename);

You can't combine qerror_report with continued action. qerror_report() should be a terminal action. You also shouldn't combine error_printf() with qerror_report().

You should restore the original image file before doing qerror_report() and just drop the error_printf()s as it's all redundant information.

Regards,

Anthony Liguori

+        ret = bdrv_open(bs, old_filename, flags, old_drv);
+        if (ret != 0) {
+            error_printf("do_snapshot_blkdev(): Unable to re-open "
+                         "original image - aborting!\n");
+            qerror_report(QERR_OPEN_FILE_FAILED, old_filename);
+            abort();
+        }
      }
  out:
      if (ret) {




reply via email to

[Prev in Thread] Current Thread [Next in Thread]