| Message ID | 049abd55f3f85bac5d299ca34f17b9abd39fedc4.1342092497.git.phrdina@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/12/2012 10:55 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina@redhat.com> > --- > hmp-commands.hx | 2 +- > hmp.c | 10 ++++++++++ > hmp.h | 1 + > qapi-schema.json | 17 +++++++++++++++++ > qmp-commands.hx | 24 ++++++++++++++++++++++++ > savevm.c | 21 +++++++++++---------- > sysemu.h | 1 - > 7 files changed, 64 insertions(+), 12 deletions(-) > + > +## > +# @delvm: This name is worse than 'loadvm' or 'savevm', in that we are NOT deleting the entire vm, but a named snapshot stored within the vm. Would naming it 'delete-vm-snapshot' make more sense (in which case the others might make more sense as 'save-vm-snapshot' and 'load-vm-snapshot')?
On 07/12/2012 07:59 PM, Eric Blake wrote: > On 07/12/2012 10:55 AM, Pavel Hrdina wrote: >> Signed-off-by: Pavel Hrdina<phrdina@redhat.com> >> --- >> hmp-commands.hx | 2 +- >> hmp.c | 10 ++++++++++ >> hmp.h | 1 + >> qapi-schema.json | 17 +++++++++++++++++ >> qmp-commands.hx | 24 ++++++++++++++++++++++++ >> savevm.c | 21 +++++++++++---------- >> sysemu.h | 1 - >> 7 files changed, 64 insertions(+), 12 deletions(-) >> + >> +## >> +# @delvm: > This name is worse than 'loadvm' or 'savevm', in that we are NOT > deleting the entire vm, but a named snapshot stored within the vm. > Would naming it 'delete-vm-snapshot' make more sense (in which case the > others might make more sense as 'save-vm-snapshot' and 'load-vm-snapshot')? > This naming looks nice. I definitely agree that it could be save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and query-vm-snapshots. Pavel
On 07/16/2012 02:12 AM, Pavel Hrdina wrote: > On 07/12/2012 07:59 PM, Eric Blake wrote: >> On 07/12/2012 10:55 AM, Pavel Hrdina wrote: >>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com> >>> --- >>> hmp-commands.hx | 2 +- >>> hmp.c | 10 ++++++++++ >>> hmp.h | 1 + >>> qapi-schema.json | 17 +++++++++++++++++ >>> qmp-commands.hx | 24 ++++++++++++++++++++++++ >>> savevm.c | 21 +++++++++++---------- >>> sysemu.h | 1 - >>> 7 files changed, 64 insertions(+), 12 deletions(-) >>> + >>> +## >>> +# @delvm: >> This name is worse than 'loadvm' or 'savevm', in that we are NOT >> deleting the entire vm, but a named snapshot stored within the vm. >> Would naming it 'delete-vm-snapshot' make more sense (in which case the >> others might make more sense as 'save-vm-snapshot' and >> 'load-vm-snapshot')? >> > This naming looks nice. I definitely agree that it could be > save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and > query-vm-snapshots. On seeing that spelled out, I wonder if the '-vm' is just noise; where we could use 'query-snapshots' instead of 'query-vm-snapshots'. Then again, we already have 'blockdev-snapshot-sync', which is an entirely different snapshot (just a block device, rather than the entire VM), so dropping -vm is probably a bad idea. Anyone else want to chime in on the bikeshed painting discussion of the best QMP name?
Am 16.07.2012 18:37, schrieb Eric Blake: > On 07/16/2012 02:12 AM, Pavel Hrdina wrote: >> On 07/12/2012 07:59 PM, Eric Blake wrote: >>> On 07/12/2012 10:55 AM, Pavel Hrdina wrote: >>>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com> >>>> --- >>>> hmp-commands.hx | 2 +- >>>> hmp.c | 10 ++++++++++ >>>> hmp.h | 1 + >>>> qapi-schema.json | 17 +++++++++++++++++ >>>> qmp-commands.hx | 24 ++++++++++++++++++++++++ >>>> savevm.c | 21 +++++++++++---------- >>>> sysemu.h | 1 - >>>> 7 files changed, 64 insertions(+), 12 deletions(-) >>>> + >>>> +## >>>> +# @delvm: >>> This name is worse than 'loadvm' or 'savevm', in that we are NOT >>> deleting the entire vm, but a named snapshot stored within the vm. >>> Would naming it 'delete-vm-snapshot' make more sense (in which case the >>> others might make more sense as 'save-vm-snapshot' and >>> 'load-vm-snapshot')? >>> >> This naming looks nice. I definitely agree that it could be >> save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and >> query-vm-snapshots. > > On seeing that spelled out, I wonder if the '-vm' is just noise; where > we could use 'query-snapshots' instead of 'query-vm-snapshots'. Then > again, we already have 'blockdev-snapshot-sync', which is an entirely > different snapshot (just a block device, rather than the entire VM), so > dropping -vm is probably a bad idea. Anyone else want to chime in on > the bikeshed painting discussion of the best QMP name? Sure. ;-) If there is a blockdev-snapshot-sync, then the counterparts for the whole VM shouldn't be called load/save/del-vm-snapshot, but vm-snapshot-load/save/del. And possibly add a -sync as well. Kevin
diff --git a/hmp-commands.hx b/hmp-commands.hx index b145612..3be9ad4 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -299,7 +299,7 @@ ETEXI .args_type = "name:s", .params = "tag|id", .help = "delete a VM snapshot from its tag or id", - .mhandler.cmd = do_delvm, + .mhandler.cmd = hmp_delvm, }, STEXI diff --git a/hmp.c b/hmp.c index 6ed4c3f..a4668ca 100644 --- a/hmp.c +++ b/hmp.c @@ -1022,3 +1022,13 @@ void hmp_loadvm(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &err); } + +void hmp_delvm(Monitor *mon, const QDict *qdict) +{ + const char *name = qdict_get_str(qdict, "name"); + Error *err = NULL; + + qmp_delvm(name, &err); + + hmp_handle_error(mon, &err); +} diff --git a/hmp.h b/hmp.h index 5623ae7..78710e2 100644 --- a/hmp.h +++ b/hmp.h @@ -66,5 +66,6 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict); void hmp_netdev_del(Monitor *mon, const QDict *qdict); void hmp_savevm(Monitor *mon, const QDict *qdict); void hmp_loadvm(Monitor *mon, const QDict *qdict); +void hmp_delvm(Monitor *mon, const QDict *qdict); #endif diff --git a/qapi-schema.json b/qapi-schema.json index 7df6a90..a22d26a 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1912,3 +1912,20 @@ # Since: 1.2 ## { 'command': 'loadvm', 'data': {'name': 'str'} } + +## +# @delvm: +# +# Delete the snapshot identified by 'tag' or 'id'. +# +# @name: tag or id of existing snapshot +# +# Returns: Nothing on success +# If no block device can accept snapshots, SnapshotNotAccepted +# If there is a writable device not supporting snapshots, +# SnapshotNotSupported +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed +# +# Since: 1.2 +## +{ 'command': 'delvm', 'data': {'name': 'str'} } diff --git a/qmp-commands.hx b/qmp-commands.hx index 02550f2..1071bb9 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1112,6 +1112,30 @@ Example: EQMP { + .name = "delvm", + .args_type = "name:s", + .params = "tag|id", + .help = "delete a VM snapshot from its tag or id", + .mhandler.cmd_new = qmp_marshal_input_delvm + }, + +SQMP +delvm +------ + +Delete the snapshot identified by 'tag' or 'id'. + +Arguments: + +- "name": tag or id of existing snapshot + +Example: + +-> { "execute": "delvm", "arguments": { "name": "my_snapshot" } } +<- { "return": {} } + +EQMP + { .name = "qmp_capabilities", .args_type = "", .params = "", diff --git a/savevm.c b/savevm.c index c113dd4..d3e8b07 100644 --- a/savevm.c +++ b/savevm.c @@ -2286,15 +2286,14 @@ void qmp_loadvm(const char *name, Error **errp) } } -void do_delvm(Monitor *mon, const QDict *qdict) +void qmp_delvm(const char *name, Error **errp) { BlockDriverState *bs, *bs1; int ret; - const char *name = qdict_get_str(qdict, "name"); bs = bdrv_snapshots(); if (!bs) { - monitor_printf(mon, "No block device supports snapshots\n"); + error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED); return; } @@ -2303,13 +2302,15 @@ void do_delvm(Monitor *mon, const QDict *qdict) if (bdrv_can_snapshot(bs1)) { ret = bdrv_snapshot_delete(bs1, name); if (ret < 0) { - if (ret == -ENOTSUP) - monitor_printf(mon, - "Snapshots not supported on device '%s'\n", - bdrv_get_device_name(bs1)); - else - monitor_printf(mon, "Error %d while deleting snapshot on " - "'%s'\n", ret, bdrv_get_device_name(bs1)); + if (ret == -ENOTSUP) { + error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED, + bdrv_get_device_name(bs1)); + return; + } else { + error_set(errp, QERR_SNAPSHOT_DELETE_FAILED, + bdrv_get_device_name(bs1), ret); + return; + } } } } diff --git a/sysemu.h b/sysemu.h index 8e65651..45e071c 100644 --- a/sysemu.h +++ b/sysemu.h @@ -69,7 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify); void qemu_add_machine_init_done_notifier(Notifier *notify); -void do_delvm(Monitor *mon, const QDict *qdict); void do_info_snapshots(Monitor *mon); void qemu_announce_self(void);
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- hmp-commands.hx | 2 +- hmp.c | 10 ++++++++++ hmp.h | 1 + qapi-schema.json | 17 +++++++++++++++++ qmp-commands.hx | 24 ++++++++++++++++++++++++ savevm.c | 21 +++++++++++---------- sysemu.h | 1 - 7 files changed, 64 insertions(+), 12 deletions(-)