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 <ibar...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
