Previous patch causes a bug into Qemu if the patch 46764fe09ca2e0f15c0981a672c166ed8cf57e72 ("virtio-serial: fix segfault on disconnect") is not include in that version of Qemu. This crash happens when device is closed inside a write operation. For SPICE character device a spice_server_char_device_wakeup is called to write data which handles both read and write pending operations. As we want to close the device but we can't do it inside the handler operation schedule a timer that will close the guest device outside this callback. The proper solution would be to patch Qemu but making sure this is not so easy so this workaround patch. Code is marked with some comments to remember to remove this hack in a safe future. Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/stream-device.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/server/stream-device.c b/server/stream-device.c index b7553c67..869d8841 100644 --- a/server/stream-device.c +++ b/server/stream-device.c @@ -52,6 +52,7 @@ struct StreamDevice { bool opened; bool flow_stopped; StreamChannel *stream_channel; + SpiceTimer *close_timer; }; struct StreamDeviceClass { @@ -63,6 +64,8 @@ static StreamDevice *stream_device_new(SpiceCharDeviceInstance *sin, RedsState * G_DEFINE_TYPE(StreamDevice, stream_device, RED_TYPE_CHAR_DEVICE) +static void char_device_set_state(RedCharDevice *char_dev, int state); + typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance *sin) SPICE_GNUC_WARN_UNUSED_RESULT; @@ -71,6 +74,16 @@ static StreamMsgHandler handle_msg_format, handle_msg_data; static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin, const char *error_msg) SPICE_GNUC_WARN_UNUSED_RESULT; +static void +close_timer_func(void *opaque) +{ + StreamDevice *dev = opaque; + + if (dev->opened && dev->has_error) { + char_device_set_state(RED_CHAR_DEVICE(dev), 0); + } +} + static bool stream_device_partial_read(StreamDevice *dev, SpiceCharDeviceInstance *sin) { @@ -88,7 +101,17 @@ stream_device_partial_read(StreamDevice *dev, SpiceCharDeviceInstance *sin) while (sif->read(sin, buf, sizeof(buf)) > 0) { continue; } - sif->state(sin, 0); + + // This code is a workaround for a Qemu bug, see patch + // "stream-device: Workaround Qemu bug closing device" + // as calling sif->state here can cause a crash schedule + // a timer and do the call in it. Remove this code when + // will be sure all Qemu versions has been patched. + RedsState *reds = red_char_device_get_server(RED_CHAR_DEVICE(dev)); + if (!dev->close_timer) { + dev->close_timer = reds_core_timer_add(reds, close_timer_func, dev); + } + reds_core_timer_start(reds, dev->close_timer, 0); return false; } @@ -340,6 +363,9 @@ stream_device_dispose(GObject *object) { StreamDevice *dev = STREAM_DEVICE(object); + RedsState *reds = red_char_device_get_server(RED_CHAR_DEVICE(dev)); + reds_core_timer_remove(reds, dev->close_timer); + if (dev->stream_channel) { // close all current connections and drop the reference red_channel_destroy(RED_CHANNEL(dev->stream_channel)); @@ -376,7 +402,7 @@ reset_channels(StreamDevice *dev) } static void -char_device_enable(RedCharDevice *char_dev) +char_device_set_state(RedCharDevice *char_dev, int state) { SpiceCharDeviceInstance *sin = NULL; g_object_get(char_dev, "sin", &sin, NULL); @@ -384,7 +410,7 @@ char_device_enable(RedCharDevice *char_dev) SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin); if (sif->state) { - sif->state(sin, 1); + sif->state(sin, state); } } @@ -413,7 +439,7 @@ stream_device_port_event(RedCharDevice *char_dev, uint8_t event) // this could prevent a possible race condition where we open the // device from the guest and it take some time to enable causing // temporary writing issues. - char_device_enable(char_dev); + char_device_set_state(char_dev, 1); } static void -- 2.14.3 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel