Re: [Qemu-devel] [PATCH 03/16] allow writing to the backing file

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On 09/02/2015 02:51 AM, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Not much description in the commit message.  I really want an
explanation of why this patch is necessary.  After all, with
'block-commit', we were able to turn on read-write mode of backing files
on an as-needed basis, without having to expose that to the end user.
Giving the end user a knob that they must tune feels a bit awkward, and
probably means we don't have the design right.

> ---
>  block.c              | 41 ++++++++++++++++++++++++++++++++++++++++-
>  qapi/block-core.json |  7 ++++++-
>  2 files changed, 46 insertions(+), 2 deletions(-)
> 

> +#define ALLOW_WRITE_BACKING_FILE    "allow-write-backing-file"
> +static QemuOptsList backing_file_opts = {
> +    .name = "backing_file",
> +    .head = QTAILQ_HEAD_INITIALIZER(backing_file_opts.head),
> +    .desc = {
> +        {
> +            .name = ALLOW_WRITE_BACKING_FILE,
> +            .type = QEMU_OPT_BOOL,
> +            .help = "allow write to backing file",

If you do add more justification for why this patch is necessary, then,

s/write/writes/

> +++ b/qapi/block-core.json
> @@ -1408,6 +1408,10 @@
>  # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
>  #                 (default: off)
>  #
> +# @allow-write-backing-file: #optional whether the backing file is opened in
> +#                            read-write mode. It is only for backing file
> +#                            (Since 2.5 default: false)
> +#

The name feels a bit long.

It sounds like it is an error to pass allow-write-backing-file for a
top-level BDS (that is, the BDS associated with a BB).  Meanwhile, the
default for any backing chain BDS is to open it read-only, regardless of
the 'read-only' setting of the parent.  But can we just allow
'read-only':false on a backing BDS to mean that the BDS starts life as
read-write, without having to add a new parameter?

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]