Change in vdsm[master]: network: wait for udev after configuring SRIOV

danken at redhat.com danken at redhat.com
Thu Apr 30 15:09:49 UTC 2015


Dan Kenigsberg has posted comments on this change.

Change subject: network: wait for udev after configuring SRIOV
......................................................................


Patch Set 5: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/40400/5/vdsm/network/api.py
File vdsm/network/api.py:

Line 542:         # This is a blocking call that should wait for all udev events to be
Line 543:         # handled. Since those include renaming devices, we should wait until
Line 544:         # udev finishes in order to report a stable hardware state in
Line 545:         # subsequent engine calls.
Line 546:         udevadm.settle(timeout=10)
> this is ideal, but I am not sure it worth the effort.
I (mistakenly?) expected that write() would block until the events are queued into udev. Are we sure that this is not the case? Ido, can you ask our kernel/udev specialists?

I don't see the benefit of calling settle() multiple times. If the race is real, we need to block until the kernel creates the devices (I am guessing that we can do this by waiting for vdsm.netlink.monitor.Monitor events), by then udev got its "change" event, and we can call settle().

But even then, we can not tell if 10 seconds are enough.. I hate leaving races behind, but as long as settle() does not return an error upon timeout, we can never solve this race.

Ido, would you open a udev bug, detailing the scenario, and asking for them to notify the caller whether `udevadm settle` time-outed? Add this bug to a TODO line here, warning about the (theoretical) race.

I am not sure whether the former issue is real, and the worse-case scenario of these issues is badly-named nics, which can be overwridden manually by "refresh capabilities" button. But still, we should do a little effort to check them out and note them as a known caveat.
Line 547: 
Line 548: 
Line 549: def _persist_numvfs(device_name, numvfs):
Line 550:     dir_path = os.path.join(constants.P_VDSM_LIB, 'virtual_functions')


-- 
To view, visit https://gerrit.ovirt.org/40400
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I04d63ca35aab585051c3d0a5f2652dbf2f91b080
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list