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 <we...@cn.fujitsu.com>
>> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com>
>> Signed-off-by: Gonglei <arei.gong...@huawei.com>
>
> 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
