[COLO,v4,02/15] allow writing to the backing file

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>
---
 block.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

On 2015/5/8 17:15, 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>
> ---
>  block.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 49d472f..8fa4f05 100644
> --- a/block.c
> +++ b/block.c
> @@ -1051,6 +1051,20 @@ out:
>      bdrv_refresh_limits(bs, NULL);
>  }
>  
> +#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",
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  /*
>   * Opens the backing file for a BlockDriverState if not yet open
>   *
> @@ -1065,6 +1079,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>      int ret = 0;
>      BlockDriverState *backing_hd;
>      Error *local_err = NULL;
> +    QemuOpts *opts = NULL;
> +    int flags;
>  
>      if (bs->backing_hd != NULL) {
>          QDECREF(options);
> @@ -1077,6 +1093,19 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>      }
>  
>      bs->open_flags &= ~BDRV_O_NO_BACKING;
> +    flags = bdrv_backing_flags(bs->open_flags);
> +
> +    opts = qemu_opts_create(&backing_file_opts, NULL, 0, &error_abort);
> +    qemu_opts_absorb_qdict(opts, options, &local_err);
> +    if (local_err) {
> +        ret = -EINVAL;
> +        goto free_exit;

Oh, This will leak memory which  local variable local_err pointed.
Besides, do we need to propagate the error info?

Regards,
-Gonglei
> +    }
> +    if (qemu_opt_get_bool(opts, ALLOW_WRITE_BACKING_FILE, false)) {
> +        flags |= BDRV_O_RDWR;
> +    }
> +    qemu_opts_del(opts);
> +
>      if (qdict_haskey(options, "file.filename")) {
>          backing_filename[0] = '\0';
>      } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
> @@ -1109,7 +1138,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>      assert(bs->backing_hd == NULL);
>      ret = bdrv_open(&backing_hd,
>                      *backing_filename ? backing_filename : NULL, NULL, options,
> -                    bdrv_backing_flags(bs->open_flags), NULL, &local_err);
> +                    flags, NULL, &local_err);
>      if (ret < 0) {
>          bdrv_unref(backing_hd);
>          backing_hd = NULL;
>

On 05/14/2015 03:16 PM, Gonglei wrote:
> On 2015/5/8 17:15, 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>
>> ---
>>  block.c | 31 ++++++++++++++++++++++++++++++-
>>  1 file changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/block.c b/block.c
>> index 49d472f..8fa4f05 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1051,6 +1051,20 @@ out:
>>      bdrv_refresh_limits(bs, NULL);
>>  }
>>  
>> +#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",
>> +        },
>> +        { /* end of list */ }
>> +    },
>> +};
>> +
>>  /*
>>   * Opens the backing file for a BlockDriverState if not yet open
>>   *
>> @@ -1065,6 +1079,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>>      int ret = 0;
>>      BlockDriverState *backing_hd;
>>      Error *local_err = NULL;
>> +    QemuOpts *opts = NULL;
>> +    int flags;
>>  
>>      if (bs->backing_hd != NULL) {
>>          QDECREF(options);
>> @@ -1077,6 +1093,19 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>>      }
>>  
>>      bs->open_flags &= ~BDRV_O_NO_BACKING;
>> +    flags = bdrv_backing_flags(bs->open_flags);
>> +
>> +    opts = qemu_opts_create(&backing_file_opts, NULL, 0, &error_abort);
>> +    qemu_opts_absorb_qdict(opts, options, &local_err);
>> +    if (local_err) {
>> +        ret = -EINVAL;
>> +        goto free_exit;
> 
> Oh, This will leak memory which  local variable local_err pointed.
> Besides, do we need to propagate the error info?

Yes, we need to propaget the error info. I also forget to free options.
Will fix it in the next version.

Thanks
Wen Congyang

> 
> Regards,
> -Gonglei
>> +    }
>> +    if (qemu_opt_get_bool(opts, ALLOW_WRITE_BACKING_FILE, false)) {
>> +        flags |= BDRV_O_RDWR;
>> +    }
>> +    qemu_opts_del(opts);
>> +
>>      if (qdict_haskey(options, "file.filename")) {
>>          backing_filename[0] = '\0';
>>      } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
>> @@ -1109,7 +1138,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>>      assert(bs->backing_hd == NULL);
>>      ret = bdrv_open(&backing_hd,
>>                      *backing_filename ? backing_filename : NULL, NULL, options,
>> -                    bdrv_backing_flags(bs->open_flags), NULL, &local_err);
>> +                    flags, NULL, &local_err);
>>      if (ret < 0) {
>>          bdrv_unref(backing_hd);
>>          backing_hd = NULL;
>>
> 
> 
> .
>