| Message ID | 1358147387-8221-12-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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 <xiawenc@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
On Mon, 14 Jan 2013 15:09:47 +0800 Wenchao Xia <xiawenc@linux.vnet.ibm.com> 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 <xiawenc@linux.vnet.ibm.com> > --- > Note: > This patch need previous hmp extention patch which enable > info sub command take qdict * as paramter. > > --- > monitor.c | 6 +++--- > savevm.c | 43 ++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 45 insertions(+), 4 deletions(-) > > diff --git a/monitor.c b/monitor.c > index d186532..cba75a2 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2583,9 +2583,9 @@ static mon_cmd_t info_cmds[] = { > }, > { > .name = "snapshots", > - .args_type = "", > - .params = "", > - .help = "show the currently saved VM snapshots", > + .args_type = "device:B?", > + .params = "[device]", > + .help = "show snapshots of whole vm or a single device", > .mhandler.cmd = do_info_snapshots, > }, > { > 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; > + } We try to restrict hmp.c to use the QMP API only (see Pavel's comment on moving this to hmp.c). If this can't be implemented using qmp_query_snapshots(), then I'd suggest to add this capability to qmp_query_snapshots() or add a new QMP command. > + > + 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; > } > > void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
On Mon, 14 Jan 2013 12:15:37 +0100 Pavel Hrdina <phrdina@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 <xiawenc@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).
> 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 <xiawenc@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.
On Tue, 15 Jan 2013 10:36:22 +0800 Wenchao Xia <xiawenc@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 <xiawenc@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.
δΊ 2013-1-15 19:05, Luiz Capitulino ει: > On Tue, 15 Jan 2013 10:36:22 +0800 > Wenchao Xia <xiawenc@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 <xiawenc@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. > OK, new hmp function will goto hmp.c
diff --git a/monitor.c b/monitor.c index d186532..cba75a2 100644 --- a/monitor.c +++ b/monitor.c @@ -2583,9 +2583,9 @@ static mon_cmd_t info_cmds[] = { }, { .name = "snapshots", - .args_type = "", - .params = "", - .help = "show the currently saved VM snapshots", + .args_type = "device:B?", + .params = "[device]", + .help = "show snapshots of whole vm or a single device", .mhandler.cmd = do_info_snapshots, }, { 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; } void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
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 <xiawenc@linux.vnet.ibm.com> --- Note: This patch need previous hmp extention patch which enable info sub command take qdict * as paramter. --- monitor.c | 6 +++--- savevm.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 4 deletions(-)