Re: [PATCH 1/2] node_device_udev: handle move events

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

 



On Thu, Apr 16, 2020 at 11:57:45AM -0400, Mark Asselstine wrote:
It is possible and common to rename some devices, this is especially
true for ethernet devices such as veth pairs.

In the udevEventHandleThread() we will be notified of this change but
currently we only process "add", "change" and "remove"
events. Renaming a device such as above results in a "move" event, not
a "remove" followed by and "add" or vise versa. This change will add
the new/destination device to our records but unfortunately there is
no usable mechanism to identify the old/source device to remove it
from the records. So this is admittedly only a partial fix.


There is, but it is only internal.  We should ask for the function
`udev_device_get_devpath_old()` to be publicized, if possible.  That should give
us the old name.

When I checked this using `udevadm monitor --property` I got two events for each
rename, one kernel event (where the old path pointed to the previous name) and
one udev event where the old path referred to the original name the device was
created with.

For example when I created a virtual network device with the name "fdsa", then
renamed it bunch of times, started udev monitoring and then renamed it from
"first" to "second" this was the output of udevadm for that one particular rename:

monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent

KERNEL[36343.246068] move     /devices/virtual/net/second (net)
ACTION=move
DEVPATH=/devices/virtual/net/second
DEVPATH_OLD=/devices/virtual/net/first
IFINDEX=3
INTERFACE=second
SEQNUM=4609
SUBSYSTEM=net

UDEV  [36343.246785] move     /devices/virtual/net/fdsa (net)
ACTION=move
DEVPATH=/devices/virtual/net/fdsa
DEVPATH_OLD=/devices/virtual/net/first
IFINDEX=3
INTERFACE=fdsa
SEQNUM=4609
SUBSYSTEM=net
USEC_INITIALIZED=136864589

Thank you for a patch like this, I did not want to change the error just because
"it's probably fine" until I can be sure that we do not lose the device (even
though someone else is clearly managing it).

So I think the patches are great, and as much as I would like to add support for
removing the old device if possible, probably using the function above, it will
need some extra work.  At least we should file a request for the udev function
to be exposed.

Anyway, I am not against these patches and I would even be fine with changing
the warning to info in 2/2, but I will wait for someone else to see this and
comment (and I'd like to know your opinion on what I wrote as well).  If that
does not happen, I will retest them against (at that point) current master and
push them.

Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>

Signed-off-by: Mark Asselstine <mark.asselstine@xxxxxxxxxxxxx>
---
src/node_device/node_device_udev.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 8451903e8a..3149de8321 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1499,6 +1499,11 @@ udevHandleOneDevice(struct udev_device *device)
    if (STREQ(action, "remove"))
        return udevRemoveOneDevice(device);

+    if (STREQ(action, "move")) {
+        /* TODO: implement a way of finding and removing the old device */
+        return udevAddOneDevice(device);
+    }
+
    return 0;
}

--
2.20.1


Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux