Change in vdsm[master]: hostdev: fix creation and deletion of multiple udev rules

danken at redhat.com danken at redhat.com
Tue Mar 10 11:31:03 UTC 2015


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 <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek 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