| Message ID | 1355725509-5429-7-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Dec 17, 2012 at 02:25:09PM +0800, Wenchao Xia wrote: > @@ -983,17 +983,22 @@ ETEXI > > { > .name = "snapshot_blkdev", > - .args_type = "reuse:-n,device:B,snapshot-file:s?,format:s?", > - .params = "[-n] device [new-image-file] [format]", > - .help = "initiates a live snapshot\n\t\t\t" > - "of device. If a new image file is specified, the\n\t\t\t" > - "new image file will become the new root image.\n\t\t\t" > - "If format is specified, the snapshot file will\n\t\t\t" > - "be created in that format. Otherwise the\n\t\t\t" > - "snapshot will be internal! (currently unsupported).\n\t\t\t" > - "The default format is qcow2. The -n flag requests QEMU\n\t\t\t" > - "to reuse the image found in new-image-file, instead of\n\t\t\t" > - "recreating it from scratch.", > + .args_type = "internal:-i,reuse:-n,device:B,snapshot-file:s?,format:s?", > + .params = "[-i] [-n] device [new-image-file] [format]", Please rename snapshot-file and new-image-file because it is now either the external snapshot filename or the internal snapshot name - it's not always a file! > + .help = "initiates a live snapshot of device.\n\t\t\t" > + " The -i flag requests QEMU to create internal snapshot\n\t\t\t" > + "instead of external one.\n\t\t\t" > + " The -n flag requests QEMU to use existing snapshot\n\t\t\t" > + "instead of creating new snapshot, which would fails if\n\t\t\t" > + "snapshot does not exist ahead.\n\t\t\t" "which fails if the snapshot does not exist already" > + " new-image-file is the snapshot's name, in external case\n\t\t\t" > + "it is the new image's name which will become the new root\n\t\t\t" > + "image and must be specified, in internal case it is the\n\t\t\t" > + "record's name and if not specified QEMU will create\n\t\t\t" > + "internal snapshot with name generated according to time.\n\t\t\t" > + " format is only valid in external case, which is the new\n\t\t\t" > + "snapshot image's format. If not sepcified default format\n\t\t\t" > + "qcow2 will be used.", "If not specified, the default format is qcow2." > .mhandler.cmd = hmp_snapshot_blkdev, > }, > > @@ -1004,6 +1009,25 @@ Snapshot device, using snapshot file as target if provided > ETEXI > > { > + .name = "snapshot_delete_blkdev", Internal snapshots can already be deleted with delvm but there is no existing command for external snapshots. > + .args_type = "internal:-i,device:B,snapshot-file:s", > + .params = "[-i] device new-image-file", > + .help = "delete a snapshot synchronous.\n\t\t\t" "synchronous" is implied for HMP commands, they are all like this so I don't think it's necessary to mention it. > + " The -i flag requests QEMU to delete internal snapshot\n\t\t\t" > + "instead of external one.\n\t\t\t" > + " new-image-file is the snapshot's name, in external case\n\t\t\t" > + "it is the image's name which is not supported now.\n\t\t\t" I'm not sure how useful this interface is. If we have no implementation for deleting external snapshots and libvirt already uses delvm, then I'd prefer you drop this command from the patch series. Later on, when there is code to implement external snapshot deletion it can be added. Then there is no risk that this command design doesn't work and we have to change it again. (Remember libvirt already uses delvm so adding snapshot_delete_blkdev without external snapshot deletion just adds code churn.) > @@ -1486,8 +1510,8 @@ ETEXI > > { > .name = "info", > - .args_type = "item:s?", > - .params = "[subcommand]", > + .args_type = "item:s?,params:s?", > + .params = "[subcommand] [params]", > .help = "show various information about the system state", > .mhandler.cmd = do_info, > }, What does this change do? > diff --git a/hmp.c b/hmp.c > index 180ba2b..f247f51 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -806,20 +806,40 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) > const char *filename = qdict_get_try_str(qdict, "snapshot-file"); > const char *format = qdict_get_try_str(qdict, "format"); > int reuse = qdict_get_try_bool(qdict, "reuse", 0); > + int internal = qdict_get_try_bool(qdict, "internal", 0); > enum NewImageMode mode; > + enum SnapshotType type; > Error *errp = NULL; > > - if (!filename) { > - /* In the future, if 'snapshot-file' is not specified, the snapshot > - will be taken internally. Today it's actually required. */ > + if ((!internal) && (!filename)) { > + /* in external case filename must be set, should we generate > + it automatically? */ Picking a filename would mainly be useful to humans. libvirt and other management tools will want full control anyway. So the question is: is there a naming policy that will be useful to most human users? If yes, please implement it. If no, please drop this comment. > diff --git a/monitor.c b/monitor.c > index c0e32d6..81de470 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -124,11 +124,14 @@ typedef struct mon_cmd_t { > void (*user_print)(Monitor *mon, const QObject *data); > union { > void (*info)(Monitor *mon); > + void (*info_qdict)(Monitor *mon, const QDict *qdict); > void (*cmd)(Monitor *mon, const QDict *qdict); > - int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data); > + int (*cmd_new)(Monitor *mon, const QDict *params, > + QObject **ret_data); > int (*cmd_async)(Monitor *mon, const QDict *params, > MonitorCompletion *cb, void *opaque); > } mhandler; > + int info_cmd_need_qdict; > int flags; The union discriminator is the flags field (e.g. MONITOR_CMD_ASYNC). Please follow that style. (If you use a boolean variable, please use the bool type instead of int.) > } mon_cmd_t; > > @@ -824,7 +827,11 @@ static void do_info(Monitor *mon, const QDict *qdict) > goto help; > } > > - cmd->mhandler.info(mon); > + if (cmd->info_cmd_need_qdict) { > + cmd->mhandler.info_qdict(mon, qdict); > + } else { > + cmd->mhandler.info(mon); > + } > return; > > help: The generic monitor changes need to go in a separate commit. > @@ -2605,10 +2612,12 @@ static mon_cmd_t info_cmds[] = { > }, > { > .name = "snapshots", > - .args_type = "", > - .params = "", > - .help = "show the currently saved VM snapshots", > - .mhandler.info = do_info_snapshots, > + .args_type = "device:B?", > + .params = "[device]", > + .help = "show the currently saved VM snapshots or snapshots on " > + "a single device.", > + .mhandler.info_qdict = do_info_snapshots, > + .info_cmd_need_qdict = 1, > }, > { > .name = "status", This change to the info snapshots command needs to go in a separate commit. > diff --git a/savevm.c b/savevm.c > index c027529..5982aa9 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -2336,7 +2336,7 @@ void do_delvm(Monitor *mon, const QDict *qdict) > } > } > > -void do_info_snapshots(Monitor *mon) > +static void do_info_snapshots_vm(Monitor *mon) > { > BlockDriverState *bs, *bs1; > QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s; > @@ -2400,6 +2400,59 @@ void do_info_snapshots(Monitor *mon) > > } > > +static void do_info_snapshots_blk(Monitor *mon, const char *device) > +{ > + BlockDriverState *bs; > + QEMUSnapshotInfo *sn_tab, *sn; > + int nb_sns, i; > + char buf[256]; > + > + /* 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 ; > + } > + > + nb_sns = bdrv_snapshot_list(bs, &sn_tab); > + if (nb_sns < 0) { > + monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n", > + device, nb_sns); > + return; > + } > + > + if (nb_sns == 0) { > + monitor_printf(mon, "There is no snapshot available.\n"); > + return; > + } > + > + monitor_printf(mon, "Device %s:\n", device); > + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL)); > + for (i = 0; i < nb_sns; i++) { > + sn = &sn_tab[i]; > + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn)); > + } > + g_free(sn_tab); > + return; > +} Return at the end of a void function is not necessary and QEMU code doesn't use it. Please use QEMU style. Stefan
> On Mon, Dec 17, 2012 at 02:25:09PM +0800, Wenchao Xia wrote: >> @@ -983,17 +983,22 @@ ETEXI >> >> { >> .name = "snapshot_blkdev", >> - .args_type = "reuse:-n,device:B,snapshot-file:s?,format:s?", >> - .params = "[-n] device [new-image-file] [format]", >> - .help = "initiates a live snapshot\n\t\t\t" >> - "of device. If a new image file is specified, the\n\t\t\t" >> - "new image file will become the new root image.\n\t\t\t" >> - "If format is specified, the snapshot file will\n\t\t\t" >> - "be created in that format. Otherwise the\n\t\t\t" >> - "snapshot will be internal! (currently unsupported).\n\t\t\t" >> - "The default format is qcow2. The -n flag requests QEMU\n\t\t\t" >> - "to reuse the image found in new-image-file, instead of\n\t\t\t" >> - "recreating it from scratch.", >> + .args_type = "internal:-i,reuse:-n,device:B,snapshot-file:s?,format:s?", >> + .params = "[-i] [-n] device [new-image-file] [format]", > > Please rename snapshot-file and new-image-file because it is now either > the external snapshot filename or the internal snapshot name - it's not > always a file! > OK. >> + .help = "initiates a live snapshot of device.\n\t\t\t" >> + " The -i flag requests QEMU to create internal snapshot\n\t\t\t" >> + "instead of external one.\n\t\t\t" >> + " The -n flag requests QEMU to use existing snapshot\n\t\t\t" >> + "instead of creating new snapshot, which would fails if\n\t\t\t" >> + "snapshot does not exist ahead.\n\t\t\t" > > "which fails if the snapshot does not exist already" > Will use this, thanks. >> + " new-image-file is the snapshot's name, in external case\n\t\t\t" >> + "it is the new image's name which will become the new root\n\t\t\t" >> + "image and must be specified, in internal case it is the\n\t\t\t" >> + "record's name and if not specified QEMU will create\n\t\t\t" >> + "internal snapshot with name generated according to time.\n\t\t\t" >> + " format is only valid in external case, which is the new\n\t\t\t" >> + "snapshot image's format. If not sepcified default format\n\t\t\t" >> + "qcow2 will be used.", > > "If not specified, the default format is qcow2." > >> .mhandler.cmd = hmp_snapshot_blkdev, >> }, >> >> @@ -1004,6 +1009,25 @@ Snapshot device, using snapshot file as target if provided >> ETEXI >> >> { >> + .name = "snapshot_delete_blkdev", > > Internal snapshots can already be deleted with delvm but there is no > existing command for external snapshots. > >> + .args_type = "internal:-i,device:B,snapshot-file:s", >> + .params = "[-i] device new-image-file", >> + .help = "delete a snapshot synchronous.\n\t\t\t" > > "synchronous" is implied for HMP commands, they are all like this so I > don't think it's necessary to mention it. > >> + " The -i flag requests QEMU to delete internal snapshot\n\t\t\t" >> + "instead of external one.\n\t\t\t" >> + " new-image-file is the snapshot's name, in external case\n\t\t\t" >> + "it is the image's name which is not supported now.\n\t\t\t" > > I'm not sure how useful this interface is. If we have no implementation > for deleting external snapshots and libvirt already uses delvm, then I'd > prefer you drop this command from the patch series. > > Later on, when there is code to implement external snapshot deletion it > can be added. Then there is no risk that this command design doesn't > work and we have to change it again. (Remember libvirt already uses > delvm so adding snapshot_delete_blkdev without external snapshot > deletion just adds code churn.) > OK, this interface will be dropped to make things simpler. >> @@ -1486,8 +1510,8 @@ ETEXI >> >> { >> .name = "info", >> - .args_type = "item:s?", >> - .params = "[subcommand]", >> + .args_type = "item:s?,params:s?", >> + .params = "[subcommand] [params]", >> .help = "show various information about the system state", >> .mhandler.cmd = do_info, >> }, > > What does this change do? > Pls ignore this, another serial was sent which extend hmp info infra first. >> diff --git a/hmp.c b/hmp.c >> index 180ba2b..f247f51 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -806,20 +806,40 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) >> const char *filename = qdict_get_try_str(qdict, "snapshot-file"); >> const char *format = qdict_get_try_str(qdict, "format"); >> int reuse = qdict_get_try_bool(qdict, "reuse", 0); >> + int internal = qdict_get_try_bool(qdict, "internal", 0); >> enum NewImageMode mode; >> + enum SnapshotType type; >> Error *errp = NULL; >> >> - if (!filename) { >> - /* In the future, if 'snapshot-file' is not specified, the snapshot >> - will be taken internally. Today it's actually required. */ >> + if ((!internal) && (!filename)) { >> + /* in external case filename must be set, should we generate >> + it automatically? */ > > Picking a filename would mainly be useful to humans. libvirt and other > management tools will want full control anyway. So the question is: is > there a naming policy that will be useful to most human users? > > If yes, please implement it. If no, please drop this comment. > It will be dropped. >> diff --git a/monitor.c b/monitor.c >> index c0e32d6..81de470 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -124,11 +124,14 @@ typedef struct mon_cmd_t { >> void (*user_print)(Monitor *mon, const QObject *data); >> union { >> void (*info)(Monitor *mon); >> + void (*info_qdict)(Monitor *mon, const QDict *qdict); >> void (*cmd)(Monitor *mon, const QDict *qdict); >> - int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data); >> + int (*cmd_new)(Monitor *mon, const QDict *params, >> + QObject **ret_data); >> int (*cmd_async)(Monitor *mon, const QDict *params, >> MonitorCompletion *cb, void *opaque); >> } mhandler; >> + int info_cmd_need_qdict; >> int flags; > > The union discriminator is the flags field (e.g. MONITOR_CMD_ASYNC). > Please follow that style. > > (If you use a boolean variable, please use the bool type instead of > int.) > There would be another way to extend hmp info command. >> } mon_cmd_t; >> >> @@ -824,7 +827,11 @@ static void do_info(Monitor *mon, const QDict *qdict) >> goto help; >> } >> >> - cmd->mhandler.info(mon); >> + if (cmd->info_cmd_need_qdict) { >> + cmd->mhandler.info_qdict(mon, qdict); >> + } else { >> + cmd->mhandler.info(mon); >> + } >> return; >> >> help: > > The generic monitor changes need to go in a separate commit. > >> @@ -2605,10 +2612,12 @@ static mon_cmd_t info_cmds[] = { >> }, >> { >> .name = "snapshots", >> - .args_type = "", >> - .params = "", >> - .help = "show the currently saved VM snapshots", >> - .mhandler.info = do_info_snapshots, >> + .args_type = "device:B?", >> + .params = "[device]", >> + .help = "show the currently saved VM snapshots or snapshots on " >> + "a single device.", >> + .mhandler.info_qdict = do_info_snapshots, >> + .info_cmd_need_qdict = 1, >> }, >> { >> .name = "status", > > This change to the info snapshots command needs to go in a separate > commit. > It will go to hmp serial. >> diff --git a/savevm.c b/savevm.c >> index c027529..5982aa9 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -2336,7 +2336,7 @@ void do_delvm(Monitor *mon, const QDict *qdict) >> } >> } >> >> -void do_info_snapshots(Monitor *mon) >> +static void do_info_snapshots_vm(Monitor *mon) >> { >> BlockDriverState *bs, *bs1; >> QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s; >> @@ -2400,6 +2400,59 @@ void do_info_snapshots(Monitor *mon) >> >> } >> >> +static void do_info_snapshots_blk(Monitor *mon, const char *device) >> +{ >> + BlockDriverState *bs; >> + QEMUSnapshotInfo *sn_tab, *sn; >> + int nb_sns, i; >> + char buf[256]; >> + >> + /* 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 ; >> + } >> + >> + nb_sns = bdrv_snapshot_list(bs, &sn_tab); >> + if (nb_sns < 0) { >> + monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n", >> + device, nb_sns); >> + return; >> + } >> + >> + if (nb_sns == 0) { >> + monitor_printf(mon, "There is no snapshot available.\n"); >> + return; >> + } >> + >> + monitor_printf(mon, "Device %s:\n", device); >> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL)); >> + for (i = 0; i < nb_sns; i++) { >> + sn = &sn_tab[i]; >> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn)); >> + } >> + g_free(sn_tab); >> + return; >> +} > > Return at the end of a void function is not necessary and QEMU code > doesn't use it. Please use QEMU style. > OK. > Stefan >
diff --git a/hmp-commands.hx b/hmp-commands.hx index 010b8c9..ee74723 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -983,17 +983,22 @@ ETEXI { .name = "snapshot_blkdev", - .args_type = "reuse:-n,device:B,snapshot-file:s?,format:s?", - .params = "[-n] device [new-image-file] [format]", - .help = "initiates a live snapshot\n\t\t\t" - "of device. If a new image file is specified, the\n\t\t\t" - "new image file will become the new root image.\n\t\t\t" - "If format is specified, the snapshot file will\n\t\t\t" - "be created in that format. Otherwise the\n\t\t\t" - "snapshot will be internal! (currently unsupported).\n\t\t\t" - "The default format is qcow2. The -n flag requests QEMU\n\t\t\t" - "to reuse the image found in new-image-file, instead of\n\t\t\t" - "recreating it from scratch.", + .args_type = "internal:-i,reuse:-n,device:B,snapshot-file:s?,format:s?", + .params = "[-i] [-n] device [new-image-file] [format]", + .help = "initiates a live snapshot of device.\n\t\t\t" + " The -i flag requests QEMU to create internal snapshot\n\t\t\t" + "instead of external one.\n\t\t\t" + " The -n flag requests QEMU to use existing snapshot\n\t\t\t" + "instead of creating new snapshot, which would fails if\n\t\t\t" + "snapshot does not exist ahead.\n\t\t\t" + " new-image-file is the snapshot's name, in external case\n\t\t\t" + "it is the new image's name which will become the new root\n\t\t\t" + "image and must be specified, in internal case it is the\n\t\t\t" + "record's name and if not specified QEMU will create\n\t\t\t" + "internal snapshot with name generated according to time.\n\t\t\t" + " format is only valid in external case, which is the new\n\t\t\t" + "snapshot image's format. If not sepcified default format\n\t\t\t" + "qcow2 will be used.", .mhandler.cmd = hmp_snapshot_blkdev, }, @@ -1004,6 +1009,25 @@ Snapshot device, using snapshot file as target if provided ETEXI { + .name = "snapshot_delete_blkdev", + .args_type = "internal:-i,device:B,snapshot-file:s", + .params = "[-i] device new-image-file", + .help = "delete a snapshot synchronous.\n\t\t\t" + " The -i flag requests QEMU to delete internal snapshot\n\t\t\t" + "instead of external one.\n\t\t\t" + " new-image-file is the snapshot's name, in external case\n\t\t\t" + "it is the image's name which is not supported now.\n\t\t\t" + "in internal case it is the snapshot record's id or name.", + .mhandler.cmd = hmp_snapshot_delete_blkdev, + }, + +STEXI +@item snapshot_delete_blkdev +@findex snapshot_delete_blkdev +Delete a snapshot on a block device. +ETEXI + + { .name = "drive_mirror", .args_type = "reuse:-n,full:-f,device:B,target:s,format:s?", .params = "[-n] [-f] device target [format]", @@ -1486,8 +1510,8 @@ ETEXI { .name = "info", - .args_type = "item:s?", - .params = "[subcommand]", + .args_type = "item:s?,params:s?", + .params = "[subcommand] [params]", .help = "show various information about the system state", .mhandler.cmd = do_info, }, diff --git a/hmp.c b/hmp.c index 180ba2b..f247f51 100644 --- a/hmp.c +++ b/hmp.c @@ -806,20 +806,40 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) const char *filename = qdict_get_try_str(qdict, "snapshot-file"); const char *format = qdict_get_try_str(qdict, "format"); int reuse = qdict_get_try_bool(qdict, "reuse", 0); + int internal = qdict_get_try_bool(qdict, "internal", 0); enum NewImageMode mode; + enum SnapshotType type; Error *errp = NULL; - if (!filename) { - /* In the future, if 'snapshot-file' is not specified, the snapshot - will be taken internally. Today it's actually required. */ + if ((!internal) && (!filename)) { + /* in external case filename must be set, should we generate + it automatically? */ error_set(&errp, QERR_MISSING_PARAMETER, "snapshot-file"); hmp_handle_error(mon, &errp); return; } - + type = internal ? SNAPSHOT_TYPE_INTERNAL : SNAPSHOT_TYPE_EXTERNAL; mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS; qmp_blockdev_snapshot_sync(device, filename, !!format, format, - true, mode, &errp); + true, mode, true, type, &errp); + hmp_handle_error(mon, &errp); +} + +void hmp_snapshot_delete_blkdev(Monitor *mon, const QDict *qdict) +{ + const char *device = qdict_get_str(qdict, "device"); + const char *filename = qdict_get_try_str(qdict, "snapshot-file"); + int internal = qdict_get_try_bool(qdict, "internal", 0); + enum SnapshotType type; + Error *errp = NULL; + + if (!internal) { + error_setg(&errp, "external snapshot delete not supported now."); + hmp_handle_error(mon, &errp); + return; + } + type = internal ? SNAPSHOT_TYPE_INTERNAL : SNAPSHOT_TYPE_EXTERNAL; + qmp_blockdev_snapshot_delete_sync(device, filename, true, type, &errp); hmp_handle_error(mon, &errp); } diff --git a/hmp.h b/hmp.h index 0ab03be..2ea67be 100644 --- a/hmp.h +++ b/hmp.h @@ -51,6 +51,7 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict); void hmp_balloon(Monitor *mon, const QDict *qdict); void hmp_block_resize(Monitor *mon, const QDict *qdict); void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); +void hmp_snapshot_delete_blkdev(Monitor *mon, const QDict *qdict); void hmp_drive_mirror(Monitor *mon, const QDict *qdict); void hmp_migrate_cancel(Monitor *mon, const QDict *qdict); void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict); diff --git a/monitor.c b/monitor.c index c0e32d6..81de470 100644 --- a/monitor.c +++ b/monitor.c @@ -124,11 +124,14 @@ typedef struct mon_cmd_t { void (*user_print)(Monitor *mon, const QObject *data); union { void (*info)(Monitor *mon); + void (*info_qdict)(Monitor *mon, const QDict *qdict); void (*cmd)(Monitor *mon, const QDict *qdict); - int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data); + int (*cmd_new)(Monitor *mon, const QDict *params, + QObject **ret_data); int (*cmd_async)(Monitor *mon, const QDict *params, MonitorCompletion *cb, void *opaque); } mhandler; + int info_cmd_need_qdict; int flags; } mon_cmd_t; @@ -824,7 +827,11 @@ static void do_info(Monitor *mon, const QDict *qdict) goto help; } - cmd->mhandler.info(mon); + if (cmd->info_cmd_need_qdict) { + cmd->mhandler.info_qdict(mon, qdict); + } else { + cmd->mhandler.info(mon); + } return; help: @@ -2605,10 +2612,12 @@ static mon_cmd_t info_cmds[] = { }, { .name = "snapshots", - .args_type = "", - .params = "", - .help = "show the currently saved VM snapshots", - .mhandler.info = do_info_snapshots, + .args_type = "device:B?", + .params = "[device]", + .help = "show the currently saved VM snapshots or snapshots on " + "a single device.", + .mhandler.info_qdict = do_info_snapshots, + .info_cmd_need_qdict = 1, }, { .name = "status", diff --git a/savevm.c b/savevm.c index c027529..5982aa9 100644 --- a/savevm.c +++ b/savevm.c @@ -2336,7 +2336,7 @@ void do_delvm(Monitor *mon, const QDict *qdict) } } -void do_info_snapshots(Monitor *mon) +static void do_info_snapshots_vm(Monitor *mon) { BlockDriverState *bs, *bs1; QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s; @@ -2400,6 +2400,59 @@ void do_info_snapshots(Monitor *mon) } +static void do_info_snapshots_blk(Monitor *mon, const char *device) +{ + BlockDriverState *bs; + QEMUSnapshotInfo *sn_tab, *sn; + int nb_sns, i; + char buf[256]; + + /* 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 ; + } + + nb_sns = bdrv_snapshot_list(bs, &sn_tab); + if (nb_sns < 0) { + monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n", + device, nb_sns); + return; + } + + if (nb_sns == 0) { + monitor_printf(mon, "There is no snapshot available.\n"); + return; + } + + monitor_printf(mon, "Device %s:\n", device); + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL)); + for (i = 0; i < nb_sns; i++) { + sn = &sn_tab[i]; + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn)); + } + g_free(sn_tab); + return; +} + +void do_info_snapshots(Monitor *mon, const QDict *qdict) +{ + /* Todo, there should be a layer rebuild qdict before enter this func. */ + const char *device = qdict_get_try_str(qdict, "params"); + if (!device) { + do_info_snapshots_vm(mon); + } else { + do_info_snapshots_blk(mon, device); + } + return; +} + void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) { qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK, diff --git a/sysemu.h b/sysemu.h index 1b6add2..a1254bf 100644 --- a/sysemu.h +++ b/sysemu.h @@ -68,7 +68,7 @@ void qemu_add_machine_init_done_notifier(Notifier *notify); void do_savevm(Monitor *mon, const QDict *qdict); int load_vmstate(const char *name); void do_delvm(Monitor *mon, const QDict *qdict); -void do_info_snapshots(Monitor *mon); +void do_info_snapshots(Monitor *mon, const QDict *qdict); void qemu_announce_self(void);
This patch add support in human monitor to create/delete/check internal snapshot on a single blk device. To make info command get parameter, added a new info handler type which can take QDict as parameter. Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- hmp-commands.hx | 50 +++++++++++++++++++++++++++++++++++++------------- hmp.c | 30 +++++++++++++++++++++++++----- hmp.h | 1 + monitor.c | 21 +++++++++++++++------ savevm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- sysemu.h | 2 +- 6 files changed, 133 insertions(+), 26 deletions(-)