From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrctQ-0007sT-32 for qemu-devel@nongnu.org; Fri, 13 Dec 2013 19:15:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VrctJ-0006IM-VS for qemu-devel@nongnu.org; Fri, 13 Dec 2013 19:15:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47974) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrctJ-0006I4-Ld for qemu-devel@nongnu.org; Fri, 13 Dec 2013 19:15:33 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rBE0FVcx017007 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 13 Dec 2013 19:15:32 -0500 Message-ID: <52ABA355.9070305@redhat.com> Date: Sat, 14 Dec 2013 01:16:21 +0100 From: Max Reitz MIME-Version: 1.0 References: <1386954633-28905-1-git-send-email-mreitz@redhat.com> <1386954633-28905-13-git-send-email-mreitz@redhat.com> <20131213201958.GZ3916@dhcp-200-207.str.redhat.com> In-Reply-To: <20131213201958.GZ3916@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 12/22] block: Allow recursive "file"s List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi On 13.12.2013 21:19, Kevin Wolf wrote: > Am 13.12.2013 um 18:10 hat Max Reitz geschrieben: >> It should be possible to use a format as a driver for a file which in >> turn requires another file, i.e., nesting file formats. >> >> Signed-off-by: Max Reitz > Hm, does this do what I think it does? > > $ ./qemu-img convert -O qcow2 /home/kwolf/images/hd.img /tmp/hd.qcow2 > $ ./qemu-img convert -f raw -O qcow2 /tmp/hd.qcow2 /tmp/hd.qcow2.qcow2 > $ x86_64-softmmu/qemu-system-x86_64 -drive driver=qcow2,file.driver=qcow2,file.file.driver=file,file.file.filename=/tmp/hd.qcow2.qcow2 > > I can't decide whether this is awesomeness or insanity, but in any case > it works with this patch. :-) > > Worth a qemu-iotests case, I think. > >> block.c | 17 ++++++++++++----- >> 1 file changed, 12 insertions(+), 5 deletions(-) >> >> diff --git a/block.c b/block.c >> index 9659eb5..9222669 100644 >> --- a/block.c >> +++ b/block.c >> @@ -948,14 +948,19 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, >> goto fail; >> } >> >> - ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err); >> + if (!drv->bdrv_file_open) { >> + ret = bdrv_open(bs, filename, options, flags, drv, &local_err); >> + options = NULL; >> + } else { >> + ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err); >> + } >> if (ret < 0) { >> error_propagate(errp, local_err); >> goto fail; >> } >> >> /* Check if any unknown options were used */ >> - if (qdict_size(options) != 0) { >> + if (options && (qdict_size(options) != 0)) { >> const QDictEntry *entry = qdict_first(options); >> error_setg(errp, "Block protocol '%s' doesn't support the option '%s'", >> drv->format_name, entry->key); >> @@ -970,10 +975,12 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, >> >> fail: >> QDECREF(options); >> - if (!bs->drv) { >> - QDECREF(bs->options); >> + if (bs) { >> + if (!bs->drv) { >> + QDECREF(bs->options); >> + } >> + bdrv_unref(bs); >> } >> - bdrv_unref(bs); >> return ret; >> } > Not sure why this hunk is needed, but anyway: Ah, I think I know. It is there because of patch 9. As stated in the cover letter, additionally to the double QDECREF on options, I noticed a possible NULL dereference of bs on error in the "if (reference)" path and fixed it in this version. In v3, it was addressed by this hunk, which somehow got into patch 12 rather than 9. I'll remove it, now that it's unnecessary. Max