Dan Kenigsberg has posted comments on this change. Change subject: hostdev: fix creation and deletion of multiple udev rules ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/38208/4//COMMIT_MSG Commit Message: Line 7: hostdev: fix creation and deletion of multiple udev rules Line 8: Line 9: When attaching multiple devices from the same group, each of the Line 10: devices needs to be detached and then later reattached to the host. To Line 11: this routine is hooked udev rule creation to give qemu access to English here is a bit clanky (/me do not understand) and it is slightly out of date. Line 12: /dev/vfio endpoint. This patch corrects behaviour of udev if the file Line 13: already exists or does not exist due to creation or deletion of VM. Line 14: Line 15: Change-Id: I83fffb2f6e42cc1746f30ca2b63c5d594de014e5 https://gerrit.ovirt.org/#/c/38208/4/vdsm/supervdsmServer File vdsm/supervdsmServer: Line 313: if not os.path.isfile(rule_file): Line 314: # If the file exists, different device from the same group has Line 315: # already been detached and we therefore can skip overwriting the Line 316: # file. Also, this file should only be created/removed via the Line 317: # means of supervdsm, therefore no races should occur. the last statement is false, as super vdsm is multithreaded. in this case, however, we do not care about the race as at the worst case, both concurrent calls would simply overwrite the same files Line 318: Line 319: rule = ('KERNEL=="{}", SUBSYSTEM=="vfio" RUN+="{} {}:{} ' Line 320: '/dev/vfio/{}"').format(iommu_group, EXT_CHOWN, Line 321: QEMU_PROCESS_USER, Line 340: error = False Line 341: Line 342: try: Line 343: os.remove(rule_file) Line 344: self.log.debug("Removing rule %s", rule_file) please move the logging out of the try-block and into the "else" clause of it. We do not want to conduse an OSError while logging with OSerror whiule removing. Line 345: except OSError as e: Line 346: if e.errno == errno.ENOENT: Line 347: # OSError with ENOENT errno here means that the rule file does Line 348: # not exist - this is expected when multiple devices in one -- To view, visit https://gerrit.ovirt.org/38208 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83fffb2f6e42cc1746f30ca2b63c5d594de014e5 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@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
