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

mzamazal at redhat.com mzamazal at redhat.com
Tue Mar 8 12:02:28 UTC 2016


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


More information about the vdsm-patches mailing list