Martin Polednik has posted comments on this change. Change subject: vdsm: introduce hostdev module ......................................................................
Patch Set 8: (2 comments) http://gerrit.ovirt.org/#/c/32313/8/vdsm/hostdev.py File vdsm/hostdev.py: Line 69: continue Line 70: Line 71: for device in VM.conf['devices']: Line 72: try: Line 73: name = device['name'] > How can we have a device without a 'name'? Is it common? Why do we swallow Some devices might not have this attribute, rng device and smartcard being an example. The warning logged below needs a bit more thinking, it should be simple continue afaik (simply ignoring unnamed devices as these are irrelevant) -- this is leftover from old code Line 74: except KeyError: Line 75: continue Line 76: Line 77: if device['device'] == 'hostdev': Line 77: if device['device'] == 'hostdev': Line 78: try: Line 79: devices[name] = vmId Line 80: except KeyError: Line 81: # VM has device which is no (longer) present > Only now do I notice the problem here: the current code cannot raise KeyErr Well the way before seemed perfect :) Line 82: # on the host - althought this should be handled by Line 83: # user, warning can help debugging libvirt errors Line 84: logging.warning('VM %s has device %s which is not ' Line 85: 'available!', vmId, device['name']) -- To view, visit http://gerrit.ovirt.org/32313 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2bba96db5be180d00cb74fb89b10c5b09e5bd180 Gerrit-PatchSet: 8 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: 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