On Mon, Jun 26, 2017 at 01:47:04PM +0200, Erik Skultety wrote:
We do perform that magical check every time an event is received,
but
unless we explicitly unref'd or disconnected the monitor, we should not
get a spurious event from a different source.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
This is just a poke patch, since I also checked libudev's source and still fail
to understand why we need that check. Unless entered by *_monitor_unref with
the last reference or with *monitor_disconnect, libudev doesn't change the fd
arbitrarily. So the only reason for us to check for that would be if we
disconnected from our side, but didn't unregister the callback in which case
it was our responsibility to clean up properly and we shouldn't get to the
callback in the first place. Thanks for comments in advance.
This check is simply a sanity check that we've not got a bug elsewhere
in the code. If we're bug free, this condition should never trigger.
This is probably better thought of as being an assert(), except we don't
use assert() hence reporting an error.
We have certainly had bugs in the past where we didn't correctly cleanup
or unregister things, so I think the check is potentially useful.
src/node_device/node_device_udev.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 174124a1b..7820c49c4 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1602,24 +1602,15 @@ nodeStateCleanup(void)
static void
udevEventHandleCallback(int watch ATTRIBUTE_UNUSED,
- int fd,
+ int fd ATTRIBUTE_UNUSED,
int events ATTRIBUTE_UNUSED,
void *data ATTRIBUTE_UNUSED)
{
struct udev_device *device = NULL;
struct udev_monitor *udev_monitor = DRV_STATE_UDEV_MONITOR(driver);
const char *action = NULL;
- int udev_fd = -1;
nodeDeviceLock();
- udev_fd = udev_monitor_get_fd(udev_monitor);
- if (fd != udev_fd) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("File descriptor returned by udev %d does not "
- "match node device file descriptor %d"),
- fd, udev_fd);
- goto cleanup;
- }
device = udev_monitor_receive_device(udev_monitor);
if (device == NULL) {
--
2.13.2
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 :|