Ping?
----- Mail original ----- > During seamless migration, after switching host, if a client was connected > during the migration, it will have data to send back to the new > qemu/spice-server instance. This is handled through MIGRATE_DATA messages. > SPICE char devices use such MIGRATE_DATA messages to restore their state. > > However, the MIGRATE_DATA message can arrive any time after the new qemu > instance has started, this can happen before or after the SPICE char > devices have been created. In order to handle this, if the migrate data > arrives early, it's stored in reds->agent_state.mig_data, and > attach_to_red_agent() will restore the agent state as appropriate. > > Unfortunately this does not work as expected as expected. If > attach_to_red_agent() is called before the MIGRATE_DATA message reaches the > server, all goes well, but if MIGRATE_DATA reaches the server before > attach_to_red_agent() gets called, then some assert() get triggered in > spice_char_device_state_restore(): > > ((null):32507): Spice-ERROR **: > char_device.c:937:spice_char_device_state_restore: assertion > `dev->num_clients == 1 && dev->wait_for_migrate_data' failed > Thread 3 (Thread 0x7f406b543700 (LWP 32543)): > Thread 2 (Thread 0x7f40697ff700 (LWP 32586)): > Thread 1 (Thread 0x7f4079b45a40 (LWP 32507)): > > What happens is that dev->wait_for_migrate_data is unconditionally cleared > when > completing handling of the MIGRATE_DATA message, so it's FALSE when > spice_char_device_state_restore() is called. Moreover, dev->num_clients is > also 0 as this is only increased by spice_char_device_start() which > spice_server_char_device_add_interface() calls after > attach_to_red_agent()/spice_char_device_state_restore() have been called. > > This commit changes the logic in spice_server_char_device_add_interface(), > and when there is migrate data pending in reds->agent_state.mig_data, we > only attempt to restore it after successfully initializing/starting the > needed char device (which will ensure that dev->num_clients is not 0). > > It also changes attach_to_red_agent() to handle the case when > reds->agent_state.mig_data is non-NULL to be the same as the case when > red_channel_waits_for_migrate_data() is TRUE. This ensures that > spice_char_device_client_add() gets called and that 'wait_for_data' is set > in the added device. > > This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1035184 > --- > Changes since v1: > - Move block calling reds_agent_state_restore() before calling > spice_char_device_start() as this should be safer. This also > groups all SUBTYPE_xxx special-casing together. I've tested that > things are still working as expected after this move. > > > server/reds.c | 25 ++++++++++++++----------- > 1 file changed, 14 insertions(+), 11 deletions(-) > > diff --git a/server/reds.c b/server/reds.c > index 0390602..bd4fea1 100644 > --- a/server/reds.c > +++ b/server/reds.c > @@ -2863,16 +2863,7 @@ static SpiceCharDeviceState > *attach_to_red_agent(SpiceCharDeviceInstance *sin) > state->read_filter.discard_all = FALSE; > reds->agent_state.plug_generation++; > > - if (reds->agent_state.mig_data) { > - spice_assert(reds->agent_state.plug_generation == 1); > - reds_agent_state_restore(reds->agent_state.mig_data); > - free(reds->agent_state.mig_data); > - reds->agent_state.mig_data = NULL; > - } else if > (!red_channel_waits_for_migrate_data(&reds->main_channel->base)) { > - /* we will assoicate the client with the char device, upon > reds_on_main_agent_start, > - * in response to MSGC_AGENT_START */ > - main_channel_push_agent_connected(reds->main_channel); > - } else { > + if (red_channel_waits_for_migrate_data(&reds->main_channel->base) || > reds->agent_state.mig_data) { > spice_debug("waiting for migration data"); > if (!spice_char_device_client_exists(reds->agent_state.base, > reds_get_client())) { > int client_added; > @@ -2889,9 +2880,13 @@ static SpiceCharDeviceState > *attach_to_red_agent(SpiceCharDeviceInstance *sin) > spice_warning("failed to add client to agent"); > reds_disconnect(); > } > - > } > + } else { > + /* we will associate the client with the char device, upon > reds_on_main_agent_start, > + * in response to MSGC_AGENT_START */ > + main_channel_push_agent_connected(reds->main_channel); > } > + > return state->base; > } > > @@ -2987,6 +2982,13 @@ static int > spice_server_char_device_add_interface(SpiceServer *s, > } else { > dev_state = spicevmc_device_connect(char_device, > SPICE_CHANNEL_PORT); > } > + } else if (strcmp(char_device->subtype, SUBTYPE_VDAGENT) == 0) { > + if (reds->agent_state.mig_data) { > + spice_assert(reds->agent_state.plug_generation == 1); > + reds_agent_state_restore(reds->agent_state.mig_data); > + free(reds->agent_state.mig_data); > + reds->agent_state.mig_data = NULL; > + } > } > > if (dev_state) { > @@ -3001,6 +3003,7 @@ static int > spice_server_char_device_add_interface(SpiceServer *s, > spice_warning("failed to create device state for %s", > char_device->subtype); > return -1; > } > + > return 0; > } > > -- > 1.9.0 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
