[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Kevin Wolf]

Am 16.12.2010 12:04, schrieb jes.soren...@redhat.com:
 From: Jes Sorensen jes.soren...@redhat.com
 
 The monitor command is:
 snapshot_blkdev device [snapshot-file] [format]
 
 Default format is qcow2. For now snapshots without a snapshot-file, eg
 internal snapshots, are not supported.
 
 Signed-off-by: Jes Sorensen jes.soren...@redhat.com
 ---
  blockdev.c  |   61 
 +++
  blockdev.h  |1 +
  hmp-commands.hx |   19 +
  3 files changed, 81 insertions(+), 0 deletions(-)
 
 diff --git a/blockdev.c b/blockdev.c
 index 3b3b82d..9d6f72c 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -516,6 +516,67 @@ void do_commit(Monitor *mon, const QDict *qdict)
  }
  }
  
 +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
 +{
 +const char *device = qdict_get_str(qdict, device);
 +const char *filename = qdict_get_try_str(qdict, snapshot_file);
 +const char *format = qdict_get_try_str(qdict, format);
 +const char format_qcow2[] = qcow2;
 +BlockDriverState *bs;
 +BlockDriver *drv, *proto_drv;
 +int ret = 0;
 +int flags;
 +
 +bs = bdrv_find(device);
 +if (!bs) {
 +qerror_report(QERR_DEVICE_NOT_FOUND, device);
 +ret = -1;
 +goto out;
 +}
 +
 +if (!format) {
 +format = format_qcow2;

Why introducing format_qcow2 instead of directly using the string
literal here?

 +}
 +
 +drv = bdrv_find_format(format);
 +if (!drv) {
 +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
 +ret = -1;
 +goto out;
 +}
 +
 +proto_drv = bdrv_find_protocol(filename);
 +if (!proto_drv) {
 +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
 +ret = -1;
 +goto out;
 +}
 +
 +ret = bdrv_img_create(filename, format, bs-filename,
 +  bs-drv-format_name, NULL, -1, bs-open_flags);
 +if (ret) {
 +goto out;
 +}
 +
 +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 :(
 + */
 +assert(ret == 0);

bdrv_commit handles this case by setting bs-drv = NULL. After this the
device will fail all requests with -ENOMEDIUM, but at least you don't
lose your VM immediately.

Kevin

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Jes Sorensen]

On 12/16/10 12:45, Kevin Wolf wrote:
 Am 16.12.2010 12:04, schrieb jes.soren...@redhat.com:
 From: Jes Sorensen jes.soren...@redhat.com

 The monitor command is:
 snapshot_blkdev device [snapshot-file] [format]

 Default format is qcow2. For now snapshots without a snapshot-file, eg
 internal snapshots, are not supported.

 Signed-off-by: Jes Sorensen jes.soren...@redhat.com
 ---
  blockdev.c  |   61 
 +++
  blockdev.h  |1 +
  hmp-commands.hx |   19 +
  3 files changed, 81 insertions(+), 0 deletions(-)

 diff --git a/blockdev.c b/blockdev.c
 index 3b3b82d..9d6f72c 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -516,6 +516,67 @@ void do_commit(Monitor *mon, const QDict *qdict)
  }
  }
  
 +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
 +{
 +const char *device = qdict_get_str(qdict, device);
 +const char *filename = qdict_get_try_str(qdict, snapshot_file);
 +const char *format = qdict_get_try_str(qdict, format);
 +const char format_qcow2[] = qcow2;
 +BlockDriverState *bs;
 +BlockDriver *drv, *proto_drv;
 +int ret = 0;
 +int flags;
 +
 +bs = bdrv_find(device);
 +if (!bs) {
 +qerror_report(QERR_DEVICE_NOT_FOUND, device);
 +ret = -1;
 +goto out;
 +}
 +
 +if (!format) {
 +format = format_qcow2;
 
 Why introducing format_qcow2 instead of directly using the string
 literal here?

It should generate the same code - I kinda liked my style better, but
I'll change it.

 +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 :(
 + */
 +assert(ret == 0);
 
 bdrv_commit handles this case by setting bs-drv = NULL. After this the
 device will fail all requests with -ENOMEDIUM, but at least you don't
 lose your VM immediately.

Well if we hit this situation something catastrophic happened. I don't
see how we can continue beyond this point really. The old image was
dropped in the swap and the new one is not accessible ... we're dead :(

Cheers,
Jes

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Kevin Wolf]

Am 13.12.2010 08:32, schrieb jes.soren...@redhat.com:
 From: Jes Sorensen jes.soren...@redhat.com
 
 The monitor command is:
 snapshot_blkdev device [snapshot-file] [format]
 
 Default format is qcow2. For now snapshots without a snapshot-file, eg
 internal snapshots, are not supported.
 
 Signed-off-by: Jes Sorensen jes.soren...@redhat.com
 ---
  blockdev.c  |   61 
 +++
  blockdev.h  |1 +
  hmp-commands.hx |   19 +
  3 files changed, 81 insertions(+), 0 deletions(-)
 
 diff --git a/blockdev.c b/blockdev.c
 index f6ac439..1ea24d7 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -514,6 +514,67 @@ void do_commit(Monitor *mon, const QDict *qdict)
  }
  }
  
 +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
 +{
 +const char *device = qdict_get_str(qdict, device);
 +const char *filename = qdict_get_try_str(qdict, snapshot_file);
 +const char *format = qdict_get_try_str(qdict, format);
 +const char format_qcow2[] = qcow2;
 +BlockDriverState *bs;
 +BlockDriver *drv, *proto_drv;
 +int ret = 0;
 +int flags;
 +
 +bs = bdrv_find(device);
 +if (!bs) {
 +qerror_report(QERR_DEVICE_NOT_FOUND, device);
 +ret = -1;
 +goto out;
 +}
 +
 +if (!format) {
 +format = format_qcow2;
 +}
 +
 +drv = bdrv_find_format(format);
 +if (!drv) {
 +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
 +ret = -1;
 +goto out;
 +}
 +
 +proto_drv = bdrv_find_protocol(filename);
 +if (!proto_drv) {
 +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
 +ret = -1;
 +goto out;
 +}
 +
 +ret = bdrv_img_create(filename, format, bs-filename,
 +  bs-drv-format_name, NULL, -1, bs-open_flags);
 +if (ret) {
 +goto out;
 +}
 +
 +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 :(
 + */
 +assert(ret == 0);
 +out:
 +if (ret) {
 +ret = 1;

I seem to remember that errors are always -1 for monitor commands.

Kevin

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Jes Sorensen]

On 12/15/10 17:55, Kevin Wolf wrote:
 +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
 +{
 +const char *device = qdict_get_str(qdict, device);
 +const char *filename = qdict_get_try_str(qdict, snapshot_file);
 +const char *format = qdict_get_try_str(qdict, format);
 +const char format_qcow2[] = qcow2;
 +BlockDriverState *bs;
 +BlockDriver *drv, *proto_drv;
 +int ret = 0;
 +int flags;
 +
 +bs = bdrv_find(device);
 +if (!bs) {
 +qerror_report(QERR_DEVICE_NOT_FOUND, device);
 +ret = -1;
 +goto out;
 +}
 +
 +if (!format) {
 +format = format_qcow2;
 +}
 +
 +drv = bdrv_find_format(format);
 +if (!drv) {
 +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
 +ret = -1;
 +goto out;
 +}
 +
 +proto_drv = bdrv_find_protocol(filename);
 +if (!proto_drv) {
 +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
 +ret = -1;
 +goto out;
 +}
 +
 +ret = bdrv_img_create(filename, format, bs-filename,
 +  bs-drv-format_name, NULL, -1, bs-open_flags);
 +if (ret) {
 +goto out;
 +}
 +
 +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 :(
 + */
 +assert(ret == 0);
 +out:
 +if (ret) {
 +ret = 1;
 
 I seem to remember that errors are always -1 for monitor commands.

I mapped it after something else, but admitted I cannot remember where -
can someone clarify?

Cheers,
Jes

[Qemu-devel] Re: [PATCH 2/3] Introduce do_snapshot_blkdev() and monitor command to handle it.

[Luiz Capitulino]

On Wed, 15 Dec 2010 17:57:25 +0100
Jes Sorensen jes.soren...@redhat.com wrote:

 On 12/15/10 17:55, Kevin Wolf wrote:
  +int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject 
  **ret_data)
  +{
  +const char *device = qdict_get_str(qdict, device);
  +const char *filename = qdict_get_try_str(qdict, snapshot_file);
  +const char *format = qdict_get_try_str(qdict, format);
  +const char format_qcow2[] = qcow2;
  +BlockDriverState *bs;
  +BlockDriver *drv, *proto_drv;
  +int ret = 0;
  +int flags;
  +
  +bs = bdrv_find(device);
  +if (!bs) {
  +qerror_report(QERR_DEVICE_NOT_FOUND, device);
  +ret = -1;
  +goto out;
  +}
  +
  +if (!format) {
  +format = format_qcow2;
  +}
  +
  +drv = bdrv_find_format(format);
  +if (!drv) {
  +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
  +ret = -1;
  +goto out;
  +}
  +
  +proto_drv = bdrv_find_protocol(filename);
  +if (!proto_drv) {
  +qerror_report(QERR_INVALID_BLOCK_FORMAT, format);
  +ret = -1;
  +goto out;
  +}
  +
  +ret = bdrv_img_create(filename, format, bs-filename,
  +  bs-drv-format_name, NULL, -1, bs-open_flags);
  +if (ret) {
  +goto out;
  +}
  +
  +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 :(
  + */
  +assert(ret == 0);
  +out:
  +if (ret) {
  +ret = 1;
  
  I seem to remember that errors are always -1 for monitor commands.
 
 I mapped it after something else, but admitted I cannot remember where -
 can someone clarify?

-1

 
 Cheers,
 Jes