Re: [Qemu-devel] [PATCH COLO v4 02/15] allow writing to the backing file

[Wen Congyang]

On 05/14/2015 03:16 PM, Gonglei wrote:
> On 2015/5/8 17:15, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <address@hidden>
>> Signed-off-by: zhanghailiang <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
>>  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;
>>
> 
> 
> .
>