Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block

[Kevin Wolf]

Am 02.04.2013 um 10:54 hat Wenchao Xia geschrieben:
> 于 2013-4-2 16:09, Kevin Wolf 写道:
> >Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben:
> >>于 2013-3-28 17:54, Kevin Wolf 写道:
> >>>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 <address@hidden>
> >>>>---
> >>>>  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.
> >>>
> >>   My bad, I forgot to set its value, the interface is intend to return
> >>negative error number and errp both on fail.
> >
> >Why do you need two separate error reporting mechanisms? Shouldn't only
> >errp be enough?
> >
> >Kevin
> >
>   Returned value can tell caller what error it is, like -ENOMEDIUM.
> Although it is not used by caller now, but I feel better to have it

No, let's remove it if there is no user. We can always add it back if we
ever need it. I doubt that we will.

Kevin