Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation

2011-10-11 Thread Federico Simoncelli 
- Original Message -
 From: Dor Laor dl...@redhat.com
 To: Federico Simoncelli fsimo...@redhat.com
 Cc: qemu-devel@nongnu.org, aba...@redhat.com, Kevin Wolf kw...@redhat.com
 Sent: Friday, October 7, 2011 12:45:06 AM
 Subject: Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to 
 avoid image creation
 
 On 10/03/2011 06:09 PM, Federico Simoncelli wrote:
  Add the new option [-n] for snapshot_blkdev to avoid the image
  creation.
  The file provided as [new-image-file] is considered as already
  initialized
  and will be used after passing a check for the backing file.
 
 Seems ok to me as a way to go around fdget and still have selinux
 gain.
 Worth to get Kevin's view too.

Hi Kevin, any news on this?

 Federico, would you like to ack or extend the design:
 http://wiki.qemu.org/Features/Snapshots

Dor, should I do it now or only after my patch is acked?

-- 
Federico

Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation

2011-10-11 Thread Dor Laor 

On 10/11/2011 01:56 PM, Federico Simoncelli wrote:

- Original Message -

From: Dor Laordl...@redhat.com
To: Federico Simoncellifsimo...@redhat.com
Cc: qemu-devel@nongnu.org, aba...@redhat.com, Kevin Wolfkw...@redhat.com
Sent: Friday, October 7, 2011 12:45:06 AM
Subject: Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid 
image creation

On 10/03/2011 06:09 PM, Federico Simoncelli wrote:

Add the new option [-n] for snapshot_blkdev to avoid the image
creation.
The file provided as [new-image-file] is considered as already
initialized
and will be used after passing a check for the backing file.


Seems ok to me as a way to go around fdget and still have selinux
gain.
Worth to get Kevin's view too.


Hi Kevin, any news on this?


Federico, would you like to ack or extend the design:
http://wiki.qemu.org/Features/Snapshots


Dor, should I do it now or only after my patch is acked?


We're not that strict in the OSS world ;) you can do it before and mark 
as a pending request.

btw: Kevin is back so he will probably drain his queue soon.

Re: [Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation

2011-10-06 Thread Dor Laor 

On 10/03/2011 06:09 PM, Federico Simoncelli wrote:

Add the new option [-n] for snapshot_blkdev to avoid the image creation.
The file provided as [new-image-file] is considered as already initialized
and will be used after passing a check for the backing file.


Seems ok to me as a way to go around fdget and still have selinux gain.
Worth to get Kevin's view too.

Federico, would you like to ack or extend the design:
http://wiki.qemu.org/Features/Snapshots



Signed-off-by: Federico Simoncellifsimo...@redhat.com
---
  blockdev.c  |   54 --
  hmp-commands.hx |7 ---
  qmp-commands.hx |4 ++--
  3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0827bf7..bd46808 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -550,8 +550,53 @@ void do_commit(Monitor *mon, const QDict *qdict)
  }
  }

+static int check_snapshot_file(const char *filename, const char *oldfilename,
+   int flags, BlockDriver *drv)
+{
+BlockDriverState *bs;
+char bak_filename[1024], *abs_filename;
+int ret = 0;
+
+bs = bdrv_new();
+if (!bs) {
+return -1;
+}
+
+ret = bdrv_open(bs, filename, flags, drv);
+if (ret) {
+qerror_report(QERR_OPEN_FILE_FAILED, filename);
+goto err0;
+}
+
+if (bs-backing_file) {
+path_combine(bak_filename, sizeof(bak_filename),
+ filename, bs-backing_file);
+
+abs_filename = realpath(bak_filename, NULL);
+if (!abs_filename) {
+ret = -1;
+goto err1;
+}
+
+if (strcmp(abs_filename, oldfilename)) {
+qerror_report(QERR_OPEN_FILE_FAILED, filename);
+ret = -1;
+}
+
+free(abs_filename);
+}
+
+err1:
+bdrv_close(bs);
+
+err0:
+bdrv_delete(bs);
+return ret;
+}
+
  int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
  {
+const int nocreate = qdict_get_try_bool(qdict, nocreate, 0);
  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);
@@ -597,8 +642,13 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
  goto out;
  }

-ret = bdrv_img_create(filename, format, bs-filename,
-  bs-drv-format_name, NULL, -1, flags);
+if (nocreate) {
+ret = check_snapshot_file(filename, bs-filename, flags, drv);
+} else {
+ret = bdrv_img_create(filename, format, bs-filename,
+  bs-drv-format_name, NULL, -1, flags);
+}
+
  if (ret) {
  goto out;
  }
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..eb9fcd4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -840,11 +840,12 @@ ETEXI

  {
  .name   = snapshot_blkdev,
-.args_type  = device:B,snapshot-file:s?,format:s?,
-.params = device [new-image-file] [format],
+.args_type  = nocreate:-n,device:B,snapshot-file:s?,format:s?,
+.params = [-n] device [new-image-file] [format],
  .help   = initiates a live snapshot\n\t\t\t
of device. If a new image file is specified, 
the\n\t\t\t
-  new image file will become the new root image.\n\t\t\t
+  new image file will be created (unless -n is\n\t\t\t
+  specified) and will become the new root image.\n\t\t\t
If format is specified, the snapshot file will\n\t\t\t
be created in that format. Otherwise the\n\t\t\t
snapshot will be internal! (currently unsupported),
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d83bce5..7af36d8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -695,8 +695,8 @@ EQMP

  {
  .name   = blockdev-snapshot-sync,
-.args_type  = device:B,snapshot-file:s?,format:s?,
-.params = device [new-image-file] [format],
+.args_type  = nocreate:-n,device:B,snapshot-file:s?,format:s?,
+.params = [-n] device [new-image-file] [format],
  .user_print = monitor_user_noop,
  .mhandler.cmd_new = do_snapshot_blkdev,
  },

[Qemu-devel] [PATCH] qemu: new option for snapshot_blkdev to avoid image creation

2011-10-03 Thread Federico Simoncelli 
Add the new option [-n] for snapshot_blkdev to avoid the image creation.
The file provided as [new-image-file] is considered as already initialized
and will be used after passing a check for the backing file.

Signed-off-by: Federico Simoncelli fsimo...@redhat.com
---
 blockdev.c  |   54 --
 hmp-commands.hx |7 ---
 qmp-commands.hx |4 ++--
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0827bf7..bd46808 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -550,8 +550,53 @@ void do_commit(Monitor *mon, const QDict *qdict)
 }
 }
 
+static int check_snapshot_file(const char *filename, const char *oldfilename,
+   int flags, BlockDriver *drv)
+{
+BlockDriverState *bs;
+char bak_filename[1024], *abs_filename;
+int ret = 0;
+
+bs = bdrv_new();
+if (!bs) {
+return -1;
+}
+
+ret = bdrv_open(bs, filename, flags, drv);
+if (ret) {
+qerror_report(QERR_OPEN_FILE_FAILED, filename);
+goto err0;
+}
+
+if (bs-backing_file) {
+path_combine(bak_filename, sizeof(bak_filename),
+ filename, bs-backing_file);
+
+abs_filename = realpath(bak_filename, NULL);
+if (!abs_filename) {
+ret = -1;
+goto err1;
+}
+
+if (strcmp(abs_filename, oldfilename)) {
+qerror_report(QERR_OPEN_FILE_FAILED, filename);
+ret = -1;
+}
+
+free(abs_filename);
+}
+
+err1:
+bdrv_close(bs);
+
+err0:
+bdrv_delete(bs);
+return ret;
+}
+
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
+const int nocreate = qdict_get_try_bool(qdict, nocreate, 0);
 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);
@@ -597,8 +642,13 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 goto out;
 }
 
-ret = bdrv_img_create(filename, format, bs-filename,
-  bs-drv-format_name, NULL, -1, flags);
+if (nocreate) {
+ret = check_snapshot_file(filename, bs-filename, flags, drv);
+} else {
+ret = bdrv_img_create(filename, format, bs-filename,
+  bs-drv-format_name, NULL, -1, flags);
+}
+
 if (ret) {
 goto out;
 }
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..eb9fcd4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -840,11 +840,12 @@ ETEXI
 
 {
 .name   = snapshot_blkdev,
-.args_type  = device:B,snapshot-file:s?,format:s?,
-.params = device [new-image-file] [format],
+.args_type  = nocreate:-n,device:B,snapshot-file:s?,format:s?,
+.params = [-n] device [new-image-file] [format],
 .help   = initiates a live snapshot\n\t\t\t
   of device. If a new image file is specified, 
the\n\t\t\t
-  new image file will become the new root image.\n\t\t\t
+  new image file will be created (unless -n is\n\t\t\t
+  specified) and will become the new root image.\n\t\t\t
   If format is specified, the snapshot file will\n\t\t\t
   be created in that format. Otherwise the\n\t\t\t
   snapshot will be internal! (currently unsupported),
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d83bce5..7af36d8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -695,8 +695,8 @@ EQMP
 
 {
 .name   = blockdev-snapshot-sync,
-.args_type  = device:B,snapshot-file:s?,format:s?,
-.params = device [new-image-file] [format],
+.args_type  = nocreate:-n,device:B,snapshot-file:s?,format:s?,
+.params = [-n] device [new-image-file] [format],
 .user_print = monitor_user_noop,
 .mhandler.cmd_new = do_snapshot_blkdev,
 },
-- 
1.7.1