[Spice-devel] 1cec1c5118b65124de6bc6f984f376ff4e297bfb ("reds: Make VDIPortState a GObject") changes

[Frediano Ziglio]

Hi,
  I think this patch is one of the most puzzling commit and caused
some regression (2 patches applied and one going).
The problem was not actually the move to GObject but that the
implementation change completely the lifetime of the underlying
RedCharDevice.
Before the patch the RedCharDevice was created when Qemu created the
device and possibly destroyed when device is destroyed but also on
some errors from the main channel.
I tried to understand what the other RedCharDevice do. SpiceVmc and
SmartCard basically create the device when Qemu create it and destroy
when Qemu destroy them (currently never). When the device is created
a channel is created. Here there are some differences. The VDI
CharDevice is already present, no matter if a Qemu device is present
or not. Also the MainChannel (the channel corresponding to the VDI
CharDevice) is always present and not created when the VDI CharDevice
is created. These 2 differences were however already present before
this patch.
One of the difference is that before an error could lead to
destroying the CharDevice basically causing all future MainChannels
to have a not working agent (as the agent requires a working CharDevice).

At the end I consider this as a good patch and in line with what other
CharDevices are working (the CharDevice is destroyed only by Qemu) however
there are cases were before CharDevice was not present and now is it.
Particularly what happens when the agent is stopped in the guest?
The guest agent is informed so won't send data?
Is it stopped when sending large data (as instance large file)? Or
will send lot of data that will be ignored by the server?
In case of errors agent_attached is set to FALSE but who will set again
this field to TRUE? Apparently this is set to TRUE only in attach_to_red_agent
called during spice_server_add_interface so this will be set back to TRUE
only during migration. Looks like this is a bug, guest should be able
to launch again the agent and variable should be set back to TRUE.

Frediano