Milan Zamazal has posted comments on this change. Change subject: virt: Add `vm' argument to underlying_device_info methods ......................................................................
Patch Set 1: (1 comment) https://gerrit.ovirt.org/#/c/53676/1//COMMIT_MSG Commit Message: 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. > In general I don't think it is a good idea. Thinking about it a bit more I don't think there is anything fundamentally wrong in passing `vm' argument here with its current public interfaces. But we need to keep device_conf argument instead of exposing the devices configuration. I think this way is fine here. We may try some redesign in future, but we shouldn't try to do that here. Moreover we might want to postpone the activity until we move to etree as that can imply some improvements to domain XML handling etc. So I'm still going to delete this patch. 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: Milan Zamazal <mzama...@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
