Re: [Qemu-devel] [PATCH] qxl: use correct rom size for revision 4
On 12/13/2012 05:40 AM, Alon Levy wrote:
On 12/06/12 16:41, Alon Levy wrote:
RHBZ 869981
Before this patch revision 4 (4 is the default) would result in a
wrong
qxl_rom size of 16384 instead of 8192 when building with
spice-protocol-0.12, due to the addition of fields in
the rom for client capabilities and monitors config that were added
between spice-protocol 0.10 and 0.12.
The solution is a bit involved, since I decided not to change
QXLRom
which is defined externally in spice-protocol. Instead for revision
4
we allocate 72 bytes for the QXLRom on the qxl_rom bar (bytes
[0,71])
and make sure no fields out of that range are accessed, via
checking of
the revision and nop-ing.
Ok, I see we tackle two issues here.
Number one is qxl accessing the new fields with revision being 4.
That needs fixing indeed. But separate patch please.
Will do.
Number two is breaking migration due to the rom size change. Can't
we
just get the rom below 8k again instead? I think we can throw away a
whole bunch of modes. Each mode is four times in the list, for
orientation = { 0, 1, 2, 3 }. orientation is never ever used
anywhere,
looks like historic leftover or something planned which was never
actually implemented.
So keeping orientation = 0 only and kick out everything else should
give
us plenty of room ...
That is indeed a better solution, but it does change functionality. I think it
is correct but I'd like to get some other opinions - Uri, Arnon, Yonit, Soren -
any problems with dropping these?
Orientation is used in the Windows Display driver. It is used to set
dmDisplayOrientation in DEVMODEW structure
(http://msdn.microsoft.com/en-us/library/windows/hardware/ff552837(v=vs.85).aspx).
So, I'm not sure we can just drop it. Moreover, we need at least 2 of
the orientations, one for AxB resolution and the other for BxA.
cheers,
Gerd
[Qemu-devel] [PATCH] qxl: reload memslots after migration, when qxl is in UNDEFINED mode
The devram memslot stays active when qxl enters UNDEFINED mode (i.e, no
primary surface). If migration has occurred while the device is in
UNDEFINED stae, the memslots have to be reloaded at the destination.
Fixes rhbz#874574
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/qxl.c b/hw/qxl.c
index 1bc2d32..96887c4 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -2146,6 +2146,7 @@ static int qxl_post_load(void *opaque, int version)
switch (newmode) {
case QXL_MODE_UNDEFINED:
+qxl_create_memslots(d);
break;
case QXL_MODE_VGA:
qxl_create_memslots(d);
--
1.7.11.7
Re: [Qemu-devel] [PATCH] spice: increase the verbosity of spice section in qemu --help
On 08/21/2012 03:31 PM, Eric Blake wrote: On 08/21/2012 04:54 AM, Yonit Halperin wrote: Added all spice options to the help string. This can be used by libvirt to determine which spice related features are supported by qemu. For older released, this is true; but for future versions of qemu, libvirt would much rather learn this information from QMP commands than from scraping -help output. Can we get at all of this information from QMP? No, we don't have qmp commands for any of spice config options. I don't think it should be in the scope of this patch.
Re: [Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop
Hi,
On 08/21/2012 09:58 AM, Gerd Hoffmann wrote:
Hi,
+#if SPICE_SERVER_VERSION= 0x000b02 /* 0.11.2 */
+static void display_start(void)
+{
+DisplayListItem *item;
+
+QLIST_FOREACH(item,display_list, link) {
+item-display-running = true;
+}
+}
I think we should simply use a global variable for 'running' here. It's
global state anyway. Having this in per-display state struct buys us
nothing and adds alot of code for updating and display list maintainance.
ok, good point.
void qemu_spice_vm_change_state_handler(void *opaque, int running,
RunState state)
{
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
SimpleSpiceDisplay *ssd = opaque;
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler,sdpy);
+#endif
If you don't register qemu_spice_vm_change_state_handler on new
spice-server you should #ifdef the whole function, not the body only.
qemu_spice_vm_change_state_handler is also called from qxl.c, as part of
its own vm_change_state handler. I didn't want to add another ifdef there.
cheers,
Gerd
[Qemu-devel] [PATCH v3 4/5] spice: add 'migrated' flag to spice info
The flag is 'true' when spice migration has completed on the src side.
It is needed for a case where libvirt dies before migration completes
and it misses the event QEVENT_SPICE_MIGRATE_COMPLETED.
When libvirt is restored and queries the migration status, it also needs
to query spice and check if its migration has completed.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hmp.c|2 ++
qapi-schema.json |5 -
ui/spice-core.c |4
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/hmp.c b/hmp.c
index a9d5675..62f4dee 100644
--- a/hmp.c
+++ b/hmp.c
@@ -413,6 +413,8 @@ void hmp_info_spice(Monitor *mon)
monitor_printf(mon, address: %s:% PRId64 [tls]\n,
info-host, info-tls_port);
}
+monitor_printf(mon, migrated: %s\n,
+ info-migrated ? true : false);
monitor_printf(mon, auth: %s\n, info-auth);
monitor_printf(mon, compiled: %s\n, info-compiled_version);
monitor_printf(mon, mouse-mode: %s\n,
diff --git a/qapi-schema.json b/qapi-schema.json
index 3d2b2d1..bf3f924 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -808,6 +808,9 @@
#
# @enabled: true if the SPICE server is enabled, false otherwise
#
+# @migrated: true if the last guest migration completed and spice
+#migration had completed as well. false otherwise.
+#
# @host: #optional The hostname the SPICE server is bound to. This depends on
#the name resolution on the host and may be an IP address.
#
@@ -833,7 +836,7 @@
# Since: 0.14.0
##
{ 'type': 'SpiceInfo',
- 'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
+ 'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port':
'int',
'*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']}
}
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 73f8a95..5d82c8d 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -46,6 +46,7 @@ static Notifier migration_state;
static const char *auth = spice;
static char *auth_passwd;
static time_t auth_expires = TIME_MAX;
+static int spice_migration_completed;
int using_spice = 0;
static QemuThread me;
@@ -310,6 +311,7 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
{
monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+spice_migration_completed = true;
}
#endif
@@ -442,6 +444,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
}
info-enabled = true;
+info-migrated = spice_migration_completed;
addr = qemu_opt_get(opts, addr);
port = qemu_opt_get_number(opts, port, 0);
@@ -495,6 +498,7 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+spice_migration_completed = true;
#else
spice_server_migrate_end(spice_server, true);
} else if (migration_has_failed(s)) {
--
1.7.7.6
[Qemu-devel] [PATCH v3 2/5] spice: notify on vm state change only via spice_server_vm_start/stop
QXLWorker-start/stop are deprecated since spice-server 0.11.2
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c |7 ---
ui/spice-core.c|4
ui/spice-display.c | 32 ++--
ui/spice-display.h |9 +++--
4 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index c2dd3b4..95bbc03 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -958,9 +958,10 @@ static void qxl_update_irq(PCIQXLDevice *d)
static void qxl_check_state(PCIQXLDevice *d)
{
QXLRam *ram = d-ram;
+int spice_display_running = qemu_spice_display_is_running(d-ssd);
-assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cmd_ring));
-assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cursor_ring));
+assert(!spice_display_running || SPICE_RING_IS_EMPTY(ram-cmd_ring));
+assert(!spice_display_running || SPICE_RING_IS_EMPTY(ram-cursor_ring));
}
static void qxl_reset_state(PCIQXLDevice *d)
@@ -1538,7 +1539,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t
events)
uint32_t old_pending;
uint32_t le_events = cpu_to_le32(events);
-assert(d-ssd.running);
+assert(qemu_spice_display_is_running(d-ssd));
old_pending = __sync_fetch_and_or(d-ram-int_pending, le_events);
if ((old_pending le_events) == le_events) {
return;
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 32de1f1..fa78cc3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -37,6 +37,7 @@
#include migration.h
#include monitor.h
#include hw/hw.h
+#include spice-display.h
/* core bits */
@@ -550,9 +551,11 @@ static void vm_change_state_handler(void *opaque, int
running,
{
#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
if (running) {
+qemu_spice_display_start();
spice_server_vm_start(spice_server);
} else {
spice_server_vm_stop(spice_server);
+qemu_spice_display_stop();
}
#endif
}
@@ -754,6 +757,7 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
spice_server = spice_server_new();
spice_server_init(spice_server, core_interface);
}
+
return spice_server_add_interface(spice_server, sin);
}
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 3e8f0b3..1c31418 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -126,18 +126,44 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
ssd-worker-wakeup(ssd-worker);
}
-void qemu_spice_start(SimpleSpiceDisplay *ssd)
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
+static void qemu_spice_start(SimpleSpiceDisplay *ssd)
{
trace_qemu_spice_start(ssd-qxl.id);
ssd-worker-start(ssd-worker);
}
-void qemu_spice_stop(SimpleSpiceDisplay *ssd)
+static void qemu_spice_stop(SimpleSpiceDisplay *ssd)
{
trace_qemu_spice_stop(ssd-qxl.id);
ssd-worker-stop(ssd-worker);
}
+#else
+
+static int spice_display_is_running;
+
+void qemu_spice_display_start(void)
+{
+spice_display_is_running = true;
+}
+
+void qemu_spice_display_stop(void)
+{
+spice_display_is_running = false;
+}
+
+#endif
+
+int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
+{
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
+return ssd-running;
+#else
+return spice_display_is_running;
+#endif
+}
+
static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
{
SimpleSpiceUpdate *update;
@@ -272,6 +298,7 @@ void qemu_spice_destroy_host_primary(SimpleSpiceDisplay
*ssd)
void qemu_spice_vm_change_state_handler(void *opaque, int running,
RunState state)
{
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
SimpleSpiceDisplay *ssd = opaque;
if (running) {
@@ -281,6 +308,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int
running,
qemu_spice_stop(ssd);
ssd-running = false;
}
+#endif
}
void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 12e50b6..672d65e 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -82,7 +82,9 @@ struct SimpleSpiceDisplay {
QXLRect dirty;
int notify;
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
int running;
+#endif
/*
* All struct members below this comment can be accessed from
@@ -129,5 +131,8 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay
*ssd, uint32_t id,
void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
uint32_t id, qxl_async_io async);
void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
-void qemu_spice_start(SimpleSpiceDisplay *ssd);
-void qemu_spice_stop(SimpleSpiceDisplay *ssd);
+#if SPICE_SERVER_VERSION = 0x000b02 /* before 0.11.2 */
+void qemu_spice_display_start(void);
+void qemu_spice_display_stop(void);
+#endif
+int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd);
--
1.7.7.6
[Qemu-devel] [PATCH v3 1/5] spice: notify spice server on vm start/stop
Spice server needs to know about the vm state in order to prevent
attempts to write to devices when they are stopped, mainly during
the non-live stage of migration.
Instead, spice will take care of restoring this writes, on the migration
target side, after migration completes.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4fc48f8..32de1f1 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -545,6 +545,18 @@ static int add_channel(const char *name, const char
*value, void *opaque)
return 0;
}
+static void vm_change_state_handler(void *opaque, int running,
+RunState state)
+{
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+if (running) {
+spice_server_vm_start(spice_server);
+} else {
+spice_server_vm_stop(spice_server);
+}
+#endif
+}
+
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
@@ -718,6 +730,8 @@ void qemu_spice_init(void)
qemu_spice_input_init();
qemu_spice_audio_init();
+qemu_add_vm_change_state_handler(vm_change_state_handler, spice_server);
+
g_free(x509_key_file);
g_free(x509_cert_file);
g_free(x509_cacert_file);
--
1.7.7.6
[Qemu-devel] [PATCH v3 3/5] spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED
When migrating, libvirt queries the migration status, and upon migration
completions, it closes the migration src. On the other hand, when
migration is completed, spice transfers data from the src to destination
via the client. This data is required for keeping the spice session
after migration, without suffering from data loss and inconsistencies.
In order to allow this data transfer, we add QEVENT for signaling
libvirt that spice migration has completed, and libvirt needs to wait
for this event before quitting the src process.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
monitor.c |1 +
monitor.h |1 +
ui/spice-core.c |9 -
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/monitor.c b/monitor.c
index ce42466..584efe0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -455,6 +455,7 @@ static const char *monitor_event_names[] = {
[QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
[QEVENT_WAKEUP] = WAKEUP,
[QEVENT_BALLOON_CHANGE] = BALLOON_CHANGE,
+[QEVENT_SPICE_MIGRATE_COMPLETED] = SPICE_MIGRATE_COMPLETED,
};
QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
diff --git a/monitor.h b/monitor.h
index 47d556b..5fc2983 100644
--- a/monitor.h
+++ b/monitor.h
@@ -43,6 +43,7 @@ typedef enum MonitorEvent {
QEVENT_SUSPEND_DISK,
QEVENT_WAKEUP,
QEVENT_BALLOON_CHANGE,
+QEVENT_SPICE_MIGRATE_COMPLETED,
/* Add to 'monitor_event_names' array in monitor.c when
* defining new events here */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index fa78cc3..73f8a95 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -285,6 +285,7 @@ typedef struct SpiceMigration {
} SpiceMigration;
static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin);
static const SpiceMigrateInterface migrate_interface = {
.base.type = SPICE_INTERFACE_MIGRATION,
@@ -292,7 +293,7 @@ static const SpiceMigrateInterface migrate_interface = {
.base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
.base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
.migrate_connect_complete = migrate_connect_complete_cb,
-.migrate_end_complete = NULL,
+.migrate_end_complete = migrate_end_complete_cb,
};
static SpiceMigration spice_migrate;
@@ -305,6 +306,11 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
}
sm-connect_complete.cb = NULL;
}
+
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
+{
+monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+}
#endif
/* config string parsing */
@@ -488,6 +494,7 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
} else if (migration_has_finished(s)) {
#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
+monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
#else
spice_server_migrate_end(spice_server, true);
} else if (migration_has_failed(s)) {
--
1.7.7.6
[Qemu-devel] [PATCH v3 0/5] add support for spice migration v3
v3: patch 2: hold a global variable for the state of spice display, instead of one per device. v2: Notify spice about vm state changes only via spice_server_vm_start/stop spice repo: http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v2 --- Hi, The following series introduces support for keeping the spice session active after migration. For more details about spice seamless migration, see http://lists.freedesktop.org/archives/spice-devel/2012-August/010412.html Spice branches with seamless migration support can be found at: spice-protocol: http://cgit.freedesktop.org/~yhalperi/spice-protocol/log/?h=seamless-migration.v1 spice-common: http://cgit.freedesktop.org/~yhalperi/spice-common/log/ spice: http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v1 spicei-gtk: http://cgit.freedesktop.org/~yhalperi/spice-gtk/log/ Regards, Yonit. Yonit Halperin (5): spice: notify spice server on vm start/stop spice: notify on vm state change only via spice_server_vm_start/stop spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED spice: add 'migrated' flag to spice info spice: adding seamless-migration option to the command line hmp.c |2 ++ hw/qxl.c |7 --- monitor.c |1 + monitor.h |1 + qapi-schema.json |5 - qemu-config.c |3 +++ qemu-options.hx|3 +++ ui/spice-core.c| 38 +- ui/spice-display.c | 32 ++-- ui/spice-display.h |9 +++-- 10 files changed, 92 insertions(+), 9 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH v3 5/5] spice: adding seamless-migration option to the command line
The seamless-migration flag is required in order to identify
whether libvirt supports the new QEVENT_SPICE_MIGRATE_COMPLETED or not
(by default the flag is off).
New libvirt versions that wait for QEVENT_SPICE_MIGRATE_COMPLETED should turn
on this flag.
When this flag is off, spice fallbacks to its old migration method, which
can result in data loss.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
qemu-config.c |3 +++
qemu-options.hx |3 +++
ui/spice-core.c |7 +++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..b7bb28f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -524,6 +524,9 @@ QemuOptsList qemu_spice_opts = {
},{
.name = playback-compression,
.type = QEMU_OPT_BOOL,
+}, {
+.name = seamless-migration,
+.type = QEMU_OPT_BOOL,
},
{ /* end of list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 47cb5bd..0727f4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -917,6 +917,9 @@ Enable/disable passing mouse events via vdagent. Default
is on.
@item playback-compression=[on|off]
Enable/disable audio stream compression (using celt 0.5.1). Default is on.
+@item seamless-migration=[on|off]
+Enable/disable spice seamless migration. Default is off.
+
@end table
ETEXI
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 5d82c8d..d447334 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -584,6 +584,9 @@ void qemu_spice_init(void)
int port, tls_port, len, addr_flags;
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+bool seamless_migration;
+#endif
qemu_thread_get_self(me);
@@ -727,6 +730,10 @@ void qemu_spice_init(void)
spice_server_set_uuid(spice_server, qemu_uuid);
#endif
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+seamless_migration = qemu_opt_get_bool(opts, seamless-migration, 0);
+spice_server_set_seamless_migration(spice_server, seamless_migration);
+#endif
if (0 != spice_server_init(spice_server, core_interface)) {
error_report(failed to initialize spice server);
exit(1);
--
1.7.7.6
[Qemu-devel] [PATCH] spice: increase the verbosity of spice section in qemu --help
Added all spice options to the help string. This can be used by libvirt
to determine which spice related features are supported by qemu.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
qemu-options.hx | 18 +-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 0727f4f..3872e70 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -835,7 +835,23 @@ Enable SDL.
ETEXI
DEF(spice, HAS_ARG, QEMU_OPTION_spice,
--spice args enable spice\n, QEMU_ARCH_ALL)
+-spice [port=port][,tls-port=secured-port][,x509-dir=dir]\n
+ [,x509-key-file=file][,x509-key-password=file]\n
+ [,x509-cert-file=file][,x509-cacert-file=file]\n
+ [,x509-dh-key-file=file][,addr=addr][,ipv4|ipv6]\n
+ [,tls-ciphers=list]\n
+ [,tls-channel=[main|display|cursor|inputs|record|playback]]\n
+
[,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n
+ [,sasl][,password=secret][,disable-ticketing]\n
+ [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n
+ [,jpeg-wan-compression=[auto|never|always]]\n
+ [,zlib-glz-wan-compression=[auto|never|always]]\n
+ [,streaming-video=[off|all|filter]][,disable-copy-paste]\n
+ [,agent-mouse=[on|off]][,playback-compression=[on|off]]\n
+ [,seamless-migration=[on|off]]\n
+ enable spice\n
+ at least one of {port, tls-port} is mandatory\n,
+QEMU_ARCH_ALL)
STEXI
@item -spice @var{option}[,@var{option}[,...]]
@findex -spice
--
1.7.7.6
[Qemu-devel] [PATCH v2 3/5] spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED
When migrating, libvirt queries the migration status, and upon migration
completions, it closes the migration src. On the other hand, when
migration is completed, spice transfers data from the src to destination
via the client. This data is required for keeping the spice session
after migration, without suffering from data loss and inconsistencies.
In order to allow this data transfer, we add QEVENT for signaling
libvirt that spice migration has completed, and libvirt needs to wait
for this event before quitting the src process.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
monitor.c |1 +
monitor.h |1 +
ui/spice-core.c |9 -
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/monitor.c b/monitor.c
index ce42466..584efe0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -455,6 +455,7 @@ static const char *monitor_event_names[] = {
[QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
[QEVENT_WAKEUP] = WAKEUP,
[QEVENT_BALLOON_CHANGE] = BALLOON_CHANGE,
+[QEVENT_SPICE_MIGRATE_COMPLETED] = SPICE_MIGRATE_COMPLETED,
};
QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
diff --git a/monitor.h b/monitor.h
index 47d556b..5fc2983 100644
--- a/monitor.h
+++ b/monitor.h
@@ -43,6 +43,7 @@ typedef enum MonitorEvent {
QEVENT_SUSPEND_DISK,
QEVENT_WAKEUP,
QEVENT_BALLOON_CHANGE,
+QEVENT_SPICE_MIGRATE_COMPLETED,
/* Add to 'monitor_event_names' array in monitor.c when
* defining new events here */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 97b45f8..ad7c866 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -295,6 +295,7 @@ typedef struct SpiceMigration {
} SpiceMigration;
static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin);
static const SpiceMigrateInterface migrate_interface = {
.base.type = SPICE_INTERFACE_MIGRATION,
@@ -302,7 +303,7 @@ static const SpiceMigrateInterface migrate_interface = {
.base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
.base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
.migrate_connect_complete = migrate_connect_complete_cb,
-.migrate_end_complete = NULL,
+.migrate_end_complete = migrate_end_complete_cb,
};
static SpiceMigration spice_migrate;
@@ -315,6 +316,11 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
}
sm-connect_complete.cb = NULL;
}
+
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
+{
+monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+}
#endif
/* config string parsing */
@@ -498,6 +504,7 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
} else if (migration_has_finished(s)) {
#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
+monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
#else
spice_server_migrate_end(spice_server, true);
} else if (migration_has_failed(s)) {
--
1.7.7.6
[Qemu-devel] [PATCH v2 1/5] spice: notify spice server on vm start/stop
Spice server needs to know about the vm state in order to prevent
attempts to write to devices when they are stopped, mainly during
the non-live stage of migration.
Instead, spice will take care of restoring this writes, on the migration
target side, after migration completes.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4fc48f8..32de1f1 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -545,6 +545,18 @@ static int add_channel(const char *name, const char
*value, void *opaque)
return 0;
}
+static void vm_change_state_handler(void *opaque, int running,
+RunState state)
+{
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+if (running) {
+spice_server_vm_start(spice_server);
+} else {
+spice_server_vm_stop(spice_server);
+}
+#endif
+}
+
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
@@ -718,6 +730,8 @@ void qemu_spice_init(void)
qemu_spice_input_init();
qemu_spice_audio_init();
+qemu_add_vm_change_state_handler(vm_change_state_handler, spice_server);
+
g_free(x509_key_file);
g_free(x509_cert_file);
g_free(x509_cacert_file);
--
1.7.7.6
[Qemu-devel] [PATCH v2 2/5] spice: notify on vm state change only via spice_server_vm_start/stop
QXLWorker-start/stop are deprecated since spice-server 0.11.2
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c| 44
ui/spice-display.c |6 ++
ui/spice-display.h |2 ++
3 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 32de1f1..97b45f8 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -37,6 +37,7 @@
#include migration.h
#include monitor.h
#include hw/hw.h
+#include spice-display.h
/* core bits */
@@ -55,6 +56,16 @@ struct SpiceTimer {
};
static QTAILQ_HEAD(, SpiceTimer) timers = QTAILQ_HEAD_INITIALIZER(timers);
+typedef struct DisplayListItem DisplayListItem;
+
+struct DisplayListItem {
+SimpleSpiceDisplay *display;
+QLIST_ENTRY(DisplayListItem) link;
+};
+static QLIST_HEAD(, DisplayListItem) display_list =
+QLIST_HEAD_INITIALIZER(display_list);
+
+
static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque)
{
SpiceTimer *timer;
@@ -545,14 +556,36 @@ static int add_channel(const char *name, const char
*value, void *opaque)
return 0;
}
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+static void display_start(void)
+{
+DisplayListItem *item;
+
+QLIST_FOREACH(item, display_list, link) {
+item-display-running = true;
+}
+}
+
+static void display_stop(void)
+{
+DisplayListItem *item;
+
+QLIST_FOREACH(item, display_list, link) {
+item-display-running = false;
+}
+}
+#endif
+
static void vm_change_state_handler(void *opaque, int running,
RunState state)
{
#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
if (running) {
+display_start();
spice_server_vm_start(spice_server);
} else {
spice_server_vm_stop(spice_server);
+display_stop();
}
#endif
}
@@ -754,6 +787,17 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
spice_server = spice_server_new();
spice_server_init(spice_server, core_interface);
}
+
+if (strcmp(sin-sif-type, SPICE_INTERFACE_QXL) == 0) {
+QXLInstance *qxl;
+DisplayListItem *item;
+
+qxl = container_of(sin, QXLInstance, base);
+item = g_malloc0(sizeof(*item));
+item-display = container_of(qxl, SimpleSpiceDisplay, qxl);
+
+QLIST_INSERT_HEAD(display_list, item, link);
+}
return spice_server_add_interface(spice_server, sin);
}
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 3e8f0b3..073a354 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -126,6 +126,7 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
ssd-worker-wakeup(ssd-worker);
}
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
void qemu_spice_start(SimpleSpiceDisplay *ssd)
{
trace_qemu_spice_start(ssd-qxl.id);
@@ -137,6 +138,7 @@ void qemu_spice_stop(SimpleSpiceDisplay *ssd)
trace_qemu_spice_stop(ssd-qxl.id);
ssd-worker-stop(ssd-worker);
}
+#endif
static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
{
@@ -272,6 +274,7 @@ void qemu_spice_destroy_host_primary(SimpleSpiceDisplay
*ssd)
void qemu_spice_vm_change_state_handler(void *opaque, int running,
RunState state)
{
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
SimpleSpiceDisplay *ssd = opaque;
if (running) {
@@ -281,6 +284,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int
running,
qemu_spice_stop(ssd);
ssd-running = false;
}
+#endif
}
void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
@@ -518,7 +522,9 @@ void qemu_spice_display_init(DisplayState *ds)
qemu_spice_add_interface(sdpy.qxl.base);
assert(sdpy.worker);
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler,
sdpy);
+#endif
qemu_spice_create_host_memslot(sdpy);
qemu_spice_create_host_primary(sdpy);
}
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 12e50b6..cf7a8e7 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -129,5 +129,7 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay
*ssd, uint32_t id,
void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
uint32_t id, qxl_async_io async);
void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
+#if SPICE_SERVER_VERSION 0x000b02 /* before 0.11.2 */
void qemu_spice_start(SimpleSpiceDisplay *ssd);
void qemu_spice_stop(SimpleSpiceDisplay *ssd);
+#endif
--
1.7.7.6
[Qemu-devel] [PATCH v2 4/5] spice: add 'migrated' flag to spice info
The flag is 'true' when spice migration has completed on the src side.
It is needed for a case where libvirt dies before migration completes
and it misses the event QEVENT_SPICE_MIGRATE_COMPLETED.
When libvirt is restored and queries the migration status, it also needs
to query spice and check if its migration has completed.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hmp.c|2 ++
qapi-schema.json |5 -
ui/spice-core.c |4
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/hmp.c b/hmp.c
index a9d5675..62f4dee 100644
--- a/hmp.c
+++ b/hmp.c
@@ -413,6 +413,8 @@ void hmp_info_spice(Monitor *mon)
monitor_printf(mon, address: %s:% PRId64 [tls]\n,
info-host, info-tls_port);
}
+monitor_printf(mon, migrated: %s\n,
+ info-migrated ? true : false);
monitor_printf(mon, auth: %s\n, info-auth);
monitor_printf(mon, compiled: %s\n, info-compiled_version);
monitor_printf(mon, mouse-mode: %s\n,
diff --git a/qapi-schema.json b/qapi-schema.json
index 3d2b2d1..bf3f924 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -808,6 +808,9 @@
#
# @enabled: true if the SPICE server is enabled, false otherwise
#
+# @migrated: true if the last guest migration completed and spice
+#migration had completed as well. false otherwise.
+#
# @host: #optional The hostname the SPICE server is bound to. This depends on
#the name resolution on the host and may be an IP address.
#
@@ -833,7 +836,7 @@
# Since: 0.14.0
##
{ 'type': 'SpiceInfo',
- 'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
+ 'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port':
'int',
'*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']}
}
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ad7c866..ad7f01a 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -46,6 +46,7 @@ static Notifier migration_state;
static const char *auth = spice;
static char *auth_passwd;
static time_t auth_expires = TIME_MAX;
+static int spice_migration_completed;
int using_spice = 0;
static QemuThread me;
@@ -320,6 +321,7 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
{
monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+spice_migration_completed = true;
}
#endif
@@ -452,6 +454,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
}
info-enabled = true;
+info-migrated = spice_migration_completed;
addr = qemu_opt_get(opts, addr);
port = qemu_opt_get_number(opts, port, 0);
@@ -505,6 +508,7 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+spice_migration_completed = true;
#else
spice_server_migrate_end(spice_server, true);
} else if (migration_has_failed(s)) {
--
1.7.7.6
[Qemu-devel] [PATCH v2 0/5] add support for spice migration v2
v2: Notify spice about vm state changes only via spice_server_vm_start/stop spice repo: http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v2 --- Hi, The following series introduces support for keeping the spice session active after migration. For more details about spice seamless migration, see http://lists.freedesktop.org/archives/spice-devel/2012-August/010412.html Spice branches with seamless migration support can be found at: spice-protocol: http://cgit.freedesktop.org/~yhalperi/spice-protocol/log/?h=seamless-migration.v1 spice-common: http://cgit.freedesktop.org/~yhalperi/spice-common/log/ spice: http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v1 spicei-gtk: http://cgit.freedesktop.org/~yhalperi/spice-gtk/log/ Regards, Yonit. Yonit Halperin (5): spice: notify spice server on vm start/stop spice: notify on vm state change only via spice_server_vm_start/stop spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED spice: add 'migrated' flag to spice info spice: adding seamless-migration option to the command line hmp.c |2 + monitor.c |1 + monitor.h |1 + qapi-schema.json |5 ++- qemu-config.c |3 ++ qemu-options.hx|3 ++ ui/spice-core.c| 78 +++- ui/spice-display.c |6 ui/spice-display.h |2 + 9 files changed, 99 insertions(+), 2 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH v2 5/5] spice: adding seamless-migration option to the command line
The seamless-migration flag is required in order to identify
whether libvirt supports the new QEVENT_SPICE_MIGRATE_COMPLETED or not
(by default the flag is off).
New libvirt versions that wait for QEVENT_SPICE_MIGRATE_COMPLETED should turn
on this flag.
When this flag is off, spice fallbacks to its old migration method, which
can result in data loss.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
qemu-config.c |3 +++
qemu-options.hx |3 +++
ui/spice-core.c |7 +++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..b7bb28f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -524,6 +524,9 @@ QemuOptsList qemu_spice_opts = {
},{
.name = playback-compression,
.type = QEMU_OPT_BOOL,
+}, {
+.name = seamless-migration,
+.type = QEMU_OPT_BOOL,
},
{ /* end of list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 47cb5bd..0727f4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -917,6 +917,9 @@ Enable/disable passing mouse events via vdagent. Default
is on.
@item playback-compression=[on|off]
Enable/disable audio stream compression (using celt 0.5.1). Default is on.
+@item seamless-migration=[on|off]
+Enable/disable spice seamless migration. Default is off.
+
@end table
ETEXI
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ad7f01a..08a6da9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -614,6 +614,9 @@ void qemu_spice_init(void)
int port, tls_port, len, addr_flags;
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+bool seamless_migration;
+#endif
qemu_thread_get_self(me);
@@ -757,6 +760,10 @@ void qemu_spice_init(void)
spice_server_set_uuid(spice_server, qemu_uuid);
#endif
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+seamless_migration = qemu_opt_get_bool(opts, seamless-migration, 0);
+spice_server_set_seamless_migration(spice_server, seamless_migration);
+#endif
if (0 != spice_server_init(spice_server, core_interface)) {
error_report(failed to initialize spice server);
exit(1);
--
1.7.7.6
[Qemu-devel] [PATCH 0/4] add support for spice migration
The following series introduces support for keeping the spice session active after migration. For more details about spice seamless migration, see http://lists.freedesktop.org/archives/spice-devel/2012-August/010412.html Spice branches with seamless migration support can be found at: spice-protocol: http://cgit.freedesktop.org/~yhalperi/spice-protocol/log/?h=seamless-migration.v1 spice-common: http://cgit.freedesktop.org/~yhalperi/spice-common/log/ spice: http://cgit.freedesktop.org/~yhalperi/spice/log/?h=seamless-migration.v1 spicei-gtk: http://cgit.freedesktop.org/~yhalperi/spice-gtk/log/ Regards, Yonit. Yonit Halperin (4): spice: notify spice server on vm start/stop spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED spice: add 'migrated' flag to spice info spice: adding seamless-migration option to the command line hmp.c|2 ++ monitor.c|1 + monitor.h|1 + qapi-schema.json |5 - qemu-config.c|3 +++ qemu-options.hx |3 +++ ui/spice-core.c | 34 +- 7 files changed, 47 insertions(+), 2 deletions(-) -- 1.7.7.6
[Qemu-devel] [PATCH 1/4] spice: notify spice server on vm start/stop
Spice server needs to know about the vm state in order to prevent
attempts to write to devices when they are stopped, mainly during
the non-live stage of migration.
Instead, spice will take care of restoring this writes, on the migration
target side, after migration completes.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4fc48f8..32de1f1 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -545,6 +545,18 @@ static int add_channel(const char *name, const char
*value, void *opaque)
return 0;
}
+static void vm_change_state_handler(void *opaque, int running,
+RunState state)
+{
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+if (running) {
+spice_server_vm_start(spice_server);
+} else {
+spice_server_vm_stop(spice_server);
+}
+#endif
+}
+
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
@@ -718,6 +730,8 @@ void qemu_spice_init(void)
qemu_spice_input_init();
qemu_spice_audio_init();
+qemu_add_vm_change_state_handler(vm_change_state_handler, spice_server);
+
g_free(x509_key_file);
g_free(x509_cert_file);
g_free(x509_cacert_file);
--
1.7.7.6
[Qemu-devel] [PATCH 4/4] spice: adding seamless-migration option to the command line
The seamless-migration flag is required in order to identify
whether libvirt supports the new QEVENT_SPICE_MIGRATE_COMPLETED or not
(by default the flag is off).
New libvirt versions that wait for QEVENT_SPICE_MIGRATE_COMPLETED should turn
on this flag.
When this flag is off, spice fallbacks to its old migration method, which
can result in data loss.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
qemu-config.c |3 +++
qemu-options.hx |3 +++
ui/spice-core.c |7 +++
3 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..b7bb28f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -524,6 +524,9 @@ QemuOptsList qemu_spice_opts = {
},{
.name = playback-compression,
.type = QEMU_OPT_BOOL,
+}, {
+.name = seamless-migration,
+.type = QEMU_OPT_BOOL,
},
{ /* end of list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 47cb5bd..0727f4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -917,6 +917,9 @@ Enable/disable passing mouse events via vdagent. Default
is on.
@item playback-compression=[on|off]
Enable/disable audio stream compression (using celt 0.5.1). Default is on.
+@item seamless-migration=[on|off]
+Enable/disable spice seamless migration. Default is off.
+
@end table
ETEXI
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4d3d05e..f5e2d5c 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -581,6 +581,9 @@ void qemu_spice_init(void)
int port, tls_port, len, addr_flags;
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+bool seamless_migration;
+#endif
qemu_thread_get_self(me);
@@ -724,6 +727,10 @@ void qemu_spice_init(void)
spice_server_set_uuid(spice_server, qemu_uuid);
#endif
+#if SPICE_SERVER_VERSION = 0x000b02 /* 0.11.2 */
+seamless_migration = qemu_opt_get_bool(opts, seamless-migration, 0);
+spice_server_set_seamless_migration(spice_server, seamless_migration);
+#endif
if (0 != spice_server_init(spice_server, core_interface)) {
error_report(failed to initialize spice server);
exit(1);
--
1.7.7.6
[Qemu-devel] [PATCH 3/4] spice: add 'migrated' flag to spice info
The flag is 'true' when spice migration has completed on the src side.
It is needed for a case where libvirt dies before migration completes
and it misses the event QEVENT_SPICE_MIGRATE_COMPLETED.
When libvirt is restored and queries the migration status, it also needs
to query spice and check if its migration has completed.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hmp.c|2 ++
qapi-schema.json |5 -
ui/spice-core.c |4
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/hmp.c b/hmp.c
index a9d5675..62f4dee 100644
--- a/hmp.c
+++ b/hmp.c
@@ -413,6 +413,8 @@ void hmp_info_spice(Monitor *mon)
monitor_printf(mon, address: %s:% PRId64 [tls]\n,
info-host, info-tls_port);
}
+monitor_printf(mon, migrated: %s\n,
+ info-migrated ? true : false);
monitor_printf(mon, auth: %s\n, info-auth);
monitor_printf(mon, compiled: %s\n, info-compiled_version);
monitor_printf(mon, mouse-mode: %s\n,
diff --git a/qapi-schema.json b/qapi-schema.json
index 3d2b2d1..bf3f924 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -808,6 +808,9 @@
#
# @enabled: true if the SPICE server is enabled, false otherwise
#
+# @migrated: true if the last guest migration completed and spice
+#migration had completed as well. false otherwise.
+#
# @host: #optional The hostname the SPICE server is bound to. This depends on
#the name resolution on the host and may be an IP address.
#
@@ -833,7 +836,7 @@
# Since: 0.14.0
##
{ 'type': 'SpiceInfo',
- 'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
+ 'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port':
'int',
'*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']}
}
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 6b4d216..4d3d05e 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -45,6 +45,7 @@ static Notifier migration_state;
static const char *auth = spice;
static char *auth_passwd;
static time_t auth_expires = TIME_MAX;
+static int spice_migration_completed;
int using_spice = 0;
static QemuThread me;
@@ -309,6 +310,7 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
{
monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+spice_migration_completed = true;
}
#endif
@@ -441,6 +443,7 @@ SpiceInfo *qmp_query_spice(Error **errp)
}
info-enabled = true;
+info-migrated = spice_migration_completed;
addr = qemu_opt_get(opts, addr);
port = qemu_opt_get_number(opts, port, 0);
@@ -494,6 +497,7 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+spice_migration_completed = true;
#else
spice_server_migrate_end(spice_server, true);
} else if (migration_has_failed(s)) {
--
1.7.7.6
[Qemu-devel] [PATCH 2/4] spice migration: add QEVENT_SPICE_MIGRATE_COMPLETED
When migrating, libvirt queries the migration status, and upon migration
completions, it closes the migration src. On the other hand, when
migration is completed, spice transfers data from the src to destination
via the client. This data is required for keeping the spice session
after migration, without suffering from data loss and inconsistencies.
In order to allow this data transfer, we add QEVENT for signaling
libvirt that spice migration has completed, and libvirt needs to wait
for this event before quitting the src process.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
monitor.c |1 +
monitor.h |1 +
ui/spice-core.c |9 -
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/monitor.c b/monitor.c
index ce42466..584efe0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -455,6 +455,7 @@ static const char *monitor_event_names[] = {
[QEVENT_SUSPEND_DISK] = SUSPEND_DISK,
[QEVENT_WAKEUP] = WAKEUP,
[QEVENT_BALLOON_CHANGE] = BALLOON_CHANGE,
+[QEVENT_SPICE_MIGRATE_COMPLETED] = SPICE_MIGRATE_COMPLETED,
};
QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
diff --git a/monitor.h b/monitor.h
index 47d556b..5fc2983 100644
--- a/monitor.h
+++ b/monitor.h
@@ -43,6 +43,7 @@ typedef enum MonitorEvent {
QEVENT_SUSPEND_DISK,
QEVENT_WAKEUP,
QEVENT_BALLOON_CHANGE,
+QEVENT_SPICE_MIGRATE_COMPLETED,
/* Add to 'monitor_event_names' array in monitor.c when
* defining new events here */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 32de1f1..6b4d216 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -284,6 +284,7 @@ typedef struct SpiceMigration {
} SpiceMigration;
static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin);
static const SpiceMigrateInterface migrate_interface = {
.base.type = SPICE_INTERFACE_MIGRATION,
@@ -291,7 +292,7 @@ static const SpiceMigrateInterface migrate_interface = {
.base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
.base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
.migrate_connect_complete = migrate_connect_complete_cb,
-.migrate_end_complete = NULL,
+.migrate_end_complete = migrate_end_complete_cb,
};
static SpiceMigration spice_migrate;
@@ -304,6 +305,11 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
}
sm-connect_complete.cb = NULL;
}
+
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
+{
+monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
+}
#endif
/* config string parsing */
@@ -487,6 +493,7 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
} else if (migration_has_finished(s)) {
#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
+monitor_protocol_event(QEVENT_SPICE_MIGRATE_COMPLETED, NULL);
#else
spice_server_migrate_end(spice_server, true);
} else if (migration_has_failed(s)) {
--
1.7.7.6
Re: [Qemu-devel] [PATCH 1/4] spice: notify spice server on vm start/stop
On 08/16/2012 12:42 PM, Gerd Hoffmann wrote: On 08/16/12 10:23, Yonit Halperin wrote: Spice server needs to know about the vm state in order to prevent attempts to write to devices when they are stopped, mainly during the non-live stage of migration. Why this new hook? qemu already notifies spice-server using QXLWorker start/stop callbacks. It notifies the QXLWorker, and it goes to the display_channel. Spice api changes anyway, by adding spice_server_set_seamless_migration, and as other channels need this notification as well, it would be nicer to explicitly notify the server about the vm start/stop and not abuse the QXLWorker notification. Another option would have been to add notifier for SpiceCharDeviceInterface as well, and then to any other new interface that will require it. Regards, Yonit. cheers, Gerd
Re: [Qemu-devel] [PATCH] spice: abort on invalid streaming cmdline params
Ack
On 08/13/2012 11:32 AM, Christophe Fergeau wrote:
When parsing its command line parameters, spice aborts when it
finds unexpected values, except for the 'streaming-video' option.
This happens because the parsing of the parameters for this option
is done using the 'name2enum' helper, which does not error out
on unknown values. Using the 'parse_name' helper makes sure we
error out in this case. Looking at git history, the use of
'name2enum' instead of 'parse_name' seems to have been an oversight,
so let's change to that now.
Fixes rhbz#831708
---
ui/spice-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 4fc48f8..bb4f585 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -344,7 +344,8 @@ static const char *stream_video_names[] = {
[ SPICE_STREAM_VIDEO_FILTER ] = filter,
};
#define parse_stream_video(_name) \
-name2enum(_name, stream_video_names, ARRAY_SIZE(stream_video_names))
+parse_name(_name, stream video control, \
+ stream_video_names, ARRAY_SIZE(stream_video_names))
static const char *compression_names[] = {
[ SPICE_IMAGE_COMPRESS_OFF ] = off,
Re: [Qemu-devel] [PATCH v2] qxl: add QXL_IO_MONITORS_CONFIG_ASYNC
Hi,
I think you should save the monitor configuration as part of the
vmstate, and recall spice_qxl_monitors_config_async in qxl_post_load.
Regards,
Yonit.
On 07/23/2012 07:33 PM, Alon Levy wrote:
bumps spice-protocol to 0.12.0 for new IO.
revision bumped to 4 for new IO support, enabled for spice-server= 0.11.1
RHBZ: 770842
Signed-off-by: Alon Levyal...@redhat.com
---
v2: fixed interface_async_complete_io to not complain about unexpected async io.
configure|2 +-
hw/qxl.c | 30 +-
hw/qxl.h |4
trace-events |1 +
4 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index cef0a71..5fcd315 100755
--- a/configure
+++ b/configure
@@ -2630,7 +2630,7 @@ EOF
spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2/dev/null)
spice_libs=$($pkg_config --libs spice-protocol spice-server 2/dev/null)
if $pkg_config --atleast-version=0.8.2 spice-server/dev/null 21 \
- $pkg_config --atleast-version=0.8.1 spice-protocol /dev/null 21 \
+ $pkg_config --atleast-version=0.12.0 spice-protocol /dev/null 21 \
compile_prog $spice_cflags $spice_libs ; then
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
diff --git a/hw/qxl.c b/hw/qxl.c
index 3a883ce..0440440 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -249,6 +249,18 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl,
qxl_async_io async)
}
}
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl)
+{
+trace_qxl_spice_monitors_config(qxl-id);
+spice_qxl_monitors_config_async(qxl-ssd.qxl,
+qxl-ram-monitors_config,
+MEMSLOT_GROUP_GUEST,
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+ QXL_IO_MONITORS_CONFIG_ASYNC));
+}
+#endif
+
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
{
trace_qxl_spice_reset_image_cache(qxl-id);
@@ -538,6 +550,7 @@ static const char *io_port_to_string(uint32_t io_port)
= QXL_IO_DESTROY_ALL_SURFACES_ASYNC,
[QXL_IO_FLUSH_SURFACES_ASYNC] = QXL_IO_FLUSH_SURFACES_ASYNC,
[QXL_IO_FLUSH_RELEASE] = QXL_IO_FLUSH_RELEASE,
+[QXL_IO_MONITORS_CONFIG_ASYNC] = QXL_IO_MONITORS_CONFIG_ASYNC,
};
return io_port_to_string[io_port];
}
@@ -819,6 +832,7 @@ static void interface_async_complete_io(PCIQXLDevice *qxl,
QXLCookie *cookie)
case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_UPDATE_AREA_ASYNC:
case QXL_IO_FLUSH_SURFACES_ASYNC:
+case QXL_IO_MONITORS_CONFIG_ASYNC:
break;
case QXL_IO_CREATE_PRIMARY_ASYNC:
qxl_create_guest_primary_complete(qxl);
@@ -1333,7 +1347,7 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port, io_port_to_string(io_port));
/* be nice to buggy guest drivers */
if (io_port= QXL_IO_UPDATE_AREA_ASYNC
-io_port= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+io_port= QXL_IO_MONITORS_CONFIG_ASYNC) {
qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
}
return;
@@ -1361,6 +1375,9 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port = QXL_IO_DESTROY_ALL_SURFACES;
goto async_common;
case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+#endif
async_common:
async = QXL_ASYNC;
qemu_mutex_lock(d-async_lock);
@@ -1490,6 +1507,11 @@ async_common:
d-mode = QXL_MODE_UNDEFINED;
qxl_spice_destroy_surfaces(d, async);
break;
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+qxl_spice_monitors_config_async(d);
+break;
+#endif
default:
qxl_set_guest_bug(d, %s: unexpected ioport=0x%x\n, __func__,
io_port);
}
@@ -1785,9 +1807,15 @@ static int qxl_init_common(PCIQXLDevice *qxl)
io_size = 16;
break;
case 3: /* qxl-3 */
+pci_device_rev = QXL_REVISION_STABLE_V10;
+io_size = 32; /* PCI region size must be pow2 */
+break;
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+case 4: /* qxl-4 */
pci_device_rev = QXL_DEFAULT_REVISION;
io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
break;
+#endif
default:
error_report(Invalid revision %d for qxl device (max %d),
qxl-revision, QXL_DEFAULT_REVISION);
diff --git a/hw/qxl.h b/hw/qxl.h
index 172baf6..c1aadaa 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -128,7 +128,11 @@ typedef struct PCIQXLDevice {
} \
} while (0)
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V12
+#else
#define QXL_DEFAULT_REVISION
Re: [Qemu-devel] [PATCH v2] qxl: add QXL_IO_MONITORS_CONFIG_ASYNC
Hi,
On 07/25/2012 08:29 PM, Alon Levy wrote:
On Wed, Jul 25, 2012 at 12:00:21PM +0300, Yonit Halperin wrote:
Hi,
I think you should save the monitor configuration as part of the vmstate,
and recall spice_qxl_monitors_config_async in qxl_post_load.
Good point, will do. Would be nice if I could differentiate same client
(that doesn't need the recalling) and new client.
The client doesn't need the recalling (if it the same client that was
connected to the src); It is the destination server which needs the
recalling, for the future clients that will get connected.
Cheers,
Yonit.
Regards,
Yonit.
On 07/23/2012 07:33 PM, Alon Levy wrote:
bumps spice-protocol to 0.12.0 for new IO.
revision bumped to 4 for new IO support, enabled for spice-server= 0.11.1
RHBZ: 770842
Signed-off-by: Alon Levyal...@redhat.com
---
v2: fixed interface_async_complete_io to not complain about unexpected async io.
configure|2 +-
hw/qxl.c | 30 +-
hw/qxl.h |4
trace-events |1 +
4 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index cef0a71..5fcd315 100755
--- a/configure
+++ b/configure
@@ -2630,7 +2630,7 @@ EOF
spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2/dev/null)
spice_libs=$($pkg_config --libs spice-protocol spice-server 2/dev/null)
if $pkg_config --atleast-version=0.8.2 spice-server/dev/null 21 \
- $pkg_config --atleast-version=0.8.1 spice-protocol /dev/null 21 \
+ $pkg_config --atleast-version=0.12.0 spice-protocol /dev/null 21
\
compile_prog $spice_cflags $spice_libs ; then
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
diff --git a/hw/qxl.c b/hw/qxl.c
index 3a883ce..0440440 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -249,6 +249,18 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl,
qxl_async_io async)
}
}
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl)
+{
+trace_qxl_spice_monitors_config(qxl-id);
+spice_qxl_monitors_config_async(qxl-ssd.qxl,
+qxl-ram-monitors_config,
+MEMSLOT_GROUP_GUEST,
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+ QXL_IO_MONITORS_CONFIG_ASYNC));
+}
+#endif
+
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
{
trace_qxl_spice_reset_image_cache(qxl-id);
@@ -538,6 +550,7 @@ static const char *io_port_to_string(uint32_t io_port)
= QXL_IO_DESTROY_ALL_SURFACES_ASYNC,
[QXL_IO_FLUSH_SURFACES_ASYNC] = QXL_IO_FLUSH_SURFACES_ASYNC,
[QXL_IO_FLUSH_RELEASE] = QXL_IO_FLUSH_RELEASE,
+[QXL_IO_MONITORS_CONFIG_ASYNC] = QXL_IO_MONITORS_CONFIG_ASYNC,
};
return io_port_to_string[io_port];
}
@@ -819,6 +832,7 @@ static void interface_async_complete_io(PCIQXLDevice *qxl,
QXLCookie *cookie)
case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_UPDATE_AREA_ASYNC:
case QXL_IO_FLUSH_SURFACES_ASYNC:
+case QXL_IO_MONITORS_CONFIG_ASYNC:
break;
case QXL_IO_CREATE_PRIMARY_ASYNC:
qxl_create_guest_primary_complete(qxl);
@@ -1333,7 +1347,7 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port, io_port_to_string(io_port));
/* be nice to buggy guest drivers */
if (io_port= QXL_IO_UPDATE_AREA_ASYNC
-io_port= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+io_port= QXL_IO_MONITORS_CONFIG_ASYNC) {
qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
}
return;
@@ -1361,6 +1375,9 @@ static void ioport_write(void *opaque, target_phys_addr_t
addr,
io_port = QXL_IO_DESTROY_ALL_SURFACES;
goto async_common;
case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+#endif
async_common:
async = QXL_ASYNC;
qemu_mutex_lock(d-async_lock);
@@ -1490,6 +1507,11 @@ async_common:
d-mode = QXL_MODE_UNDEFINED;
qxl_spice_destroy_surfaces(d, async);
break;
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+case QXL_IO_MONITORS_CONFIG_ASYNC:
+qxl_spice_monitors_config_async(d);
+break;
+#endif
default:
qxl_set_guest_bug(d, %s: unexpected ioport=0x%x\n, __func__,
io_port);
}
@@ -1785,9 +1807,15 @@ static int qxl_init_common(PCIQXLDevice *qxl)
io_size = 16;
break;
case 3: /* qxl-3 */
+pci_device_rev = QXL_REVISION_STABLE_V10;
+io_size = 32; /* PCI region size must be pow2 */
+break;
+#if SPICE_SERVER_VERSION= 0x000b01 /* 0.11.1 */
+case 4: /* qxl-4 */
pci_device_rev = QXL_DEFAULT_REVISION;
io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
break;
+#endif
default:
error_report(Invalid revision %d for qxl
Re: [Qemu-devel] [RFC PATCH 0/5] asynchronous migration state change handlers
Hi, I would like to add some more points to Gerd's explanation: On 06/05/2012 04:15 PM, Gerd Hoffmann wrote: Hi, Absolutely not. This is hideously ugly and affects a bunch of code. Spice is *not* getting a hook in migration where it gets to add arbitrary amounts of downtime to the migration traffic. That's a terrible idea. I'd like to be more constructive in my response, but you aren't explaining the problem well enough for me to offer an alternative solution. You need to find another way to solve this problem. Actually, this is not the first time we address you with this issues. For example: http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg01805.html (The first part of the above discussion is not directly related to the current one). I'll try to explain in more details: As Gerd mentioned, migrating the spice connection smoothly requires the src server to keep running and send/receive data to/from the client, after migration has already completed, till the client completely transfers to the target. The suggested patch series only delays the migration state change from ACTIVE to COMPLETED/ERROR/CANCELED, till spice signals it has completed its part in migration. As I see it, if spice connection does exists, its migration should be treated as a non separate part of the whole migration process, and thus, the migration state shouldn't change from ACTIVE, till spice has completed its part. Hence, I don't think we should have a qmp event for signaling libvirt about spice migration. The second challenge we are facing, which I addressed in the plans part of the cover-letter, and on which I think you (anthony) actually replied, is how to tackle migrating spice data from the src server to the target server. Such data can be usb/smartcard packets sent from a device connected on the client, to the server, and that haven't reached the device. Or partial data that has been read from a guest character device and that haven't been sent to the client. Other data can be internal server-client state data we would wish to keep on the server in order to avoid establishing the connection to the target from scratch, and possibly also suffer from a slower responsiveness at start. In the cover-letter I suggested to transfer spice migration data via the vmstate infrastructure. The other alternative which we also discussed in the link above, is to transfer the data via the client. The latter also requires holding the src process alive after migration completion, in order to manage to complete transferring the data from the src to the client. The vmstate option has the advantages of faster data transfer (src-dst, instead of src-client-dst), and in addition employing an already existing reliable mechanism for data migration. The disadvantage is that in order to have an updated vmstate we need to communicate with spice client and get all in-flight data before saving the vmstate. So, we can either busy wait on the relevant fds during the pre_save of the vmstates, or have async pre_save, so that the main loop will be active (but I think that it can be risky once the non-live phase started), or have an async notifier for changing from live to non-live phase, (spice will be able to update the vmstates during this notification handler). Of course, we would in any case use a timeout in order to prevent too long delay. To summarize, since we can still use the client to transfer data from the src to the target (instead of using vmstate), the major requirement of spice, is to keep the src running after migration has completed. Yonit. Very short version: The requirement is simply to not kill qemu on the source side until the source spice-server has finished session handover to the target spice-server. Long version: spice-client connects automatically to the target machine, so the user ideally doesn't notice that his virtual machine was just migrated over to another host. Today this happens via switch-host, which is a simple message asking the spice client to connect to the new host. We want move to seamless migration model where we don't start over from scratch, but hand over the session from the source to the target. Advantage is that various state cached in spice-client will stay valid and doesn't need to be retransmitted. It also requires a handshake between spice-servers on source and target. libvirt killing qemu on the source host before the handshake is done isn't exactly helpful. [ Side note: In theory this issue exists even today: in case the data pipe to the client is full spice-server will queue up the switch-host message and qemu might be killed before it is sent out. In practice it doesn't happen though because it goes through the low-traffic main channel so the socket buffers usually have enougth space. ] So, the big question is how to tackle the issue? Option (1): Wait until spice-server is done before signaling completion to libvirt.
Re: [Qemu-devel] [RFC PATCH 0/5] asynchronous migration state change handlers
On 06/06/2012 12:22 PM, Anthony Liguori wrote: On 06/06/2012 05:10 PM, Yonit Halperin wrote: Hi, I would like to add some more points to Gerd's explanation: On 06/05/2012 04:15 PM, Gerd Hoffmann wrote: Hi, Absolutely not. This is hideously ugly and affects a bunch of code. Spice is *not* getting a hook in migration where it gets to add arbitrary amounts of downtime to the migration traffic. That's a terrible idea. I'd like to be more constructive in my response, but you aren't explaining the problem well enough for me to offer an alternative solution. You need to find another way to solve this problem. Actually, this is not the first time we address you with this issues. For example: http://lists.gnu.org/archive/html/qemu-devel/2012-03/msg01805.html (The first part of the above discussion is not directly related to the current one). I'll try to explain in more details: As Gerd mentioned, migrating the spice connection smoothly requires the src server to keep running and send/receive data to/from the client, after migration has already completed, till the client completely transfers to the target. The suggested patch series only delays the migration state change from ACTIVE to COMPLETED/ERROR/CANCELED, till spice signals it has completed its part in migration. As I see it, if spice connection does exists, its migration should be treated as a non separate part of the whole migration process, and thus, the migration state shouldn't change from ACTIVE, till spice has completed its part. Hence, I don't think we should have a qmp event for signaling libvirt about spice migration. Spice client migration has nothing to do with guest migration. Trying to abuse QEMU to support it is not acceptable. The second challenge we are facing, which I addressed in the plans part of the cover-letter, and on which I think you (anthony) actually replied, is how to tackle migrating spice data from the src server to the target server. Such data can be usb/smartcard packets sent from a device connected on the client, to the server, and that haven't reached the device. Or partial data that has been read from a guest character device and that haven't been sent to the client. Other data can be internal server-client state data we would wish to keep on the server in order to avoid establishing the connection to the target from scratch, and possibly also suffer from a slower responsiveness at start. In the cover-letter I suggested to transfer spice migration data via the vmstate infrastructure. The other alternative which we also discussed in the link above, is to transfer the data via the client. The latter also requires holding the src process alive after migration completion, in order to manage to complete transferring the data from the src to the client. -- To summarize, since we can still use the client to transfer data from the src to the target (instead of using vmstate), the major requirement of spice, is to keep the src running after migration has completed. So send a QMP event and call it a day. Using a QMP event is making spice seamless migration dependent on libvirt version. Delaying the status change to migration completed, (1) doesn't affect qemu migration time, the migration has already completed, and (2) will allow spice to seamlessly migrate, no matter which libvirt version is used. Yonit. Regards, Anthony Liguori Yonit. Very short version: The requirement is simply to not kill qemu on the source side until the source spice-server has finished session handover to the target spice-server. Long version: spice-client connects automatically to the target machine, so the user ideally doesn't notice that his virtual machine was just migrated over to another host. Today this happens via switch-host, which is a simple message asking the spice client to connect to the new host. We want move to seamless migration model where we don't start over from scratch, but hand over the session from the source to the target. Advantage is that various state cached in spice-client will stay valid and doesn't need to be retransmitted. It also requires a handshake between spice-servers on source and target. libvirt killing qemu on the source host before the handshake is done isn't exactly helpful. [ Side note: In theory this issue exists even today: in case the data pipe to the client is full spice-server will queue up the switch-host message and qemu might be killed before it is sent out. In practice it doesn't happen though because it goes through the low-traffic main channel so the socket buffers usually have enougth space. ] So, the big question is how to tackle the issue? Option (1): Wait until spice-server is done before signaling completion to libvirt. This is what this patch series implements. Advantage is that it is completely transparent for libvirt, thats why I like it. Disadvantage is that it indeed adds a small delay for the spice-server handshake. The target qemu doesn't process main
[Qemu-devel] [RFC PATCH 5/5] spice: turn spice migration end handler to be async
Instead of immediatly calling the notifier completion callback,
the notification handler assigns a completion callback
to spice's migration interface. Spice should call this
callback when it completes handling the migration state change.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 29 ++---
1 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index d85c212..053f06f 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -282,9 +282,14 @@ typedef struct SpiceMigration {
MonitorCompletion *cb;
void *opaque;
} connect_complete;
+struct {
+NotifiedCompletionFunc *cb;
+void *opaque;
+} end_complete;
} SpiceMigration;
static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin);
static const SpiceMigrateInterface migrate_interface = {
.base.type = SPICE_INTERFACE_MIGRATION,
@@ -292,7 +297,7 @@ static const SpiceMigrateInterface migrate_interface = {
.base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
.base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
.migrate_connect_complete = migrate_connect_complete_cb,
-.migrate_end_complete = NULL,
+.migrate_end_complete = migrate_end_complete_cb,
};
static SpiceMigration spice_migrate;
@@ -305,6 +310,15 @@ static void
migrate_connect_complete_cb(SpiceMigrateInstance *sin)
}
sm-connect_complete.cb = NULL;
}
+
+static void migrate_end_complete_cb(SpiceMigrateInstance *sin)
+{
+SpiceMigration *sm = container_of(sin, SpiceMigration, sin);
+if (sm-end_complete.cb) {
+sm-end_complete.cb(migrate_end_notifier, sm-end_complete.opaque);
+}
+sm-end_complete.cb = NULL;
+}
#endif
/* config string parsing */
@@ -492,16 +506,17 @@ static void migrate_end_notify_func(AsyncNotifier
*notifier, void *data,
void *cb_data)
{
bool success_end = *(bool *)data;
+
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_migrate.end_complete.cb = complete_cb;
+spice_migrate.end_complete.opaque = cb_data;
+spice_server_migrate_end(spice_server, success_end);
+#else
if (success_end) {
-#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
-#else
-spice_server_migrate_end(spice_server, true);
-} else {
-spice_server_migrate_end(spice_server, false);
-#endif
}
complete_cb(notifier, cb_data);
+#endif
}
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
--
1.7.7.6
[Qemu-devel] [RFC PATCH 4/5] migration: replace migration state change notifier with async notifiers
The patch replaces the existing state change notifier list, with two
explicit notifier lists for migration start and migration end. Unlike
the previous notifier list, the current notifications take place before
the actual status change, and also allow async handlers. Hence, it is
possible to delay the migration progress, till the async handlers have
completed.
Note that this patch still leaves the registered notifier handlers synchronous,
i.e.,
they call the notifier completion callback immediately.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
migration.c | 84 +-
migration.h |8 -
ui/spice-core.c | 31 ++--
3 files changed, 84 insertions(+), 39 deletions(-)
diff --git a/migration.c b/migration.c
index c86611d..869a8ab 100644
--- a/migration.c
+++ b/migration.c
@@ -51,8 +51,15 @@ enum {
#define MAX_THROTTLE (32 20) /* Migration speed throttling */
-static NotifierList migration_state_notifiers =
-NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
+static void migrate_start(void *opaque);
+static void migrate_end(void *opaque);
+
+static NotifierList migration_start_notifiers =
+ASYNC_NOTIFIER_LIST_INITIALIZER(migration_start_notifiers,
+migrate_start, NULL);
+static NotifierList migration_end_notifiers =
+ASYNC_NOTIFIER_LIST_INITIALIZER(migration_end_notifiers,
+migrate_end, NULL);
/* When we add fault tolerance, we could have several
migrations at once. For now we don't need to add
@@ -187,31 +194,44 @@ static int migrate_fd_cleanup(MigrationState *s)
return ret;
}
-static void migrate_end(MigrationState *s, int end_state)
+static void migrate_notify_end(MigrationState *s, int end_state)
+{
+bool migrate_success = (end_state == MIG_STATE_COMPLETED);
+
+if (!s-end_was_notified) {
+s-end_state = end_state;
+migration_end_notifiers.complete_cb = migrate_end;
+migration_end_notifiers.complete_opaque = s;
+s-end_was_notified = true;
+notifier_list_notify(migration_end_notifiers, migrate_success);
+}
+}
+
+static void migrate_end(void *opaque)
{
-s-state = end_state;
+MigrationState *s = opaque;
+
+s-state = s-end_state;
if (s-state == MIG_STATE_COMPLETED) {
runstate_set(RUN_STATE_POSTMIGRATE);
} else if (s-state == MIG_STATE_ERROR s-start_vm_in_error) {
vm_start();
}
-notifier_list_notify(migration_state_notifiers, s);
}
void migrate_fd_error(MigrationState *s)
{
-DPRINTF(setting error state\n);
migrate_fd_cleanup(s);
-migrate_end(s, MIG_STATE_ERROR);
+migrate_notify_end(s, MIG_STATE_ERROR);
}
static void migrate_fd_completed(MigrationState *s)
{
DPRINTF(setting completed state\n);
if (migrate_fd_cleanup(s) 0) {
-migrate_end(s, MIG_STATE_ERROR);
+migrate_notify_end(s, MIG_STATE_ERROR);
} else {
-migrate_end(s, MIG_STATE_COMPLETED);
+migrate_notify_end(s, MIG_STATE_COMPLETED);
}
}
@@ -281,14 +301,16 @@ static void migrate_fd_put_ready(void *opaque)
static void migrate_fd_cancel(MigrationState *s)
{
-if (s-state != MIG_STATE_ACTIVE)
+if (s-state != MIG_STATE_ACTIVE ||
+notifier_list_async_waiting(migration_end_notifiers)) {
return;
+}
DPRINTF(cancelling migration\n);
qemu_savevm_state_cancel(s-file);
migrate_fd_cleanup(s);
-migrate_end(s, MIG_STATE_CANCELLED);
+migrate_notify_end(s, MIG_STATE_CANCELLED);
}
static void migrate_fd_wait_for_unfreeze(void *opaque)
@@ -322,14 +344,19 @@ static int migrate_fd_close(void *opaque)
return s-close(s);
}
-void add_migration_state_change_notifier(Notifier *notify)
+void migration_add_start_notifier(AsyncNotifier *notify)
+{
+notifier_list_add_async(migration_start_notifiers, notify);
+}
+
+void migration_add_end_notifier(AsyncNotifier *notify)
{
-notifier_list_add(migration_state_notifiers, notify);
+notifier_list_add_async(migration_end_notifiers, notify);
}
-void remove_migration_state_change_notifier(Notifier *notify)
+void migration_remove_state_notifer(AsyncNotifier *notifier)
{
-notifier_remove(notify-base);
+notifier_remove(notifier-base);
}
bool migration_is_active(MigrationState *s)
@@ -401,13 +428,15 @@ void migrate_del_blocker(Error *reason)
migration_blockers = g_slist_remove(migration_blockers, reason);
}
-static void migrate_start(MigrationState *s, Error **errp)
+static void migrate_start(void *opaque)
{
+MigrationState *s = opaque;
int ret;
+Error *err = NULL;
switch (s-protocol) {
case MIGRATION_PROTOCOL_TCP:
-ret = tcp_start_outgoing_migration(s, s-protocol_param, errp);
+ret = tcp_start_outgoing_migration(s, s-protocol_param, err);
break;
#if !defined(WIN32)
case MIGRATION_PROTOCOL_EXEC
[Qemu-devel] [RFC PATCH 3/5] migration: moving migration completion code to a separated routine
Preparation for asynchronous migration state change notifiers.
In the following patch the migrate_end routine will be used as
the completion callback of the migration end notifiers list.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
migration.c | 31 ---
migration.h |1 +
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/migration.c b/migration.c
index 91c807d..c86611d 100644
--- a/migration.c
+++ b/migration.c
@@ -187,24 +187,32 @@ static int migrate_fd_cleanup(MigrationState *s)
return ret;
}
+static void migrate_end(MigrationState *s, int end_state)
+{
+s-state = end_state;
+if (s-state == MIG_STATE_COMPLETED) {
+runstate_set(RUN_STATE_POSTMIGRATE);
+} else if (s-state == MIG_STATE_ERROR s-start_vm_in_error) {
+vm_start();
+}
+notifier_list_notify(migration_state_notifiers, s);
+}
+
void migrate_fd_error(MigrationState *s)
{
DPRINTF(setting error state\n);
-s-state = MIG_STATE_ERROR;
-notifier_list_notify(migration_state_notifiers, s);
migrate_fd_cleanup(s);
+migrate_end(s, MIG_STATE_ERROR);
}
static void migrate_fd_completed(MigrationState *s)
{
DPRINTF(setting completed state\n);
if (migrate_fd_cleanup(s) 0) {
-s-state = MIG_STATE_ERROR;
+migrate_end(s, MIG_STATE_ERROR);
} else {
-s-state = MIG_STATE_COMPLETED;
-runstate_set(RUN_STATE_POSTMIGRATE);
+migrate_end(s, MIG_STATE_COMPLETED);
}
-notifier_list_notify(migration_state_notifiers, s);
}
static void migrate_fd_put_notify(void *opaque)
@@ -257,7 +265,7 @@ static void migrate_fd_put_ready(void *opaque)
if (ret 0) {
migrate_fd_error(s);
} else if (ret == 1) {
-int old_vm_running = runstate_is_running();
+s-start_vm_in_error = runstate_is_running();
DPRINTF(done iterating\n);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
@@ -268,11 +276,6 @@ static void migrate_fd_put_ready(void *opaque)
} else {
migrate_fd_completed(s);
}
-if (s-state != MIG_STATE_COMPLETED) {
-if (old_vm_running) {
-vm_start();
-}
-}
}
}
@@ -283,11 +286,9 @@ static void migrate_fd_cancel(MigrationState *s)
DPRINTF(cancelling migration\n);
-s-state = MIG_STATE_CANCELLED;
-notifier_list_notify(migration_state_notifiers, s);
qemu_savevm_state_cancel(s-file);
-
migrate_fd_cleanup(s);
+migrate_end(s, MIG_STATE_CANCELLED);
}
static void migrate_fd_wait_for_unfreeze(void *opaque)
diff --git a/migration.h b/migration.h
index 5ad67d7..6a0f49f 100644
--- a/migration.h
+++ b/migration.h
@@ -35,6 +35,7 @@ struct MigrationState
int shared;
int protocol;
char *protocol_param;
+bool start_vm_in_error;
};
void process_incoming_migration(QEMUFile *f);
--
1.7.7.6
[Qemu-devel] [RFC PATCH 0/5] asynchronous migration state change handlers
Hi, I'm sending this patch series again. This time with an additional patch for setting a migrate_end notifier completion callback for spice migration interface. I've also added more detailed commit messages. This patch series introduces async handlers for notifiers, and integrates them with migration state change notifications. Asynchronous migration completion notifier is essential for allowing spice to cleanly complete the src server connection to the client, and transfer the connection to the target. Currently, as soon as the migration completes, the src qemu can be closed by the management, and spice cannot complete the spice-connection migration. In order to support spice seamless migration, next to these patches, I plan to add: (1) notifier for switching from the live phase of the migration to the non-live phase, before completing savevm. Spice will use this notification to finalize the connection to the client: send and receive all in-flight data. (2) add vmstates for spice data that need to be migrated, e.g., usb/agent/smartcard buffers that were sent from the client and haven't been written to device yet. We would also want to migrate data that will allow us to continue the new spice connection from the same point the old one stopped. Without requiring special treatment in the client side. Regards, Yonit. Yonit Halperin (5): notifiers: add support for async notifiers handlers migration: moving migration start code to a separated routine migration: moving migration completion code to a separated routine migration: replace migration state change notifier with async notifiers spice: turn spice migration end handler to be async input.c |2 +- migration.c | 154 --- migration.h | 11 +++- notify.c| 78 ++-- notify.h| 55 ++-- qemu-timer.c|2 +- ui/spice-core.c | 58 +++-- vl.c|2 +- 8 files changed, 290 insertions(+), 72 deletions(-) -- 1.7.7.6
[Qemu-devel] [RFC PATCH 1/5] notifiers: add support for async notifiers handlers
This patch defines 2 subtypes of notifiers, sync and async. Both of
them can be added to a notifiers list.
The patch adds optional complete_cb to the notifiers list. complete_cb is called
when all the async notifiers have completed.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
input.c |2 +-
migration.c |2 +-
notify.c | 78 +++---
notify.h | 55 +---
qemu-timer.c |2 +-
vl.c |2 +-
6 files changed, 128 insertions(+), 13 deletions(-)
diff --git a/input.c b/input.c
index 6968b31..06f6f9f 100644
--- a/input.c
+++ b/input.c
@@ -274,5 +274,5 @@ void qemu_add_mouse_mode_change_notifier(Notifier *notify)
void qemu_remove_mouse_mode_change_notifier(Notifier *notify)
{
-notifier_remove(notify);
+notifier_remove(notify-base);
}
diff --git a/migration.c b/migration.c
index 3f485d3..acaf293 100644
--- a/migration.c
+++ b/migration.c
@@ -320,7 +320,7 @@ void add_migration_state_change_notifier(Notifier *notify)
void remove_migration_state_change_notifier(Notifier *notify)
{
-notifier_remove(notify);
+notifier_remove(notify-base);
}
bool migration_is_active(MigrationState *s)
diff --git a/notify.c b/notify.c
index 12282a6..c67e50e 100644
--- a/notify.c
+++ b/notify.c
@@ -19,23 +19,93 @@
void notifier_list_init(NotifierList *list)
{
QLIST_INIT(list-notifiers);
+QLIST_INIT(list-wait_notifiers);
}
void notifier_list_add(NotifierList *list, Notifier *notifier)
{
-QLIST_INSERT_HEAD(list-notifiers, notifier, node);
+notifier-base.type = NOTIFIER_TYPE_SYNC;
+QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
}
-void notifier_remove(Notifier *notifier)
+void notifier_list_add_async(NotifierList *list, AsyncNotifier *notifier)
+{
+notifier-base.type = NOTIFIER_TYPE_ASYNC;
+QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
+}
+
+void notifier_remove(BaseNotifier *notifier)
{
QLIST_REMOVE(notifier, node);
}
+static void notified_complete_cb(AsyncNotifier *notifier, void *opaque)
+{
+NotifierList *list = opaque;
+
+QLIST_REMOVE(notifier, wait_node);
+
+if (QLIST_EMPTY(list-wait_notifiers) !list-during_notify) {
+if (list-complete_cb) {
+list-complete_cb(list-complete_opaque);
+}
+}
+}
+
void notifier_list_notify(NotifierList *list, void *data)
{
-Notifier *notifier, *next;
+BaseNotifier *notifier, *next;
+bool async = false;
+
+if (notifier_list_async_waiting(list)) {
+AsyncNotifier *wait_notifier, *wait_next;
+
+fprintf(stderr, %s: previous notify hasn't completed\n, __func__);
+QLIST_FOREACH_SAFE(wait_notifier, list-wait_notifiers,
+ wait_node, wait_next) {
+QLIST_REMOVE(wait_notifier, wait_node);
+}
+}
+
+list-during_notify = true;
QLIST_FOREACH_SAFE(notifier, list-notifiers, node, next) {
-notifier-notify(notifier, data);
+switch (notifier-type) {
+case NOTIFIER_TYPE_SYNC:
+{
+Notifier *sync_notifier;
+
+sync_notifier = container_of(notifier, Notifier, base);
+sync_notifier-notify(sync_notifier, data);
+break;
+}
+case NOTIFIER_TYPE_ASYNC:
+{
+AsyncNotifier *async_notifier;
+
+async = true;
+async_notifier = container_of(notifier, AsyncNotifier, base);
+QLIST_INSERT_HEAD(list-wait_notifiers,
+ async_notifier,
+ wait_node);
+async_notifier-notify_async(async_notifier, data,
+ notified_complete_cb, list);
+break;
+}
+default:
+fprintf(stderr, %s: invalid notifier type %d\n, __func__,
+notifier-type);
+break;
+}
}
+
+list-during_notify = false;
+if ((!async || !notifier_list_async_waiting(list)) list-complete_cb) {
+list-complete_cb(list-complete_opaque);
+}
+}
+
+bool notifier_list_async_waiting(NotifierList *list)
+{
+return !QLIST_EMPTY(list-wait_notifiers);
}
diff --git a/notify.h b/notify.h
index 03cf26c..8660920 100644
--- a/notify.h
+++ b/notify.h
@@ -16,28 +16,73 @@
#include qemu-queue.h
+typedef enum NotifierType {
+NOTIFIER_TYPE_NONE,
+NOTIFIER_TYPE_SYNC,
+NOTIFIER_TYPE_ASYNC,
+} NotifierType;
+
+typedef struct BaseNotifier BaseNotifier;
+
+struct BaseNotifier {
+QLIST_ENTRY(BaseNotifier) node;
+NotifierType type;
+};
typedef struct Notifier Notifier;
struct Notifier
{
+BaseNotifier base;
void (*notify)(Notifier *notifier, void *data);
-QLIST_ENTRY(Notifier) node;
};
+typedef struct AsyncNotifier AsyncNotifier;
+typedef void (NotifiedCompletionFunc)(AsyncNotifier
[Qemu-devel] [RFC PATCH 2/5] migration: moving migration start code to a separated routine
Preparation for asynchronous migration state change notifiers.
In a following patch the migrate_start routine will be used as
the completion callback of the migration start notifiers list.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
migration.c | 73 +-
migration.h |2 +
2 files changed, 58 insertions(+), 17 deletions(-)
diff --git a/migration.c b/migration.c
index acaf293..91c807d 100644
--- a/migration.c
+++ b/migration.c
@@ -41,6 +41,14 @@ enum {
MIG_STATE_COMPLETED,
};
+enum {
+ MIGRATION_PROTOCOL_ERROR,
+ MIGRATION_PROTOCOL_TCP,
+ MIGRATION_PROTOCOL_EXEC,
+ MIGRATION_PROTOCOL_UNIX,
+ MIGRATION_PROTOCOL_FD,
+};
+
#define MAX_THROTTLE (32 20) /* Migration speed throttling */
static NotifierList migration_state_notifiers =
@@ -361,13 +369,16 @@ void migrate_fd_connect(MigrationState *s)
migrate_fd_put_ready(s);
}
-static MigrationState *migrate_init(int blk, int inc)
+static MigrationState *migrate_init(int protocol, const char *protocol_param,
+int blk, int inc)
{
MigrationState *s = migrate_get_current();
int64_t bandwidth_limit = s-bandwidth_limit;
memset(s, 0, sizeof(*s));
s-bandwidth_limit = bandwidth_limit;
+s-protocol = protocol;
+s-protocol_param = g_strdup(protocol_param);
s-blk = blk;
s-shared = inc;
@@ -389,13 +400,50 @@ void migrate_del_blocker(Error *reason)
migration_blockers = g_slist_remove(migration_blockers, reason);
}
+static void migrate_start(MigrationState *s, Error **errp)
+{
+int ret;
+
+switch (s-protocol) {
+case MIGRATION_PROTOCOL_TCP:
+ret = tcp_start_outgoing_migration(s, s-protocol_param, errp);
+break;
+#if !defined(WIN32)
+case MIGRATION_PROTOCOL_EXEC:
+ret = exec_start_outgoing_migration(s, s-protocol_param);
+break;
+case MIGRATION_PROTOCOL_UNIX:
+ret = unix_start_outgoing_migration(s, s-protocol_param);
+break;
+case MIGRATION_PROTOCOL_FD:
+ret = fd_start_outgoing_migration(s, s-protocol_param);
+break;
+#endif
+default:
+ret = -EPROTONOSUPPORT;
+}
+
+g_free(s-protocol_param);
+s-protocol_param = NULL;
+
+if (ret 0) {
+if (!error_is_set(errp)) {
+DPRINTF(migration failed: %s\n, strerror(-ret));
+/* FIXME: we should return meaningful errors */
+error_set(errp, QERR_UNDEFINED_ERROR);
+}
+return;
+}
+notifier_list_notify(migration_state_notifiers, s);
+}
+
void qmp_migrate(const char *uri, bool has_blk, bool blk,
bool has_inc, bool inc, bool has_detach, bool detach,
Error **errp)
{
MigrationState *s = migrate_get_current();
const char *p;
-int ret;
+int migrate_protocol;
if (s-state == MIG_STATE_ACTIVE) {
error_set(errp, QERR_MIGRATION_ACTIVE);
@@ -411,33 +459,24 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
return;
}
-s = migrate_init(blk, inc);
-
if (strstart(uri, tcp:, p)) {
-ret = tcp_start_outgoing_migration(s, p, errp);
+migrate_protocol = MIGRATION_PROTOCOL_TCP;
#if !defined(WIN32)
} else if (strstart(uri, exec:, p)) {
-ret = exec_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_EXEC;
} else if (strstart(uri, unix:, p)) {
-ret = unix_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_UNIX;
} else if (strstart(uri, fd:, p)) {
-ret = fd_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_FD;
#endif
} else {
error_set(errp, QERR_INVALID_PARAMETER_VALUE, uri, a valid
migration protocol);
return;
}
+s = migrate_init(migrate_protocol, p, blk, inc);
-if (ret 0) {
-if (!error_is_set(errp)) {
-DPRINTF(migration failed: %s\n, strerror(-ret));
-/* FIXME: we should return meaningful errors */
-error_set(errp, QERR_UNDEFINED_ERROR);
-}
-return;
-}
+migrate_start(s, errp);
-notifier_list_notify(migration_state_notifiers, s);
}
void qmp_migrate_cancel(Error **errp)
diff --git a/migration.h b/migration.h
index 2e9ca2e..5ad67d7 100644
--- a/migration.h
+++ b/migration.h
@@ -33,6 +33,8 @@ struct MigrationState
void *opaque;
int blk;
int shared;
+int protocol;
+char *protocol_param;
};
void process_incoming_migration(QEMUFile *f);
--
1.7.7.6
[Qemu-devel] [RFC PATCH 0/4] asynchronous migration state change handlers
Hi, This patch series introduces async handlers for notifiers, and integrates them with migration state change notifications. Asynchronous migration completion notifier is essential for allowing spice to cleanly complete the src server connection to the client and transfer it to the target. Currently, as soon as the migration is completed, the src qemu can be closed by the management, and spice cannot complete the spice-connection migration. In order to support spice seamless migration, next to these patches, I would also like to add asynchronous vmstate pre_save. This will allow spice to employ vmstate for migrating spice in-flight data, e.g., usb buffers that were sent from the client and reached the server after the vm was stopped. Regards, Yonit. Yonit Halperin (4): notifiers: add support for async notifiers handlers migration: moving migration start code to a separated routine migration: moving migration completion code to a separated routine migration: replace migration state change notifier with async notifiers input.c |2 +- migration.c | 154 --- migration.h | 11 +++- notify.c| 79 +++-- notify.h| 55 ++-- qemu-timer.c|2 +- ui/spice-core.c | 31 vl.c|2 +- 8 files changed, 270 insertions(+), 66 deletions(-) -- 1.7.7.6
[Qemu-devel] [RFC PATCH 1/4] notifiers: add support for async notifiers handlers
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
input.c |2 +-
migration.c |2 +-
notify.c | 79 +++---
notify.h | 55 ---
qemu-timer.c |2 +-
vl.c |2 +-
6 files changed, 129 insertions(+), 13 deletions(-)
diff --git a/input.c b/input.c
index 6968b31..06f6f9f 100644
--- a/input.c
+++ b/input.c
@@ -274,5 +274,5 @@ void qemu_add_mouse_mode_change_notifier(Notifier *notify)
void qemu_remove_mouse_mode_change_notifier(Notifier *notify)
{
-notifier_remove(notify);
+notifier_remove(notify-base);
}
diff --git a/migration.c b/migration.c
index 3f485d3..acaf293 100644
--- a/migration.c
+++ b/migration.c
@@ -320,7 +320,7 @@ void add_migration_state_change_notifier(Notifier *notify)
void remove_migration_state_change_notifier(Notifier *notify)
{
-notifier_remove(notify);
+notifier_remove(notify-base);
}
bool migration_is_active(MigrationState *s)
diff --git a/notify.c b/notify.c
index 12282a6..dde190e 100644
--- a/notify.c
+++ b/notify.c
@@ -19,23 +19,94 @@
void notifier_list_init(NotifierList *list)
{
QLIST_INIT(list-notifiers);
+QLIST_INIT(list-wait_notifiers);
}
void notifier_list_add(NotifierList *list, Notifier *notifier)
{
-QLIST_INSERT_HEAD(list-notifiers, notifier, node);
+notifier-base.type = NOTIFIER_TYPE_SYNC;
+QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
}
-void notifier_remove(Notifier *notifier)
+void notifier_list_add_async(NotifierList *list, AsyncNotifier *notifier)
+{
+notifier-base.type = NOTIFIER_TYPE_ASYNC;
+QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
+}
+
+void notifier_remove(BaseNotifier *notifier)
{
QLIST_REMOVE(notifier, node);
}
+static void notified_complete_cb(AsyncNotifier *notifier, void *opaque)
+{
+NotifierList *list = opaque;
+
+QLIST_REMOVE(notifier, wait_node);
+
+if (QLIST_EMPTY(list-wait_notifiers) !list-during_notify) {
+if (list-complete_cb) {
+list-complete_cb(list-complete_opaque);
+}
+}
+}
+
void notifier_list_notify(NotifierList *list, void *data)
{
-Notifier *notifier, *next;
+BaseNotifier *notifier, *next;
+bool async = false;
+
+if (notifier_list_async_waiting(list)) {
+AsyncNotifier *wait_notifier, *wait_next;
+
+fprintf(stderr, %s: previous notify hasn't completed\n, __func__);
+QLIST_FOREACH_SAFE(wait_notifier, list-wait_notifiers,
+ wait_node, wait_next) {
+QLIST_REMOVE(wait_notifier, wait_node);
+}
+
+}
+
+list-during_notify = true;
QLIST_FOREACH_SAFE(notifier, list-notifiers, node, next) {
-notifier-notify(notifier, data);
+switch (notifier-type) {
+case NOTIFIER_TYPE_SYNC:
+{
+Notifier *sync_notifier;
+
+sync_notifier = container_of(notifier, Notifier, base);
+sync_notifier-notify(sync_notifier, data);
+break;
+}
+case NOTIFIER_TYPE_ASYNC:
+{
+AsyncNotifier *async_notifier;
+
+async = true;
+async_notifier = container_of(notifier, AsyncNotifier, base);
+QLIST_INSERT_HEAD(list-wait_notifiers,
+ async_notifier,
+ wait_node);
+async_notifier-notify_async(async_notifier, data,
+ notified_complete_cb, list);
+break;
+}
+default:
+fprintf(stderr, %s: invalid notifier type %d\n, __func__,
+notifier-type);
+break;
+}
}
+
+list-during_notify = false;
+if ((!async || !notifier_list_async_waiting(list)) list-complete_cb) {
+list-complete_cb(list-complete_opaque);
+}
+}
+
+bool notifier_list_async_waiting(NotifierList *list)
+{
+return !QLIST_EMPTY(list-wait_notifiers);
}
diff --git a/notify.h b/notify.h
index 03cf26c..8660920 100644
--- a/notify.h
+++ b/notify.h
@@ -16,28 +16,73 @@
#include qemu-queue.h
+typedef enum NotifierType {
+NOTIFIER_TYPE_NONE,
+NOTIFIER_TYPE_SYNC,
+NOTIFIER_TYPE_ASYNC,
+} NotifierType;
+
+typedef struct BaseNotifier BaseNotifier;
+
+struct BaseNotifier {
+QLIST_ENTRY(BaseNotifier) node;
+NotifierType type;
+};
typedef struct Notifier Notifier;
struct Notifier
{
+BaseNotifier base;
void (*notify)(Notifier *notifier, void *data);
-QLIST_ENTRY(Notifier) node;
};
+typedef struct AsyncNotifier AsyncNotifier;
+typedef void (NotifiedCompletionFunc)(AsyncNotifier *notifier, void *opaque);
+
+struct AsyncNotifier {
+BaseNotifier base;
+void (*notify_async)(AsyncNotifier *notifier, void *data,
+ NotifiedCompletionFunc *complete_cb, void *cb_data);
+QLIST_ENTRY
[Qemu-devel] [RFC PATCH 2/4] migration: moving migration start code to a separated routine
Preparation for asynchronous migration state change notifiers.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
migration.c | 73 +-
migration.h |2 +
2 files changed, 58 insertions(+), 17 deletions(-)
diff --git a/migration.c b/migration.c
index acaf293..91c807d 100644
--- a/migration.c
+++ b/migration.c
@@ -41,6 +41,14 @@ enum {
MIG_STATE_COMPLETED,
};
+enum {
+ MIGRATION_PROTOCOL_ERROR,
+ MIGRATION_PROTOCOL_TCP,
+ MIGRATION_PROTOCOL_EXEC,
+ MIGRATION_PROTOCOL_UNIX,
+ MIGRATION_PROTOCOL_FD,
+};
+
#define MAX_THROTTLE (32 20) /* Migration speed throttling */
static NotifierList migration_state_notifiers =
@@ -361,13 +369,16 @@ void migrate_fd_connect(MigrationState *s)
migrate_fd_put_ready(s);
}
-static MigrationState *migrate_init(int blk, int inc)
+static MigrationState *migrate_init(int protocol, const char *protocol_param,
+int blk, int inc)
{
MigrationState *s = migrate_get_current();
int64_t bandwidth_limit = s-bandwidth_limit;
memset(s, 0, sizeof(*s));
s-bandwidth_limit = bandwidth_limit;
+s-protocol = protocol;
+s-protocol_param = g_strdup(protocol_param);
s-blk = blk;
s-shared = inc;
@@ -389,13 +400,50 @@ void migrate_del_blocker(Error *reason)
migration_blockers = g_slist_remove(migration_blockers, reason);
}
+static void migrate_start(MigrationState *s, Error **errp)
+{
+int ret;
+
+switch (s-protocol) {
+case MIGRATION_PROTOCOL_TCP:
+ret = tcp_start_outgoing_migration(s, s-protocol_param, errp);
+break;
+#if !defined(WIN32)
+case MIGRATION_PROTOCOL_EXEC:
+ret = exec_start_outgoing_migration(s, s-protocol_param);
+break;
+case MIGRATION_PROTOCOL_UNIX:
+ret = unix_start_outgoing_migration(s, s-protocol_param);
+break;
+case MIGRATION_PROTOCOL_FD:
+ret = fd_start_outgoing_migration(s, s-protocol_param);
+break;
+#endif
+default:
+ret = -EPROTONOSUPPORT;
+}
+
+g_free(s-protocol_param);
+s-protocol_param = NULL;
+
+if (ret 0) {
+if (!error_is_set(errp)) {
+DPRINTF(migration failed: %s\n, strerror(-ret));
+/* FIXME: we should return meaningful errors */
+error_set(errp, QERR_UNDEFINED_ERROR);
+}
+return;
+}
+notifier_list_notify(migration_state_notifiers, s);
+}
+
void qmp_migrate(const char *uri, bool has_blk, bool blk,
bool has_inc, bool inc, bool has_detach, bool detach,
Error **errp)
{
MigrationState *s = migrate_get_current();
const char *p;
-int ret;
+int migrate_protocol;
if (s-state == MIG_STATE_ACTIVE) {
error_set(errp, QERR_MIGRATION_ACTIVE);
@@ -411,33 +459,24 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
return;
}
-s = migrate_init(blk, inc);
-
if (strstart(uri, tcp:, p)) {
-ret = tcp_start_outgoing_migration(s, p, errp);
+migrate_protocol = MIGRATION_PROTOCOL_TCP;
#if !defined(WIN32)
} else if (strstart(uri, exec:, p)) {
-ret = exec_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_EXEC;
} else if (strstart(uri, unix:, p)) {
-ret = unix_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_UNIX;
} else if (strstart(uri, fd:, p)) {
-ret = fd_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_FD;
#endif
} else {
error_set(errp, QERR_INVALID_PARAMETER_VALUE, uri, a valid
migration protocol);
return;
}
+s = migrate_init(migrate_protocol, p, blk, inc);
-if (ret 0) {
-if (!error_is_set(errp)) {
-DPRINTF(migration failed: %s\n, strerror(-ret));
-/* FIXME: we should return meaningful errors */
-error_set(errp, QERR_UNDEFINED_ERROR);
-}
-return;
-}
+migrate_start(s, errp);
-notifier_list_notify(migration_state_notifiers, s);
}
void qmp_migrate_cancel(Error **errp)
diff --git a/migration.h b/migration.h
index 2e9ca2e..5ad67d7 100644
--- a/migration.h
+++ b/migration.h
@@ -33,6 +33,8 @@ struct MigrationState
void *opaque;
int blk;
int shared;
+int protocol;
+char *protocol_param;
};
void process_incoming_migration(QEMUFile *f);
--
1.7.7.6
[Qemu-devel] [RFC PATCH 3/4] migration: moving migration completion code to a separated routine
Preparation for asynchronous migration state change notifiers.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
migration.c | 31 ---
migration.h |1 +
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/migration.c b/migration.c
index 91c807d..c86611d 100644
--- a/migration.c
+++ b/migration.c
@@ -187,24 +187,32 @@ static int migrate_fd_cleanup(MigrationState *s)
return ret;
}
+static void migrate_end(MigrationState *s, int end_state)
+{
+s-state = end_state;
+if (s-state == MIG_STATE_COMPLETED) {
+runstate_set(RUN_STATE_POSTMIGRATE);
+} else if (s-state == MIG_STATE_ERROR s-start_vm_in_error) {
+vm_start();
+}
+notifier_list_notify(migration_state_notifiers, s);
+}
+
void migrate_fd_error(MigrationState *s)
{
DPRINTF(setting error state\n);
-s-state = MIG_STATE_ERROR;
-notifier_list_notify(migration_state_notifiers, s);
migrate_fd_cleanup(s);
+migrate_end(s, MIG_STATE_ERROR);
}
static void migrate_fd_completed(MigrationState *s)
{
DPRINTF(setting completed state\n);
if (migrate_fd_cleanup(s) 0) {
-s-state = MIG_STATE_ERROR;
+migrate_end(s, MIG_STATE_ERROR);
} else {
-s-state = MIG_STATE_COMPLETED;
-runstate_set(RUN_STATE_POSTMIGRATE);
+migrate_end(s, MIG_STATE_COMPLETED);
}
-notifier_list_notify(migration_state_notifiers, s);
}
static void migrate_fd_put_notify(void *opaque)
@@ -257,7 +265,7 @@ static void migrate_fd_put_ready(void *opaque)
if (ret 0) {
migrate_fd_error(s);
} else if (ret == 1) {
-int old_vm_running = runstate_is_running();
+s-start_vm_in_error = runstate_is_running();
DPRINTF(done iterating\n);
qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
@@ -268,11 +276,6 @@ static void migrate_fd_put_ready(void *opaque)
} else {
migrate_fd_completed(s);
}
-if (s-state != MIG_STATE_COMPLETED) {
-if (old_vm_running) {
-vm_start();
-}
-}
}
}
@@ -283,11 +286,9 @@ static void migrate_fd_cancel(MigrationState *s)
DPRINTF(cancelling migration\n);
-s-state = MIG_STATE_CANCELLED;
-notifier_list_notify(migration_state_notifiers, s);
qemu_savevm_state_cancel(s-file);
-
migrate_fd_cleanup(s);
+migrate_end(s, MIG_STATE_CANCELLED);
}
static void migrate_fd_wait_for_unfreeze(void *opaque)
diff --git a/migration.h b/migration.h
index 5ad67d7..6a0f49f 100644
--- a/migration.h
+++ b/migration.h
@@ -35,6 +35,7 @@ struct MigrationState
int shared;
int protocol;
char *protocol_param;
+bool start_vm_in_error;
};
void process_incoming_migration(QEMUFile *f);
--
1.7.7.6
[Qemu-devel] [RFC PATCH 4/4] migration: replace migration state change notifier with async notifiers
Note that this patch leaves the current notifier handlers synchronous, i.e.,
they call the notifier completion callback immediately.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
migration.c | 84 +-
migration.h |8 -
ui/spice-core.c | 31 ++--
3 files changed, 84 insertions(+), 39 deletions(-)
diff --git a/migration.c b/migration.c
index c86611d..869a8ab 100644
--- a/migration.c
+++ b/migration.c
@@ -51,8 +51,15 @@ enum {
#define MAX_THROTTLE (32 20) /* Migration speed throttling */
-static NotifierList migration_state_notifiers =
-NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
+static void migrate_start(void *opaque);
+static void migrate_end(void *opaque);
+
+static NotifierList migration_start_notifiers =
+ASYNC_NOTIFIER_LIST_INITIALIZER(migration_start_notifiers,
+migrate_start, NULL);
+static NotifierList migration_end_notifiers =
+ASYNC_NOTIFIER_LIST_INITIALIZER(migration_end_notifiers,
+migrate_end, NULL);
/* When we add fault tolerance, we could have several
migrations at once. For now we don't need to add
@@ -187,31 +194,44 @@ static int migrate_fd_cleanup(MigrationState *s)
return ret;
}
-static void migrate_end(MigrationState *s, int end_state)
+static void migrate_notify_end(MigrationState *s, int end_state)
+{
+bool migrate_success = (end_state == MIG_STATE_COMPLETED);
+
+if (!s-end_was_notified) {
+s-end_state = end_state;
+migration_end_notifiers.complete_cb = migrate_end;
+migration_end_notifiers.complete_opaque = s;
+s-end_was_notified = true;
+notifier_list_notify(migration_end_notifiers, migrate_success);
+}
+}
+
+static void migrate_end(void *opaque)
{
-s-state = end_state;
+MigrationState *s = opaque;
+
+s-state = s-end_state;
if (s-state == MIG_STATE_COMPLETED) {
runstate_set(RUN_STATE_POSTMIGRATE);
} else if (s-state == MIG_STATE_ERROR s-start_vm_in_error) {
vm_start();
}
-notifier_list_notify(migration_state_notifiers, s);
}
void migrate_fd_error(MigrationState *s)
{
-DPRINTF(setting error state\n);
migrate_fd_cleanup(s);
-migrate_end(s, MIG_STATE_ERROR);
+migrate_notify_end(s, MIG_STATE_ERROR);
}
static void migrate_fd_completed(MigrationState *s)
{
DPRINTF(setting completed state\n);
if (migrate_fd_cleanup(s) 0) {
-migrate_end(s, MIG_STATE_ERROR);
+migrate_notify_end(s, MIG_STATE_ERROR);
} else {
-migrate_end(s, MIG_STATE_COMPLETED);
+migrate_notify_end(s, MIG_STATE_COMPLETED);
}
}
@@ -281,14 +301,16 @@ static void migrate_fd_put_ready(void *opaque)
static void migrate_fd_cancel(MigrationState *s)
{
-if (s-state != MIG_STATE_ACTIVE)
+if (s-state != MIG_STATE_ACTIVE ||
+notifier_list_async_waiting(migration_end_notifiers)) {
return;
+}
DPRINTF(cancelling migration\n);
qemu_savevm_state_cancel(s-file);
migrate_fd_cleanup(s);
-migrate_end(s, MIG_STATE_CANCELLED);
+migrate_notify_end(s, MIG_STATE_CANCELLED);
}
static void migrate_fd_wait_for_unfreeze(void *opaque)
@@ -322,14 +344,19 @@ static int migrate_fd_close(void *opaque)
return s-close(s);
}
-void add_migration_state_change_notifier(Notifier *notify)
+void migration_add_start_notifier(AsyncNotifier *notify)
+{
+notifier_list_add_async(migration_start_notifiers, notify);
+}
+
+void migration_add_end_notifier(AsyncNotifier *notify)
{
-notifier_list_add(migration_state_notifiers, notify);
+notifier_list_add_async(migration_end_notifiers, notify);
}
-void remove_migration_state_change_notifier(Notifier *notify)
+void migration_remove_state_notifer(AsyncNotifier *notifier)
{
-notifier_remove(notify-base);
+notifier_remove(notifier-base);
}
bool migration_is_active(MigrationState *s)
@@ -401,13 +428,15 @@ void migrate_del_blocker(Error *reason)
migration_blockers = g_slist_remove(migration_blockers, reason);
}
-static void migrate_start(MigrationState *s, Error **errp)
+static void migrate_start(void *opaque)
{
+MigrationState *s = opaque;
int ret;
+Error *err = NULL;
switch (s-protocol) {
case MIGRATION_PROTOCOL_TCP:
-ret = tcp_start_outgoing_migration(s, s-protocol_param, errp);
+ret = tcp_start_outgoing_migration(s, s-protocol_param, err);
break;
#if !defined(WIN32)
case MIGRATION_PROTOCOL_EXEC:
@@ -425,17 +454,18 @@ static void migrate_start(MigrationState *s, Error **errp)
}
g_free(s-protocol_param);
-s-protocol_param = NULL;
-
if (ret 0) {
-if (!error_is_set(errp)) {
-DPRINTF(migration failed: %s\n, strerror(-ret));
-/* FIXME: we should return meaningful errors */
-error_set(errp, QERR_UNDEFINED_ERROR
[Qemu-devel] [PATCH] monitor: fix client_migrate_info error handling
Report QERR_INVALID_PARAMETER when port is missing. Otherwise
QERR_UNDEFINED_ERROR will occur.
rhbz #795652
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
monitor.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/monitor.c b/monitor.c
index d57e7bf..8f46031 100644
--- a/monitor.c
+++ b/monitor.c
@@ -880,6 +880,11 @@ static int client_migrate_info(Monitor *mon, const QDict
*qdict,
return -1;
}
+if (port == -1 tls_port == -1) {
+qerror_report(QERR_MISSING_PARAMETER, port/tls-port);
+return -1;
+}
+
ret = qemu_spice_migrate_info(hostname, port, tls_port, subject,
cb, opaque);
if (ret != 0) {
--
1.7.7.6
Re: [Qemu-devel] [Spice-devel] seamless migration with spice
Hi, On 03/13/2012 08:40 AM, Gerd Hoffmann wrote: On 03/12/12 19:45, Yonit Halperin wrote: Hi, On 03/12/2012 03:50 PM, Gerd Hoffmann wrote: Hi, Can you explain/exemplify, why sending data as a blob (either by (a) or (b)), that is verified only by the two ends that actually use it, is a problem? It tends to be not very robust. Especially when the creating/parsing is done ad-hoc and the format changes now and then due to more info needing to be stored later on. The qemu migration format which has almost no structure breaks now and then because of that. Thus I'd prefer to not go down this route when creating something new. cheers, Gerd Exposing spice server internals to the client/qemu seems to me more vulnerable then sending it as a blob. That also depends on what and how much we need to transfer. Nonetheless, it introduces more complexity to backward compatibility support and it will need to involve not only the capabilities/versions of the server but also those of the qemu/client Backward compatibility isn't that easy both ways. It is not easy when you have 2 components, and it is much less easy when you have 3 or 4 components. So why make it more complicated if you can avoid it. Especially since there is no functional reason for making the qemu/client capabilities/versions dependent on the server internal data. .Which reminds me, that we also need capabilities negotiation for the migration protocol between the src and the destination. If this is a hard requirement then using the vmstate channel isn't going to work. The vmstate is a one-way channel, no way to negotiate anything between source and target. We can do this via the client. Regards, Yonit. cheers, Gerd
Re: [Qemu-devel] [Spice-devel] seamless migration with spice
On 03/12/2012 11:46 AM, Gerd Hoffmann wrote: Hi, The problem with (b) is, that iirc the way b was implemented in the past was still the big blob approach, but then pass the blob through the client, which means an evil client could modify it, causing all sorts of interesting behavior inside spice-server. Since we're re-implementing this to me the send a blob through the client approach is simply not acceptable from a security pov, also see my previous mail in this thread. Agree. It should be a normal spice message which goes through the spice marshaller for parsing sanity checking. I disagree. Note that there is more info to send over then just which surfaces / images are cached by the client. There also is things like partial complete agent channel messages, including how much bytes must be read to complete the command, etc. Is there a complete list of the session state we need to save? IMHO (b) would only be acceptable if the data send through the client stops becoming a blob. Using (a) to send a blob isn't better. Gerd/Hans, Can you explain/exemplify, why sending data as a blob (either by (a) or (b)), that is verified only by the two ends that actually use it, is a problem? Lets say the client/qemu are completely aware of the migration data, what prevent it from harming it then? Instead the client could simply send a list of all surface ids, etc. which it has cached after it connects to / starts using the new host. Note that the old hosts needs to send nothing for this, this is info the client already has, also removing the need for synchronization. Yes, some session state is known to the client anyway so we don't need a source- target channel for them. As for certain other data, such as (but not limited to) partially parsed agent messages, these should be send through the regular vmstate methods IMHO. That isn't easy to handle via vmstate, at least as soon as this goes beyond a fixed number of fields aka 'migrate over this struct for me'. Think multiple spice clients connected at the same time. 1) Do (a), sending everything that way 2) Do (a) sending non client state that way; and let the client send state like which surfaces it has cached when the new session starts. I think we have to look at each piece of state information needed by the target and look how to handle this best. cheers, Gerd ___ Spice-devel mailing list spice-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Qemu-devel] [Spice-devel] seamless migration with spice
Hi, On 03/12/2012 03:50 PM, Gerd Hoffmann wrote: Hi, Can you explain/exemplify, why sending data as a blob (either by (a) or (b)), that is verified only by the two ends that actually use it, is a problem? It tends to be not very robust. Especially when the creating/parsing is done ad-hoc and the format changes now and then due to more info needing to be stored later on. The qemu migration format which has almost no structure breaks now and then because of that. Thus I'd prefer to not go down this route when creating something new. cheers, Gerd Exposing spice server internals to the client/qemu seems to me more vulnerable then sending it as a blob. Nonetheless, it introduces more complexity to backward compatibility support and it will need to involve not only the capabilities/versions of the server but also those of the qemu/client. Which reminds me, that we also need capabilities negotiation for the migration protocol between the src and the destination. Regards, Yonit.
[Qemu-devel] seamless migration with spice
Hi, We would like to implement seamless migration for Spice, i.e., keeping the currently opened spice client session valid after migration. Today, the spice client establishes the connection to the destination before migration starts, and when migration completes, the client's session is moved to the destination, but all the session data is being reset. We face 2 main challenges when coming to implement seamless migration: (1) Spice client must establish the connection to the destination before the spice password expires. However, during migration, qemu main loop is not processed, and when migration completes, the password might have already expired. Today we solve this by the async command client_migrate_info, which is expected to be called before migration starts. The command is completed once spice client has connected to the destination (or a timeout). Since async monitor commands are no longer supported, we are looking for a new solution. The straightforward solution would be to process the main loop on the destination side during migration. (2) In order to restore the source-client spice session in the destination, we need to pass data from the source to the destination. Example for such data: in flight copy paste data, in flight usb data We want to pass the data from the source spice server to the destination, via Spice client. This introduces a possible race: after migration completes, the source qemu can be killed before the spice-server completes transferring the migration data to the client. Possible solutions: - Have an async migration state notifiers. The migration state will change after all the notifiers complete callbacks are called. - libvirt will wait for qmp event corresponding to spice completing its migration, and only then will kill the source qemu process. Any thoughts? Thanks, Yonit.
Re: [Qemu-devel] seamless migration with spice
Hi. On 03/11/2012 05:36 PM, Anthony Liguori wrote: On 03/11/2012 10:25 AM, Alon Levy wrote: On Sun, Mar 11, 2012 at 09:18:17AM -0500, Anthony Liguori wrote: On 03/11/2012 08:16 AM, Yonit Halperin wrote: Hi, We would like to implement seamless migration for Spice, i.e., keeping the currently opened spice client session valid after migration. Today, the spice client establishes the connection to the destination before migration starts, and when migration completes, the client's session is moved to the destination, but all the session data is being reset. We face 2 main challenges when coming to implement seamless migration: (1) Spice client must establish the connection to the destination before the spice password expires. However, during migration, qemu main loop is not processed, and when migration completes, the password might have already expired. Today we solve this by the async command client_migrate_info, which is expected to be called before migration starts. The command is completed once spice client has connected to the destination (or a timeout). Since async monitor commands are no longer supported, we are looking for a new solution. We need to fix async monitor commands. Luiz sent a note our to qemu-devel recently on this topic. I'm not sure we'll get there for 1.1 but if we do a 3 month release cycle for 1.2, then that's a pretty reasonable target IMHO. What about the second part? it's independant of the async issue. Isn't this a client problem? The client has this state, no? No, part of the data is server specific. If the state is stored in the server, wouldn't it be marshaled as part of the server's migration state? We currently don't restore the server state. That is the problem we want to solve. I meant that the server state can be marshaled from the source to the client, and from the client to the destination. The client serves as the mediator. Another option that we thought about was using save/load vmstate. Regards, Yonit. I read that as the client needs to marshal it's own local state in the session and restore it in the new session. Regards, Anthony Liguori Regards, Anthony Liguori The straightforward solution would be to process the main loop on the destination side during migration. (2) In order to restore the source-client spice session in the destination, we need to pass data from the source to the destination. Example for such data: in flight copy paste data, in flight usb data We want to pass the data from the source spice server to the destination, via Spice client. This introduces a possible race: after migration completes, the source qemu can be killed before the spice-server completes transferring the migration data to the client. Possible solutions: - Have an async migration state notifiers. The migration state will change after all the notifiers complete callbacks are called. - libvirt will wait for qmp event corresponding to spice completing its migration, and only then will kill the source qemu process. Any thoughts? Thanks, Yonit.
[Qemu-devel] [PATCH v2 1/2] qxl: set only off-screen surfaces dirty instead of the whole vram
We used to assure the guest surfaces were saved before migration by
setting the whole vram dirty. This patch sets dirty only the areas
that are actually used in the vram.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c | 53 -
1 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index bc03c1d..df55de1 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1006,7 +1006,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
qxl_spice_destroy_surfaces(d, QXL_SYNC);
}
-/* called from spice server thread context only */
+/* can be also called from spice server thread context */
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
{
uint64_t phys = le64_to_cpu(pqxl);
@@ -1465,6 +1465,46 @@ static void qxl_hw_text_update(void *opaque,
console_ch_t *chardata)
}
}
+static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
+{
+intptr_t vram_start;
+int i;
+
+if (qxl-mode != QXL_MODE_NATIVE) {
+return;
+}
+
+/* dirty the primary surface */
+qxl_set_dirty(qxl-vga.vram, qxl-shadow_rom.draw_area_offset,
+ qxl-shadow_rom.surface0_area_size);
+
+vram_start = (intptr_t)memory_region_get_ram_ptr(qxl-vram_bar);
+
+/* dirty the off-screen surfaces */
+for (i = 0; i NUM_SURFACES; i++) {
+QXLSurfaceCmd *cmd;
+intptr_t surface_offset;
+int surface_size;
+
+if (qxl-guest_surfaces.cmds[i] == 0) {
+continue;
+}
+
+cmd = qxl_phys2virt(qxl, qxl-guest_surfaces.cmds[i],
+MEMSLOT_GROUP_GUEST);
+assert(cmd-type == QXL_SURFACE_CMD_CREATE);
+surface_offset = (intptr_t)qxl_phys2virt(qxl,
+ cmd-u.surface_create.data,
+ MEMSLOT_GROUP_GUEST);
+surface_offset -= vram_start;
+surface_size = cmd-u.surface_create.height *
+ abs(cmd-u.surface_create.stride);
+dprint(qxl, 3, %s: dirty surface %d, offset %d, size %d\n, __func__,
+ i, (int)surface_offset, surface_size);
+qxl_set_dirty(qxl-vram_bar, surface_offset, surface_size);
+}
+}
+
static void qxl_vm_change_state_handler(void *opaque, int running,
RunState state)
{
@@ -1478,14 +1518,9 @@ static void qxl_vm_change_state_handler(void *opaque,
int running,
* called
*/
qxl_update_irq(qxl);
-} else if (qxl-mode == QXL_MODE_NATIVE) {
-/* dirty all vram (which holds surfaces) and devram (primary surface)
- * to make sure they are saved */
-/* FIXME #1: should go out during live stage */
-/* FIXME #2: we only need to save the areas which are actually used */
-qxl_set_dirty(qxl-vram_bar, 0, qxl-vram_size);
-qxl_set_dirty(qxl-vga.vram, qxl-shadow_rom.draw_area_offset,
- qxl-shadow_rom.surface0_area_size);
+} else {
+/* make sure surfaces are saved before migration */
+qxl_dirty_surfaces(qxl);
}
}
--
1.7.7.6
[Qemu-devel] [PATCH v2 2/2] qxl: make sure primary surface is saved on migration also in compat mode
RHBZ #790083
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index df55de1..10137f9 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1470,7 +1470,7 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
intptr_t vram_start;
int i;
-if (qxl-mode != QXL_MODE_NATIVE) {
+if (qxl-mode != QXL_MODE_NATIVE qxl-mode != QXL_MODE_COMPAT) {
return;
}
--
1.7.7.6
Re: [Qemu-devel] [PATCH] qxl: don't render stuff when the vm is stopped.
Hi,
On 02/15/2012 03:11 PM, Gerd Hoffmann wrote:
This patch fixes the local qxl renderer to not kick spice-server in case
the vm is stopped. First it is pointless because we render evevything
when the vm is stopped. Thus there is nothing to render anyway because
a stopped guest can hardly queue more commands.
hmm...When the vm is stopped we render only commands that we already
read. We don't do more reading from the command ring. So there may be
other pending commands that were not rendered.
That is why I suggested allowing rendering when the vm is stopped. But
not allowing it during loading the vm during migration.
Regards,
Yonit.
Second we avoid
triggering an assert in spice-server.
With this patch applied it is possible to take screen shots (via
screendump monitor command) from a qxl gpu even in case the guest
is stopped.
Signed-off-by: Gerd Hoffmannkra...@redhat.com
---
hw/qxl-render.c | 12 +---
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 133d093..7084143 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -121,19 +121,17 @@ void qxl_render_update(PCIQXLDevice *qxl)
dpy_resize(vga-ds);
}
-if (!qxl-guest_primary.commands) {
-return;
-}
-qxl-guest_primary.commands = 0;
-
update.left = 0;
update.right = qxl-guest_primary.surface.width;
update.top= 0;
update.bottom = qxl-guest_primary.surface.height;
memset(dirty, 0, sizeof(dirty));
-qxl_spice_update_area(qxl, 0,update,
- dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC);
+if (runstate_is_running() qxl-guest_primary.commands) {
+qxl-guest_primary.commands = 0;
+qxl_spice_update_area(qxl, 0,update,
+ dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC);
+}
if (redraw) {
memset(dirty, 0, sizeof(dirty));
dirty[0] = update;
Re: [Qemu-devel] [PATCH] qxl: don't render stuff when the vm is stopped.
On 02/15/2012 03:36 PM, Gerd Hoffmann wrote: On 02/15/12 14:22, Yonit Halperin wrote: Hi, On 02/15/2012 03:11 PM, Gerd Hoffmann wrote: This patch fixes the local qxl renderer to not kick spice-server in case the vm is stopped. First it is pointless because we render evevything when the vm is stopped. Thus there is nothing to render anyway because a stopped guest can hardly queue more commands. hmm...When the vm is stopped we render only commands that we already read. We don't do more reading from the command ring. Ah, ok. So there may be other pending commands that were not rendered. That is why I suggested allowing rendering when the vm is stopped. But not allowing it during loading the vm during migration. I'd prefer to keep things simple: let the spice worker run when the guest runs, no exceptions. We may leave some unrendered commands in the ring then. Ok. Is that a problem for some reason? Note that the guest may have more commands queued which spice-server simply doesn't see yet due to the ring being full. If we stop the guest the wrong moment we can end up with a half-done screen update operation no matter what. Ok. Then ack after correcting the patch comment. Thanks, Yonit. cheers, Gerd
[Qemu-devel] [PATCH] qxl: make sure primary surface is saved on migration also in compat mode
RHBZ #790083
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c | 17 -
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index bc03c1d..a2a3380 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1478,14 +1478,21 @@ static void qxl_vm_change_state_handler(void *opaque,
int running,
* called
*/
qxl_update_irq(qxl);
-} else if (qxl-mode == QXL_MODE_NATIVE) {
-/* dirty all vram (which holds surfaces) and devram (primary surface)
+} else {
+/* dirty all vram (which holds surfaces) and the primary surface
* to make sure they are saved */
/* FIXME #1: should go out during live stage */
/* FIXME #2: we only need to save the areas which are actually used */
-qxl_set_dirty(qxl-vram_bar, 0, qxl-vram_size);
-qxl_set_dirty(qxl-vga.vram, qxl-shadow_rom.draw_area_offset,
- qxl-shadow_rom.surface0_area_size);
+switch (qxl-mode) {
+case QXL_MODE_NATIVE:
+qxl_set_dirty(qxl-vram_bar, 0, qxl-vram_size);
+case QXL_MODE_COMPAT:
+qxl_set_dirty(qxl-vga.vram, qxl-shadow_rom.draw_area_offset,
+ qxl-shadow_rom.surface0_area_size);
+break;
+default:
+break;
+}
}
}
--
1.7.7.6
Re: [Qemu-devel] [PATCH] qxl: make sure primary surface is saved on migration also in compat mode
On 02/14/2012 10:35 AM, Gerd Hoffmann wrote: On 02/14/12 09:10, Yonit Halperin wrote: RHBZ #790083 Signed-off-by: Yonit Halperinyhalp...@redhat.com You are doing two things in one patch: (a) fix the compat mode bug, which also matches the patch description, and (b) skip vram when it is unused (in compat mode). I'd love to see (b) done in a different way: simply walk all surfaces and tag them dirty. Will have the same effect for compat mode (no surfaces used - nothing tagged dirty) and additionally it will (in native mode) only migrate over the vram areas which are actually filled with surfaces. I can do it, by retrieving the surfaces addresses from the tracked guest commands. However, if we already do it, it would be even better if we just dirty only the areas that are actually modified by the update_area calls. The problem is that (1) spice-server updates surfaces also without request from driver. We can add a cb to the interface or use the async_complete cb with a special flag (2) async_complete cb is called from spice server context. We can add a pipe for update_area dirty events, and make sure that it is handled, before migration moves from the live stage. Yonit. thanks, Gerd
Re: [Qemu-devel] [Spice-devel] [PATCH] qxl: make sure primary surface is saved on migration also in compat mode
On 02/14/2012 11:10 AM, Yonit Halperin wrote: On 02/14/2012 10:35 AM, Gerd Hoffmann wrote: On 02/14/12 09:10, Yonit Halperin wrote: RHBZ #790083 Signed-off-by: Yonit Halperinyhalp...@redhat.com You are doing two things in one patch: (a) fix the compat mode bug, which also matches the patch description, and (b) skip vram when it is unused (in compat mode). I'd love to see (b) done in a different way: simply walk all surfaces and tag them dirty. Will have the same effect for compat mode (no surfaces used - nothing tagged dirty) and additionally it will (in native mode) only migrate over the vram areas which are actually filled with surfaces. I can do it, by retrieving the surfaces addresses from the tracked guest commands. However, if we already do it, it would be even better if we just dirty only the areas that are actually modified by the update_area calls. The problem is that (1) spice-server updates surfaces also without request from driver. We can add a cb to the interface or use the async_complete cb with a special flag (2) async_complete cb is called from spice server context. We can add a pipe for update_area dirty events, and make sure that it is handled, before migration moves from the live stage. Giving it another thought, I don't really like it because surfaces are destroyed with high frequency. So it will be a waste. So doing it just before migration sounds better. Yonit. thanks, Gerd ___ Spice-devel mailing list spice-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Qemu-devel] [PATCH] qxl: make sure primary surface is saved on migration also in compat mode
On 02/14/2012 11:24 AM, Gerd Hoffmann wrote: Hi, I can do it, by retrieving the surfaces addresses from the tracked guest commands. Exactly. However, if we already do it, it would be even better if we just dirty only the areas that are actually modified by the update_area calls. The problem is that (1) spice-server updates surfaces also without request from driver. On worker-stop() for example, which renderes all outstanding commands so all state is flushed to the surfaces (and thereby device memory). This is done on vm_stop too, so I wouldn't be surprised if most surfaces are dirtied anyway at this point. Getting notifications about spice-server touching surfaces doesn't buy us much then. We also render drawables that are not hidden by other ones, when we lack free memory in the driver. In addition inter-surfaces dependencies are handled in a brute-force manner by rendering old drawables (I plan to improve this). But anyway, as I mentioned it the preceding email, since most of the surfaces are destroyed (at least with the current drivers), I prefer not to dirty them live. So I'll make the change you originally suggested. Regards, Yonit. cheers, Gerd
[Qemu-devel] [PATCH] server: support IPV6 addresses in channel events sent to qemu
RHBZ #788444
CC: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 21 +
server/spice.h |6 ++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index 2492a89..828ba65 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2005,7 +2005,7 @@ static void reds_get_spice_ticket(RedLinkInfo *link)
#if HAVE_SASL
static char *addr_to_string(const char *format,
-struct sockaddr *sa,
+struct sockaddr_storage *sa,
socklen_t salen) {
char *addr;
char host[NI_MAXHOST];
@@ -2013,7 +2013,7 @@ static char *addr_to_string(const char *format,
int err;
size_t addrlen;
-if ((err = getnameinfo(sa, salen,
+if ((err = getnameinfo((struct sockaddr *)sa, salen,
host, sizeof(host),
serv, sizeof(serv),
NI_NUMERICHOST | NI_NUMERICSERV)) != 0) {
@@ -2402,11 +2402,13 @@ static void reds_start_auth_sasl(RedLinkInfo *link)
RedsSASL *sasl = link-stream-sasl;
/* Get local remote client addresses in form IPADDR;PORT */
-if (!(localAddr = addr_to_string(%s;%s, link-stream-info.laddr,
link-stream-info.llen))) {
+if (!(localAddr = addr_to_string(%s;%s, link-stream-info.laddr_ext,
+ link-stream-info.llen_ext))) {
goto error;
}
-if (!(remoteAddr = addr_to_string(%s;%s, link-stream-info.paddr,
link-stream-info.plen))) {
+if (!(remoteAddr = addr_to_string(%s;%s, link-stream-info.paddr_ext,
+ link-stream-info.plen_ext))) {
free(localAddr);
goto error;
}
@@ -2717,10 +2719,21 @@ static RedLinkInfo *reds_init_client_connection(int
socket)
stream-socket = socket;
/* gather info + send event */
+
+/* deprecated fields. Filling them for backward compatibility */
stream-info.llen = sizeof(stream-info.laddr);
stream-info.plen = sizeof(stream-info.paddr);
getsockname(stream-socket, (struct sockaddr*)(stream-info.laddr),
stream-info.llen);
getpeername(stream-socket, (struct sockaddr*)(stream-info.paddr),
stream-info.plen);
+
+stream-info.flags |= SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT;
+stream-info.llen_ext = sizeof(stream-info.laddr_ext);
+stream-info.plen_ext = sizeof(stream-info.paddr_ext);
+getsockname(stream-socket, (struct sockaddr*)(stream-info.laddr_ext),
+stream-info.llen_ext);
+getpeername(stream-socket, (struct sockaddr*)(stream-info.paddr_ext),
+stream-info.plen_ext);
+
reds_stream_channel_event(stream, SPICE_CHANNEL_EVENT_CONNECTED);
openssl_init(link);
diff --git a/server/spice.h b/server/spice.h
index c582e6c..7397655 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -54,6 +54,7 @@ typedef struct SpiceCoreInterface SpiceCoreInterface;
#define SPICE_CHANNEL_EVENT_DISCONNECTED 3
#define SPICE_CHANNEL_EVENT_FLAG_TLS (1 0)
+#define SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT (1 1)
typedef struct SpiceWatch SpiceWatch;
typedef void (*SpiceWatchFunc)(int fd, int event, void *opaque);
@@ -66,9 +67,14 @@ typedef struct SpiceChannelEventInfo {
int type;
int id;
int flags;
+/* deprecated, can't hold ipv6 addresses, kept for backward compatibility
*/
struct sockaddr laddr;
struct sockaddr paddr;
socklen_t llen, plen;
+/* should be used if (flags SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) */
+struct sockaddr_storage laddr_ext;
+struct sockaddr_storage paddr_ext;
+socklen_t llen_ext, plen_ext;
} SpiceChannelEventInfo;
struct SpiceCoreInterface {
--
1.7.7.6
[Qemu-devel] [PATCH] spice: support ipv6 channel address in monitor events and in spice info
RHBZ #788444
CC: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 37 -
1 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 5639c6f..60fd6c3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -220,10 +220,23 @@ static void channel_event(int event,
SpiceChannelEventInfo *info)
}
client = qdict_new();
-add_addr_info(client, info-paddr, info-plen);
-
server = qdict_new();
-add_addr_info(server, info-laddr, info-llen);
+
+#ifdef SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT
+if (info-flags SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) {
+add_addr_info(client, (struct sockaddr *)info-paddr_ext,
+ info-plen_ext);
+add_addr_info(server, (struct sockaddr *)info-laddr_ext,
+ info-llen_ext);
+} else {
+fprintf(stderr, spice: %s, extended address is expected\n,
+__func__);
+#endif
+add_addr_info(client, info-paddr, info-plen);
+add_addr_info(server, info-laddr, info-llen);
+#ifdef SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT
+}
+#endif
if (event == SPICE_CHANNEL_EVENT_INITIALIZED) {
qdict_put(server, auth, qstring_from_str(auth));
@@ -376,16 +389,30 @@ static SpiceChannelList *qmp_query_spice_channels(void)
QTAILQ_FOREACH(item, channel_list, link) {
SpiceChannelList *chan;
char host[NI_MAXHOST], port[NI_MAXSERV];
+struct sockaddr *paddr;
+socklen_t plen;
chan = g_malloc0(sizeof(*chan));
chan-value = g_malloc0(sizeof(*chan-value));
-getnameinfo(item-info-paddr, item-info-plen,
+#ifdef SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT
+if (item-info-flags SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) {
+paddr = (struct sockaddr *)item-info-paddr_ext;
+plen = item-info-plen_ext;
+} else {
+#endif
+paddr = item-info-paddr;
+plen = item-info-plen;
+#ifdef SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT
+}
+#endif
+
+getnameinfo(paddr, plen,
host, sizeof(host), port, sizeof(port),
NI_NUMERICHOST | NI_NUMERICSERV);
chan-value-host = g_strdup(host);
chan-value-port = g_strdup(port);
-chan-value-family =
g_strdup(inet_strfamily(item-info-paddr.sa_family));
+chan-value-family = g_strdup(inet_strfamily(paddr-sa_family));
chan-value-connection_id = item-info-connection_id;
chan-value-channel_type = item-info-type;
--
1.7.7.6
Re: [Qemu-devel] [Spice-devel] client_migrate_info - do we need a new command?
Hi,
On 12/13/2011 08:19 PM, Anthony Liguori wrote:
In our call today, Avi asked that we evaluate whether the interface for
client_migrate_info is the Right Interface before we introduce a new
command to work around the fact that async commands are broken.
I looked into this today and here's what I came to.
1) What are the failure scenarios?
The issue is qerror_report(). Roughly speaking, qerror_report either
prints to stderr or it associates an error with the current monitor
command.
The problem with this is that qerror_report() is used all over the code
base today and if an error occurs in a device that has nothing to do
with the command, instead of printing to stderr, the command will fail
with a bizarre error reason (even though it really succeeded).
2) Does the command have the right semantics?
The command has the following doc:
client_migrate_info
--
Set the spice/vnc connection info for the migration target. The spice/vnc
server will ask the spice/vnc client to automatically reconnect using the
new parameters (if specified) once the vm migration finished successfully.
Arguments:
- protocol: protocol: spice or vnc (json-string)
- hostname: migration target hostname (json-string)
- port: spice/vnc tcp port for plaintext channels (json-int, optional)
- tls-port: spice tcp port for tls-secured channels (json-int, optional)
- cert-subject: server certificate subject (json-string, optional)
Example:
- { execute: client_migrate_info,
arguments: { protocol: spice,
hostname: virt42.lab.kraxel.org,
port: 1234 } }
- { return: {} }
Originally, the command was a normal sync command and my understanding
is that it simply posted notification to the clients. Apparently, users
of the interface need to actually know when the client has Ack'd this
operation because otherwise it's racy since a disconnect may occur
before the client processes the redirection.
It's racy because the migration can start before the client manages to
connect to the migration target. And since the target is unresponsive
during migration, the client will manage to connect to it only after
migration completes; but that can take a while, and the client's ticket
might expire till then.
OTOH, that means that what we really need is 1) tell connected clients
that they need to redirect 2) notification when/if connected clients are
prepared to redirect.
The trouble with using a async command for this is that the time between
(1) (2) may be arbitrarily long. Since most QMP clients today always
use a NULL tag, that effectively means the monitor is blocked for an
arbitrarily long time while this operation is in flight.
I don't know if libspice uses a timeout for this operation,
We use a timeout of 10 Sec
but if it
doesn't, this could block arbitrarily long. Even with tagging, we don't
have a way to cancel in flight commands so blocking for arbitrary time
periods is problematic.
I think splitting this into two commands, one that requests the clients
to redirect and then an event that lets a tool know that the clients are
ready to migrate ends up being nicer. It means that we never end up with
a blocked QMP session and clients are more likely to properly deal with
the fact that an event may take arbitrarily long to happen.
Clients can also implement their own cancel logic by choosing to stop
waiting for an event to happen and then ignoring spurious events.
So regardless of the async issue, I think splitting this command is the
right thing to do long term.
I just want to emphasize that using client_migrate_info for connecting
to to target is more of a workaround. IMHO, the more complete solution
would have been (similar to the one we have in Rhel5):
1) Add a migration notifier for pre-starting migration. Introduce
completion cb to these notifiers.
2) actually start migration only after the completion cb is called.
Then, the client_migrate_info can go back to be sync.
If we already plan to make changes, maybe they should be aimed to such a
solution.
Btw, if we also had such notifier for pre-finish migration (before
starting the target vm), we could even turn the client migration to be
really seamless again.
Regards,
Yonit.
Regards,
Anthony Liguori
___
Spice-devel mailing list
spice-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Qemu-devel] [Spice-devel] Possible SPICE/QEMU deadlock on fast client reconnect
Hi, Sounds like https://bugzilla.redhat.com/show_bug.cgi?id=746950 and https://bugs.freedesktop.org/show_bug.cgi?id=41858 Alon's patch: http://lists.freedesktop.org/archives/spice-devel/2011-September /005369.html Should solve it. Cheers, Yonit. On 10/21/2011 05:26 PM, Daniel P. Berrange wrote: In testing my patches for 'add_client' support with SPICE, I noticed deadlock in the QEMU/SPICE code. It only happens if I close a SPICE client and then immediately reconnect within about 1 second. If I wait a couple of seconds before reconnecting the SPICE client I don't see the deadlock. I'm using QEMU SPICE git master, and GDB has this to say (gdb) thread apply all bt Thread 3 (Thread 0x7f269e142700 (LWP 17233)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:165 #1 0x004e80e9 in qemu_cond_wait (cond=optimized out, mutex=optimized out) at qemu-thread-posix.c:113 #2 0x00556469 in qemu_kvm_wait_io_event (env=optimized out) at /home/berrange/src/virt/qemu/cpus.c:626 #3 qemu_kvm_cpu_thread_fn (arg=0x29217a0) at /home/berrange/src/virt/qemu/cpus.c:661 #4 0x003a83207b31 in start_thread (arg=0x7f269e142700) at pthread_create.c:305 #5 0x003a82edfd2d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115 Thread 2 (Thread 0x7f26822fc700 (LWP 17234)): #0 __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:140 #1 0x003a83209ad6 in _L_lock_752 () from /lib64/libpthread.so.0 #2 0x003a832099d7 in __pthread_mutex_lock (mutex=0x1182f60) at pthread_mutex_lock.c:65 #3 0x004e7ec9 in qemu_mutex_lock (mutex=optimized out) at qemu-thread-posix.c:54 #4 0x00507df5 in channel_event (event=3, info=0x2a3c610) at ui/spice-core.c:234 #5 0x7f269f77be87 in reds_stream_free (s=0x2a3c590) at reds.c:4073 #6 0x7f269f75b64f in red_channel_client_disconnect (rcc=0x7f267c069ec0) at red_channel.c:961 #7 0x7f269f75a131 in red_peer_handle_incoming (handler=0x7f267c06a5a0, stream=0x2a3c590) at red_channel.c:150 #8 red_channel_client_receive (rcc=0x7f267c069ec0) at red_channel.c:158 #9 0x7f269f761fbc in handle_channel_events (in_listener=0x7f267c06a5f8, events=optimized out) at red_worker.c:9566 #10 0x7f269f776672 in red_worker_main (arg=optimized out) at red_worker.c:10813 #11 0x003a83207b31 in start_thread (arg=0x7f26822fc700) at pthread_create.c:305 #12 0x003a82edfd2d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115 Thread 1 (Thread 0x7f269f72e9c0 (LWP 17232)): #0 0x003a8320e01d in read () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f269f75daaa in receive_data (n=4, in_buf=0x7fffe7a5a02c, fd=18) at red_worker.h:150 #2 read_message (message=0x7fffe7a5a02c, fd=18) at red_worker.h:163 #3 red_dispatcher_disconnect_cursor_peer (rcc=0x7f267c0c0f60) at red_dispatcher.c:157 #4 0x7f269f75c20d in red_client_destroy (client=0x2a35400) at red_channel.c:1189 #5 0x7f269f778335 in reds_client_disconnect (client=0x2a35400) at reds.c:624 ---Typereturn to continue, or qreturn to quit--- #6 0x7f269f75a131 in red_peer_handle_incoming (handler=0x2a35b50, stream=0x2b037d0) at red_channel.c:150 #7 red_channel_client_receive (rcc=0x2a35470) at red_channel.c:158 #8 0x7f269f75b1e8 in red_channel_client_event (fd=optimized out, event=optimized out, data=optimized out) at red_channel.c:774 #9 0x0045e561 in fd_trampoline (chan=optimized out, cond=optimized out, opaque=optimized out) at iohandler.c:97 #10 0x003a84e427ed in g_main_dispatch (context=0x27fc510) at gmain.c:2441 #11 g_main_context_dispatch (context=0x27fc510) at gmain.c:3014 #12 0x004c3da3 in glib_select_poll (err=false, xfds=0x7fffe7a5a2e0, wfds=0x7fffe7a5a260, rfds= 0x7fffe7a5a1e0) at /home/berrange/src/virt/qemu/vl.c:1495 #13 main_loop_wait (nonblocking=optimized out) at /home/berrange/src/virt/qemu/vl.c:1541 #14 0x0040fdd1 in main_loop () at /home/berrange/src/virt/qemu/vl.c:1570 #15 main (argc=optimized out, argv=optimized out, envp=optimized out) at /home/berrange/src/virt/qemu/vl.c:3563 (gdb)
[Qemu-devel] [PATCH] qxl: fix guest cursor tracking
(1) If the guest cursor command is empty, don't reload it after migration.
(2) Cleaning the guest cursor when it is released by
the spice server. In addition, explicitly reset the
cursor in spice upon destroying the primary surface
(was done by spice-server implicitly). This will prevent
access to pci memory that was released.
RHBZ: 744518
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c | 16
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 03848ed..c9b60a2 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -238,6 +238,9 @@ void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
{
qxl-ssd.worker-reset_cursor(qxl-ssd.worker);
+qemu_mutex_lock(qxl-track_lock);
+qxl-guest_cursor = 0;
+qemu_mutex_unlock(qxl-track_lock);
}
@@ -402,7 +405,9 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct
QXLCommandExt *ext)
{
QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext-cmd.data, ext-group_id);
if (cmd-type == QXL_CURSOR_SET) {
+qemu_mutex_lock(qxl-track_lock);
qxl-guest_cursor = ext-cmd.data;
+qemu_mutex_unlock(qxl-track_lock);
}
break;
}
@@ -1067,6 +1072,7 @@ static int qxl_destroy_primary(PCIQXLDevice *d,
qxl_async_io async)
d-mode = QXL_MODE_UNDEFINED;
qemu_spice_destroy_primary_surface(d-ssd, 0, async);
+qxl_spice_reset_cursor(d);
return 1;
}
@@ -1710,10 +1716,12 @@ static int qxl_post_load(void *opaque, int version)
cmds[out].group_id = MEMSLOT_GROUP_GUEST;
out++;
}
-cmds[out].cmd.data = d-guest_cursor;
-cmds[out].cmd.type = QXL_CMD_CURSOR;
-cmds[out].group_id = MEMSLOT_GROUP_GUEST;
-out++;
+if (d-guest_cursor) {
+cmds[out].cmd.data = d-guest_cursor;
+cmds[out].cmd.type = QXL_CMD_CURSOR;
+cmds[out].group_id = MEMSLOT_GROUP_GUEST;
+out++;
+}
qxl_spice_loadvm_commands(d, cmds, out);
g_free(cmds);
--
1.7.6.4
[Qemu-devel] [PATCH v2 1/2] spice: turn client_migrate_info to async
RHBZ 737921
Spice client is required to connect to the migration target before/as migration
starts. Since after migration starts, the target qemu is blocked and cannot
accept new spice client
we trigger the connection to the target upon client_migrate_info command.
client_migrate_info completion cb will be called after spice client has been
connected to the target (or a timeout). See following patches and spice patches.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hmp-commands.hx |3 ++-
monitor.c |6 --
qmp-commands.hx |3 ++-
ui/qemu-spice.h | 14 +++---
ui/spice-core.c | 10 +++---
5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..6f390a0 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -827,7 +827,8 @@ ETEXI
.params = protocol hostname port tls-port cert-subject,
.help = send migration info to spice/vnc client,
.user_print = monitor_user_noop,
-.mhandler.cmd_new = client_migrate_info,
+.mhandler.cmd_async = client_migrate_info,
+.flags = MONITOR_CMD_ASYNC,
},
STEXI
diff --git a/monitor.c b/monitor.c
index df0f622..0374dcc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1221,7 +1221,8 @@ static int add_graphics_client(Monitor *mon, const QDict
*qdict, QObject **ret_d
return -1;
}
-static int client_migrate_info(Monitor *mon, const QDict *qdict, QObject
**ret_data)
+static int client_migrate_info(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque)
{
const char *protocol = qdict_get_str(qdict, protocol);
const char *hostname = qdict_get_str(qdict, hostname);
@@ -1236,7 +1237,8 @@ static int client_migrate_info(Monitor *mon, const QDict
*qdict, QObject **ret_d
return -1;
}
-ret = qemu_spice_migrate_info(hostname, port, tls_port, subject);
+ret = qemu_spice_migrate_info(hostname, port, tls_port, subject,
+ cb, opaque);
if (ret != 0) {
qerror_report(QERR_UNDEFINED_ERROR);
return -1;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 27cc66e..321fb10 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -578,7 +578,8 @@ EQMP
.params = protocol hostname port tls-port cert-subject,
.help = send migration info to spice/vnc client,
.user_print = monitor_user_noop,
-.mhandler.cmd_new = client_migrate_info,
+.mhandler.cmd_async = client_migrate_info,
+.flags = MONITOR_CMD_ASYNC,
},
SQMP
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index f34be69..c35b29c 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -25,6 +25,7 @@
#include qemu-option.h
#include qemu-config.h
#include qemu-char.h
+#include monitor.h
extern int using_spice;
@@ -37,7 +38,8 @@ int qemu_spice_set_passwd(const char *passwd,
bool fail_if_connected, bool
disconnect_if_connected);
int qemu_spice_set_pw_expire(time_t expires);
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
-const char *subject);
+const char *subject,
+MonitorCompletion cb, void *opaque);
void do_info_spice_print(Monitor *mon, const QObject *data);
void do_info_spice(Monitor *mon, QObject **ret_data);
@@ -45,6 +47,7 @@ void do_info_spice(Monitor *mon, QObject **ret_data);
int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr);
#else /* CONFIG_SPICE */
+#include monitor.h
#define using_spice 0
static inline int qemu_spice_set_passwd(const char *passwd,
@@ -57,8 +60,13 @@ static inline int qemu_spice_set_pw_expire(time_t expires)
{
return -1;
}
-static inline int qemu_spice_migrate_info(const char *h, int p, int t, const
char *s)
-{ return -1; }
+static inline int qemu_spice_migrate_info(const char *h, int p, int t,
+ const char *s,
+ MonitorCompletion cb, void *opaque)
+{
+cb(opaque, NULL);
+return -1;
+}
#endif /* CONFIG_SPICE */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3cbc721..50c0d7d 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -457,10 +457,14 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
}
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
-const char *subject)
+const char *subject,
+MonitorCompletion *cb, void *opaque)
{
-return spice_server_migrate_info(spice_server, hostname,
- port, tls_port, subject);
+int ret;
+ret = spice_server_migrate_info(spice_server, hostname,
+port, tls_port, subject);
+cb(opaque, NULL
[Qemu-devel] [PATCH v2 2/2] spice: support the new migration interface (spice 0.8.3)
- call spice_server_migrate_(start|end|connect).
- register spice_migrate_connect completion callback
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 56 ++-
1 files changed, 55 insertions(+), 1 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 50c0d7d..457cf61 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -288,6 +288,38 @@ static SpiceCoreInterface core_interface = {
#endif
};
+#ifdef SPICE_INTERFACE_MIGRATION
+typedef struct SpiceMigration {
+SpiceMigrateInstance sin;
+struct {
+MonitorCompletion *cb;
+void *opaque;
+} connect_complete;
+} SpiceMigration;
+
+static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+
+static const SpiceMigrateInterface migrate_interface = {
+.base.type = SPICE_INTERFACE_MIGRATION,
+.base.description = migration,
+.base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
+.base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
+.migrate_connect_complete = migrate_connect_complete_cb,
+.migrate_end_complete = NULL,
+};
+
+static SpiceMigration spice_migrate;
+
+static void migrate_connect_complete_cb(SpiceMigrateInstance *sin)
+{
+SpiceMigration *sm = container_of(sin, SpiceMigration, sin);
+if (sm-connect_complete.cb) {
+sm-connect_complete.cb(sm-connect_complete.opaque, NULL);
+}
+sm-connect_complete.cb = NULL;
+}
+#endif
+
/* config string parsing */
static int name2enum(const char *string, const char *table[], int entries)
@@ -449,9 +481,19 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
{
int state = get_migration_state();
-if (state == MIG_STATE_COMPLETED) {
+if (state == MIG_STATE_ACTIVE) {
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_server_migrate_start(spice_server);
+#endif
+} else if (state == MIG_STATE_COMPLETED) {
#if SPICE_SERVER_VERSION = 0x000701 /* 0.7.1 */
+#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
+#else
+spice_server_migrate_end(spice_server, true);
+} else if (state == MIG_STATE_CANCELLED || state == MIG_STATE_ERROR) {
+spice_server_migrate_end(spice_server, false);
+#endif
#endif
}
}
@@ -461,9 +503,16 @@ int qemu_spice_migrate_info(const char *hostname, int
port, int tls_port,
MonitorCompletion *cb, void *opaque)
{
int ret;
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_migrate.connect_complete.cb = cb;
+spice_migrate.connect_complete.opaque = opaque;
+ret = spice_server_migrate_connect(spice_server, hostname,
+ port, tls_port, subject);
+#else
ret = spice_server_migrate_info(spice_server, hostname,
port, tls_port, subject);
cb(opaque, NULL);
+#endif
return ret;
}
@@ -654,6 +703,11 @@ void qemu_spice_init(void)
migration_state.notify = migration_state_notifier;
add_migration_state_change_notifier(migration_state);
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_migrate.sin.base.sif = migrate_interface.base;
+spice_migrate.connect_complete.cb = NULL;
+qemu_spice_add_interface(spice_migrate.sin.base);
+#endif
qemu_spice_input_init();
qemu_spice_audio_init();
--
1.7.6.4
[Qemu-devel] [PATCH v2 0/2] spice migration interface v2 (RHBZ 737921)
Same as the previous series with a small fix to allow compliation without Spice disabled. Yonit Spice client is required to connect to the migration target before/as migration starts. Previously, it connected upon migration completion, however, the ticket was set in the beginning, thus when migration time was ticket_expiration_time, spice failed to connect to the target. Since the migration target is blocked after migration starts, we execute spice-client connection to the target before migration, upon client_migrate_info. We wait till the client is connected to the target, or till a timeout occurs. In order to not block the iothread, this patch turns client_migrate_info to asynchronous. In addition, we changed the spice api: (1) client_migrate_info need to call spice_server_migrate_connect (2) spice_server_migrate_start/end need to be called upon migration start/end ** spice_server_start and the migrate_end_complete callback, were added for future use, in case we implement a real seamless spice migration Yonit Halperin (2): spice: turn client_migrate_info to async spice: support the new migration interface (spice 0.8.3) hmp-commands.hx |3 +- monitor.c |6 +++- qmp-commands.hx |3 +- ui/qemu-spice.h | 14 +-- ui/spice-core.c | 66 +++--- 5 files changed, 81 insertions(+), 11 deletions(-) -- 1.7.6.4
Re: [Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)
ACK
On 10/17/2011 12:24 PM, Alon Levy wrote:
If we migrate when the device is not in a native state the guest
still believes the slots are created, and will cause operations
that reference the slots, causing a panic: virtual address out of range
on the first of them. Easy to see by migrating in vga mode (with
a driver loaded, for instance windows cmd window in full screen mode)
and then exiting vga mode back to native mode will cause said panic.
Fixed by doing the slot recreation unconditionally at post_load
Signed-off-by: Alon Levyal...@redhat.com
---
hw/qxl.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 03848ed..4e9f39f 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1684,6 +1684,14 @@ static int qxl_post_load(void *opaque, int version)
qxl_mode_to_string(d-mode));
newmode = d-mode;
d-mode = QXL_MODE_UNDEFINED;
+for (i = 0; i NUM_MEMSLOTS; i++) {
+if (!d-guest_slots[i].active) {
+continue;
+}
+dprint(d, 1, %s: restoring guest slot %d delta %PRIu64\n,
+ __func__, i, d-guest_slots[i].delta);
+qxl_add_memslot(d, i, d-guest_slots[i].delta, QXL_SYNC);
+}
switch (newmode) {
case QXL_MODE_UNDEFINED:
break;
@@ -1691,12 +1699,6 @@ static int qxl_post_load(void *opaque, int version)
qxl_enter_vga_mode(d);
break;
case QXL_MODE_NATIVE:
-for (i = 0; i NUM_MEMSLOTS; i++) {
-if (!d-guest_slots[i].active) {
-continue;
-}
-qxl_add_memslot(d, i, 0, QXL_SYNC);
-}
qxl_create_guest_primary(d, 1, QXL_SYNC);
/* replay surface-create and cursor-set commands */
Re: [Qemu-devel] [Spice-devel] viewing continuous guest virtual memory as continuous in qemu
On 10/02/2011 03:24 PM, Alon Levy wrote: Hi, I'm trying to acheive the $subject. Some background: currently spice relies on a preallocated pci bar for both surfaces and for VGA framebuffer + commands. I have been trying to get rid of the surfaces bar. To do that I allocate memory in the guest and then translate it for spice-server consumption using cpu_physical_memory_map. AFAIU this works only when the guest allocates a continuous range of physical pages. This is a large requirement from the guest, which I'd like to drop. So I would like to have the guest use a regular allocator, generating for instance two sequential pages in virtual memory that are scattered in physical memory. Those two physical guest page addresses (gp1 and gp2) correspond to two host virtual memory addresses (hv1, hv2). I would now like to provide to spice-server a single virtual address p that maps to those two pages in sequence. I don't want to handle my own scatter-gather list, I would like to have this mapping done once so I can use an existing library that requires a single pointer (for instance pixman or libGL) to do the rendering. Is there any way to acheive that without host kernel support, in user space, i.e. in qemu? or with an existing host kernel device? I'd appreciate any help, Alon ___ Spice-devel mailing list spice-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel Hi, won't there be an overhead for rendering on a non continuous surface? Will it be worthwhile comparing to not creating the surface? BTW. We should test if the split to vram (surfaces) and devram (commands and others) is more efficient than having one section. Even if it is more efficient, we can remove the split and give to the surfaces higher allocation priority on a part of the pci bar. Anyway, by default, we can try allocating surfaces on the guest RAM. If it fails, we can try to allocate on the pci-bar. Cheers, Yonit.
Re: [Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine
On 09/22/2011 05:01 PM, Alon Levy wrote:
On Wed, Sep 21, 2011 at 06:51:18PM +0300, Yonit Halperin wrote:
Signed-off-by: Yonit Halperinyhalp...@redhat.com
---
server/reds.c | 68 +++-
1 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index bea0eb0..e7388a0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2612,12 +2612,48 @@ static void inputs_init()
reds_register_channel(channel);
}
+static void reds_send_input_channel_insecure_warn()
+{
+RedsOutItem *item;
+SpiceMsgNotify notify;
+char *mess = keyboard channel is insecure;
+const int mess_len = strlen(mess);
+
+item = new_out_item(SPICE_MSG_NOTIFY);
+
+notify.time_stamp = get_time_stamp();
+notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
+notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
+notify.what = SPICE_WARN_GENERAL;
+notify.message_len = mess_len;
+
+spice_marshall_msg_notify(item-m,notify);
+spice_marshaller_add(item-m, (uint8_t *)mess, mess_len + 1);
+
+reds_push_pipe_item(item);
+}
+
+static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg,
RedsStream *stream)
+{
+uint32_t *caps;
+
+ASSERT(channel);
+ASSERT(link_msg);
+ASSERT(stream);
+
+if (link_msg-channel_type == SPICE_CHANNEL_INPUTS !stream-ssl) {
+reds_send_input_channel_insecure_warn();
+}
+caps = (uint32_t *)((uint8_t *)link_msg + link_msg-caps_offset);
+channel-link(channel, stream, FALSE, link_msg-num_common_caps,
+ link_msg-num_common_caps ? caps : NULL,
link_msg-num_channel_caps,
+ link_msg-num_channel_caps ? caps +
link_msg-num_common_caps : NULL);
+}
+
static void reds_handle_other_links(RedLinkInfo *link)
{
Channel *channel;
-RedsStream *stream;
SpiceLinkMess *link_mess;
-uint32_t *caps;
link_mess = link-link_mess;
@@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link)
}
reds_send_link_result(link, SPICE_LINK_ERR_OK);
+// TODO: if mig_target, where this call should be? here or only when the
links are handled
Both? add a prefix argument to reds_show_new_channel maybe.
Hi, my concern was more about the red_channel_event called from
reds_show_new_channel. I think it should be called only once, and at the
same place it is in these patches - when the client makes the initial
connection to the target, i.e., before migration.
reds_show_new_channel(link, reds-link_id);
-if (link_mess-channel_type == SPICE_CHANNEL_INPUTS !link-stream-ssl)
{
-RedsOutItem *item;
-SpiceMsgNotify notify;
-char *mess = keyboard channel is insecure;
-const int mess_len = strlen(mess);
-
-item = new_out_item(SPICE_MSG_NOTIFY);
-
-notify.time_stamp = get_time_stamp();
-notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
-notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
-notify.what = SPICE_WARN_GENERAL;
-notify.message_len = mess_len;
+reds_stream_remove_watch(link-stream);
-spice_marshall_msg_notify(item-m,notify);
-spice_marshaller_add(item-m, (uint8_t *)mess, mess_len + 1);
+reds_channel_do_link(channel, link-link_mess, link-stream);
Would be nice to consistently use link_mess or link-link_mess, not both.
+free(link_mess);
-reds_push_pipe_item(item);
-}
-stream = link-stream;
-reds_stream_remove_watch(stream);
link-stream = NULL;
link-link_mess = NULL;
reds_link_free(link);
-caps = (uint32_t *)((uint8_t *)link_mess + link_mess-caps_offset);
-channel-link(channel, stream, reds-mig_target,
link_mess-num_common_caps,
- link_mess-num_common_caps ? caps : NULL,
link_mess-num_channel_caps,
- link_mess-num_channel_caps ? caps +
link_mess-num_common_caps : NULL);
-free(link_mess);
}
static void reds_handle_link(RedLinkInfo *link)
--
1.7.4.4
[Qemu-devel] [PATCH 0/2] spice migration interface (RHBZ 737921)
Spice client is required to connect to the migration target before/as migration starts. Previously, it connected upon migration completion, however, the ticket was set in the beginning, thus when migration time was ticket_expiration_time, spice failed to connect to the target. Since the migration target is blocked after migration starts, we execute spice-client connection to the target before migration, upon client_migrate_info. We wait till the client is connected to the target, or till a timeout occurs. In order to not block the iothread, this patch turns client_migrate_info to asynchronous. In addition, we changed the spice api: (1) client_migrate_info need to call spice_server_migrate_connect (2) spice_server_migrate_start/end need to be called upon migration start/end ** spice_server_start and the migrate_end_complete callback, were added for future use, in case we implement a real seamless spice migration Spice server patches will be emailed next. They are for the 0.8 spice branch, but will be ported to upstream soon. Yonit Halperin (2): spice: turn client_migrate_info to async spice: support the new migration interface (spice 0.8.3) hmp-commands.hx |3 +- monitor.c |6 +++- qmp-commands.hx |3 +- ui/qemu-spice.h | 13 +- ui/spice-core.c | 66 +++--- 5 files changed, 81 insertions(+), 10 deletions(-) -- 1.7.4.4
[Qemu-devel] [PATCH 2/2] spice: support the new migration interface (spice 0.8.3)
- call spice_server_migrate_(start|end|connect).
- register spice_migrate_connect completion callback
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
ui/spice-core.c | 56 ++-
1 files changed, 55 insertions(+), 1 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 50c0d7d..457cf61 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -288,6 +288,38 @@ static SpiceCoreInterface core_interface = {
#endif
};
+#ifdef SPICE_INTERFACE_MIGRATION
+typedef struct SpiceMigration {
+SpiceMigrateInstance sin;
+struct {
+MonitorCompletion *cb;
+void *opaque;
+} connect_complete;
+} SpiceMigration;
+
+static void migrate_connect_complete_cb(SpiceMigrateInstance *sin);
+
+static const SpiceMigrateInterface migrate_interface = {
+.base.type = SPICE_INTERFACE_MIGRATION,
+.base.description = migration,
+.base.major_version = SPICE_INTERFACE_MIGRATION_MAJOR,
+.base.minor_version = SPICE_INTERFACE_MIGRATION_MINOR,
+.migrate_connect_complete = migrate_connect_complete_cb,
+.migrate_end_complete = NULL,
+};
+
+static SpiceMigration spice_migrate;
+
+static void migrate_connect_complete_cb(SpiceMigrateInstance *sin)
+{
+SpiceMigration *sm = container_of(sin, SpiceMigration, sin);
+if (sm-connect_complete.cb) {
+sm-connect_complete.cb(sm-connect_complete.opaque, NULL);
+}
+sm-connect_complete.cb = NULL;
+}
+#endif
+
/* config string parsing */
static int name2enum(const char *string, const char *table[], int entries)
@@ -449,9 +481,19 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
{
int state = get_migration_state();
-if (state == MIG_STATE_COMPLETED) {
+if (state == MIG_STATE_ACTIVE) {
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_server_migrate_start(spice_server);
+#endif
+} else if (state == MIG_STATE_COMPLETED) {
#if SPICE_SERVER_VERSION = 0x000701 /* 0.7.1 */
+#ifndef SPICE_INTERFACE_MIGRATION
spice_server_migrate_switch(spice_server);
+#else
+spice_server_migrate_end(spice_server, true);
+} else if (state == MIG_STATE_CANCELLED || state == MIG_STATE_ERROR) {
+spice_server_migrate_end(spice_server, false);
+#endif
#endif
}
}
@@ -461,9 +503,16 @@ int qemu_spice_migrate_info(const char *hostname, int
port, int tls_port,
MonitorCompletion *cb, void *opaque)
{
int ret;
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_migrate.connect_complete.cb = cb;
+spice_migrate.connect_complete.opaque = opaque;
+ret = spice_server_migrate_connect(spice_server, hostname,
+ port, tls_port, subject);
+#else
ret = spice_server_migrate_info(spice_server, hostname,
port, tls_port, subject);
cb(opaque, NULL);
+#endif
return ret;
}
@@ -654,6 +703,11 @@ void qemu_spice_init(void)
migration_state.notify = migration_state_notifier;
add_migration_state_change_notifier(migration_state);
+#ifdef SPICE_INTERFACE_MIGRATION
+spice_migrate.sin.base.sif = migrate_interface.base;
+spice_migrate.connect_complete.cb = NULL;
+qemu_spice_add_interface(spice_migrate.sin.base);
+#endif
qemu_spice_input_init();
qemu_spice_audio_init();
--
1.7.4.4
[Qemu-devel] [PATCH spice-protocol] Release 0.8.2
semi-seamless migration RHBZ 738262
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
NEWS |4
configure.ac |2 +-
spice/enums.h|2 ++
spice/protocol.h |6 +-
4 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index f238abc..bb11ed2 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Major changes in 0.8.2
+==
+* Add support for semi-seamless migration
+
Major changes in 0.8.1
==
* Add support for volume change
diff --git a/configure.ac b/configure.ac
index 989cb00..4ec1990 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@ AC_PREREQ([2.57])
m4_define([SPICE_MAJOR], 0)
m4_define([SPICE_MINOR], 8)
-m4_define([SPICE_MICRO], 1)
+m4_define([SPICE_MICRO], 2)
AC_INIT(spice-protocol, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [],
spice-protocol)
diff --git a/spice/enums.h b/spice/enums.h
index 756df56..fcfc3bb 100644
--- a/spice/enums.h
+++ b/spice/enums.h
@@ -364,6 +364,7 @@ enum {
SPICE_MSG_MAIN_AGENT_DATA,
SPICE_MSG_MAIN_AGENT_TOKEN,
SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST,
+SPICE_MSG_MAIN_MIGRATE_END,
SPICE_MSG_END_MAIN
};
@@ -377,6 +378,7 @@ enum {
SPICE_MSGC_MAIN_AGENT_START,
SPICE_MSGC_MAIN_AGENT_DATA,
SPICE_MSGC_MAIN_AGENT_TOKEN,
+SPICE_MSGC_MAIN_MIGRATE_END,
SPICE_MSGC_END_MAIN
};
diff --git a/spice/protocol.h b/spice/protocol.h
index 40027be..ddfe84b 100644
--- a/spice/protocol.h
+++ b/spice/protocol.h
@@ -37,7 +37,7 @@
#define SPICE_MAGIC (*(uint32_t*)REDQ)
#define SPICE_VERSION_MAJOR 2
-#define SPICE_VERSION_MINOR 0
+#define SPICE_VERSION_MINOR 1
// Encryption Ticketing Parameters
#define SPICE_MAX_PASSWORD_LENGTH 60
@@ -111,6 +111,10 @@ enum {
SPICE_RECORD_CAP_VOLUME,
};
+enum {
+SPICE_MAIN_CAP_SEMI_SEAMLESS_MIGRATE,
+};
+
#include spice/end-packed.h
#endif /* _H_SPICE_PROTOCOL */
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 11/13] server: turn spice_server_migrate_start into a valid call
We will add a qemu call to spice_server_migrate_start when migration starts.
For now, it does nothing, but we may need this notification in the future.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index 6d2269c..76aa0ed 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -5143,16 +5143,10 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_info(SpiceServer *s, const char* des
SPICE_GNUC_VISIBLE int spice_server_migrate_start(SpiceServer *s)
{
ASSERT(reds == s);
-
-if (1) {
-/* seamless doesn't work, fixing needs protocol change. */
-return -1;
-}
-
+red_printf();
if (!reds-mig_spice) {
return -1;
}
-reds_mig_started();
return 0;
}
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 13/13] Release 0.8.3
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
NEWS |7 +++
configure.ac |2 +-
server/spice-server.syms |4
server/spice.h |2 +-
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index ee6ceec..18168cb 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,10 @@
+Major changes in 0.8.3:
+===
+* server Bug fixes (RHBZ): 718713
+* client Bug fixes (RHBZ): 726441, 727969, 728252
+* server: semi-seamless migration support (RHBZ 738266)
+* require spice-protocol = 0.8.2
+
Major changes in 0.8.2:
===
* server: sasl support (fdo bz 34795)
diff --git a/configure.ac b/configure.ac
index e169f36..7fb636f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@ AC_PREREQ([2.57])
m4_define([SPICE_MAJOR], 0)
m4_define([SPICE_MINOR], 8)
-m4_define([SPICE_MICRO], 2)
+m4_define([SPICE_MICRO], 3)
AC_INIT(spice, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [], spice)
diff --git a/server/spice-server.syms b/server/spice-server.syms
index 826c562..62cdc18 100644
--- a/server/spice-server.syms
+++ b/server/spice-server.syms
@@ -81,3 +81,7 @@ global:
spice_qxl_destroy_surface_async;
spice_qxl_flush_surfaces_async;
} SPICE_SERVER_0.8.1;
+
+SPICE_SERVER_0.8.3 {
+spice_server_migrate_connect;
+} SPICE_SERVER_0.8.2;
diff --git a/server/spice.h b/server/spice.h
index 42ddbc6..ce2d149 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -22,7 +22,7 @@
#include sys/socket.h
#include spice/qxl_dev.h
-#define SPICE_SERVER_VERSION 0x000802 /* release 0.8.2 */
+#define SPICE_SERVER_VERSION 0x000803 /* release 0.8.3 */
/* interface base type */
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 04/13] server, proto: tell the client to connect to the migration target before migraton starts
(1) send SPICE_MSG_MAIN_MIGRATE_BEGIN upon spice_server_migrate_connect
(2) wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR), or a timeout,
in order
to complete client_migrate_info monitor command
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
common/messages.h |2 +
server/reds.c | 148 +++-
spice.proto |5 +-
3 files changed, 115 insertions(+), 40 deletions(-)
diff --git a/common/messages.h b/common/messages.h
index 6fcd8be..16ae05b 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -66,6 +66,8 @@ typedef struct SpiceMsgMainMigrationBegin {
uint16_t pub_key_type;
uint32_t pub_key_size;
uint8_t *pub_key_data;
+uint32_t cert_subject_size;
+uint8_t *cert_subject_data;
} SpiceMsgMainMigrationBegin;
typedef struct SpiceMsgMainMigrationSwitchHost {
diff --git a/server/reds.c b/server/reds.c
index 99d52f9..845b0ee 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -271,6 +271,7 @@ typedef struct RedsState {
VDIPortState agent_state;
InputsState *inputs_state;
+int client_semi_mig_cap;
int mig_wait_connect;
int mig_wait_disconnect;
int mig_inprogress;
@@ -280,6 +281,7 @@ typedef struct RedsState {
IncomingHandler in_handler;
RedsOutgoingData outgoing;
Channel *channels;
+Channel main_channel;
int mouse_mode;
int is_client_mouse_allowed;
int dispatcher_allows_client_mouse;
@@ -378,6 +380,7 @@ static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
static void reds_push();
static void reds_out_item_free(RedsOutItem *item);
+static void migrate_timeout(void *opaque);
static ChannelSecurityOptions *channels_security = NULL;
static int default_channel_security =
@@ -644,6 +647,12 @@ static void reds_shatdown_channels()
static void reds_mig_cleanup()
{
if (reds-mig_inprogress) {
+if (reds-mig_wait_connect) {
+SpiceMigrateInterface *sif;
+ASSERT(migration_interface);
+sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
+sif-migrate_connect_complete(migration_interface);
+}
reds-mig_inprogress = FALSE;
reds-mig_wait_connect = FALSE;
reds-mig_wait_disconnect = FALSE;
@@ -1700,7 +1709,7 @@ static void reds_main_handle_message(void *opaque, size_t
size, uint32_t type, v
reds_send_channels();
break;
case SPICE_MSGC_MAIN_MIGRATE_CONNECTED:
-red_printf(connected);
+red_printf(client connected to migration target);
if (reds-mig_wait_connect) {
reds_mig_cleanup();
}
@@ -1891,6 +1900,23 @@ static int sync_write(RedsStream *stream, const void
*in_buf, size_t n)
return TRUE;
}
+static void reds_channel_set_caps(Channel *channel, int cap, int active)
+{
+int nbefore, n;
+
+nbefore = channel-num_caps;
+n = cap / 32;
+channel-num_caps = MAX(channel-num_caps, n + 1);
+channel-caps = spice_renew(uint32_t, channel-caps, channel-num_caps);
+memset(channel-caps + nbefore, 0,
+ (channel-num_caps - nbefore) * sizeof(uint32_t));
+if (active) {
+channel-caps[n] |= (1 cap);
+} else {
+channel-caps[n] = ~(1 cap);
+}
+}
+
static void reds_channel_set_common_caps(Channel *channel, int cap, int active)
{
int nbefore, n;
@@ -1933,7 +1959,6 @@ static int reds_send_link_ack(RedLinkInfo *link)
{
SpiceLinkHeader header;
SpiceLinkReply ack;
-Channel caps = { 0, };
Channel *channel;
BUF_MEM *bmBuf;
BIO *bio;
@@ -1948,7 +1973,8 @@ static int reds_send_link_ack(RedLinkInfo *link)
channel = reds_find_channel(link-link_mess-channel_type, 0);
if (!channel) {
-channel = caps;
+ASSERT(link-link_mess-channel_type == SPICE_CHANNEL_MAIN);
+channel = reds-main_channel;
}
reds_channel_init_auth_caps(channel); /* make sure common caps are set */
@@ -1989,7 +2015,6 @@ static int reds_send_link_ack(RedLinkInfo *link)
ret = TRUE;
end:
-reds_channel_dispose(caps);
BIO_free(bio);
return ret;
}
@@ -2042,28 +2067,41 @@ static void reds_start_net_test()
}
}
+static int test_capability(uint32_t *caps, uint32_t num_caps, uint32_t cap)
+{
+uint32_t index = cap / 32;
+if (num_caps index + 1) {
+return FALSE;
+}
+
+return (caps[index] (1 (cap % 32))) != 0;
+}
+
static void reds_handle_main_link(RedLinkInfo *link)
{
+SpiceLinkMess *link_mess = link-link_mess;
uint32_t connection_id;
+uint32_t *channel_caps;
+uint32_t num_channel_caps;
red_printf();
reds_disconnect();
-if (!link-link_mess-connection_id) {
+if (!link_mess-connection_id) {
reds_send_link_result(link, SPICE_LINK_ERR_OK);
while((connection_id = rand()) == 0);
reds-agent_state.num_tokens = 0;
memcpy((reds-taTicket), taTicket, sizeof(reds-taTicket
[Qemu-devel] [PATCH 1/2] spice: turn client_migrate_info to async
RHBZ 737921
Spice client is required to connect to the migration target before/as migration
starts. Since after migration starts, the target qemu is blocked and cannot
accept new spice client
we trigger the connection to the target upon client_migrate_info command.
client_migrate_info completion cb will be called after spice client has been
connected to the target (or a timeout). See following patches and spice patches.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hmp-commands.hx |3 ++-
monitor.c |6 --
qmp-commands.hx |3 ++-
ui/qemu-spice.h | 13 +++--
ui/spice-core.c | 10 +++---
5 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9e1cca8..6f390a0 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -827,7 +827,8 @@ ETEXI
.params = protocol hostname port tls-port cert-subject,
.help = send migration info to spice/vnc client,
.user_print = monitor_user_noop,
-.mhandler.cmd_new = client_migrate_info,
+.mhandler.cmd_async = client_migrate_info,
+.flags = MONITOR_CMD_ASYNC,
},
STEXI
diff --git a/monitor.c b/monitor.c
index df0f622..0374dcc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1221,7 +1221,8 @@ static int add_graphics_client(Monitor *mon, const QDict
*qdict, QObject **ret_d
return -1;
}
-static int client_migrate_info(Monitor *mon, const QDict *qdict, QObject
**ret_data)
+static int client_migrate_info(Monitor *mon, const QDict *qdict,
+ MonitorCompletion cb, void *opaque)
{
const char *protocol = qdict_get_str(qdict, protocol);
const char *hostname = qdict_get_str(qdict, hostname);
@@ -1236,7 +1237,8 @@ static int client_migrate_info(Monitor *mon, const QDict
*qdict, QObject **ret_d
return -1;
}
-ret = qemu_spice_migrate_info(hostname, port, tls_port, subject);
+ret = qemu_spice_migrate_info(hostname, port, tls_port, subject,
+ cb, opaque);
if (ret != 0) {
qerror_report(QERR_UNDEFINED_ERROR);
return -1;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 27cc66e..321fb10 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -578,7 +578,8 @@ EQMP
.params = protocol hostname port tls-port cert-subject,
.help = send migration info to spice/vnc client,
.user_print = monitor_user_noop,
-.mhandler.cmd_new = client_migrate_info,
+.mhandler.cmd_async = client_migrate_info,
+.flags = MONITOR_CMD_ASYNC,
},
SQMP
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index f34be69..0edc47b 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -25,6 +25,7 @@
#include qemu-option.h
#include qemu-config.h
#include qemu-char.h
+#include monitor.h
extern int using_spice;
@@ -37,7 +38,8 @@ int qemu_spice_set_passwd(const char *passwd,
bool fail_if_connected, bool
disconnect_if_connected);
int qemu_spice_set_pw_expire(time_t expires);
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
-const char *subject);
+const char *subject,
+MonitorCompletion cb, void *opaque);
void do_info_spice_print(Monitor *mon, const QObject *data);
void do_info_spice(Monitor *mon, QObject **ret_data);
@@ -57,7 +59,14 @@ static inline int qemu_spice_set_pw_expire(time_t expires)
{
return -1;
}
-static inline int qemu_spice_migrate_info(const char *h, int p, int t, const
char *s)
+static inline int qemu_spice_migrate_info(const char *h, int p, int t,
+ const char *s,
+ MonitorCompletion cb, void *opaque)
+{
+cb(opaque, NULL);
+return -1;
+}
+
{ return -1; }
#endif /* CONFIG_SPICE */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 3cbc721..50c0d7d 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -457,10 +457,14 @@ static void migration_state_notifier(Notifier *notifier,
void *data)
}
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
-const char *subject)
+const char *subject,
+MonitorCompletion *cb, void *opaque)
{
-return spice_server_migrate_info(spice_server, hostname,
- port, tls_port, subject);
+int ret;
+ret = spice_server_migrate_info(spice_server, hostname,
+port, tls_port, subject);
+cb(opaque, NULL);
+return ret;
}
static int add_channel(const char *name, const char *value, void *opaque)
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 00/13] semi-seamless migration v2 (RHBZ #738266, 725009)
same as the previous version with the following changes: (1) I'm sending only the spice-server patches. I will send the client fixes later. (and then I will move the Release 0.8.3 patch to the head). (2) migration interface changes: * spice_server_migrate_connect was added. It is called on client_migrate_info cmd, if spice-server is new (instead of spice_server_migrate_info). * migrate_end_complete callback was added to the migration interface. In case we will need it for future seamless migration implementation * bump release to 0.8.3 (3) fall back to the switch host scheme in case of client failure to connect the target (4) fix accessing the wrong ticket in the target side (legacy from old seamless migration code) (5) fix not calling to migrate_connect_complete callback when no client is connected reminder: Here is a summary of the new migration scheme (copied from the commit msg of the first patch) migration source side - (1) spice_server_migrate_connect (*): tell client to link to the target side - send SPICE_MSG_MAIN_MIGRATE_BEGIN. It will be called on client_migrate_info cmd. client_migrate_info is asynchronous. (2) Complete client_migrate_info only when the client has been connected to the target - wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR) or a timeout. (3) spice_server_migrate_end: tell client migration it can switch to the target - send SPICE_MSG_MAIN_MIGRATE_END. (4) client cleans up all data related to the connection to the source and switches to the target. It sends SPICE_MSGC_MAIN_MIGRATE_END. migration target side - (1) the server identifies itself as a migraiton target since the client is linked with (connection_id != 0) (2) server doesn't start the channels' logic (channel-link) till it receives SPICE_MSGC_MAIN_MIGRATE_END from the client. * After migration starts, the target qemu is blocked and cannot accept new spice client connections. Thus, we trigger the connection to the target upon client_migrate_info command. Yonit Halperin (13): server/spice.h: semi-seamless migration interface, RHBZ #738266 server: handle migration interface addition configure: spice-protocol = 0.8.2 (semi-seamless migration protocol) server,proto: tell the client to connect to the migration target before migraton starts spice.proto: add SPICE_MSG_MAIN_MIGRATE_END SPICE_MSGC_MAIN_MIGRATE_END server: send SPICE_MSG_MAIN_MIGRATE_END on spice_server_migrate_end server: move SPICE_MSG_MAIN_INIT sending code to a separate routine server: move the linking of channels to a separate routine server: handling semi-seamless migration in the target side server: call migrate_connect_complete callback when no client is connected server: turn spice_server_migrate_start into a valid call server: fall back to switch host scheme in case semi-seamless connection to target fails Release 0.8.3 NEWS|7 + common/messages.h |2 + configure.ac|4 +- server/reds.c | 545 +-- server/reds.h |4 + server/spice-experimental.h |3 - server/spice-server.syms|4 + server/spice.h | 29 +++- spice.proto |9 +- 9 files changed, 474 insertions(+), 133 deletions(-) -- 1.7.4.4
[Qemu-devel] [PATCH spice-server 07/13] server: move SPICE_MSG_MAIN_INIT sending code to a separate routine
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 56 +---
1 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index e088b08..bea0eb0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2078,6 +2078,35 @@ static int test_capability(uint32_t *caps, uint32_t
num_caps, uint32_t cap)
return (caps[index] (1 (cap % 32))) != 0;
}
+static void reds_main_channel_init(int do_net_test)
+{
+RedsOutItem *item;
+SpiceMsgMainInit init;
+
+item = new_out_item(SPICE_MSG_MAIN_INIT);
+init.session_id = reds-link_id;
+init.display_channels_hint = red_dispatcher_count();
+init.current_mouse_mode = reds-mouse_mode;
+init.supported_mouse_modes = SPICE_MOUSE_MODE_SERVER;
+if (reds-is_client_mouse_allowed) {
+init.supported_mouse_modes |= SPICE_MOUSE_MODE_CLIENT;
+}
+init.agent_connected = !!vdagent;
+init.agent_tokens = REDS_AGENT_WINDOW_SIZE;
+reds-agent_state.num_client_tokens = REDS_AGENT_WINDOW_SIZE;
+init.multi_media_time = reds_get_mm_time() - MM_TIME_DELTA;
+init.ram_hint = red_dispatcher_qxl_ram_size();
+
+spice_marshall_msg_main_init(item-m, init);
+
+reds_push_pipe_item(item);
+if (do_net_test) {
+reds_start_net_test();
+}
+/* Now that we have a client, forward any pending agent data */
+while (read_from_vdi_port());
+}
+
static void reds_handle_main_link(RedLinkInfo *link)
{
SpiceLinkMess *link_mess = link-link_mess;
@@ -2144,29 +2173,10 @@ static void reds_handle_main_link(RedLinkInfo *link)
reds_main_event, NULL);
if (!reds-mig_target) {
-RedsOutItem *item;
-SpiceMsgMainInit init;
-
-item = new_out_item(SPICE_MSG_MAIN_INIT);
-init.session_id = connection_id;
-init.display_channels_hint = red_dispatcher_count();
-init.current_mouse_mode = reds-mouse_mode;
-init.supported_mouse_modes = SPICE_MOUSE_MODE_SERVER;
-if (reds-is_client_mouse_allowed) {
-init.supported_mouse_modes |= SPICE_MOUSE_MODE_CLIENT;
-}
-init.agent_connected = !!vdagent;
-init.agent_tokens = REDS_AGENT_WINDOW_SIZE;
-reds-agent_state.num_client_tokens = REDS_AGENT_WINDOW_SIZE;
-init.multi_media_time = reds_get_mm_time() - MM_TIME_DELTA;
-init.ram_hint = red_dispatcher_qxl_ram_size();
-
-spice_marshall_msg_main_init(item-m, init);
-
-reds_push_pipe_item(item);
-reds_start_net_test();
-/* Now that we have a client, forward any pending agent data */
-while (read_from_vdi_port());
+reds_main_channel_init(TRUE);
+}
+else {
+ASSERT(reds-client_semi_mig_cap);
}
}
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 01/13] server/spice.h: semi-seamless migration interface, RHBZ #738266
semi-seamless migration details:
migration source side
-
(1) spice_server_migrate_connect (*): tell client to link
to the target side - send SPICE_MSG_MAIN_MIGRATE_BEGIN.
This should be called upon client_migrate_info cmd.
client_migrate_info is asynchronous.
(2) Complete spice_server_migrate_connect only when the client has been
connected
to the target - wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR)
or a timeout.
(3) spice_server_migrate_end: tell client migration it can switch to the target
- send
SPICE_MSG_MAIN_MIGRATE_END.
(4) client cleans up all data related to the connection to the source and
switches to the target.
It sends SPICE_MSGC_MAIN_MIGRATE_END.
migration target side
-
(1) the server identifies itself as a migraiton target since the client is
linked with (connection_id != 0)
(2) server doesn't start the channels' logic (channel-link) till it receives
SPICE_MSGC_MAIN_MIGRATE_END
from the client.
* After migration starts, the target qemu is blocked and cannot accept new
spice client
connections. Thus, we trigger the connection to the target upon
client_migrate_info
command.
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c |9 +
server/spice-experimental.h |3 ---
server/spice.h | 27 ++-
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index f082c53..9a983f8 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -4854,6 +4854,15 @@ SPICE_GNUC_VISIBLE int
spice_server_set_agent_copypaste(SpiceServer *s, int enab
return 0;
}
+/* semi-seamless client migration */
+SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const
char* dest,
+int port, int secure_port,
+const char* cert_subject)
+{
+red_printf(not implemented yet);
+return 0;
+}
+
SPICE_GNUC_VISIBLE int spice_server_migrate_info(SpiceServer *s, const char*
dest,
int port, int secure_port,
const char* cert_subject)
diff --git a/server/spice-experimental.h b/server/spice-experimental.h
index 482ac44..6997aa0 100644
--- a/server/spice-experimental.h
+++ b/server/spice-experimental.h
@@ -29,16 +29,13 @@ void spice_server_net_wire_recv_packet(SpiceNetWireInstance
*sin,
const uint8_t *pkt, int len);
/* spice seamless client migration (broken) */
-
enum {
SPICE_MIGRATE_CLIENT_NONE = 1,
SPICE_MIGRATE_CLIENT_WAITING,
SPICE_MIGRATE_CLIENT_READY,
};
-int spice_server_migrate_start(SpiceServer *s);
int spice_server_migrate_client_state(SpiceServer *s);
-int spice_server_migrate_end(SpiceServer *s, int completed);
#endif // __SPICE_EXPERIMENTAL_H__
diff --git a/server/spice.h b/server/spice.h
index ac5a41e..42ddbc6 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -469,11 +469,36 @@ int spice_server_set_agent_copypaste(SpiceServer *s, int
enable);
int spice_server_get_sock_info(SpiceServer *s, struct sockaddr *sa, socklen_t
*salen);
int spice_server_get_peer_info(SpiceServer *s, struct sockaddr *sa, socklen_t
*salen);
-/* spice switch-host client migration */
+/* migration interface */
+#define SPICE_INTERFACE_MIGRATION migration
+#define SPICE_INTERFACE_MIGRATION_MAJOR 1
+#define SPICE_INTERFACE_MIGRATION_MINOR 1
+typedef struct SpiceMigrateInterface SpiceMigrateInterface;
+typedef struct SpiceMigrateInstance SpiceMigrateInstance;
+typedef struct SpiceMigrateState SpiceMigrateState;
+
+struct SpiceMigrateInterface {
+SpiceBaseInterface base;
+void (*migrate_connect_complete)(SpiceMigrateInstance *sin);
+void (*migrate_end_complete)(SpiceMigrateInstance *sin);
+};
+
+struct SpiceMigrateInstance {
+SpiceBaseInstance base;
+SpiceMigrateState *st;
+};
+/* spice switch-host client migration */
int spice_server_migrate_info(SpiceServer *s, const char* dest,
int port, int secure_port,
const char* cert_subject);
int spice_server_migrate_switch(SpiceServer *s);
+/* spice (semi-)seamless client migration */
+int spice_server_migrate_connect(SpiceServer *s, const char* dest,
+ int port, int secure_port,
+ const char* cert_subject);
+int spice_server_migrate_start(SpiceServer *s);
+int spice_server_migrate_end(SpiceServer *s, int completed);
+
#endif
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 09/13] server: handling semi-seamless migration in the target side
(1) not sending anything to the client till we recieve SPICE_MSGC_MIGRATE_END
(2) start a new migration (handle client_migrate_info) only after
SPICE_MSGC_MIGRATE_END
from the previous migration has been received
(3) use the correct ticket
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 137 -
1 files changed, 116 insertions(+), 21 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index e7388a0..ca4e1d1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -258,6 +258,13 @@ typedef struct RedsStatValue {
#endif
typedef struct RedsMigSpice RedsMigSpice;
+typedef struct RedsMigPendingLink RedsMigPendingLink;
+
+struct RedsMigPendingLink {
+RedsMigPendingLink *next;
+SpiceLinkMess *link_msg;
+RedsStream *stream;
+};
typedef struct RedsState {
int listen_socket;
@@ -274,10 +281,13 @@ typedef struct RedsState {
int client_semi_mig_cap;
int mig_wait_connect;
int mig_wait_disconnect;
+int mig_wait_prev_complete;
int mig_inprogress;
int expect_migrate;
int mig_target;
RedsMigSpice *mig_spice;
+RedsMigPendingLink *mig_pending_links;
+int num_mig_links;
int num_of_channels;
IncomingHandler in_handler;
RedsOutgoingData outgoing;
@@ -293,7 +303,6 @@ typedef struct RedsState {
SpiceTimer *vdi_port_write_timer;
int vdi_port_write_timer_started;
-TicketAuthentication taTicket;
SSL_CTX *ctx;
#ifdef RED_STATISTICS
@@ -382,6 +391,9 @@ static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
static void reds_push();
static void reds_out_item_free(RedsOutItem *item);
static void migrate_timeout(void *opaque);
+static void reds_mig_pending_links_free(void);
+static void reds_handle_client_migrate_complete(void);
+static void reds_mig_started(void);
static ChannelSecurityOptions *channels_security = NULL;
static int default_channel_security =
@@ -648,7 +660,7 @@ static void reds_shatdown_channels()
static void reds_mig_cleanup()
{
if (reds-mig_inprogress) {
-if (reds-mig_wait_connect) {
+if (reds-mig_wait_connect || reds-mig_wait_prev_complete) {
SpiceMigrateInterface *sif;
ASSERT(migration_interface);
sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
@@ -657,8 +669,13 @@ static void reds_mig_cleanup()
reds-mig_inprogress = FALSE;
reds-mig_wait_connect = FALSE;
reds-mig_wait_disconnect = FALSE;
+reds-mig_wait_prev_complete = FALSE;
core-timer_cancel(reds-mig_timer);
}
+if (reds-num_mig_links) {
+ASSERT(reds-mig_target);
+reds_mig_pending_links_free();
+}
}
static void reds_reset_vdp()
@@ -925,6 +942,11 @@ static void reds_send_channels()
Channel *channel;
int i;
+if (reds-mig_target) {
+red_printf(warning: unexpected SPICE_MSGC_MAIN_ATTACH_CHANNELS during
migration);
+return;
+}
+
item = new_out_item(SPICE_MSG_MAIN_CHANNELS_LIST);
channels_info = (SpiceMsgChannels *)spice_malloc(sizeof(SpiceMsgChannels)
+ reds-num_of_channels * sizeof(SpiceChannelId));
channels_info-num_of_channels = reds-num_of_channels;
@@ -1008,7 +1030,7 @@ static void reds_send_mouse_mode()
SpiceMsgMainMouseMode mouse_mode;
RedsOutItem *item;
-if (!reds-stream) {
+if (!reds-stream || reds-mig_target) {
return;
}
@@ -1077,6 +1099,7 @@ static void reds_agent_remove()
SpiceCharDeviceInstance *sin = vdagent;
SpiceCharDeviceInterface *sif;
+// TODO: is this cond needed
if (!reds-mig_target) {
reds_reset_vdp();
}
@@ -1103,7 +1126,7 @@ static void reds_send_tokens()
SpiceMsgMainAgentTokens tokens;
RedsOutItem *item;
-if (!reds-stream) {
+if (!reds-stream || reds-mig_target) {
return;
}
@@ -1798,6 +1821,18 @@ static void reds_main_handle_message(void *opaque,
size_t size, uint32_t type, v
break;
case SPICE_MSGC_DISCONNECTING:
break;
+case SPICE_MSGC_MAIN_MIGRATE_END:
+if (!reds-mig_target) {
+red_printf(unexpected SPICE_MSGC_MIGRATE_END, not target);
+return;
+}
+if (!reds-client_semi_mig_cap) {
+red_printf(unexpected SPICE_MSGC_MIGRATE_END
+ ,client does not support semi-seamless migration);
+return;
+}
+reds_handle_client_migrate_complete();
+break;
default:
red_printf(unexpected type %d, type);
}
@@ -2122,14 +2157,10 @@ static void reds_handle_main_link(RedLinkInfo *link)
reds_send_link_result(link, SPICE_LINK_ERR_OK);
while((connection_id = rand()) == 0);
reds-agent_state.num_tokens = 0;
-memcpy((reds-taTicket), taTicket, sizeof(reds-taTicket));
reds-mig_target = FALSE;
} else {
-if (link_mess-connection_id != reds-link_id
[Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 68 +++-
1 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index bea0eb0..e7388a0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2612,12 +2612,48 @@ static void inputs_init()
reds_register_channel(channel);
}
+static void reds_send_input_channel_insecure_warn()
+{
+RedsOutItem *item;
+SpiceMsgNotify notify;
+char *mess = keyboard channel is insecure;
+const int mess_len = strlen(mess);
+
+item = new_out_item(SPICE_MSG_NOTIFY);
+
+notify.time_stamp = get_time_stamp();
+notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
+notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
+notify.what = SPICE_WARN_GENERAL;
+notify.message_len = mess_len;
+
+spice_marshall_msg_notify(item-m, notify);
+spice_marshaller_add(item-m, (uint8_t *)mess, mess_len + 1);
+
+reds_push_pipe_item(item);
+}
+
+static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg,
RedsStream *stream)
+{
+uint32_t *caps;
+
+ASSERT(channel);
+ASSERT(link_msg);
+ASSERT(stream);
+
+if (link_msg-channel_type == SPICE_CHANNEL_INPUTS !stream-ssl) {
+reds_send_input_channel_insecure_warn();
+}
+caps = (uint32_t *)((uint8_t *)link_msg + link_msg-caps_offset);
+channel-link(channel, stream, FALSE, link_msg-num_common_caps,
+ link_msg-num_common_caps ? caps : NULL,
link_msg-num_channel_caps,
+ link_msg-num_channel_caps ? caps +
link_msg-num_common_caps : NULL);
+}
+
static void reds_handle_other_links(RedLinkInfo *link)
{
Channel *channel;
-RedsStream *stream;
SpiceLinkMess *link_mess;
-uint32_t *caps;
link_mess = link-link_mess;
@@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link)
}
reds_send_link_result(link, SPICE_LINK_ERR_OK);
+// TODO: if mig_target, where this call should be? here or only when the
links are handled
reds_show_new_channel(link, reds-link_id);
-if (link_mess-channel_type == SPICE_CHANNEL_INPUTS !link-stream-ssl)
{
-RedsOutItem *item;
-SpiceMsgNotify notify;
-char *mess = keyboard channel is insecure;
-const int mess_len = strlen(mess);
-
-item = new_out_item(SPICE_MSG_NOTIFY);
-
-notify.time_stamp = get_time_stamp();
-notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
-notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
-notify.what = SPICE_WARN_GENERAL;
-notify.message_len = mess_len;
+reds_stream_remove_watch(link-stream);
-spice_marshall_msg_notify(item-m, notify);
-spice_marshaller_add(item-m, (uint8_t *)mess, mess_len + 1);
+reds_channel_do_link(channel, link-link_mess, link-stream);
+free(link_mess);
-reds_push_pipe_item(item);
-}
-stream = link-stream;
-reds_stream_remove_watch(stream);
link-stream = NULL;
link-link_mess = NULL;
reds_link_free(link);
-caps = (uint32_t *)((uint8_t *)link_mess + link_mess-caps_offset);
-channel-link(channel, stream, reds-mig_target,
link_mess-num_common_caps,
- link_mess-num_common_caps ? caps : NULL,
link_mess-num_channel_caps,
- link_mess-num_channel_caps ? caps +
link_mess-num_common_caps : NULL);
-free(link_mess);
}
static void reds_handle_link(RedLinkInfo *link)
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 12/13] server: fall back to switch host scheme in case semi-seamless connection to target fails
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 18 +-
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index 76aa0ed..54c06d1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -283,6 +283,7 @@ typedef struct RedsState {
int mig_wait_disconnect;
int mig_wait_prev_complete;
int mig_inprogress;
+int mig_connect_ok;
int expect_migrate;
int mig_target;
RedsMigSpice *mig_spice;
@@ -1736,6 +1737,7 @@ static void reds_main_handle_message(void *opaque, size_t
size, uint32_t type, v
case SPICE_MSGC_MAIN_MIGRATE_CONNECTED:
red_printf(client connected to migration target);
if (reds-mig_wait_connect) {
+reds-mig_connect_ok = TRUE;
reds_mig_cleanup();
}
break;
@@ -1743,6 +1745,7 @@ static void reds_main_handle_message(void *opaque, size_t
size, uint32_t type, v
// TODO: fall into switch host in case of connect error or timeout
red_printf(mig connect error);
if (reds-mig_wait_connect) {
+reds-mig_connect_ok = FALSE;
reds_mig_cleanup();
}
break;
@@ -2172,6 +2175,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
reds-mig_inprogress = FALSE;
reds-mig_wait_connect = FALSE;
reds-mig_wait_disconnect = FALSE;
+reds-mig_connect_ok = FALSE;
reds-stream = link-stream;
reds-in_handler.shut = FALSE;
@@ -4151,8 +4155,6 @@ static void reds_mig_connect(void)
reds_push_pipe_item(item);
-reds_mig_release();
-
reds-mig_wait_connect = TRUE;
core-timer_start(reds-mig_timer, MIGRATE_TIMEOUT);
}
@@ -4194,6 +4196,7 @@ static void reds_mig_started(void)
reds_listen_stop();
sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
+reds-mig_connect_ok = FALSE;
if (reds-stream == NULL) {
red_printf(not connected to stream);
@@ -4227,7 +4230,7 @@ static void reds_mig_finished(int completed)
RedsOutItem *item;
red_printf();
-
+reds_mig_release();
if (reds-stream == NULL) {
red_printf(no stream connected);
return;
@@ -4298,7 +4301,12 @@ static void migrate_timeout(void *opaque)
{
red_printf();
ASSERT(reds-mig_wait_connect || reds-mig_wait_disconnect ||
reds-mig_wait_prev_complete);
-reds_mig_disconnect();
+if (reds-mig_wait_connect) {
+reds-mig_connect_ok = FALSE;
+reds_mig_cleanup();
+} else {
+reds_mig_disconnect();
+}
}
static void key_modifiers_sender(void *opaque)
@@ -5188,7 +5196,7 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_end(SpiceServer *s, int completed)
goto complete;
}
-if (reds-client_semi_mig_cap) {
+if (reds-client_semi_mig_cap reds-mig_connect_ok) {
reds_mig_finished(completed);
} else {
ret = spice_server_migrate_switch(s);
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 05/13] spice.proto: add SPICE_MSG_MAIN_MIGRATE_END SPICE_MSGC_MAIN_MIGRATE_END
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
spice.proto |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/spice.proto b/spice.proto
index d5b954e..235ec95 100644
--- a/spice.proto
+++ b/spice.proto
@@ -219,6 +219,8 @@ channel MainChannel : BaseChannel {
uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
} @ctype(SpiceMsgMainMigrationSwitchHost) migrate_switch_host;
+Empty migrate_end;
+
client:
message {
uint64 cache_size;
@@ -243,6 +245,8 @@ channel MainChannel : BaseChannel {
message {
uint32 num_tokens;
} @ctype(SpiceMsgcMainAgentTokens) agent_token;
+
+Empty migrate_end;
};
enum8 clip_type {
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 06/13] server: send SPICE_MSG_MAIN_MIGRATE_END on spice_server_migrate_end
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 86 +
1 files changed, 68 insertions(+), 18 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index 845b0ee..e088b08 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -275,6 +275,7 @@ typedef struct RedsState {
int mig_wait_connect;
int mig_wait_disconnect;
int mig_inprogress;
+int expect_migrate;
int mig_target;
RedsMigSpice *mig_spice;
int num_of_channels;
@@ -4038,13 +4039,21 @@ static void reds_mig_continue(void)
core-timer_start(reds-mig_timer, MIGRATE_TIMEOUT);
}
-static void reds_mig_started(void)
+static void reds_listen_start(void)
{
-red_printf();
-ASSERT(reds-mig_spice);
+ASSERT(reds);
+if (reds-listen_watch != NULL) {
+core-watch_update_mask(reds-listen_watch, SPICE_WATCH_EVENT_READ);
+}
-reds-mig_inprogress = TRUE;
+if (reds-secure_listen_watch != NULL) {
+core-watch_update_mask(reds-secure_listen_watch,
SPICE_WATCH_EVENT_READ);
+}
+}
+static void reds_listen_stop(void)
+{
+ASSERT(reds);
if (reds-listen_watch != NULL) {
core-watch_update_mask(reds-listen_watch, 0);
}
@@ -4052,6 +4061,14 @@ static void reds_mig_started(void)
if (reds-secure_listen_watch != NULL) {
core-watch_update_mask(reds-secure_listen_watch, 0);
}
+}
+
+static void reds_mig_started(void)
+{
+red_printf();
+ASSERT(reds-mig_spice);
+
+reds-mig_inprogress = TRUE;
if (reds-stream == NULL) {
red_printf(not connected to stream);
@@ -4071,13 +4088,6 @@ static void reds_mig_finished(int completed)
RedsOutItem *item;
red_printf();
-if (reds-listen_watch != NULL) {
-core-watch_update_mask(reds-listen_watch, SPICE_WATCH_EVENT_READ);
-}
-
-if (reds-secure_listen_watch != NULL) {
-core-watch_update_mask(reds-secure_listen_watch,
SPICE_WATCH_EVENT_READ);
-}
if (reds-stream == NULL) {
red_printf(no stream connected);
@@ -4086,22 +4096,25 @@ static void reds_mig_finished(int completed)
reds-mig_inprogress = TRUE;
if (completed) {
+#ifdef SPICE_SEAMLESS_MIGRATION
Channel *channel;
SpiceMsgMigrate migrate;
-reds-mig_wait_disconnect = TRUE;
-core-timer_start(reds-mig_timer, MIGRATE_TIMEOUT);
-
item = new_out_item(SPICE_MSG_MIGRATE);
migrate.flags = SPICE_MIGRATE_NEED_FLUSH |
SPICE_MIGRATE_NEED_DATA_TRANSFER;
spice_marshall_msg_migrate(item-m, migrate);
-
reds_push_pipe_item(item);
channel = reds-channels;
while (channel) {
channel-migrate(channel);
channel = channel-next;
}
+#else
+item = new_out_item(SPICE_MSG_MAIN_MIGRATE_END);
+reds_push_pipe_item(item);
+#endif
+reds-mig_wait_disconnect = TRUE;
+core-timer_start(reds-mig_timer, MIGRATE_TIMEOUT);
} else {
item = new_out_item(SPICE_MSG_MAIN_MIGRATE_CANCEL);
reds_push_pipe_item(item);
@@ -4116,6 +4129,8 @@ static void reds_mig_switch(void)
RedsOutItem *item;
if (s == NULL) {
+red_printf(warning: migration target was not set. disconnecting
client);
+reds_disconnect();
return;
}
@@ -4956,6 +4971,12 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_connect(SpiceServer *s, const char*
ASSERT(migration_interface);
ASSERT(reds == s);
+
+if (reds-expect_migrate reds-client_semi_mig_cap) {
+red_printf(warning: consecutive calls without migration. Canceling
previous call);
+reds_mig_finished(FALSE);
+}
+
sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
@@ -4963,6 +4984,9 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_connect(SpiceServer *s, const char*
return -1;
}
+reds-expect_migrate = TRUE;
+reds_listen_stop();
+
if (reds-client_semi_mig_cap) {
reds_mig_started();
} else {
@@ -4975,13 +4999,13 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_info(SpiceServer *s, const char* des
int port, int secure_port,
const char* cert_subject)
{
+red_printf();
ASSERT(!migration_interface);
ASSERT(reds == s);
if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
return -1;
}
-
return 0;
}
@@ -5018,20 +5042,46 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_client_state(SpiceServer *s)
SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
{
SpiceMigrateInterface *sif;
+int ret = 0;
+
+red_printf();
+
ASSERT(migration_interface);
ASSERT(reds == s);
-reds_mig_finished(completed);
+
+reds_listen_start();
sif
[Qemu-devel] [PATCH spice-server 10/13] server: call migrate_connect_complete callback when no client is connected
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 72 ++---
1 files changed, 43 insertions(+), 29 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index ca4e1d1..6d2269c 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -761,6 +761,7 @@ static void reds_disconnect()
reds-net_test_id = 0;
reds-net_test_stage = NET_TEST_STAGE_INVALID;
reds-in_handler.end_pos = 0;
+reds-expect_migrate = FALSE;
bitrate_per_sec = ~0;
latency = 0;
@@ -1739,6 +1740,7 @@ static void reds_main_handle_message(void *opaque, size_t
size, uint32_t type, v
}
break;
case SPICE_MSGC_MAIN_MIGRATE_CONNECT_ERROR:
+// TODO: fall into switch host in case of connect error or timeout
red_printf(mig connect error);
if (reds-mig_wait_connect) {
reds_mig_cleanup();
@@ -4124,7 +4126,7 @@ static void reds_mig_release(void)
}
}
-static void reds_mig_continue(void)
+static void reds_mig_connect(void)
{
RedsMigSpice *s = reds-mig_spice;
SpiceMsgMainMigrationBegin migrate;
@@ -4181,22 +4183,43 @@ static void reds_listen_stop(void)
static void reds_mig_started(void)
{
+SpiceMigrateInterface *sif = NULL;
+
red_printf();
ASSERT(reds-mig_spice);
-reds-mig_inprogress = TRUE;
+if (!migration_interface) {
+return;
+}
+
+reds_listen_stop();
+sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
if (reds-stream == NULL) {
red_printf(not connected to stream);
-goto error;
+reds_mig_release();
+sif-migrate_connect_complete(migration_interface);
+return;
}
-reds_mig_continue();
-return;
+reds-expect_migrate = TRUE;
+if (reds-client_semi_mig_cap) {
+if (reds-mig_target) {
+red_printf(previous spice migration hasn't completed yet. Waiting
for client);
+reds-mig_wait_prev_complete = TRUE;
+core-timer_start(reds-mig_timer, MIGRATE_TIMEOUT);
+return;
+}
+} else if (sif) {
+// switch host msg will be sent after migration completes
+sif-migrate_connect_complete(migration_interface);
+return;
+}
-error:
-reds_mig_release();
-reds_mig_disconnect();
+reds-mig_inprogress = TRUE;
+
+reds_mig_connect();
+return;
}
static void reds_mig_finished(int completed)
@@ -5060,6 +5083,7 @@ static int reds_set_migration_dest_info(const char* dest,
RedsMigSpice *spice_migration = NULL;
reds_mig_release();
+
if ((port == -1 secure_port == -1) || !dest) {
return FALSE;
}
@@ -5082,7 +5106,6 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_connect(SpiceServer *s, const char*
int port, int secure_port,
const char* cert_subject)
{
-SpiceMigrateInterface *sif;
red_printf();
ASSERT(migration_interface);
ASSERT(reds == s);
@@ -5092,27 +5115,14 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_connect(SpiceServer *s, const char*
reds_mig_finished(FALSE);
}
-sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
-
if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
+SpiceMigrateInterface *sif;
+sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
sif-migrate_connect_complete(migration_interface);
return -1;
}
+reds_mig_started();
-reds-expect_migrate = TRUE;
-reds_listen_stop();
-
-if (reds-client_semi_mig_cap) {
-if (!reds-mig_target) {
-reds_mig_started();
-} else {
-red_printf(previous spice migration hasn't completed yet. Waiting
for client);
-reds-mig_wait_prev_complete = TRUE;
-core-timer_start(reds-mig_timer, MIGRATE_TIMEOUT);
-}
-} else {
-sif-migrate_connect_complete(migration_interface);
-}
return 0;
}
@@ -5166,20 +5176,24 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_end(SpiceServer *s, int completed)
int ret = 0;
red_printf();
-
ASSERT(migration_interface);
ASSERT(reds == s);
reds_listen_start();
sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
-if (!reds-expect_migrate reds-stream) {
-red_printf(spice_server_migrate_info was not called, disconnecting
client);
+
+if (!reds-stream) {
+ret = 0;
+goto complete;
+}
+
+if (!reds-expect_migrate) {
+red_printf(spice_server_migrate_info failed or was not called,
disconnecting client);
reds_disconnect();
ret = -1;
goto complete;
}
-reds-expect_migrate = FALSE;
if (reds-client_semi_mig_cap
[Qemu-devel] [PATCH spice-server 02/13] server: handle migration interface addition
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
server/reds.c | 29 +
server/reds.h |4
2 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/server/reds.c b/server/reds.c
index 9a983f8..99d52f9 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -73,6 +73,7 @@ static SpiceKbdInstance *keyboard = NULL;
static SpiceMouseInstance *mouse = NULL;
static SpiceTabletInstance *tablet = NULL;
static SpiceCharDeviceInstance *vdagent = NULL;
+static SpiceMigrateInstance *migration_interface = NULL;
#define MIGRATION_NOTIFY_SPICE_KEY spice_mig_ext
@@ -4345,6 +4346,20 @@ SPICE_GNUC_VISIBLE int
spice_server_add_interface(SpiceServer *s,
red_printf(unsupported net wire interface);
return -1;
#endif
+} else if (strcmp(interface-type, SPICE_INTERFACE_MIGRATION) == 0) {
+red_printf(SPICE_INTERFACE_MIGRATION);
+if (migration_interface) {
+red_printf(already have migration);
+return -1;
+}
+
+if (interface-major_version != SPICE_INTERFACE_MIGRATION_MAJOR ||
+interface-minor_version SPICE_INTERFACE_MIGRATION_MINOR) {
+red_printf(unsupported migration interface);
+return -1;
+}
+migration_interface = SPICE_CONTAINEROF(sin, SpiceMigrateInstance,
base);
+migration_interface-st = spice_new0(SpiceMigrateState, 1);
}
return 0;
@@ -4859,7 +4874,15 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_connect(SpiceServer *s, const char*
int port, int secure_port,
const char* cert_subject)
{
+SpiceMigrateInterface *sif;
+red_printf();
+ASSERT(migration_interface);
+ASSERT(reds == s);
+
red_printf(not implemented yet);
+sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
+sif-migrate_connect_complete(migration_interface);
+
return 0;
}
@@ -4920,8 +4943,14 @@ SPICE_GNUC_VISIBLE int
spice_server_migrate_client_state(SpiceServer *s)
SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
{
+SpiceMigrateInterface *sif;
+ASSERT(migration_interface);
ASSERT(reds == s);
reds_mig_finished(completed);
+sif = SPICE_CONTAINEROF(migration_interface-base.sif,
SpiceMigrateInterface, base);
+if (sif-migrate_end_complete) {
+sif-migrate_end_complete(migration_interface);
+}
return 0;
}
diff --git a/server/reds.h b/server/reds.h
index 463c94f..b60681a 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -119,6 +119,10 @@ struct SpiceNetWireState {
struct TunnelWorker *worker;
};
+struct SpiceMigrateState {
+int dummy;
+};
+
void reds_channel_dispose(Channel *channel);
ssize_t reds_stream_read(RedsStream *s, void *buf, size_t nbyte);
--
1.7.4.4
[Qemu-devel] [PATCH spice-server 03/13] configure: spice-protocol = 0.8.2 (semi-seamless migration protocol)
Signed-off-by: Yonit Halperin yhalp...@redhat.com --- configure.ac |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index 3a86515..e169f36 100644 --- a/configure.ac +++ b/configure.ac @@ -126,7 +126,7 @@ fi dnl = dnl Check deps -PKG_CHECK_MODULES(PROTOCOL, spice-protocol = 0.8.1) +PKG_CHECK_MODULES(PROTOCOL, spice-protocol = 0.8.2) AC_SUBST(PROTOCOL_CFLAGS) AC_CHECK_LIBM -- 1.7.4.4
[Qemu-devel] [PATCH] spice: set qxl-ssd.running=true before telling spice to start, RHBZ #733993
If qxl-ssd.running=true is set after telling spice to start, the spice server
thread can call qxl_send_events while qxl-ssd.running is still false. This
leads to
assert(d-ssd.running).
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
Since it looks like the purpose of the assert in qxl_send_event is preventing
changes
in the guest when the vm is stopped, I think it is not necessary for
ssd.running to be
exactly synchronized with the spice server status, but just be true before
the spice worker starts.
ui/spice-display.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 683d454..3224f99 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -260,11 +260,12 @@ void qemu_spice_vm_change_state_handler(void *opaque, int
running, int reason)
SimpleSpiceDisplay *ssd = opaque;
if (running) {
+ssd-running = true;
qemu_spice_start(ssd);
} else {
qemu_spice_stop(ssd);
+ssd-running = false;
}
-ssd-running = running;
}
void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
--
1.7.4.4
Re: [Qemu-devel] [Spice-devel] [PATCH] server: don't call reds_stream_free from worker thread context
On 09/02/2011 06:19 PM, Gerd Hoffmann wrote:
reds_stream_free() may call the channel_event callback which is not
supposed to be callsed from worker thread context. This patch moves
the reds_stream_free call for the display channel from the worker to
the dispatcher to fix this issue.
[ Note: not tested yet, against 0.8 branch, sending out for review
comments nevertheless ]
Signed-off-by: Gerd Hoffmannkra...@redhat.com
---
server/red_dispatcher.c |5 +
server/red_worker.c |3 +--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index f74b13e..801a575 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -51,6 +51,7 @@ struct RedDispatcher {
int y_res;
int use_hardware_cursor;
RedDispatcher *next;
+RedsStream *stream;
RedWorkerMessage async_message;
pthread_mutex_t async_lock;
QXLDevSurfaceCreate surface_create;
@@ -81,6 +82,7 @@ static void red_dispatcher_set_peer(Channel *channel,
RedsStream *stream, int mi
red_printf();
dispatcher = (RedDispatcher *)channel-data;
+dispatcher-stream = stream;
RedWorkerMessage message = RED_WORKER_MESSAGE_DISPLAY_CONNECT;
write_message(dispatcher-channel,message);
send_data(dispatcher-channel,stream, sizeof(RedsStream *));
@@ -93,6 +95,9 @@ static void red_dispatcher_shutdown_peer(Channel *channel)
red_printf();
RedWorkerMessage message = RED_WORKER_MESSAGE_DISPLAY_DISCONNECT;
write_message(dispatcher-channel,message);
+read_message(dispatcher-channel,message);
+ASSERT(message == RED_WORKER_MESSAGE_READY);
+reds_stream_free(dispatcher-stream);
Hi,
RED_WORKER_MESSAGE_DISPLAY_DISCONNECT is not the only place that
triggers red_disconnect_channel (and as a result,
reds_stream_free(dispatcher-stream)). red_disconnect_channel is called
also when there is an error upon receive/send and also when timeouts
related to the client occur (e.g., in flush_display_commands).
We probably better make the dispatcher bi-directional, i.e., not only
push messages to the worker, but also listen.
Cheers,
Yonit.
}
static void red_dispatcher_migrate(Channel *channel)
diff --git a/server/red_worker.c b/server/red_worker.c
index 5f07803..f77b0f2 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -8486,8 +8486,6 @@ static void red_disconnect_channel(RedChannel *channel)
{
channel_release_res(channel);
red_pipe_clear(channel);
-reds_stream_free(channel-stream);
-channel-stream = NULL;
channel-send_data.blocked = FALSE;
channel-send_data.size = channel-send_data.pos = 0;
spice_marshaller_reset(channel-send_data.marshaller);
@@ -10060,6 +10058,7 @@ static void handle_dev_input(EventListener *listener,
uint32_t events)
case RED_WORKER_MESSAGE_CURSOR_DISCONNECT:
red_printf(cursor disconnect);
red_disconnect_cursor((RedChannel *)worker-cursor_channel);
+write_ready = 1;
break;
case RED_WORKER_MESSAGE_CURSOR_MIGRATE:
red_printf(cursor migrate);
[Qemu-devel] [PATCH v3 2/2] qxl: s/qxl_set_irq/qxl_update_irq/
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index de65a40..f00b5d3 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -808,7 +808,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
qxl_destroy_primary(d, QXL_SYNC);
}
-static void qxl_set_irq(PCIQXLDevice *d)
+static void qxl_update_irq(PCIQXLDevice *d)
{
uint32_t pending = le32_to_cpu(d-ram-int_pending);
uint32_t mask= le32_to_cpu(d-ram-int_mask);
@@ -1209,7 +1209,7 @@ async_common:
qemu_spice_wakeup(d-ssd);
break;
case QXL_IO_UPDATE_IRQ:
-qxl_set_irq(d);
+qxl_update_irq(d);
break;
case QXL_IO_NOTIFY_OOM:
if (!SPICE_RING_IS_EMPTY(d-ram-release_ring)) {
@@ -1359,7 +1359,7 @@ static void pipe_read(void *opaque)
do {
len = read(d-pipe[0], dummy, sizeof(dummy));
} while (len == sizeof(dummy));
-qxl_set_irq(d);
+qxl_update_irq(d);
}
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
@@ -1373,7 +1373,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t
events)
return;
}
if (pthread_self() == d-main) {
-qxl_set_irq(d);
+qxl_update_irq(d);
} else {
if (write(d-pipe[1], d, 1) != 1) {
dprint(d, 1, %s: write to pipe failed\n, __FUNCTION__);
@@ -1465,10 +1465,10 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
if (running) {
/*
* if qxl_send_events was called from spice server context before
- * migration ended, qxl_set_irq for these events might not have been
+ * migration ended, qxl_update_irq for these events might not have been
* called
*/
- qxl_set_irq(qxl);
+ qxl_update_irq(qxl);
} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
--
1.7.4.4
[Qemu-devel] [PATCH v3 1/2] qxl: send interrupt after migration in case ram-int_pending != 0, RHBZ #732949
if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram-int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
hw/qxl.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..de65a40 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
qxl_set_irq(d);
}
-/* called from spice server thread context only */
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
{
uint32_t old_pending;
@@ -1463,7 +1462,14 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
PCIQXLDevice *qxl = opaque;
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
-if (!running qxl-mode == QXL_MODE_NATIVE) {
+if (running) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been
+ * called
+ */
+ qxl_set_irq(qxl);
+} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
/* FIXME #1: should go out during live stage */
--
1.7.4.4
Re: [Qemu-devel] [PATCH v2 1/2] qxl: send interrupt after migration in case ram-int_pending != 0, RHBZ #732949
On 09/01/2011 10:36 PM, Michael S. Tsirkin wrote:
On Wed, Aug 31, 2011 at 03:37:33PM +0300, Yonit Halperin wrote:
if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt.
This is a general issue with interrupt migration, and PCI core has code
to handle this, migrating interrupts. So rather than work around this
in qxl I'd like us to first understand whether there really exists such
a problem, since if yes it would affect other devices.
Could you help with that please?
I think this issue is spice-specific: the problem is that when a
spice_server thread issues a request for interrupt, the request is
passed to the qemu thread through a pipe. This pipe status is not saved
during migration. Thus, any pending interrupt request are purged when
migration completes.
In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram-int_pending was set.
Maybe this is the only issue?
A way to check would be to call
uint32_t pending = le32_to_cpu(d-ram-int_pending);
uint32_t mask= le32_to_cpu(d-ram-int_mask);
int level = !!(pending mask);
qxl_ring_set_dirty(d);
instead of qxl_set_irq, and see if that is enough.
I was talking about the check in qxl_send_events
Note: I don't object to reusing qxl_set_irq in
production, just let us make sure we don't hide bugs.
As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
You need to sign off :)
---
hw/qxl.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..c7edc60 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
qxl_set_irq(d);
}
-/* called from spice server thread context only */
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
{
uint32_t old_pending;
@@ -1463,7 +1462,13 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
PCIQXLDevice *qxl = opaque;
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
-if (!running qxl-mode == QXL_MODE_NATIVE) {
+if (running) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been
called
+ */
+ qxl_set_irq(qxl);
+} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
/* FIXME #1: should go out during live stage */
--
1.7.4.4
[Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram-int_pending != 0, RHBZ #732949
if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram-int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
---
hw/qxl.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..5dfda11 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
qxl_set_irq(d);
}
-/* called from spice server thread context only */
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
{
uint32_t old_pending;
@@ -1463,7 +1462,15 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
PCIQXLDevice *qxl = opaque;
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
-if (!running qxl-mode == QXL_MODE_NATIVE) {
+if (running) {
+if (qxl-ram-int_pending) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have
been called
+ */
+qxl_set_irq(qxl);
+}
+} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
/* FIXME #1: should go out during live stage */
--
1.7.4.4
Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram-int_pending != 0, RHBZ #732949
On 08/31/2011 01:23 PM, Gerd Hoffmann wrote:
On 08/31/11 10:20, Yonit Halperin wrote:
if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram-int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
- if (!running qxl-mode == QXL_MODE_NATIVE) {
+ if (running) {
+ if (qxl-ram-int_pending) {
+ /*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been
called
+ */
+ qxl_set_irq(qxl);
+ }
You can call qxl_set_irq unconditionally,
it checks for int_pending anyway.
Hi,
qxl_set_irq doesn't test int_pending, but it will call qemu_set_irq
with level=0 if !int_pending.
cheers,
Gerd
[Qemu-devel] [PATCH v2 1/2] qxl: send interrupt after migration in case ram-int_pending != 0, RHBZ #732949
if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram-int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
---
hw/qxl.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..c7edc60 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
qxl_set_irq(d);
}
-/* called from spice server thread context only */
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
{
uint32_t old_pending;
@@ -1463,7 +1462,13 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
PCIQXLDevice *qxl = opaque;
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
-if (!running qxl-mode == QXL_MODE_NATIVE) {
+if (running) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been
called
+ */
+ qxl_set_irq(qxl);
+} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
/* FIXME #1: should go out during live stage */
--
1.7.4.4
[Qemu-devel] [PATCH v2 2/2] qxl: s/qxl_set_irq/qxl_update_irq/
---
hw/qxl.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index c7edc60..3ddca88 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -808,7 +808,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
qxl_destroy_primary(d, QXL_SYNC);
}
-static void qxl_set_irq(PCIQXLDevice *d)
+static void qxl_update_irq(PCIQXLDevice *d)
{
uint32_t pending = le32_to_cpu(d-ram-int_pending);
uint32_t mask= le32_to_cpu(d-ram-int_mask);
@@ -1209,7 +1209,7 @@ async_common:
qemu_spice_wakeup(d-ssd);
break;
case QXL_IO_UPDATE_IRQ:
-qxl_set_irq(d);
+qxl_update_irq(d);
break;
case QXL_IO_NOTIFY_OOM:
if (!SPICE_RING_IS_EMPTY(d-ram-release_ring)) {
@@ -1359,7 +1359,7 @@ static void pipe_read(void *opaque)
do {
len = read(d-pipe[0], dummy, sizeof(dummy));
} while (len == sizeof(dummy));
-qxl_set_irq(d);
+qxl_update_irq(d);
}
static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
@@ -1373,7 +1373,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t
events)
return;
}
if (pthread_self() == d-main) {
-qxl_set_irq(d);
+qxl_update_irq(d);
} else {
if (write(d-pipe[1], d, 1) != 1) {
dprint(d, 1, %s: write to pipe failed\n, __FUNCTION__);
@@ -1465,9 +1465,9 @@ static void qxl_vm_change_state_handler(void *opaque, int
running, int reason)
if (running) {
/*
* if qxl_send_events was called from spice server context before
- * migration ended, qxl_set_irq for these events might not have been
called
+ * migration ended, qxl_update_irq for these events might not have
been called
*/
- qxl_set_irq(qxl);
+ qxl_update_irq(qxl);
} else if (qxl-mode == QXL_MODE_NATIVE) {
/* dirty all vram (which holds surfaces) and devram (primary surface)
* to make sure they are saved */
--
1.7.4.4
[Qemu-devel] [PATCH] qxl: allowing the command rings to be not empty when spice worker is stopped RHBZ #728984
same as 8927cfbba232e28304734f7afd463c1b84134031, but for qxl_check_state, that
was
triggered by qxl_pre_load (which calls qxl_hard_reset, which calls
qxl_soft_reset),
and caused the migration target to crash.
---
hw/qxl.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index db7ae7a..7991e70 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -821,17 +821,15 @@ static void qxl_check_state(PCIQXLDevice *d)
{
QXLRam *ram = d-ram;
-assert(SPICE_RING_IS_EMPTY(ram-cmd_ring));
-assert(SPICE_RING_IS_EMPTY(ram-cursor_ring));
+assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cmd_ring));
+assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cursor_ring));
}
static void qxl_reset_state(PCIQXLDevice *d)
{
-QXLRam *ram = d-ram;
QXLRom *rom = d-rom;
-assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cmd_ring));
-assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cursor_ring));
+qxl_check_state(d);
d-shadow_rom.update_id = cpu_to_le32(0);
*rom = d-shadow_rom;
qxl_rom_set_dirty(d);
--
1.7.4.4
[Qemu-devel] [PATCH] qxl: upon reset, if spice worker is stopped, the command rings can be not empty
Spice worker does no longer process commands when it is stopped. Otherwise, it might crash during migration when attempting to process commands while the guest is not completely loaded. Cc: Alon Levy al...@redhat.com --- hw/qxl.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 0b9a4c7..a6fb7f0 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -656,8 +656,8 @@ static void qxl_reset_state(PCIQXLDevice *d) QXLRam *ram = d-ram; QXLRom *rom = d-rom; -assert(SPICE_RING_IS_EMPTY(ram-cmd_ring)); -assert(SPICE_RING_IS_EMPTY(ram-cursor_ring)); +assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cmd_ring)); +assert(!d-ssd.running || SPICE_RING_IS_EMPTY(ram-cursor_ring)); d-shadow_rom.update_id = cpu_to_le32(0); *rom = d-shadow_rom; qxl_rom_set_dirty(d); -- 1.7.4.4
[Qemu-devel] [PATCH] qxl: make sure primary surface is saved on migration
---
hw/qxl.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 2bb36c6..9fdeffb 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1165,11 +1165,14 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
if (!running qxl-mode == QXL_MODE_NATIVE) {
-/* dirty all vram (which holds surfaces) to make sure it is saved */
+/* dirty all vram (which holds surfaces) and devram (primary surface)
+ * to make sure they are saved */
/* FIXME #1: should go out during live stage */
/* FIXME #2: we only need to save the areas which are actually used */
-ram_addr_t addr = qxl-vram_offset;
-qxl_set_dirty(addr, addr + qxl-vram_size);
+ram_addr_t vram_addr = qxl-vram_offset;
+ram_addr_t devram_addr = qxl-vga.vram_offset;
+qxl_set_dirty(vram_addr, vram_addr + qxl-vram_size);
+qxl_set_dirty(devram_addr, devram_addr + qxl-vga.vram_size);
}
}
--
1.7.4.4
[Qemu-devel] [PATCHv2] qxl: make sure primary surface is saved on migration
---
hw/qxl.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 2bb36c6..cff95a4 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1165,11 +1165,14 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
if (!running qxl-mode == QXL_MODE_NATIVE) {
-/* dirty all vram (which holds surfaces) to make sure it is saved */
+/* dirty all vram (which holds surfaces) and devram (primary surface)
+ * to make sure they are saved */
/* FIXME #1: should go out during live stage */
/* FIXME #2: we only need to save the areas which are actually used */
-ram_addr_t addr = qxl-vram_offset;
-qxl_set_dirty(addr, addr + qxl-vram_size);
+ram_addr_t vram_addr = qxl-vram_offset;
+ram_addr_t devram_addr = qxl-vga.vram_offset;
+qxl_set_dirty(vram_addr, vram_addr + qxl-vram_size);
+qxl_set_dirty(devram_addr, devram_addr + qxl-rom-surface0_area_size);
}
}
--
1.7.4.4
[Qemu-devel] [PATCHv3] qxl: make sure primary surface is saved on migration
---
hw/qxl.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 2bb36c6..b3a3507 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1165,11 +1165,14 @@ static void qxl_vm_change_state_handler(void *opaque,
int running, int reason)
qemu_spice_vm_change_state_handler(qxl-ssd, running, reason);
if (!running qxl-mode == QXL_MODE_NATIVE) {
-/* dirty all vram (which holds surfaces) to make sure it is saved */
+/* dirty all vram (which holds surfaces) and devram (primary surface)
+ * to make sure they are saved */
/* FIXME #1: should go out during live stage */
/* FIXME #2: we only need to save the areas which are actually used */
-ram_addr_t addr = qxl-vram_offset;
-qxl_set_dirty(addr, addr + qxl-vram_size);
+ram_addr_t vram_addr = qxl-vram_offset;
+ram_addr_t surface0_addr = qxl-vga.vram_offset +
qxl-shadow_rom.draw_area_offset;
+qxl_set_dirty(vram_addr, vram_addr + qxl-vram_size);
+qxl_set_dirty(surface0_addr, surface0_addr +
qxl-shadow_rom.surface0_area_size);
}
}
--
1.7.4.4
Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest S3S4 support
On 06/29/2011 02:38 PM, Alon Levy wrote: On Wed, Jun 29, 2011 at 12:25:00PM +0200, Gerd Hoffmann wrote: On 06/29/11 11:21, Alon Levy wrote: On Wed, Jun 29, 2011 at 11:01:11AM +0200, Gerd Hoffmann wrote: Hi, I think it will receive them after migration, since the command ring was stored. Our confusion here is because you think there is still seemless migration. Unfortunately it doesn't work right now, unless you plan to fix it the only form of migration right now is switch-host, and for that those commands will be lost, the new connection will receive images for each surface. If you treat the client as seemless you are completely right. The spice server needs this too so it can render the surfaces correctly before sending the surface images to the client (or send the old surfaces and the commands on top of that). That is one difference between qemu migration and S3 state: For qemu migration it is no problem to have unprocessed commands in the rings, they will simply be processed once the spice server state is restored. When the guest driver restores the state when it comes back from S3 it needs the command rings to do so, thats why they must be flushed before entering S3 ... You mean it needs the command rings to be empty before, since they are lost during the reset, right? One more reason. Wasn't aware there is a reset anyway, was thinking hah. The reset is the whole mess.. otherwise S3 would have been trivial, and actually disabling the reset was the first thing we did, but of course it doesn't solve S4 in that case. more about the command ordering. Without reset spice-server would first process the old commands (which may reference non-existing surfaces), then the new commands which re-recreate all state, which is simply the wrong order. With reset the old commands just get lost which causes rendering bugs. Is it an option to have the driver just remove the commands from the ring (and resubmit after resume)? I suspect it isn't as there is no race-free way to do that, right? Right - the whole ring assumes that the same side removes. of course we can add an IO for that (two - FREEZE and UNFREEZE). But I think this is the wrong approach. Instead, rendering all the commands, and dropping the wait for the client. Right now if we flush we do actually wait for the client, but I plan to remove this later. (we do this right now for update_area as well and that's much higher frequency). cheers, Gerd To conclude, we still need to flush the command ring before stop. We don't want to change migration. So we still need to change spice server api. Gerd?
Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest S3S4 support
Sorry for the late response, wasn't available. I'm afraid that (1) and (2) will indeed wakeup the worker, but will not assure emptying the command ring, as it depends on the client pipe size. - Original Message - From: Alon Levy al...@redhat.com To: Gerd Hoffmann kra...@redhat.com Cc: qemu-devel@nongnu.org, yhalp...@redhat.com Sent: Wednesday, June 22, 2011 11:57:54 AM Subject: Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest S3S4 support On Wed, Jun 22, 2011 at 11:13:19AM +0200, Gerd Hoffmann wrote: Hi, worker call. We can add a I/O command to ask qxl to push the release queue head to the release ring. So you suggest to replace QXL_IO_UPDATE_MEM with what, two io commands instead of using the val parameter? I'd like to (a) avoid updating the libspice-server API if possible and (b) have one I/O command for each logical step. So going into S3 could look like this: (0) stop putting new commands into the rings (1) QXL_IO_NOTIFY_CMD (2) QXL_IO_NOTIFY_CURSOR qxl calls notify(), to make the worker thread empty the command rings before it processes the next dispatcher request. (3) QXl_IO_FLUSH_SURFACES (to be implemented) qxl calls stop()+start(), spice-server renders all surfaces, thereby flushing state to device memory. (4) QXL_IO_DESTROY_ALL_SURFACES zap surfaces (5) QXL_IO_FLUSH_RELEASE (to be implemented) push release queue head into the release ring, so the guest will see it and can release everything. (1)+(2)+(4) exist already. (3)+(5) can be done without libspice-server changes. Looks workable? yeah. Yonit? cheers, Gerd
Re: [Qemu-devel] [PATCH 2/2] qxl: add QXL_IO_UPDATE_MEM for guest S3S4 support
On 06/20/2011 07:32 PM, Alon Levy wrote: On Mon, Jun 20, 2011 at 05:50:32PM +0200, Gerd Hoffmann wrote: On 06/20/11 17:11, Alon Levy wrote: On Mon, Jun 20, 2011 at 04:07:59PM +0200, Gerd Hoffmann wrote: What is the difference to one worker-stop() + worker-start() cycle? ok, stop+start won't disconnect any clients either. But does stop render all waiting commands? I'll have to look, I don't know if it does. It does. This is what qemu uses to flush all spice server state to device memory on migration. I don't see that STOP flushes the command rings. We must read and empty the command rings before going to S3. What is the reason for deleting all surfaces? Making sure all references are dropped to pci memory in devram. Ah, because the spice server keeps a reference to the create command until the surface is destroyed, right? Actually right, so my correction stands corrected. There is is QXL_IO_DESTROY_ALL_SURFACES + worker-destroy_surfaces() ... Regarding QXL_IO_DESTROY_ALL_SURFACES, it destroys the primary surface too, which is a little special, that's another difference - update_mem destroys everything except the primary. I know I tried to destroy the primary but it didn't work right, don't recall why right now, so I guess I'll have to retry. I guess it is because DrvAssertMode(disable) destroyed the primary surface in a separate call. Don't think we need to separate the calls any more (we probably needed it when we thought S3 and resolution changes will have different paths). The QXL_IO_UPDATE_MEM command does too much special stuff IMHO. I also think we don't need to extend the libspice-server API. We can add a I/O command which renders everything to device memory via stop+start. We can zap all surfaces with the existing command + Yes, start+stop work nicely, didn't realize (saw it before, assumed it wouldn't be good enough), just need to destroy the surfaces too. worker call. We can add a I/O command to ask qxl to push the release queue head to the release ring. So you suggest to replace QXL_IO_UPDATE_MEM with what, two io commands instead of using the val parameter? QXL_IO_UPDATE_MEM QXL_IO_FLUSH_RELEASE ? Comments? cheers, Gerd
