diff mbox

[4/6] add reuse field

Message ID 1330522650-15357-5-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Feb. 29, 2012, 1:37 p.m. UTC
In some cases it can be useful to use an existing file as the new image
in a snapshot.  Add this capability to blockdev-transaction.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c       |   18 +++++++++++-------
 qapi-schema.json |    3 ++-
 qmp-commands.hx  |    7 +++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

Comments

Eric Blake Feb. 29, 2012, 6:50 p.m. UTC | #1
On 02/29/2012 06:37 AM, Paolo Bonzini wrote:
> In some cases it can be useful to use an existing file as the new image
> in a snapshot.  Add this capability to blockdev-transaction.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  blockdev.c       |   18 +++++++++++-------
>  qapi-schema.json |    3 ++-
>  qmp-commands.hx  |    7 +++++++
>  3 files changed, 20 insertions(+), 8 deletions(-)
> @@ -805,13 +807,15 @@ void qmp_blockdev_transaction(BlockdevActionList *dev_list,
>          }
>  
>          /* create new image w/backing file */
> -        ret = bdrv_img_create(new_image_file, format,
> -                              states->old_bs->filename,
> -                              states->old_bs->drv->format_name,
> -                              NULL, -1, flags);
> -        if (ret) {
> -            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
> -            goto delete_and_fail;
> +        if (do_snapshot) {
> +            ret = bdrv_img_create(new_image_file, format,
> +                                  states->old_bs->filename,
> +                                  states->old_bs->drv->format_name,
> +                                  NULL, -1, flags);
> +            if (ret) {
> +                error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
> +                goto delete_and_fail;
> +            }

Is there any sanity checking that we should be doing if the 'reuse' flag
was provided, or is this a case of 'the user better be giving us the
right information, and it's their fault if things break'?

> +++ b/qapi-schema.json
> @@ -1127,7 +1127,8 @@
>  # @format: #optional the format of the snapshot image, default is 'qcow2'.
>  ##
>  { 'type': 'BlockdevSnapshot',
> -  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
> +  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
> +            '*reuse': 'bool' } }
>  
>  ##
>  # @BlockdevAction
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ee05ec2..6728495 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -704,6 +704,10 @@ the original disks pre-snapshot attempt are used.
>  A list of dictionaries is accepted, that contains the actions to be performed.
>  For snapshots this is the device, the file to use for the new snapshot,
>  and the format.  The default format, if not specified, is qcow2.
> +Image files can be created by QEMU, or it can be created externally.

'files' is plural, 'it' is singular.  Perhaps this wording is better?

Each new snapshot defaults to being created by QEMU (wiping any contents
if the file already exists), but it is also possible to reuse an
externally-created file.
Kevin Wolf March 1, 2012, 10:22 a.m. UTC | #2
Am 29.02.2012 19:50, schrieb Eric Blake:
> On 02/29/2012 06:37 AM, Paolo Bonzini wrote:
>> In some cases it can be useful to use an existing file as the new image
>> in a snapshot.  Add this capability to blockdev-transaction.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  blockdev.c       |   18 +++++++++++-------
>>  qapi-schema.json |    3 ++-
>>  qmp-commands.hx  |    7 +++++++
>>  3 files changed, 20 insertions(+), 8 deletions(-)
>> @@ -805,13 +807,15 @@ void qmp_blockdev_transaction(BlockdevActionList *dev_list,
>>          }
>>  
>>          /* create new image w/backing file */
>> -        ret = bdrv_img_create(new_image_file, format,
>> -                              states->old_bs->filename,
>> -                              states->old_bs->drv->format_name,
>> -                              NULL, -1, flags);
>> -        if (ret) {
>> -            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
>> -            goto delete_and_fail;
>> +        if (do_snapshot) {
>> +            ret = bdrv_img_create(new_image_file, format,
>> +                                  states->old_bs->filename,
>> +                                  states->old_bs->drv->format_name,
>> +                                  NULL, -1, flags);
>> +            if (ret) {
>> +                error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
>> +                goto delete_and_fail;
>> +            }
> 
> Is there any sanity checking that we should be doing if the 'reuse' flag
> was provided, or is this a case of 'the user better be giving us the
> right information, and it's their fault if things break'?

The latter, qemu can't really perform meaningful checks here, especially
once we allow using file descriptors instead of file names.

(Would this command actually provide a hackish way for using FDs with
backing files even without -blockdev? Hm... Better forget it right now,
I never told anyone about this! :-))

>> +++ b/qapi-schema.json
>> @@ -1127,7 +1127,8 @@
>>  # @format: #optional the format of the snapshot image, default is 'qcow2'.
>>  ##
>>  { 'type': 'BlockdevSnapshot',
>> -  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
>> +  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
>> +            '*reuse': 'bool' } }
>>  
>>  ##
>>  # @BlockdevAction
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index ee05ec2..6728495 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -704,6 +704,10 @@ the original disks pre-snapshot attempt are used.
>>  A list of dictionaries is accepted, that contains the actions to be performed.
>>  For snapshots this is the device, the file to use for the new snapshot,
>>  and the format.  The default format, if not specified, is qcow2.
>> +Image files can be created by QEMU, or it can be created externally.
> 
> 'files' is plural, 'it' is singular.  Perhaps this wording is better?
> 
> Each new snapshot defaults to being created by QEMU (wiping any contents
> if the file already exists), but it is also possible to reuse an
> externally-created file.

And let's add a warning that you're on your own if you override qemu's
snapshot creation.

Kevin
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 3c35553..61e20f7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -740,6 +740,7 @@  void qmp_blockdev_transaction(BlockdevActionList *dev_list,
     const char *device;
     const char *format;
     const char *new_image_file = NULL;
+    bool do_snapshot;
 
     QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionStates) snap_bdrv_states;
     QSIMPLEQ_INIT(&snap_bdrv_states);
@@ -759,6 +760,7 @@  void qmp_blockdev_transaction(BlockdevActionList *dev_list,
         case BLOCKDEV_ACTION_KIND_SNAPSHOT:
             device = dev_info->snapshot->device;
             new_image_file = dev_info->snapshot->snapshot_file;
+            do_snapshot = !dev_info->snapshot->has_reuse || !dev_info->snapshot->reuse;
             format = dev_info->snapshot->format;
             assert(!(dev_info->snapshot->has_format && format));
             break;
@@ -805,13 +807,15 @@  void qmp_blockdev_transaction(BlockdevActionList *dev_list,
         }
 
         /* create new image w/backing file */
-        ret = bdrv_img_create(new_image_file, format,
-                              states->old_bs->filename,
-                              states->old_bs->drv->format_name,
-                              NULL, -1, flags);
-        if (ret) {
-            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
-            goto delete_and_fail;
+        if (do_snapshot) {
+            ret = bdrv_img_create(new_image_file, format,
+                                  states->old_bs->filename,
+                                  states->old_bs->drv->format_name,
+                                  NULL, -1, flags);
+            if (ret) {
+                error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+                goto delete_and_fail;
+            }
         }
 
         /* We will manually add the backing_hd field to the bs later */
diff --git a/qapi-schema.json b/qapi-schema.json
index d2fbee3..78df122 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1127,7 +1127,8 @@ 
 # @format: #optional the format of the snapshot image, default is 'qcow2'.
 ##
 { 'type': 'BlockdevSnapshot',
-  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } }
+  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
+            '*reuse': 'bool' } }
 
 ##
 # @BlockdevAction
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ee05ec2..6728495 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -704,6 +704,10 @@  the original disks pre-snapshot attempt are used.
 A list of dictionaries is accepted, that contains the actions to be performed.
 For snapshots this is the device, the file to use for the new snapshot,
 and the format.  The default format, if not specified, is qcow2.
+Image files can be created by QEMU, or it can be created externally.
+In the latter case, the contents of the image must be the same as the
+current contents of the device.  Typically this is achieved by using
+the current image file as the backing file for the image.
 
 Arguments:
 
@@ -715,6 +719,8 @@  actions array:
       - "device": device name to snapshot (json-string)
       - "snapshot-file": name of new image file (json-string)
       - "format": format of new image (json-string, optional)
+      - "reuse": whether QEMU should look for an existing image file
+        (json-bool, optional, default false)
 
 Example:
 
@@ -725,6 +731,7 @@  Example:
                                          "format": "qcow2" } },
          { 'type': 'snapshot, 'data' : { "device": "ide-hd1",
                                          "snapshot-file": "/some/place/my-image2",
+                                         "reuse": true,
                                          "format": "qcow2",
                                          "op": "snapshot" } } ] } }
 <- { "return": {} }