[Qemu-devel] Re: [PATCH v2] Improve error handling in do_snapshot_blkdev()

[Kevin Wolf]

Am 03.03.2011 14:13, schrieb address@hidden:
> 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 |   30 ++++++++++++++++++++++++------
>  1 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 0690cc8..f52fe8f 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,12 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
> QObject **ret_data)
>          goto out;
>      }
>  
> +    strncpy(old_filename, bs->filename, sizeof(old_filename));
> +    old_filename[1023] = '\0';

qemu has pstrcpy() from cutils.c for this.

> +
> +    old_drv = bs->drv;
> +    flags = bs->open_flags;
> +
>      if (!format) {
>          format = "qcow2";
>      }
> @@ -610,7 +617,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 +625,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",

That should probably be a colon in "%s: Attempting..." Also, is the
leading space intentional?

Kevin