Francesco Romani has posted comments on this change. Change subject: virt: Add `vm' argument to underlying_device_info methods ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/53676/1//COMMIT_MSG Commit Message: Line 6: Line 7: virt: Add `vm' argument to underlying_device_info methods Line 8: Line 9: One of the methods (storage) needs it to access VM related Line 10: information (arch). It's perhaps better to pass the whole Vm instance and vm.log also, but this is easier to replace Line 11: instead of adding more other arguments. Perhaps we could remove some of Line 12: the other underlying_device_info arguments when we've got `vm'. Perhaps Line 13: we should take a different approach. It's for discussion, but for now Line 14: we take this path to make storage device handling movable out of Vm Line 11: instead of adding more other arguments. Perhaps we could remove some of Line 12: the other underlying_device_info arguments when we've got `vm'. Perhaps Line 13: we should take a different approach. It's for discussion, but for now Line 14: we take this path to make storage device handling movable out of Vm Line 15: class. I think it is better to add only the specific argument in the place we need it. To extend all the other methods adding an unused argument feels wrong. We should instead go in the direction of decouplig the bits that needs extra argument (handle unknown device at the end of the storage's method). For the sake of the practicality *and to avoid stalling this patchset* I'm ok adding one exception to the storage method, however. Line 16: Line 17: Change-Id: Ide9e8d105d6c14101f9ce367409af729a075b2df -- To view, visit https://gerrit.ovirt.org/53676 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ide9e8d105d6c14101f9ce367409af729a075b2df Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches