Re: [Qemu-devel] [PATCH 1/8] migration: split hmp_savevm to migrate_savevm and hmp_savevm wrapper
On 01/18/2016 06:47 PM, Markus Armbruster wrote:
"Denis V. Lunev" writes:
This would be useful in the next step when QMP version of this call will
be introduced. The patch also moves snapshot name generation to the
hmp specific code as QMP version of this code will require the name
on the protocol level.
Addition of migration_savevm to migration/migration.h is temporary. It
will be removed in the next patch with QMP level change.
Signed-off-by: Denis V. Lunev
CC: Juan Quintela
CC: Eric Blake
CC: Amit Shah
CC: Markus Armbruster
---
hmp.c | 26
include/migration/migration.h | 3 +++
migration/savevm.c| 46 +--
3 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/hmp.c b/hmp.c
index c2b2c16..e7bab75 100644
--- a/hmp.c
+++ b/hmp.c
@@ -18,6 +18,7 @@
#include "net/eth.h"
#include "sysemu/char.h"
#include "sysemu/block-backend.h"
+#include "sysemu/sysemu.h"
#include "qemu/option.h"
#include "qemu/timer.h"
#include "qmp-commands.h"
@@ -32,6 +33,7 @@
#include "ui/console.h"
#include "block/qapi.h"
#include "qemu-io.h"
+#include "migration/migration.h"
#ifdef CONFIG_SPICE
#include
@@ -2386,3 +2388,27 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict
*qdict)
qapi_free_RockerOfDpaGroupList(list);
}
+
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+const char *name = qdict_get_try_str(qdict, "name");
+char name_buf[64];
+
+if (name == NULL) {
I'd prefer !name.
+qemu_timeval tv;
+struct tm tm;
+
+/* cast below needed for OpenBSD where tv_sec is still 'long' */
+localtime_r((const time_t *)&tv.tv_sec, &tm);
I'm afraid this uses tv.tv_sec uninitialized. Not sure how this
survived testing :)
the name is generated :) somehow
OK, this should be definitely fixed
Aside: the ugly cast comes from commit d7d9b52. It works when the
system conforms to POSIX (tv_sec is time_t), or when tv_sec's type is
essentially the same as time_t (supposedly the case in old OpenBSD). As
far as I can tell, OpenBSD was fixed in 2013.
this is copied from the original code. I do not think
that this should be fixed in this commit. We could
make additional commit removing this ugly cast.
I don't want to have this patch reverted if something
goes wrong during merge.
+strftime(name_buf, sizeof(name_buf), "vm-%Y%m%d%H%M%S", &tm);
+
+name = name_buf;
+}
+
+migrate_savevm(name, &local_err);
+
+if (local_err != NULL) {
+error_report_err(local_err);
+}
+}
diff --git a/include/migration/migration.h b/include/migration/migration.h
index d9494b8..73c8bb1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -277,6 +277,8 @@ int migrate_compress_threads(void);
int migrate_decompress_threads(void);
bool migrate_use_events(void);
+void migrate_savevm(const char *name, Error **errp);
+
/* Sending on the return path - generic and then for each message type */
void migrate_send_rp_message(MigrationIncomingState *mis,
enum mig_rp_message_type message_type,
@@ -321,4 +323,5 @@ int ram_save_queue_pages(MigrationState *ms, const char
*rbname,
PostcopyState postcopy_state_get(void);
/* Set the state and return the old state */
PostcopyState postcopy_state_set(PostcopyState new_state);
+
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..308302a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
return ret;
}
-void hmp_savevm(Monitor *mon, const QDict *qdict)
+void migrate_savevm(const char *name, Error **errp)
{
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
@@ -1914,29 +1914,27 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
int saved_vm_running;
uint64_t vm_state_size;
qemu_timeval tv;
-struct tm tm;
-const char *name = qdict_get_try_str(qdict, "name");
Error *local_err = NULL;
AioContext *aio_context;
if (!bdrv_all_can_snapshot(&bs)) {
-monitor_printf(mon, "Device '%s' is writable but does not "
- "support snapshots.\n", bdrv_get_device_name(bs));
+error_setg(errp,
+ "Device '%s' is writable but does not support snapshots",
+ bdrv_get_device_name(bs));
return;
}
/* Delete old snapshots of the same name */
-if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
-monitor_printf(mon,
- "Error while deleting snapshot on device '%s': %s\n",
- bdrv_get_device_name(bs1), error_get_pretty(local_err));
+if (bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
+error_setg(errp, "Error while deleting sna
Re: [Qemu-devel] [PATCH 1/8] migration: split hmp_savevm to migrate_savevm and hmp_savevm wrapper
"Denis V. Lunev" writes:
> This would be useful in the next step when QMP version of this call will
> be introduced. The patch also moves snapshot name generation to the
> hmp specific code as QMP version of this code will require the name
> on the protocol level.
>
> Addition of migration_savevm to migration/migration.h is temporary. It
> will be removed in the next patch with QMP level change.
>
> Signed-off-by: Denis V. Lunev
> CC: Juan Quintela
> CC: Eric Blake
> CC: Amit Shah
> CC: Markus Armbruster
> ---
> hmp.c | 26
> include/migration/migration.h | 3 +++
> migration/savevm.c| 46
> +--
> 3 files changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index c2b2c16..e7bab75 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -18,6 +18,7 @@
> #include "net/eth.h"
> #include "sysemu/char.h"
> #include "sysemu/block-backend.h"
> +#include "sysemu/sysemu.h"
> #include "qemu/option.h"
> #include "qemu/timer.h"
> #include "qmp-commands.h"
> @@ -32,6 +33,7 @@
> #include "ui/console.h"
> #include "block/qapi.h"
> #include "qemu-io.h"
> +#include "migration/migration.h"
>
> #ifdef CONFIG_SPICE
> #include
> @@ -2386,3 +2388,27 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const
> QDict *qdict)
>
> qapi_free_RockerOfDpaGroupList(list);
> }
> +
> +void hmp_savevm(Monitor *mon, const QDict *qdict)
> +{
> +Error *local_err = NULL;
> +const char *name = qdict_get_try_str(qdict, "name");
> +char name_buf[64];
> +
> +if (name == NULL) {
I'd prefer !name.
> +qemu_timeval tv;
> +struct tm tm;
> +
> +/* cast below needed for OpenBSD where tv_sec is still 'long' */
> +localtime_r((const time_t *)&tv.tv_sec, &tm);
I'm afraid this uses tv.tv_sec uninitialized. Not sure how this
survived testing :)
Aside: the ugly cast comes from commit d7d9b52. It works when the
system conforms to POSIX (tv_sec is time_t), or when tv_sec's type is
essentially the same as time_t (supposedly the case in old OpenBSD). As
far as I can tell, OpenBSD was fixed in 2013.
> +strftime(name_buf, sizeof(name_buf), "vm-%Y%m%d%H%M%S", &tm);
> +
> +name = name_buf;
> +}
> +
> +migrate_savevm(name, &local_err);
> +
> +if (local_err != NULL) {
> +error_report_err(local_err);
> +}
> +}
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index d9494b8..73c8bb1 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -277,6 +277,8 @@ int migrate_compress_threads(void);
> int migrate_decompress_threads(void);
> bool migrate_use_events(void);
>
> +void migrate_savevm(const char *name, Error **errp);
> +
> /* Sending on the return path - generic and then for each message type */
> void migrate_send_rp_message(MigrationIncomingState *mis,
> enum mig_rp_message_type message_type,
> @@ -321,4 +323,5 @@ int ram_save_queue_pages(MigrationState *ms, const char
> *rbname,
> PostcopyState postcopy_state_get(void);
> /* Set the state and return the old state */
> PostcopyState postcopy_state_set(PostcopyState new_state);
> +
> #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0ad1b93..308302a 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
> return ret;
> }
>
> -void hmp_savevm(Monitor *mon, const QDict *qdict)
> +void migrate_savevm(const char *name, Error **errp)
> {
> BlockDriverState *bs, *bs1;
> QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
> @@ -1914,29 +1914,27 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
> int saved_vm_running;
> uint64_t vm_state_size;
> qemu_timeval tv;
> -struct tm tm;
> -const char *name = qdict_get_try_str(qdict, "name");
> Error *local_err = NULL;
> AioContext *aio_context;
>
> if (!bdrv_all_can_snapshot(&bs)) {
> -monitor_printf(mon, "Device '%s' is writable but does not "
> - "support snapshots.\n", bdrv_get_device_name(bs));
> +error_setg(errp,
> + "Device '%s' is writable but does not support snapshots",
> + bdrv_get_device_name(bs));
> return;
> }
>
> /* Delete old snapshots of the same name */
> -if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
> -monitor_printf(mon,
> - "Error while deleting snapshot on device '%s': %s\n",
> - bdrv_get_device_name(bs1),
> error_get_pretty(local_err));
> +if (bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
> +error_setg(errp, "Error while deleting snapshot on device '%s': %s",
> + bdrv_get_device_name(bs1), error_get_pretty(local_err));
Before the patch, we delete snapshots only when name
[Qemu-devel] [PATCH 1/8] migration: split hmp_savevm to migrate_savevm and hmp_savevm wrapper
This would be useful in the next step when QMP version of this call will
be introduced. The patch also moves snapshot name generation to the
hmp specific code as QMP version of this code will require the name
on the protocol level.
Addition of migration_savevm to migration/migration.h is temporary. It
will be removed in the next patch with QMP level change.
Signed-off-by: Denis V. Lunev
CC: Juan Quintela
CC: Eric Blake
CC: Amit Shah
CC: Markus Armbruster
---
hmp.c | 26
include/migration/migration.h | 3 +++
migration/savevm.c| 46 +--
3 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/hmp.c b/hmp.c
index c2b2c16..e7bab75 100644
--- a/hmp.c
+++ b/hmp.c
@@ -18,6 +18,7 @@
#include "net/eth.h"
#include "sysemu/char.h"
#include "sysemu/block-backend.h"
+#include "sysemu/sysemu.h"
#include "qemu/option.h"
#include "qemu/timer.h"
#include "qmp-commands.h"
@@ -32,6 +33,7 @@
#include "ui/console.h"
#include "block/qapi.h"
#include "qemu-io.h"
+#include "migration/migration.h"
#ifdef CONFIG_SPICE
#include
@@ -2386,3 +2388,27 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict
*qdict)
qapi_free_RockerOfDpaGroupList(list);
}
+
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+const char *name = qdict_get_try_str(qdict, "name");
+char name_buf[64];
+
+if (name == NULL) {
+qemu_timeval tv;
+struct tm tm;
+
+/* cast below needed for OpenBSD where tv_sec is still 'long' */
+localtime_r((const time_t *)&tv.tv_sec, &tm);
+strftime(name_buf, sizeof(name_buf), "vm-%Y%m%d%H%M%S", &tm);
+
+name = name_buf;
+}
+
+migrate_savevm(name, &local_err);
+
+if (local_err != NULL) {
+error_report_err(local_err);
+}
+}
diff --git a/include/migration/migration.h b/include/migration/migration.h
index d9494b8..73c8bb1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -277,6 +277,8 @@ int migrate_compress_threads(void);
int migrate_decompress_threads(void);
bool migrate_use_events(void);
+void migrate_savevm(const char *name, Error **errp);
+
/* Sending on the return path - generic and then for each message type */
void migrate_send_rp_message(MigrationIncomingState *mis,
enum mig_rp_message_type message_type,
@@ -321,4 +323,5 @@ int ram_save_queue_pages(MigrationState *ms, const char
*rbname,
PostcopyState postcopy_state_get(void);
/* Set the state and return the old state */
PostcopyState postcopy_state_set(PostcopyState new_state);
+
#endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..308302a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
return ret;
}
-void hmp_savevm(Monitor *mon, const QDict *qdict)
+void migrate_savevm(const char *name, Error **errp)
{
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
@@ -1914,29 +1914,27 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
int saved_vm_running;
uint64_t vm_state_size;
qemu_timeval tv;
-struct tm tm;
-const char *name = qdict_get_try_str(qdict, "name");
Error *local_err = NULL;
AioContext *aio_context;
if (!bdrv_all_can_snapshot(&bs)) {
-monitor_printf(mon, "Device '%s' is writable but does not "
- "support snapshots.\n", bdrv_get_device_name(bs));
+error_setg(errp,
+ "Device '%s' is writable but does not support snapshots",
+ bdrv_get_device_name(bs));
return;
}
/* Delete old snapshots of the same name */
-if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
-monitor_printf(mon,
- "Error while deleting snapshot on device '%s': %s\n",
- bdrv_get_device_name(bs1), error_get_pretty(local_err));
+if (bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
+error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+ bdrv_get_device_name(bs1), error_get_pretty(local_err));
error_free(local_err);
return;
}
bs = bdrv_all_find_vmstate_bs();
if (bs == NULL) {
-monitor_printf(mon, "No block device can accept snapshots\n");
+error_setg(errp, "No block device can accept snapshots");
return;
}
aio_context = bdrv_get_aio_context(bs);
@@ -1945,7 +1943,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
ret = global_state_store();
if (ret) {
-monitor_printf(mon, "Error saving global state\n");
+error_setg(errp, "Error saving global state");
return;
}
vm_stop(RUN_STATE_SAVE_VM);
@@ -1960,39 +1958,31 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
