qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/3]


From: Sameeh Jubran
Subject: Re: [Qemu-devel] [PATCH 0/3]
Date: Fri, 27 Oct 2017 11:08:46 +0300

On Fri, Oct 27, 2017 at 2:51 AM, Michael Roth <address@hidden>
wrote:

> Quoting Sameeh Jubran (2017-08-13 10:58:46)
> > From: Sameeh Jubran <address@hidden>
> >
> > 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>.*


reply via email to

[Prev in Thread] Current Thread [Next in Thread]