[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 11/18] block: Generic file creation fallback
From: |
Max Reitz |
Subject: |
Re: [PULL 11/18] block: Generic file creation fallback |
Date: |
Tue, 25 Feb 2020 11:19:16 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 25.02.20 11:05, Peter Maydell wrote:
> On Thu, 20 Feb 2020 at 16:11, Max Reitz <address@hidden> wrote:
>>
>> If a protocol driver does not support image creation, we can see whether
>> maybe the file exists already. If so, just truncating it will be
>> sufficient.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> Message-Id: <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
>
> Hi; Coverity thinks there's a memory leak in the error
> codepaths in this function (CID 1419884): is it right?
Yes, it is, I’ll write a patch.
>> +static int bdrv_create_file_fallback(const char *filename, BlockDriver *drv,
>> + QemuOpts *opts, Error **errp)
>> +{
>> + BlockBackend *blk;
>> + QDict *options = qdict_new();
>
> We create the QDict here...
>
>> + int64_t size = 0;
>> + char *buf = NULL;
>> + PreallocMode prealloc;
>> Error *local_err = NULL;
>> int ret;
>>
>> + size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
>> + buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
>> + prealloc = qapi_enum_parse(&PreallocMode_lookup, buf,
>> + PREALLOC_MODE_OFF, &local_err);
>> + g_free(buf);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return -EINVAL;
>
> ...but here and in other error return paths we don't
> free it (I think that might need a qobject_unref() but
> am not sure).
>
>> + }
>> +
>> + if (prealloc != PREALLOC_MODE_OFF) {
>> + error_setg(errp, "Unsupported preallocation mode '%s'",
>> + PreallocMode_str(prealloc));
>> + return -ENOTSUP;
>> + }
>
> (You could probably postpone qdict_new() to here to avoid
> having to change the error handling paths above this point, but
> you still need to deal with the error path for blk_new_open failing.)
Indeed. Or maybe put the unref() under the out label and set @options
to NULL after it’s captured by @blk. I’ll see.
>> +
>> + qdict_put_str(options, "driver", drv->format_name);
>> +
>> + blk = blk_new_open(filename, NULL, options,
>> + BDRV_O_RDWR | BDRV_O_RESIZE, errp);
>> + if (!blk) {
>> + error_prepend(errp, "Protocol driver '%s' does not support image "
>> + "creation, and opening the image failed: ",
>> + drv->format_name);
>> + return -EINVAL;
>> + }
>
> I guess for error-paths after blk_new_open() succeeds
> that the blk object owns the options dictionary and
> will deal with unreffing it for us?
Yes.
Max
>> +
>> + size = create_file_fallback_truncate(blk, size, errp);
>> + if (size < 0) {
>> + ret = size;
>> + goto out;
>> + }
>> +
>> + ret = create_file_fallback_zero_first_sector(blk, size, errp);
>> + if (ret < 0) {
>> + goto out;
>> + }
>> +
>> + ret = 0;
>> +out:
>> + blk_unref(blk);
>> + return ret;
>> +}
>
> thanks
> -- PMM
>
signature.asc
Description: OpenPGP digital signature
- [PULL 05/18] qapi: Allow getting flat output from 'query-named-block-nodes', (continued)
- [PULL 05/18] qapi: Allow getting flat output from 'query-named-block-nodes', Max Reitz, 2020/02/20
- [PULL 06/18] qemu-img: Add --target-is-zero to convert, Max Reitz, 2020/02/20
- [PULL 07/18] block: always fill entire LUKS header space with zeros, Max Reitz, 2020/02/20
- [PULL 08/18] block/backup-top: fix flags handling, Max Reitz, 2020/02/20
- [PULL 09/18] iotests/279: Fix for non-qcow2 formats, Max Reitz, 2020/02/20
- [PULL 10/18] block/nbd: Fix hang in .bdrv_close(), Max Reitz, 2020/02/20
- [PULL 11/18] block: Generic file creation fallback, Max Reitz, 2020/02/20
- [PULL 12/18] file-posix: Drop hdev_co_create_opts(), Max Reitz, 2020/02/20
- [PULL 13/18] iscsi: Drop iscsi_co_create_opts(), Max Reitz, 2020/02/20
- [PULL 14/18] iotests: Add test for image creation fallback, Max Reitz, 2020/02/20
- [PULL 15/18] qemu-img: Fix convert -n -B for backing-less targets, Max Reitz, 2020/02/20
- [PULL 16/18] iotests: Test convert -n -B to backing-less target, Max Reitz, 2020/02/20
- [PULL 17/18] block: Fix VM size field width in snapshot dump, Max Reitz, 2020/02/20
- [PULL 18/18] iotests: Test snapshot -l field separation, Max Reitz, 2020/02/20
- Re: [PULL 00/18] Block patches, Peter Maydell, 2020/02/21