Martin Polednik has posted comments on this change.

Change subject: hostdev: move pci device code to separate class
......................................................................


Patch Set 8:

(3 comments)

https://gerrit.ovirt.org/#/c/57956/8//COMMIT_MSG
Commit Message:

PS8, Line 35: [2] This is true only because reattach, in it's current version, 
does
            : more then just reattach the device. It modifies the permissions 
on the
            : udev device by creating udev rule. The rule should be removed. The
            : removal process must be split from reattach call.
> I forgot about https://gerrit.ovirt.org/#/c/59042/5 so my question switches
Should be doable unless git explodes.


https://gerrit.ovirt.org/#/c/57956/8/vdsm/virt/vmdevices/hostdevice.py
File vdsm/virt/vmdevices/hostdevice.py:

PS8, Line 210:     def teardown(self):
             :         pass
> seems unneeded, you do the same thing (aka nothing) core.Base already does.
True, fixed. Leftover from facade approach.


Line 264: 
Line 265:         return hostdev
Line 266: 
Line 267:     @classmethod
Line 268:     def update_device_info(cls, vm, device_conf, x):
> +1 to both comments
I'll try to work my naming skills to figure out something reasonable.
Line 269:         alias = 
x.getElementsByTagName('alias')[0].getAttribute('name')
Line 270:         address = vmxml.device_address(x)
Line 271:         source = x.getElementsByTagName('source')[0]
Line 272:         device = pci_address_to_name(**vmxml.device_address(source))


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb93c2434ff827406bbf4ee0af30f5726f6af
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Martin Polednik <[email protected]>
Gerrit-Reviewer: Milan Zamazal <[email protected]>
Gerrit-Reviewer: Tomas Golembiovsky <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to