From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtYfC-0005tV-Hx for qemu-devel@nongnu.org; Wed, 26 Nov 2014 04:13:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtYf7-0007Ku-Ee for qemu-devel@nongnu.org; Wed, 26 Nov 2014 04:13:30 -0500 Message-ID: <547599A1.5080100@redhat.com> Date: Wed, 26 Nov 2014 10:13:05 +0100 From: Max Reitz MIME-Version: 1.0 References: <1416924485-13304-1-git-send-email-mreitz@redhat.com> <1416924485-13304-2-git-send-email-mreitz@redhat.com> <87h9xm4e8u.fsf@blackfin.pond.sub.org> In-Reply-To: <87h9xm4e8u.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/12] block: qcow2 driver may not be found List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , Peter Lieven , qemu-devel@nongnu.org, Stefan Hajnoczi , qemu-stable@nongnu.org On 2014-11-26 at 08:23, Markus Armbruster wrote: > Max Reitz writes: > >> Albeit absolutely impossible right now, bdrv_find_format("qcow2") may >> fail. bdrv_append_temp_snapshot() should heed that case. > Impossible because we always compile in bdrv_qcow2. Right now we do, right. >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Max Reitz >> --- >> block.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/block.c b/block.c >> index 866c8b4..b31fb67 100644 >> --- a/block.c >> +++ b/block.c >> @@ -1320,6 +1320,12 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) >> } >> >> bdrv_qcow2 = bdrv_find_format("qcow2"); >> + if (!bdrv_qcow2) { >> + error_setg(errp, "Failed to locate qcow2 driver"); >> + ret = -ENOENT; >> + goto out; >> + } >> + >> opts = qemu_opts_create(bdrv_qcow2->create_opts, NULL, 0, >> &error_abort); >> qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); > This dynamic qcow2 driver lookup business is silly. Compiling without > qcow2 would be a massive loss of functionality, and I wouldn't bet a > nickel on the error paths it would pring to live. True. > Even sillier lookups: "file" in bdrv_find_protocol(), and "raw" in > find_image_format(). > > Statically linking to them would be simpler and more honest. > > Aside: similar silliness exists around QemuOpts. > > Patch looks correct. Well, at least it will silence Coverity... Max