Re: [Qemu-devel] [PATCH 08/11] drive-backup: added support for data compression

[Eric Blake]

On 05/31/2016 03:15 AM, Denis V. Lunev wrote:
> From: Pavel Butsykin <address@hidden>
> 
> The idea is simple - backup is "written-once" data. It is written block
> by block and it is large enough. It would be nice to save storage
> space and compress it.
> 
> The patch adds a flag to the qmp/hmp drive-backup command which enables
> block compression. Compression should be implemented in the format driver
> to enable this feature.
> 
> There are some limitations of the format driver to allow compressed writes.
> We can write data only once. Though for backup this is perfectly fine.
> These limitations are maintained by the driver and the error will be
> reported if we are doing something wrong.
> 

> @@ -3309,6 +3315,7 @@ void qmp_drive_backup(const char *device, const char 
> *target,
>                        bool has_mode, enum NewImageMode mode,
>                        bool has_speed, int64_t speed,
>                        bool has_bitmap, const char *bitmap,
> +                      bool has_compress, bool compress,
>                        bool has_on_source_error, BlockdevOnError 
> on_source_error,
>                        bool has_on_target_error, BlockdevOnError 
> on_target_error,
>                        Error **errp)

Might be nice to simplify this signature once my qapi 'box' patches land.


> +++ b/qapi/block-core.json
> @@ -888,6 +888,9 @@
>  #          Must be present if sync is "incremental", must NOT be present
>  #          otherwise. (Since 2.4)
>  #
> +# @compress: #optional the compression data blocks (if the target format
> +#            supports it; default: false).

Missing a '(since 2.7)' notation.

> +++ b/qmp-commands.hx
> @@ -1186,7 +1186,8 @@ EQMP
>      {
>          .name       = "drive-backup",
>          .args_type  = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
> -                      "bitmap:s?,on-source-error:s?,on-target-error:s?",
> +                      "bitmap:s?,compress:b?,"
> +                      "on-source-error:s?,on-target-error:s?",
>          .mhandler.cmd_new = qmp_marshal_drive_backup,
>      },
>  
> @@ -1220,6 +1221,8 @@ Arguments:
>  - "mode": whether and how QEMU should create a new image
>            (NewImageMode, optional, default 'absolute-paths')
>  - "speed": the maximum speed, in bytes per second (json-int, optional)
> +- "compress": the compression data blocks (if the target format supports it).
> +              (json-bool, optional, default false)

Reads awkwardly; maybe "compress": true to compress data, if the target
format supports it. (json-bool, optional, default false)


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature