From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47718) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL9XI-0002XM-47 for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:54:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UL9XG-0003Se-TT for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:54:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35354) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UL9XG-0003SO-Lm for qemu-devel@nongnu.org; Thu, 28 Mar 2013 05:54:18 -0400 Date: Thu, 28 Mar 2013 10:54:12 +0100 From: Kevin Wolf Message-ID: <20130328095412.GA3077@dhcp-200-207.str.redhat.com> References: <1363961953-13561-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1363961953-13561-12-git-send-email-xiawenc@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1363961953-13561-12-git-send-email-xiawenc@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben: > Now image info will be retrieved as an embbed json object inside > BlockDeviceInfo, backing chain info and all related internal snapshot > info can be got in the enhanced recursive structure of ImageInfo. > > Signed-off-by: Wenchao Xia > --- > block/qapi.c | 39 ++++++++++++++++++++++++++-- > include/block/qapi.h | 4 ++- > qapi-schema.json | 5 +++- > qmp-commands.hx | 67 +++++++++++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 109 insertions(+), 6 deletions(-) > > diff --git a/block/qapi.c b/block/qapi.c > index df73f5b..9051947 100644 > --- a/block/qapi.c > +++ b/block/qapi.c > @@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs, > return 0; > } > > -BlockInfo *bdrv_query_info(BlockDriverState *bs) > +/* return 0 on success, and @p_info will be set only on success. */ > +int bdrv_query_info(BlockDriverState *bs, > + BlockInfo **p_info, > + Error **errp) > { > BlockInfo *info = g_malloc0(sizeof(*info)); > + BlockDriverState *bs0; > + ImageInfo **p_image_info; > + int ret = 0; ret is never changed, so this function always returns 0. I would suggest to drop ret and make the function return type void. > info->device = g_strdup(bs->device_name); > info->type = g_strdup("unknown"); > info->locked = bdrv_dev_is_medium_locked(bs); > @@ -256,8 +262,29 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs) > info->inserted->iops_wr = > bs->io_limits.iops[BLOCK_IO_LIMIT_WRITE]; > } > + > + bs0 = bs; > + p_image_info = &info->inserted->image; > + while (1) { > + if (bdrv_query_image_info(bs0, p_image_info, errp)) { > + goto err; > + } > + if (bs0->drv && bs0->backing_hd) { > + bs0 = bs0->backing_hd; > + (*p_image_info)->has_backing_image = true; > + p_image_info = &((*p_image_info)->backing_image); > + } else { > + break; > + } > + } > } > - return info; > + > + *p_info = info; > + return 0; > + > + err: > + qapi_free_BlockInfo(info); > + return ret; > } > > SnapshotInfoList *qmp_query_snapshots(Error **errp) > @@ -284,11 +311,17 @@ BlockInfoList *qmp_query_block(Error **errp) > > while ((bs = bdrv_next(bs))) { > BlockInfoList *info = g_malloc0(sizeof(*info)); > - info->value = bdrv_query_info(bs); > + if (bdrv_query_info(bs, &info->value, errp)) { > + goto err; > + } Consequently, you've got the error handling wrong here, the if condition is never true. It should look more or less like this (the syntax details may be wrong): Error *local_err; bdrv_query_info(bs, &info->value, &local_err); if (error_is_set(local_err)) { error_propagate(err, local_err); goto err; } Kevin