From: Peter Maydell <peter.maydell@linaro.org>
To: Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
QEMU Developers <qemu-devel@nongnu.org>,
Qemu-block <qemu-block@nongnu.org>
Subject: Re: [PULL 11/18] block: Generic file creation fallback
Date: Tue, 25 Feb 2020 10:05:50 +0000 [thread overview]
Message-ID: <CAFEAcA8YFHy9uf5WXP0qwkRkcxgC1ufOYDXQExsV8AVgU5OReQ@mail.gmail.com> (raw)
In-Reply-To: <20200220160710.533297-12-mreitz@redhat.com>
On Thu, 20 Feb 2020 at 16:11, Max Reitz <mreitz@redhat.com> 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 <mreitz@redhat.com>
> Message-Id: <20200122164532.178040-3-mreitz@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
Hi; Coverity thinks there's a memory leak in the error
codepaths in this function (CID 1419884): is it right?
> +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.)
> +
> + 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?
> +
> + 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
next prev parent reply other threads:[~2020-02-25 10:07 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 16:06 [PULL 00/18] Block patches Max Reitz
2020-02-20 16:06 ` [PULL 01/18] docs: improve qcow2 spec about extending image header Max Reitz
2020-02-20 16:06 ` [PULL 02/18] docs: qcow2: introduce compression type feature Max Reitz
2020-02-20 16:06 ` [PULL 03/18] iotests: Remove the superfluous 2nd check for the availability of quorum Max Reitz
2020-02-20 16:06 ` [PULL 04/18] iotests/147: Fix drive parameters Max Reitz
2020-02-20 16:06 ` [PULL 05/18] qapi: Allow getting flat output from 'query-named-block-nodes' Max Reitz
2020-02-20 16:06 ` [PULL 06/18] qemu-img: Add --target-is-zero to convert Max Reitz
2020-02-20 16:36 ` Eric Blake
2020-02-20 20:18 ` David Edmondson
2020-02-20 16:06 ` [PULL 07/18] block: always fill entire LUKS header space with zeros Max Reitz
2020-02-20 16:07 ` [PULL 08/18] block/backup-top: fix flags handling Max Reitz
2020-02-20 16:07 ` [PULL 09/18] iotests/279: Fix for non-qcow2 formats Max Reitz
2020-02-20 16:07 ` [PULL 10/18] block/nbd: Fix hang in .bdrv_close() Max Reitz
2020-02-20 16:07 ` [PULL 11/18] block: Generic file creation fallback Max Reitz
2020-02-25 10:05 ` Peter Maydell [this message]
2020-02-25 10:19 ` Max Reitz
2020-02-20 16:07 ` [PULL 12/18] file-posix: Drop hdev_co_create_opts() Max Reitz
2020-02-20 16:07 ` [PULL 13/18] iscsi: Drop iscsi_co_create_opts() Max Reitz
2020-02-20 16:07 ` [PULL 14/18] iotests: Add test for image creation fallback Max Reitz
2020-02-20 16:07 ` [PULL 15/18] qemu-img: Fix convert -n -B for backing-less targets Max Reitz
2020-02-20 16:07 ` [PULL 16/18] iotests: Test convert -n -B to backing-less target Max Reitz
2020-02-20 16:07 ` [PULL 17/18] block: Fix VM size field width in snapshot dump Max Reitz
2020-02-20 16:07 ` [PULL 18/18] iotests: Test snapshot -l field separation Max Reitz
2020-02-21 14:20 ` [PULL 00/18] Block patches Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFEAcA8YFHy9uf5WXP0qwkRkcxgC1ufOYDXQExsV8AVgU5OReQ@mail.gmail.com \
--to=peter.maydell@linaro.org \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).