Am 24.04.2013 um 17:32 hat Pavel Hrdina geschrieben:
> QMP command vm-snapshot-save takes one parameter name and the name is
> mandatory. The command returns SnapshotInfo on success, otherwise it returns
> an error message. If there is a snapshot with the same name it also returns
> an error message and if you want to overwrite that snapshot, you will have to
> at first call vm-snapshot-delete.
>
> HMP command savevm now has one optional parameter name and one flag -f.
> If the name parameter isn't provided the HMP command will create new one for
> internally used vm-snapshot-save. You can also specify the -f flag for
> overwrite
> existing snapshot which will internally use vm-snapshot-delete before
> vm-snapshot-save, otherwise it will print an error message if there is already
> a snapshot with the same name. If something else goes wrong, an error message
> will be printed.
>
> These improves behavior of the command to let you select only the name of the
> snapshot you want to create. This will ensure that if you want snapshot with
> the name '2', it will not rewrite or fail if there is any snapshot with id
> '2'.
>
> Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1491,3 +1491,52 @@ void hmp_vm_snapshot_load(Monitor *mon, const QDict
> *qdict)
> qapi_free_SnapshotInfo(info);
> hmp_handle_error(mon, &local_err);
> }
> +
> +void hmp_vm_snapshot_save(Monitor *mon, const QDict *qdict)
> +{
> + const char *name = qdict_get_try_str(qdict, "name");
> + bool force = qdict_get_try_bool(qdict, "force", 0);
> + Error *local_err = NULL;
> + SnapshotInfo *info = NULL;
> + qemu_timeval tv;
> + struct tm tm;
> + char tmp_name[256];
> +
> + if (!name) {
> + localtime_r((const time_t *)&tv.tv_sec, &tm);
> + strftime(tmp_name, sizeof(tmp_name), "vm-%Y%m%d%H%M%S", &tm);
> + name = tmp_name;
> + }
> +
> + if (force) {
> + info = qmp_vm_snapshot_delete(true, name, false, NULL, &local_err);
> + /* We don't need print info about deleted snapshot. It still needs to
> + * be freed. */
> + qapi_free_SnapshotInfo(info);
> + if (error_is_set(&local_err)) {
> + hmp_handle_error(mon, &local_err);
> + return;
> + }
> + }
Deleting a snapshot that doesn't exist returns an error. This means that
you can use -f _only_ to overwrite a snapshot. I think this is
unexpected, with -f all cases that work without -f should keep working.
> +
> + info = qmp_vm_snapshot_save(name, &local_err);
> +
> + if (info) {
> + char buf[256];
> + QEMUSnapshotInfo sn = {
> + .vm_state_size = info->vm_state_size,
> + .date_sec = info->date_sec,
> + .date_nsec = info->date_nsec,
> + .vm_clock_nsec = info->vm_clock_sec * 1000000000 +
> + info->vm_clock_nsec,
> + };
> + pstrcpy(sn.id_str, sizeof(sn.id_str), info->id);
> + pstrcpy(sn.name, sizeof(sn.name), info->name);
> + monitor_printf(mon, "Created snapshot's info:\n");
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf),
> NULL));
> + monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf),
> &sn));
> + }
> +
> + qapi_free_SnapshotInfo(info);
> + hmp_handle_error(mon, &local_err);
> +}
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2398,36 +2369,24 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> sn->date_nsec = tv.tv_usec * 1000;
> sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
>
> - if (name) {
> - if (bdrv_snapshot_find(bs, old_sn, name, NULL, NULL, true)) {
> - pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
> - pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
> - } else {
> - pstrcpy(sn->name, sizeof(sn->name), name);
> - }
> - } else {
> - /* cast below needed for OpenBSD where tv_sec is still 'long' */
> - localtime_r((const time_t *)&tv.tv_sec, &tm);
> - strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm);
> - }
> -
> - /* Delete old snapshots of the same name */
> - if (name && del_existing_snapshots(mon, name) < 0) {
> + if (bdrv_snapshot_find(bs, old_sn, name, NULL, NULL, false)) {
> + error_setg(errp, "Snapshot '%s' exists", name);
This is only checked for the VM state device. For all other devices the
case isn't handled any more.
> goto the_end;
> + } else {
> + pstrcpy(sn->name, sizeof(sn->name), name);
> }
>
> /* save the VM state */
> f = qemu_fopen_bdrv(bs, 1);
> if (!f) {
> - monitor_printf(mon, "Could not open VM state file\n");
> + error_setg(errp, "Failed to open '%s' file",
> bdrv_get_device_name(bs));
> goto the_end;
> }
> qemu_savevm_state(f, &local_err);
> vm_state_size = qemu_ftell(f);
> qemu_fclose(f);
> if (error_is_set(&local_err)) {
> - qerror_report_err(local_err);
> - error_free(local_err);
> + error_propagate(errp, local_err);
> goto the_end;
> }
>
> @@ -2440,18 +2399,29 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
> bdrv_snapshot_create(bs1, sn, &local_err);
> if (error_is_set(&local_err)) {
> - qerror_report(ERROR_CLASS_GENERIC_ERROR,
> - "Failed to create snapshot for device '%s':
> %s",
> - bdrv_get_device_name(bs1),
> - error_get_pretty(local_err));
> + error_setg(errp, "Failed to create snapshot for "
> + "device '%s': %s", bdrv_get_device_name(bs1),
> + error_get_pretty(local_err));
> error_free(local_err);
> + goto the_end;
> }
> }
> }
>
> + info = g_malloc0(sizeof(SnapshotInfo));
> + info->id = g_strdup(sn->id_str);
> + info->name = g_strdup(sn->name);
> + info->date_nsec = sn->date_nsec;
> + info->date_sec = sn->date_sec;
> + info->vm_state_size = vm_state_size;
> + info->vm_clock_nsec = sn->vm_clock_nsec % 1000000000;
> + info->vm_clock_sec = sn->vm_clock_nsec / 1000000000;
You could consider using a compound literal for better readability:
info = g_malloc(...);
*info = (SnapshotInfo) {
.id = g_strdup(sn->id_str),
.name = g_strdup(sn->name),
.date_nsec = sn->date_nsec,
...
};
> the_end:
> if (saved_vm_running)
> vm_start();
> +
> + return info;
> }
Kevin
