On Tue, 15 Jan 2013 10:36:22 +0800
Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote:
> > On Mon, 2013-01-14 at 15:09 +0800, Wenchao Xia wrote:
> >> This patch use block layer API to qmp snapshot info on a block
> >> device, then use the same code dumping vm snapshot info, to print
> >> in monitor.
> >>
> >> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com>
> >> ---
> >> Note:
> >> This patch need previous hmp extention patch which enable
> >> info sub command take qdict * as paramter.
> >
> >> diff --git a/savevm.c b/savevm.c
> >> index cabdcb6..cd474e9 100644
> >> --- a/savevm.c
> >> +++ b/savevm.c
> >> @@ -2354,9 +2354,50 @@ static void do_info_snapshots_vm(Monitor *mon)
> >> return;
> >> }
> >>
> >> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
> >> +{
> >> + Error *err = NULL;
> >> + SnapshotInfoList *list;
> >> + BlockDriverState *bs;
> >> +
> >> + /* find the target bs */
> >> + bs = bdrv_find(device);
> >> + if (!bs) {
> >> + monitor_printf(mon, "Device '%s' not found.\n", device);
> >> + return ;
> >> + }
> >> +
> >> + if (!bdrv_can_snapshot(bs)) {
> >> + monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
> >> + return ;
> >> + }
> >> +
> >> + list = bdrv_query_snapshot_infolist(bs, NULL, NULL, &err);
> >> + if (error_is_set(&err)) {
> >> + hmp_handle_error(mon, &err);
> >> + return;
> >> + }
> >> +
> >> + if (list == NULL) {
> >> + monitor_printf(mon, "There is no snapshot available.\n");
> >> + return;
> >> + }
> >> +
> >> + monitor_printf(mon, "Device '%s':\n", device);
> >> + monitor_dump_snapshotinfolist(mon, list);
> >> + qapi_free_SnapshotInfoList(list);
> >> + return;
> >> +}
> >> +
> >> void do_info_snapshots(Monitor *mon, const QDict *qdict)
> >> {
> >> - do_info_snapshots_vm(mon);
> >> + const char *device = qdict_get_try_str(qdict, "device");
> >> + if (!device) {
> >> + do_info_snapshots_vm(mon);
> >> + } else {
> >> + do_info_snapshots_blk(mon, device);
> >> + }
> >> + return;
> >> }
> >>
> >
> > I think that you should move these functions into hmp.c file. This also
> > applies to previous patch and according to this changes you don't have
> > to export hmp_handle_error() function which should be used only in hmp.c
> >
> > Pavel
> >
> It seems there are other "do_info_**" function in other .c files,
> so I suggest a different serial later which move those functions, if
> necessary.
The other functions are probably old hmp code. That is, code that is not
converted to make QMP calls. Generally, new hmp code goes into hmp.c unless
there's a good reason to put them in a different file.
So, please move it to hmp.c.
