Ping. On Fri, Oct 27, 2017 at 10:08 AM, Sameeh Jubran <sam...@daynix.com> wrote:
> > > On Fri, Oct 27, 2017 at 2:51 AM, Michael Roth <mdr...@linux.vnet.ibm.com> > wrote: > >> Quoting Sameeh Jubran (2017-08-13 10:58:46) >> > From: Sameeh Jubran <sjub...@redhat.com> >> > >> > This series fixes qemu-ga's behaviour upon facing a missing >> serial/serial >> > driver by listening to the serial device's events. >> > >> > For more info on why this series is needed checkout the commit message >> > of the third patch and the following bugzilla: >> https://bugzilla.redhat.com/show_bug.cgi?id=990629. >> > >> > Sameeh Jubran (3): >> > qga: Channel: Add functions for checking serial status >> > qga: main: make qga config and socket activation global >> > qga: Prevent qemu-ga exit if serial doesn't exist >> >> Hi Sameeh, >> >> The event handling stuff is spiffy and could be useful for other use-cases >> (e.g. cpu/mem hotplug events that can be consumed by management), but >> since >> the actual bug here is somewhat of an edge case (we *could* just tell >> people that installing the agent before virtio-serial drivers is a bug, >> or that unplugging the agent's communication channel is a bad idea), >> I'm not too comfortable with adding this much complexity unless there's >> a stronger argument for it. >> > I can relate to your concerns, it is somehow an edge case but I think that > this > is the elegant way to handle it instead of just polling forever. This > patch series > is more related to Windows than Linux as this edge case is much more common > on Windows since when the virtio-serial driver is installed sometimes > usually > it requires a post-installation reboot and when the system is up, qemu-ga > runs before > the virtio-serial driver is fully configured and it fails to load and then > another reboot is needed. > > >> >> There's also a couple issues I had with this series as it stands, namely >> the lack of a ./configure check for udev (which could cause build >> breakage in some environments), and a lot of spillage of GAConfig into >> qga/channel-*, which I think could be avoided. >> > I think we can use the --retry with linux clients and use the device > notifications > API provided by Windows as it is supported since xp. > >> >> I've sent an alternative series that I think we should consider as it >> uses a much simpler mechanism to implement this support (basically >> just periodically retrying the channel if it doesn't exist, or if it >> disappears for whatever reason). I've tested it on Windows, but would >> be good to confirm that it adequately addresses the use-case you were >> looking at. Thanks! >> > I haven't tested it yet, but I think it might solve the issue. Your series > is much > simpler and less intrusive to the code but I don't think this is the right > approach. > >> >> > >> > Makefile | 4 + >> > qga/channel-posix.c | 54 ++++++++++ >> > qga/channel-win32.c | 60 +++++++++++ >> > qga/channel.h | 9 ++ >> > qga/main.c | 284 ++++++++++++++++++++++++++++++ >> ++++++++++++++++------ >> > qga/service-win32.h | 4 + >> > 6 files changed, 385 insertions(+), 30 deletions(-) >> > >> > -- >> > 2.9.4 >> > >> >> > > > -- > Respectfully, > *Sameeh Jubran* > *Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>* > *Software Engineer @ Daynix <http://www.daynix.com>.* > -- Respectfully, *Sameeh Jubran* *Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>* *Software Engineer @ Daynix <http://www.daynix.com>.*
