On Mon, 14 Jan 2013 12:15:37 +0100
Pavel Hrdina <phrd...@redhat.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
Exactly, I was going to say the same thing. Also note that you'll need
better names for do_info_snapshots_vm() and do_info_snapshots_blk() if
you make them public (or you could move them to hmp.c as well).
