[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 03/16] allow writing to the backing
From: |
Wen Congyang |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 03/16] allow writing to the backing file |
Date: |
Wed, 9 Sep 2015 17:19:18 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 09/03/2015 12:06 AM, Eric Blake wrote:
> On 09/02/2015 02:51 AM, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <address@hidden>
>> Signed-off-by: zhanghailiang <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>
> Not much description in the commit message. I really want an
> explanation of why this patch is necessary. After all, with
For COLO, we have such backing chain:
secondary disk <-- hidden disk <-- active disk
secondary disk is top BDS(use bacing reference), so it can be opened in
read-write mode.
But hidden disk is read only, and we need to write to hidden disk(backup job
will write data to it).
> '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?
>
We have discussed it before:
http://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg04468.html
Thanks
Wen Congyang