From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57733) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVQV4-00018U-BG for qemu-devel@nongnu.org; Sat, 20 Sep 2014 15:39:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XVQUy-0001Yh-0p for qemu-devel@nongnu.org; Sat, 20 Sep 2014 15:39:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60933) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVQUx-0001Y1-OM for qemu-devel@nongnu.org; Sat, 20 Sep 2014 15:39:11 -0400 Message-ID: <541DD7CE.5070106@redhat.com> Date: Sat, 20 Sep 2014 21:38:54 +0200 From: Max Reitz MIME-Version: 1.0 References: <1410891148-28849-1-git-send-email-armbru@redhat.com> <1410891148-28849-5-git-send-email-armbru@redhat.com> In-Reply-To: <1410891148-28849-5-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 04/23] block: Connect BlockBackend and DriveInfo List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, benoit.canet@nodalink.com, stefanha@redhat.com On 16.09.2014 20:12, Markus Armbruster wrote: > Make the BlockBackend own the DriveInfo. Change blockdev_init() to > return the BlockBackend instead of the DriveInfo. > > Signed-off-by: Markus Armbruster > --- > block.c | 2 -- > block/block-backend.c | 38 ++++++++++++++++++++++++ > blockdev.c | 73 ++++++++++++++++++++++++----------------------- > include/sysemu/blockdev.h | 4 +++ > 4 files changed, 79 insertions(+), 38 deletions(-) [snip] > diff --git a/blockdev.c b/blockdev.c > index 5da6028..722d083 100644 > --- a/blockdev.c > +++ b/blockdev.c [snip] > @@ -920,19 +922,18 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) > } > > /* Actual block device init: Functionality shared with blockdev-add */ > - dinfo = blockdev_init(filename, bs_opts, &local_err); > + blk = blockdev_init(filename, bs_opts, &local_err); > bs_opts = NULL; > - if (dinfo == NULL) { > - if (local_err) { > - error_report("%s", error_get_pretty(local_err)); > - error_free(local_err); > - } > + if (!blk) { > + error_report("%s", error_get_pretty(local_err)); > + error_free(local_err); > goto fail; Not all of blockdev_init() sets errp on failure. Try "qemu-system-x86_64 -drive format=help" and you'll see a segfault with this patch applied. So either you can make blockdev_init() do things right, which is, set errp for format=help instead of dumping the list to stdout (but I'm not even sure whether that's really correct, because it's not really an error), or you keep the "if" around error_report() and error_free() here. Looks good otherwise, though. Max