Change in vdsm[master]: virt: Add `vm' argument to underlying_device_info methods

fromani at redhat.com fromani at redhat.com
Fri Feb 19 08:27:47 UTC 2016


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 <mzamazal at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list