[libvirt RFC PATCH] util: vireventglibwatch: watch for G_IO_HUP and G_IO_ERR
To more closely match the previous usage in virEventPollDispatchHandles, where called the handle callback for any revents returned by poll. This should fix the virtlogd error on subsequent domain startup: error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/f28live.log': Device or resource busy as well as virtlogd spinning caused by virLogHandlerDomainLogFileEvent never being called on hangup. Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: f8ab47cb4491dd72d866c1a96a9d94b8c3341de9 Fixes: 946a25274c46ffff46323c62f567ae7e753aa921 --- src/util/vireventglibwatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c index 7694e74f23..178707f6b7 100644 --- a/src/util/vireventglibwatch.c +++ b/src/util/vireventglibwatch.c @@ -89,11 +89,11 @@ GSource *virEventGLibCreateSocketWatch(int fd, sizeof(virEventGLibFDSource)); ssource = (virEventGLibFDSource *)source; - ssource->condition = condition; + ssource->condition = condition | G_IO_HUP | G_IO_ERR; ssource->fd = fd; ssource->pollfd.fd = fd; - ssource->pollfd.events = condition; + ssource->pollfd.events = condition | G_IO_HUP | G_IO_ERR; g_source_add_poll(source, &ssource->pollfd); -- 2.24.1
On Wed, 2020-02-19 at 01:31 +0100, Ján Tomko wrote:
To more closely match the previous usage in virEventPollDispatchHandles, where called the handle callback for any revents returned by poll.
This should fix the virtlogd error on subsequent domain startup: error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/f28live.log': Device or resource busy as well as virtlogd spinning caused by virLogHandlerDomainLogFileEvent never being called on hangup.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: f8ab47cb4491dd72d866c1a96a9d94b8c3341de9 Fixes: 946a25274c46ffff46323c62f567ae7e753aa921 --- src/util/vireventglibwatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Dan and Pavel should confirm this is the correct way to address the issue, but it makes the problem go away on my machine so Tested-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
On 2/19/20 1:31 AM, Ján Tomko wrote:
To more closely match the previous usage in virEventPollDispatchHandles, where called the handle callback for any revents returned by poll.
This should fix the virtlogd error on subsequent domain startup: error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/f28live.log': Device or resource busy as well as virtlogd spinning caused by virLogHandlerDomainLogFileEvent never being called on hangup.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: f8ab47cb4491dd72d866c1a96a9d94b8c3341de9 Fixes: 946a25274c46ffff46323c62f567ae7e753aa921 --- src/util/vireventglibwatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c index 7694e74f23..178707f6b7 100644 --- a/src/util/vireventglibwatch.c +++ b/src/util/vireventglibwatch.c @@ -89,11 +89,11 @@ GSource *virEventGLibCreateSocketWatch(int fd, sizeof(virEventGLibFDSource)); ssource = (virEventGLibFDSource *)source;
- ssource->condition = condition; + ssource->condition = condition | G_IO_HUP | G_IO_ERR; ssource->fd = fd;
ssource->pollfd.fd = fd; - ssource->pollfd.events = condition; + ssource->pollfd.events = condition | G_IO_HUP | G_IO_ERR;
g_source_add_poll(source, &ssource->pollfd);
Yeah, to docs says we need to put HUP and ERR explicitly on the list of events. From [1]: gushort events: a bitwise combination from GIOCondition, specifying which events should be polled for. Typically for reading from a file descriptor you would use G_IO_IN | G_IO_HUP | G_IO_ERR, and for writing you would use G_IO_OUT | G_IO_ERR. 1: https://developer.gnome.org/glib/unstable/glib-The-Main-Event-Loop.html#GPol... Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Wed, Feb 19, 2020 at 01:31:22AM +0100, Ján Tomko wrote:
To more closely match the previous usage in virEventPollDispatchHandles, where called the handle callback for any revents returned by poll.
This should fix the virtlogd error on subsequent domain startup: error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/f28live.log': Device or resource busy as well as virtlogd spinning caused by virLogHandlerDomainLogFileEvent never being called on hangup.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: f8ab47cb4491dd72d866c1a96a9d94b8c3341de9 Fixes: 946a25274c46ffff46323c62f567ae7e753aa921 --- src/util/vireventglibwatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c index 7694e74f23..178707f6b7 100644 --- a/src/util/vireventglibwatch.c +++ b/src/util/vireventglibwatch.c @@ -89,11 +89,11 @@ GSource *virEventGLibCreateSocketWatch(int fd, sizeof(virEventGLibFDSource)); ssource = (virEventGLibFDSource *)source;
- ssource->condition = condition; + ssource->condition = condition | G_IO_HUP | G_IO_ERR; ssource->fd = fd;
ssource->pollfd.fd = fd; - ssource->pollfd.events = condition; + ssource->pollfd.events = condition | G_IO_HUP | G_IO_ERR;
g_source_add_poll(source, &ssource->pollfd);
This is something I knew about but forgot to fix beforef pushing the original patches. At the OS level poll() will always return HUP/ERR events and we relied on this historically. GLib uses poll() and so will also see HUP/ERR events but unless you requested G_IO_ERR/HUP, the callback will never be invoked to dispatch the event. This is what leads to the 100% CPU burn behaviour. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Ján Tomko -
Michal Privoznik