| Message ID | 03cf631acdeac4654b1d7d8505d9d5f133ea6e6c.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> > --- > +++ b/qapi-schema.json > @@ -1868,3 +1868,25 @@ > # Since: 0.14.0 > ## > { 'command': 'netdev_del', 'data': {'id': 'str'} } > + > +## > +# @savevm: > +# > +# Create a snapshot of the whole virtual machine. If 'tag' is provided, > +# it is used as human readable identifier. If there is already a snapshot > +# with the same tag or ID, it is replaced. > +# > +# @name: tag or id of new or existing snapshot Needs an #optional designation, given the syntax below. > +# > +# Returns: Nothing on success > +# If there is a writable device not supporting snapshots, > +# SnapshotNotSupported > +# If no block device can accept snapshots, SnapshotNotAccepted > +# If an error occures while creating a snapshot, SnapshotCreateFailed s/occures/occurs/ > +# If open a block device for vm state fail, SnapshotOpenFailed > +# If an error uccures while writing vm state, SnapshotWriteFailed s/uccures/occurs/ > +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed The notion of blindly overwriting the existing snapshot of the same name seems a bit dangerous; should we take this opportunity to enhance the command, and add a force flag, where things fail if the flag is false but the name already exists, and where the reuse only happens if the flag is present? (In fact, it would make my life in libvirt easier, as I have an action item to make libvirt reject attempts to create a snapshot with tag named '1' if an existing snapshot already has an id of '1'.) > +# > +# Since: 1.2 > +## > +{ 'command': 'savevm', 'data': {'*name': 'str'} } > \ No newline at end of file Fix that. > @@ -1061,6 +1061,32 @@ Example: > > EQMP > { > + .name = "savevm", > + .args_type = "name:s?", > + .params = "name", > + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", > + .mhandler.cmd_new = qmp_marshal_input_savevm > + }, > + > +SQMP > +savevm I know the HMP command is short, for ease of typing; but since 'savevm' is not an English word, should we name the QMP command 'save-vm'?
On 07/12/2012 07:53 PM, Eric Blake wrote: > On 07/12/2012 10:55 AM, Pavel Hrdina wrote: >> Signed-off-by: Pavel Hrdina<phrdina@redhat.com> >> --- >> +++ b/qapi-schema.json >> @@ -1868,3 +1868,25 @@ >> # Since: 0.14.0 >> ## >> { 'command': 'netdev_del', 'data': {'id': 'str'} } >> + >> +## >> +# @savevm: >> +# >> +# Create a snapshot of the whole virtual machine. If 'tag' is provided, >> +# it is used as human readable identifier. If there is already a snapshot >> +# with the same tag or ID, it is replaced. >> +# >> +# @name: tag or id of new or existing snapshot > Needs an #optional designation, given the syntax below. > >> +# >> +# Returns: Nothing on success >> +# If there is a writable device not supporting snapshots, >> +# SnapshotNotSupported >> +# If no block device can accept snapshots, SnapshotNotAccepted >> +# If an error occures while creating a snapshot, SnapshotCreateFailed > s/occures/occurs/ > >> +# If open a block device for vm state fail, SnapshotOpenFailed >> +# If an error uccures while writing vm state, SnapshotWriteFailed > s/uccures/occurs/ > >> +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed > The notion of blindly overwriting the existing snapshot of the same name > seems a bit dangerous; should we take this opportunity to enhance the > command, and add a force flag, where things fail if the flag is false > but the name already exists, and where the reuse only happens if the > flag is present? (In fact, it would make my life in libvirt easier, as > I have an action item to make libvirt reject attempts to create a > snapshot with tag named '1' if an existing snapshot already has an id of > '1'.) This sounds reasonable and I think that this could be also good for HMP, not only for QMP. If there isn't anyone who disagree, I'll implement it. Pavel >> +# >> +# Since: 1.2 >> +## >> +{ 'command': 'savevm', 'data': {'*name': 'str'} } >> \ No newline at end of file > Fix that. > >> @@ -1061,6 +1061,32 @@ Example: >> >> EQMP >> { >> + .name = "savevm", >> + .args_type = "name:s?", >> + .params = "name", >> + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", >> + .mhandler.cmd_new = qmp_marshal_input_savevm >> + }, >> + >> +SQMP >> +savevm > I know the HMP command is short, for ease of typing; but since 'savevm' > is not an English word, should we name the QMP command 'save-vm'? >
On Thu, 12 Jul 2012 18:55:15 +0200 Pavel Hrdina <phrdina@redhat.com> wrote: > Signed-off-by: Pavel Hrdina <phrdina@redhat.com> > --- > hmp-commands.hx | 2 +- > hmp.c | 10 ++++++++++ > hmp.h | 1 + > qapi-schema.json | 22 ++++++++++++++++++++++ > qerror.c | 24 ++++++++++++++++++++++++ > qerror.h | 18 ++++++++++++++++++ > qmp-commands.hx | 26 ++++++++++++++++++++++++++ > savevm.c | 29 ++++++++++++++--------------- > sysemu.h | 1 - > 9 files changed, 116 insertions(+), 17 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index f5d9d91..e8c5325 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -267,7 +267,7 @@ ETEXI > .args_type = "name:s?", > .params = "[tag|id]", > .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", > - .mhandler.cmd = do_savevm, > + .mhandler.cmd = hmp_savevm, > }, > > STEXI > diff --git a/hmp.c b/hmp.c > index 4c6d4ae..491599d 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1002,3 +1002,13 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict) > qmp_netdev_del(id, &err); > hmp_handle_error(mon, &err); > } > + > +void hmp_savevm(Monitor *mon, const QDict *qdict) > +{ > + const char *name = qdict_get_try_str(qdict, "name"); > + Error *err = NULL; > + > + qmp_savevm(!!name, name, &err); > + > + hmp_handle_error(mon, &err); > +} > diff --git a/hmp.h b/hmp.h > index 79d138d..dc6410b 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict); > void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict); > 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); > > #endif > diff --git a/qapi-schema.json b/qapi-schema.json > index 1ab5dbd..4db1447 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1868,3 +1868,25 @@ > # Since: 0.14.0 > ## > { 'command': 'netdev_del', 'data': {'id': 'str'} } > + > +## > +# @savevm: Let's call it save-vm. > +# > +# Create a snapshot of the whole virtual machine. If 'tag' is provided, > +# it is used as human readable identifier. If there is already a snapshot > +# with the same tag or ID, it is replaced. Please, document that the vm is automatically stopped and resumed, and that this can take a long time. > +# > +# @name: tag or id of new or existing snapshot I don't like the idea of making 'name' optional in QMP. We could (and should) have it as optional in HMP though. This means that we should move the code that auto-generates it to HMP. Also, I remember an old discussion about distinguishing between 'tag' and 'id'. I remember it was difficult to do, so we could choose not to do it, but let's re-evaluate this again. > +# > +# Returns: Nothing on success > +# If there is a writable device not supporting snapshots, > +# SnapshotNotSupported > +# If no block device can accept snapshots, SnapshotNotAccepted > +# If an error occures while creating a snapshot, SnapshotCreateFailed > +# If open a block device for vm state fail, SnapshotOpenFailed > +# If an error uccures while writing vm state, SnapshotWriteFailed > +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed I don't think we should re-create all these errors, more comments on that below. > +# > +# Since: 1.2 > +## > +{ 'command': 'savevm', 'data': {'*name': 'str'} } > \ No newline at end of file > diff --git a/qerror.c b/qerror.c > index 92c4eff..4e6efa1 100644 > --- a/qerror.c > +++ b/qerror.c > @@ -309,6 +309,30 @@ static const QErrorStringTable qerror_table[] = { > .desc = "Could not start VNC server on %(target)", > }, > { > + .error_fmt = QERR_SNAPSHOT_CREATE_FAILED, > + .desc = "Error '%(errno)' while creating snapshot on '%(device)'", > + }, > + { > + .error_fmt = QERR_SNAPSHOT_DELETE_FAILED, > + .desc = "Error '%(errno)' while deleting snapshot on '%(device)'", > + }, > + { > + .error_fmt = QERR_SNAPSHOT_NOT_ACCEPTED, > + .desc = "No block device can accept snapshots", > + }, > + { > + .error_fmt = QERR_SNAPSHOT_NOT_SUPPORTED, > + .desc = "Device '%(device)' is writable but does not support snapshots", > + }, > + { > + .error_fmt = QERR_SNAPSHOT_OPEN_FAILED, > + .desc = "Error while opening snapshot on '%(device)'", > + }, > + { > + .error_fmt = QERR_SNAPSHOT_WRITE_FAILED, > + .desc = "Error '%(errno)' while writing snapshot on '%(device)'", > + }, > + { > .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS, > .desc = "Connection can not be completed immediately", > }, > diff --git a/qerror.h b/qerror.h > index b4c8758..bc47f4a 100644 > --- a/qerror.h > +++ b/qerror.h > @@ -251,6 +251,24 @@ QError *qobject_to_qerror(const QObject *obj); > #define QERR_VNC_SERVER_FAILED \ > "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" > > +#define QERR_SNAPSHOT_CREATE_FAILED \ > + "{ 'class': 'SnapshotCreateFailed', 'data': { 'device': %s, 'errno': %d } }" > + > +#define QERR_SNAPSHOT_DELETE_FAILED \ > + "{ 'class': 'SnapshotDeleteFailed', 'data': { 'device': %s, 'errno': %d } }" > + > +#define QERR_SNAPSHOT_NOT_ACCEPTED \ > + "{ 'class': 'SnapshotNotAccepted', 'data': {} }" > + > +#define QERR_SNAPSHOT_NOT_SUPPORTED \ > + "{ 'class': 'SnapshotNotSupported', 'data': { 'device': %s } }" > + > +#define QERR_SNAPSHOT_OPEN_FAILED \ > + "{ 'class': 'SnapshotOpenFailed', 'data': { 'device': %s } }" > + > +#define QERR_SNAPSHOT_WRITE_FAILED \ > + "{ 'class': 'SnapshotWriteFailed', 'data': { 'device': %s, 'errno': %d } }" > + > #define QERR_SOCKET_CONNECT_IN_PROGRESS \ > "{ 'class': 'SockConnectInprogress', 'data': {} }" > > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 2e1a38e..a1c82f7 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1061,6 +1061,32 @@ Example: > > EQMP > { > + .name = "savevm", > + .args_type = "name:s?", > + .params = "name", > + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", > + .mhandler.cmd_new = qmp_marshal_input_savevm > + }, > + > +SQMP > +savevm > +------ > + > +Create a snapshot of the whole virtual machine. If 'tag' is provided, > +it is used as human readable identifier. If there is already a snapshot > +with the same tag or ID, it is replaced. > + > +Arguments: > + > +- "name": tag or id of new or existing snapshot > + > +Example: > + > +-> { "execute": "savevm", "arguments": { "name": "my_snapshot" } } > +<- { "return": {} } > + > +EQMP > + { > .name = "qmp_capabilities", > .args_type = "", > .params = "", > diff --git a/savevm.c b/savevm.c > index a15c163..9fc1b53 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -2033,7 +2033,7 @@ static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, > /* > * Deletes snapshots of a given name in all opened images. > */ > -static int del_existing_snapshots(Monitor *mon, const char *name) > +static int del_existing_snapshots(Error **errp, const char *name) > { > BlockDriverState *bs; > QEMUSnapshotInfo sn1, *snapshot = &sn1; > @@ -2046,9 +2046,8 @@ static int del_existing_snapshots(Monitor *mon, const char *name) > { > ret = bdrv_snapshot_delete(bs, name); > if (ret < 0) { > - monitor_printf(mon, > - "Error while deleting snapshot on '%s'\n", > - bdrv_get_device_name(bs)); > + error_set(errp, QERR_SNAPSHOT_DELETE_FAILED, > + bdrv_get_device_name(bs), ret); The Right Thing to do here is to change bdrv_snapshot_delete() to take an Error argument and let it set the real error cause. Three considerations: 1. The QMP command shouldn't delete existing snapshots by default. Either, we add a 'force' argument to it or don't delete snapshots in save-vm at all (in which case a mngt app would have to delete the snapshots with the same name manually, I prefer this approach btw) 2. This has to be done in a separate series 3. It's a good idea to wait for my series improving error_set() to be merged before doing this > return -1; > } > } > @@ -2057,7 +2056,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name) > return 0; > } > > -void do_savevm(Monitor *mon, const QDict *qdict) > +void qmp_savevm(bool has_name, const char *name, Error **errp) > { > BlockDriverState *bs, *bs1; > QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1; > @@ -2072,7 +2071,6 @@ void do_savevm(Monitor *mon, const QDict *qdict) > struct timeval tv; > struct tm tm; > #endif > - const char *name = qdict_get_try_str(qdict, "name"); > > /* Verify if there is a device that doesn't support snapshots and is writable */ > bs = NULL; > @@ -2083,15 +2081,15 @@ void do_savevm(Monitor *mon, const QDict *qdict) > } > > if (!bdrv_can_snapshot(bs)) { > - monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n", > - bdrv_get_device_name(bs)); > + error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED, > + bdrv_get_device_name(bs)); Please, use QERR_NOT_SUPPORTED instead. I know it doesn't allow any arguments today, but I'm working on a series which will allow you to pass any arguments you wish. > return; > } > } > > bs = bdrv_snapshots(); > if (!bs) { > - monitor_printf(mon, "No block device can accept snapshots\n"); > + error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED); I think we should use QERR_NOT_SUPPORTED here too. > return; > } > > @@ -2112,7 +2110,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) > #endif > sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock); > > - if (name) { > + if (has_name) { > ret = bdrv_snapshot_find(bs, old_sn, name); > if (ret >= 0) { > pstrcpy(sn->name, sizeof(sn->name), old_sn->name); > @@ -2133,21 +2131,22 @@ void do_savevm(Monitor *mon, const QDict *qdict) > } > > /* Delete old snapshots of the same name */ > - if (name && del_existing_snapshots(mon, name) < 0) { > + if (has_name && del_existing_snapshots(errp, name) < 0) { > goto the_end; > } The QMP command should not delete existing snapshots, as said above. > > /* save the VM state */ > f = qemu_fopen_bdrv(bs, 1); > if (!f) { > - monitor_printf(mon, "Could not open VM state file\n"); > + error_set(errp, QERR_SNAPSHOT_OPEN_FAILED, bdrv_get_device_name(bs)); Please, use QERR_OPEN_FILE_FAILED instead. The filename should be the backing file name. > goto the_end; > } > ret = qemu_savevm_state(f); > vm_state_size = qemu_ftell(f); > qemu_fclose(f); > if (ret < 0) { > - monitor_printf(mon, "Error %d while writing VM\n", ret); > + error_set(errp, QERR_SNAPSHOT_WRITE_FAILED, > + bdrv_get_device_name(bs), ret); The Right Thing to do here it to convert qemu_savevm_sstate() to take an Error argument. The same considerations I wrote above about del_existing_snapshots() apply here, except that you can report IOError for most qemu_savevm_state() errors. > goto the_end; > } > > @@ -2160,8 +2159,8 @@ void do_savevm(Monitor *mon, const QDict *qdict) > sn->vm_state_size = (bs == bs1 ? vm_state_size : 0); > ret = bdrv_snapshot_create(bs1, sn); > if (ret < 0) { > - monitor_printf(mon, "Error while creating snapshot on '%s'\n", > - bdrv_get_device_name(bs1)); > + error_set(errp, QERR_SNAPSHOT_CREATE_FAILED, > + bdrv_get_device_name(bs1), ret); Here too, bdrv_snapshot_create() should be changed to take an Error argument. > } > } > } > diff --git a/sysemu.h b/sysemu.h > index 6540c79..95d1207 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_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);
On Mon, 16 Jul 2012 10:12:08 +0200 Pavel Hrdina <phrdina@redhat.com> wrote: > On 07/12/2012 07:53 PM, Eric Blake wrote: > > On 07/12/2012 10:55 AM, Pavel Hrdina wrote: > >> Signed-off-by: Pavel Hrdina<phrdina@redhat.com> > >> --- > >> +++ b/qapi-schema.json > >> @@ -1868,3 +1868,25 @@ > >> # Since: 0.14.0 > >> ## > >> { 'command': 'netdev_del', 'data': {'id': 'str'} } > >> + > >> +## > >> +# @savevm: > >> +# > >> +# Create a snapshot of the whole virtual machine. If 'tag' is provided, > >> +# it is used as human readable identifier. If there is already a snapshot > >> +# with the same tag or ID, it is replaced. > >> +# > >> +# @name: tag or id of new or existing snapshot > > Needs an #optional designation, given the syntax below. > > > >> +# > >> +# Returns: Nothing on success > >> +# If there is a writable device not supporting snapshots, > >> +# SnapshotNotSupported > >> +# If no block device can accept snapshots, SnapshotNotAccepted > >> +# If an error occures while creating a snapshot, SnapshotCreateFailed > > s/occures/occurs/ > > > >> +# If open a block device for vm state fail, SnapshotOpenFailed > >> +# If an error uccures while writing vm state, SnapshotWriteFailed > > s/uccures/occurs/ > > > >> +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed > > The notion of blindly overwriting the existing snapshot of the same name > > seems a bit dangerous; should we take this opportunity to enhance the > > command, and add a force flag, where things fail if the flag is false > > but the name already exists, and where the reuse only happens if the > > flag is present? (In fact, it would make my life in libvirt easier, as > > I have an action item to make libvirt reject attempts to create a > > snapshot with tag named '1' if an existing snapshot already has an id of > > '1'.) > This sounds reasonable and I think that this could be also good for HMP, > not only for QMP. > If there isn't anyone who disagree, I'll implement it. I agree with it. > > Pavel > >> +# > >> +# Since: 1.2 > >> +## > >> +{ 'command': 'savevm', 'data': {'*name': 'str'} } > >> \ No newline at end of file > > Fix that. > > > >> @@ -1061,6 +1061,32 @@ Example: > >> > >> EQMP > >> { > >> + .name = "savevm", > >> + .args_type = "name:s?", > >> + .params = "name", > >> + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", > >> + .mhandler.cmd_new = qmp_marshal_input_savevm > >> + }, > >> + > >> +SQMP > >> +savevm > > I know the HMP command is short, for ease of typing; but since 'savevm' > > is not an English word, should we name the QMP command 'save-vm'? > > > > >
diff --git a/hmp-commands.hx b/hmp-commands.hx index f5d9d91..e8c5325 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -267,7 +267,7 @@ ETEXI .args_type = "name:s?", .params = "[tag|id]", .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", - .mhandler.cmd = do_savevm, + .mhandler.cmd = hmp_savevm, }, STEXI diff --git a/hmp.c b/hmp.c index 4c6d4ae..491599d 100644 --- a/hmp.c +++ b/hmp.c @@ -1002,3 +1002,13 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict) qmp_netdev_del(id, &err); hmp_handle_error(mon, &err); } + +void hmp_savevm(Monitor *mon, const QDict *qdict) +{ + const char *name = qdict_get_try_str(qdict, "name"); + Error *err = NULL; + + qmp_savevm(!!name, name, &err); + + hmp_handle_error(mon, &err); +} diff --git a/hmp.h b/hmp.h index 79d138d..dc6410b 100644 --- a/hmp.h +++ b/hmp.h @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict); void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict); 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); #endif diff --git a/qapi-schema.json b/qapi-schema.json index 1ab5dbd..4db1447 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1868,3 +1868,25 @@ # Since: 0.14.0 ## { 'command': 'netdev_del', 'data': {'id': 'str'} } + +## +# @savevm: +# +# Create a snapshot of the whole virtual machine. If 'tag' is provided, +# it is used as human readable identifier. If there is already a snapshot +# with the same tag or ID, it is replaced. +# +# @name: tag or id of new or existing snapshot +# +# Returns: Nothing on success +# If there is a writable device not supporting snapshots, +# SnapshotNotSupported +# If no block device can accept snapshots, SnapshotNotAccepted +# If an error occures while creating a snapshot, SnapshotCreateFailed +# If open a block device for vm state fail, SnapshotOpenFailed +# If an error uccures while writing vm state, SnapshotWriteFailed +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed +# +# Since: 1.2 +## +{ 'command': 'savevm', 'data': {'*name': 'str'} } \ No newline at end of file diff --git a/qerror.c b/qerror.c index 92c4eff..4e6efa1 100644 --- a/qerror.c +++ b/qerror.c @@ -309,6 +309,30 @@ static const QErrorStringTable qerror_table[] = { .desc = "Could not start VNC server on %(target)", }, { + .error_fmt = QERR_SNAPSHOT_CREATE_FAILED, + .desc = "Error '%(errno)' while creating snapshot on '%(device)'", + }, + { + .error_fmt = QERR_SNAPSHOT_DELETE_FAILED, + .desc = "Error '%(errno)' while deleting snapshot on '%(device)'", + }, + { + .error_fmt = QERR_SNAPSHOT_NOT_ACCEPTED, + .desc = "No block device can accept snapshots", + }, + { + .error_fmt = QERR_SNAPSHOT_NOT_SUPPORTED, + .desc = "Device '%(device)' is writable but does not support snapshots", + }, + { + .error_fmt = QERR_SNAPSHOT_OPEN_FAILED, + .desc = "Error while opening snapshot on '%(device)'", + }, + { + .error_fmt = QERR_SNAPSHOT_WRITE_FAILED, + .desc = "Error '%(errno)' while writing snapshot on '%(device)'", + }, + { .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS, .desc = "Connection can not be completed immediately", }, diff --git a/qerror.h b/qerror.h index b4c8758..bc47f4a 100644 --- a/qerror.h +++ b/qerror.h @@ -251,6 +251,24 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_VNC_SERVER_FAILED \ "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" +#define QERR_SNAPSHOT_CREATE_FAILED \ + "{ 'class': 'SnapshotCreateFailed', 'data': { 'device': %s, 'errno': %d } }" + +#define QERR_SNAPSHOT_DELETE_FAILED \ + "{ 'class': 'SnapshotDeleteFailed', 'data': { 'device': %s, 'errno': %d } }" + +#define QERR_SNAPSHOT_NOT_ACCEPTED \ + "{ 'class': 'SnapshotNotAccepted', 'data': {} }" + +#define QERR_SNAPSHOT_NOT_SUPPORTED \ + "{ 'class': 'SnapshotNotSupported', 'data': { 'device': %s } }" + +#define QERR_SNAPSHOT_OPEN_FAILED \ + "{ 'class': 'SnapshotOpenFailed', 'data': { 'device': %s } }" + +#define QERR_SNAPSHOT_WRITE_FAILED \ + "{ 'class': 'SnapshotWriteFailed', 'data': { 'device': %s, 'errno': %d } }" + #define QERR_SOCKET_CONNECT_IN_PROGRESS \ "{ 'class': 'SockConnectInprogress', 'data': {} }" diff --git a/qmp-commands.hx b/qmp-commands.hx index 2e1a38e..a1c82f7 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1061,6 +1061,32 @@ Example: EQMP { + .name = "savevm", + .args_type = "name:s?", + .params = "name", + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created", + .mhandler.cmd_new = qmp_marshal_input_savevm + }, + +SQMP +savevm +------ + +Create a snapshot of the whole virtual machine. If 'tag' is provided, +it is used as human readable identifier. If there is already a snapshot +with the same tag or ID, it is replaced. + +Arguments: + +- "name": tag or id of new or existing snapshot + +Example: + +-> { "execute": "savevm", "arguments": { "name": "my_snapshot" } } +<- { "return": {} } + +EQMP + { .name = "qmp_capabilities", .args_type = "", .params = "", diff --git a/savevm.c b/savevm.c index a15c163..9fc1b53 100644 --- a/savevm.c +++ b/savevm.c @@ -2033,7 +2033,7 @@ static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, /* * Deletes snapshots of a given name in all opened images. */ -static int del_existing_snapshots(Monitor *mon, const char *name) +static int del_existing_snapshots(Error **errp, const char *name) { BlockDriverState *bs; QEMUSnapshotInfo sn1, *snapshot = &sn1; @@ -2046,9 +2046,8 @@ static int del_existing_snapshots(Monitor *mon, const char *name) { ret = bdrv_snapshot_delete(bs, name); if (ret < 0) { - monitor_printf(mon, - "Error while deleting snapshot on '%s'\n", - bdrv_get_device_name(bs)); + error_set(errp, QERR_SNAPSHOT_DELETE_FAILED, + bdrv_get_device_name(bs), ret); return -1; } } @@ -2057,7 +2056,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name) return 0; } -void do_savevm(Monitor *mon, const QDict *qdict) +void qmp_savevm(bool has_name, const char *name, Error **errp) { BlockDriverState *bs, *bs1; QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1; @@ -2072,7 +2071,6 @@ void do_savevm(Monitor *mon, const QDict *qdict) struct timeval tv; struct tm tm; #endif - const char *name = qdict_get_try_str(qdict, "name"); /* Verify if there is a device that doesn't support snapshots and is writable */ bs = NULL; @@ -2083,15 +2081,15 @@ void do_savevm(Monitor *mon, const QDict *qdict) } if (!bdrv_can_snapshot(bs)) { - monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n", - bdrv_get_device_name(bs)); + error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED, + bdrv_get_device_name(bs)); return; } } bs = bdrv_snapshots(); if (!bs) { - monitor_printf(mon, "No block device can accept snapshots\n"); + error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED); return; } @@ -2112,7 +2110,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) #endif sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock); - if (name) { + if (has_name) { ret = bdrv_snapshot_find(bs, old_sn, name); if (ret >= 0) { pstrcpy(sn->name, sizeof(sn->name), old_sn->name); @@ -2133,21 +2131,22 @@ void do_savevm(Monitor *mon, const QDict *qdict) } /* Delete old snapshots of the same name */ - if (name && del_existing_snapshots(mon, name) < 0) { + if (has_name && del_existing_snapshots(errp, name) < 0) { goto the_end; } /* save the VM state */ f = qemu_fopen_bdrv(bs, 1); if (!f) { - monitor_printf(mon, "Could not open VM state file\n"); + error_set(errp, QERR_SNAPSHOT_OPEN_FAILED, bdrv_get_device_name(bs)); goto the_end; } ret = qemu_savevm_state(f); vm_state_size = qemu_ftell(f); qemu_fclose(f); if (ret < 0) { - monitor_printf(mon, "Error %d while writing VM\n", ret); + error_set(errp, QERR_SNAPSHOT_WRITE_FAILED, + bdrv_get_device_name(bs), ret); goto the_end; } @@ -2160,8 +2159,8 @@ void do_savevm(Monitor *mon, const QDict *qdict) sn->vm_state_size = (bs == bs1 ? vm_state_size : 0); ret = bdrv_snapshot_create(bs1, sn); if (ret < 0) { - monitor_printf(mon, "Error while creating snapshot on '%s'\n", - bdrv_get_device_name(bs1)); + error_set(errp, QERR_SNAPSHOT_CREATE_FAILED, + bdrv_get_device_name(bs1), ret); } } } diff --git a/sysemu.h b/sysemu.h index 6540c79..95d1207 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_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);
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- hmp-commands.hx | 2 +- hmp.c | 10 ++++++++++ hmp.h | 1 + qapi-schema.json | 22 ++++++++++++++++++++++ qerror.c | 24 ++++++++++++++++++++++++ qerror.h | 18 ++++++++++++++++++ qmp-commands.hx | 26 ++++++++++++++++++++++++++ savevm.c | 29 ++++++++++++++--------------- sysemu.h | 1 - 9 files changed, 116 insertions(+), 17 deletions(-)