Mark Wu has uploaded a new change for review.
Change subject: [WIP] MOM Integration ......................................................................
[WIP] MOM Integration
This patch tries to integrate MOM with vdsm. For the descprition of how this patch works, please see Adam's mail: https://fedorahosted.org/pipermail/vdsm-devel/2012-February/000628.html
Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f --- M vdsm.spec.in M vdsm/API.py M vdsm/Makefile.am M vdsm/clientIF.py M vdsm/config.py.in M vdsm/ksm.py M vdsm/libvirtvm.py A vdsm/mom.conf A vdsm/mom.policy M vdsm/supervdsmServer.py A vdsm/vdsMOM.py M vdsm/vm.py 12 files changed, 271 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/2367/1 -- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 1: (4 inline comments)
.................................................... File vdsm/clientIF.py Line 91: self.ksmMonitor = ksm.KsmMonitorThread(self) You can combine this if clause into the above if statement.
.................................................... File vdsm/config.py.in Line 146: Hmm, I am not sure that we actually need this config option. Eventually we will always want to use MOM when it is installed so the only safety check we need right now is one that re-enables the KSM thread when MOM fails to load.
.................................................... File vdsm/mom.conf Line 29: You can remove Balloon since the current vdsm policy is not using the Balloon Controller.
.................................................... File vdsm/vdsMOM.py Line 48: There has been a recent move to stop using the traceback.format_exc() function in logging calls. Please change to:
self._cif.log.error("MOM initialization failed", exc_info=True)
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com
Adam Litke has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 3: (1 inline comment)
.................................................... File vdsm/vdsMOM.py Line 22: import traceback You do not need to import traceback anymore.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: shu ming sming56@gmail.com
Doron Fediuck has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (4 inline comments)
Hi Mark, Looks good. Still some minor issues I commented on inline.
From vdsm's point of view, we may want to consider a better
interface / api for monitoring (possibly also executing) some factors. In this context see my comment in clientIF.py.
.................................................... File vdsm/clientIF.py Line 92: Although this works, I'm troubled from developer's point of view. This means that on each and every point we had and will have regarding ksm, the developer needs to know whether to call self.ksmMonitor or self.vdsMOM.
Any thoughts about it?
Looking forward I think it will be better to have an interface (Protocol / API) both ksm and mom will implement, so the developer may call the same thing all over the code. Does this make sense?
.................................................... File vdsm.spec.in Line 351: cp %{_sysconfdir}/%{vdsm_name}/mom.conf %{_sysconfdir}/mom.conf Mark, Can you please explain this part;
Usually, rpm handles configuration files quite well, so no real need for such intervention.
If the existing mom.conf belongs to someone else (mom rpm?), then it's considered a bad practice to overrun it in this way. ie- one rpm should not mv / rm another rpm's files.
So if I understand correctly, and you're running over the original mom's conf, I suggest do one of the following:
1. Change mom's default conf file name to mom.conf.samle (or template), and then it's perfectly fine to cp or generate vdsm conf file for mom.
2. If vdsm creates its own mom-vdsm.conf, mom should be able to be started with a flag specifying the relevant conf file.
.................................................... File vdsm/supervdsmServer.py Line 192: There's a trailing white tab here, which git really hates...
.................................................... File vdsm/vdsMOM.py Line 35: self.MOM = mom.MOM("/etc/vdsm/mom.conf") This should probably be taken from vdsm.conf file, thus improving a previous packaging issue I commented on.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming sming56@gmail.com
Adam Litke has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (2 inline comments)
.................................................... File vdsm/clientIF.py Line 92: Hi Doron. We are looking to deprecate the ksmMonitorThread but would like to preserve compatibility for one short generation in case a distro is unable to ship MOM right away. But after this one generation, all ksm operations will be routed through MOM.
.................................................... File vdsm.spec.in Line 351: cp %{_sysconfdir}/%{vdsm_name}/mom.conf %{_sysconfdir}/mom.conf Mark. MOM allows you to pass in a configuration file in its constructor function. So, just install the mom.conf into the vdsm conf directory and use that path when initializing MOM.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming sming56@gmail.com
Doron Fediuck has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 92: Hi Adam, Unsure how good or bad KSM integration is. It may be nice to have it as a fallback, unless Dan / Ayal think otherwise.
The main thing I see here, is a need to keep it unified. Even if you decide to upgrade mom in the future into turbo-mom, we'd still need to keep some sort of an interface, allowing developers to use a single monitoring/perf-tuning entity.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming sming56@gmail.com
Adam Litke has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 92: The interface is MOM and its policy. Note that MOM needs to perform all monitoring and tuning via vdsm APIs in this configuration so vdsm will have internal mechanisms for doing everything that mom currently is doing.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming sming56@gmail.com
Doron Fediuck has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 92: Adam, the interface I was referring to is a more generic one, than a specific MOM policy. Yet I'm willing to put this aside for now, and revisit after the current generation.
Depricating ksmMonitorThread should be more than just a cleanup, and I'll be waiting there.
So for the time being, this can remain as Mark wrote it.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming sming56@gmail.com
Ayal Baron has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (7 inline comments)
Note I have not reviewed the patch, just replying to Doron's comment so I cannot rate the patch yet.
.................................................... File vdsm/clientIF.py Line 92: Mom is a policy engine, it doesn't mandate a specific policy so if the policy changes in the future, we need not change the API (or the code at all) just the policy passed to mom, so I'm not sure what you're suggesting.
.................................................... File vdsm/Makefile.am Line 40: vdsMOM.py \ Again, please keep alphabetical sort.
Line 134: mom.conf and again
Line 156: install-data-libvirtpass \ would be nice to sort this list
Line 174: uninstall-data-libvirtpass \ same here
.................................................... File vdsm.spec.in Line 493: %{_sysconfdir}/%{vdsm_name}/mom.policy please sort alphabetically
Line 578: %{_datadir}/%{vdsm_name}/vdsMOM.py* same here
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Doron Fediuck has posted comments on this change.
Change subject: [WIP] MOM Integration ......................................................................
Patch Set 6: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 92: AFAIR, MOM is Memory Overcommitment Manager, and the policy rules will come from ovirt-engine. My initial comment was from the dev's point of view, who (after this change) will need to check whether ksm or mom is in use. Going forward a single entity in vdsm side will make it simpler, and we can later decide if it represents only MOM or something more generic. If you want to continue discussing it, I suggest move it to some mailing list, although I already said we can leave this for now and revisit in next generation.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7:
Change from patch set 6: - fix file list sorting problem in Makefile.am and vdsm.spec.in according to Ayal's comments - fix mom.conf packaging issue according to Doron's comments
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Doron Fediuck has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7: I would prefer that you didn't submit this
(14 inline comments)
Thanks for pursuing this integration, particularly when ksmtune usage for Fedora is broken.
Could you think of a unittest for this new feature?
.................................................... File vdsm/clientIF.py Line 85: # If MOM is not available, we need failback to KsmMonitorThread. s/failback/to fall back/
.................................................... File vdsm/ksm.py Line 102: def tune(params): I'm not very fond of basket args like this "params". why haven't you listed the 3 valid args in the function's signature?
Line 103: allowed = set(('ksm_run', 'ksm_sleep_millisecs', 'ksm_pages_to_scan')) ALLOWED is a constant
Line 104: if set(params.keys()) > allowed: smells like something you should
assert
Line 106: for (k, v) in params.items(): not really important for such a short dict, but iteritems() is quicker.
Line 109: f.write(str(v)) use 'with' for auto-close on exception.
Line 111: except IOError: is this expected? if not, log.error().
Line 113: return True it is more pythonic to return None on success, and raise an exception on failure.
.................................................... File vdsm/supervdsmServer.py Line 190: def ksmTune(self, tuningParams): where is this new verb used? in a future patch? if it's via a hidden path from the MOM object, I could really use some commenting.
.................................................... File vdsm/vdsMOM.py Line 25: def __init__(self, cif): Saggi would tell you that the single configurable of this function should be an arg to this constructor.
Line 33: # it must be initialized by calling setConnection() with the vdsm cif object. please keep lines short and add new modules to PEP8WHITELIST
Line 34: vdsmInterface.setConnection(self._cif) _cif is not the proper API to interact with Vdsm. API.py is. Please change MOM to conform to this, if possible.
Line 37: self.MOM = mom.MOM(conf) self.MOM is not a class nor a public constant. it should probably be called self._mom.
Line 40: self.running = True is there any reason to return an object if this has failed? it seems cleaner to avoid self.running and raise the exception as-is.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7:
Hi Dan, Thanks a lot for the review. I am going to revise the patch as per your comments. And the unittest will be included in next version too. The only problem needing more discussions is that how mom interacts with vdsm. Currently, vdsm passes the vdsm internal instance of clientIF to mom to enable mom call vdsm API functions defined in API.py. Let's think MOM is kind of plugin for vdsm, then it would make sense that passing cif to mom. Without cif, mom would have to interact with vdsm via xmlrpc. In that case, mom works like a standalone application on system. So which idea do you like more? Any other suggestions? Thanks!
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7:
How about moving MOM development to gerrit.ovirt.org? ;-)
I think that mom should receive an API object (even API.Global()!) that it needs for its operation. Even passing BindingXMLRPC() object is more APIish than the internal clientIF object.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7:
I would also like to request that a set of tests be added to the functional test suite to check the MOM integration code.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 7: I would prefer that you didn't submit this
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com
Xu He Jie has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 9: I would prefer that you didn't submit this
(5 inline comments)
.................................................... File vdsm/ksm.py Line 30: momBase.__init__(self, log, self.run, 'KsmMonitor') I know this work. I guess only use one way, overriding run() method or set target, will be more clean
Line 39: self.start() It looks can't start thread at base class. But move start out of __init__, and just overriding start() method will be clean. is right?
.................................................... File vdsm/vdsMOM.py Line 25: class momBase(threading.Thread): the name should be MomBase
Line 45: class vdsMOM(momBase): same with above
Line 48: import mom why not import mom at top of file
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 9: I would prefer that you didn't submit this
(10 inline comments)
.................................................... File Makefile.am Line 56: vdsm_cli/vdsClient.py \ Please help keep these sorted.
Line 60: No need to add whitespace here.
.................................................... File vdsm/clientIF.py Line 70: self.memOvercommit = None This variable name should make it more obvious what it is. How about naming it MOMController ?
.................................................... File vdsm/ksm.py Line 28: class KsmMonitorThread(momBase): I am not sure I like having this thread inherit from a MomBase class. The KsmMonitorThread is on its way to deprecation so we should leave this class as-is. I would rather see if statements in the few areas of the code that check if MOM is initialized and fall back to ksmThread as necessary.
.................................................... File vdsm/libvirtvm.py Line 2050: self.cif.memOvercommit.adjust() I don't understand this adjust call. It seems to be a no-op.
.................................................... File vdsm/mom.conf Line 45: policy: /etc/vdsm/mom.policy Maybe build this value using autoconf and use the @confdir@ variable.
Line 51: log: /var/log/vdsm/mom.log Same as above
.................................................... File vdsm/vdsMOM.py Line 25: class momBase(threading.Thread): This is not a great abstraction in my opinion. The KsmMonitorThread really has nothing to do with MOM so I would prefer that we don't try to merge the two things together. Is there any other reason to have this class around?
.................................................... File vdsm/vm.py Line 571: if self.lastStatus != 'Don' and 'recover' not in self.conf: accidental bug addition: s/Don/Down/
Line 572: self.cif.memOvercommit.adjust() Just add an if statement here to see if you should use MOM or KsmMonitorThread. Basically, if ksmMonitor is not None, call its adjust method.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 9:
One additional comment. You will need to submit a patch to MOM to change the initialization sequence since you no longer need to receive a reference to clientIF.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Xu He Jie has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 9: (2 inline comments)
.................................................... File vdsm/Makefile.am Line 241: $(DESTDIR)$(vdsmconfdir)/mom.policy use tab instead of space
Line 243: $(DESTDIR)$(vdsmconfdir)/mom.conf use tab instead of space
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 9:
Adam, The MOM patch is already done in my local repo. I wrote it to test this patch. I will submit later.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 10: I would prefer that you didn't submit this
(4 inline comments)
.................................................... File vdsm/constants.py.in Line 165: # is this really needed to be exposed in the global constants.py? I'd rather hide it in ksm.py.
.................................................... File vdsm/vdsmMOM.py Line 22: from vdsm.config import config due to the filename change it is a bit hard to check if all my questions were answered.
at least one, has not - this module should not have direct access to vdsm.config.config. VdsmMOM can take momconf as an arg.
that's better for testability and good relations with Saggi.
Line 30: class VdsmMOM(threading.Thread): I do not have good suggestions, but I'm not happy with the name. We all know it is a part of the vdsm project, so the Vdsm prefix does not really help.
Line 32: def __init__(self, log): I think the signature would be prettier if "log" is dropped, and replaced with a logging.getLogger() call.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 11: I would prefer that you didn't submit this
(2 inline comments)
thanks, Mark, for the update. I have very few stylistic comments.
.................................................... File vdsm/ksm.py Line 109: KSM_PARAMS = ('run', 'sleep_millisecs', 'pages_to_scan') do you envisage someone outside this module using this tupple? if not, keep _PRIVATE, or in-function.
.................................................... File vdsm/MOM.py Line 30: class momThread(threading.Thread): style: class names should begin with Upper case (yeah, we have ugly baggage not conforming, but let's improve!)
the name of the module, though, would better be lower-case mom.py (I think).
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 11: (1 inline comment)
.................................................... File vdsm/MOM.py Line 44: ret['ksmState'] = stats['ksm_run'] I suspect that a subtle change if api is hidden here: ksmMonitor returns True/False for ksmState.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 11: (2 inline comments)
.................................................... File vdsm/MOM.py Line 30: class momThread(threading.Thread): we can't use the name mom.py, because it's the name of the python module MOM provides. If using mom.py, it will fail to import mom /usr/lib/python2.7/site-packages/mom
The original file name is vdsmMOM.py or vdsMOM.py. As you mentioned before, the prefix is useless because it's in vdsm project. I didn't figure out a better name than MOM.py. If you have any suggestion, I will change it. Thanks!
Line 44: ret['ksmState'] = stats['ksm_run'] Thanks for pointing out it!
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 12:
Other questions except the file name(MOM.py) has been answered.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Ayal Baron has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 13: I would prefer that you didn't submit this
(5 inline comments)
Minor issues
.................................................... File vdsm/ksm.py Line 114: raise Exception('Invalid KSM tuning parameter: %s=%s' % (k, v)) you could also make sure that values are proper (run in [0,1,2] etc)
.................................................... File vdsm/libvirtvm.py Line 2067: if self.cif.ksmMonitor: for symmetry perhaps this should be if not self.mom
.................................................... File vdsm/vm.py Line 572: and self.cif.ksmMonitor: again, perhaps not self.mom
Line 574: # behaviors on VM start/destroy. Because the tuning can be s/. B/, b/
Line 575: # done automatically acccording its statistics data. s/its statistics/to its statistical/
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 14:
Ayal, All problems you pointed out has been fixed in the new patch. Many thanks for the review!
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Xu He Jie has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 14: I would prefer that you didn't submit this
(1 inline comment)
minor issue
.................................................... File vdsm/clientIF.py Line 112: if self.mom: if any exception raised before _prepareMON, self.mom will be non-exist. this will overlap the real exception
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 14: (1 inline comment)
.................................................... File vdsm/clientIF.py Line 112: if self.mom: Yes, you're right. I will fix it in next patch. Thanks for the review!
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 15: Looks good to me, but someone else must approve
(2 inline comments)
this patch has waited long enough.
.................................................... File vdsm/ksm.py Line 109: KSM_PARAMS = {'run': 3, 'sleep_millisecs': 0x100000000, I still do not think KSM_PARAMS should be public.
Line 114: if k not in KSM_PARAMS.keys(): use iterkeys() when there is no reason to create a new list.
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Doron Fediuck has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 17: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 17: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 17: Verified
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: MOM Integration ......................................................................
Patch Set 17:
Congrats
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: MOM Integration ......................................................................
MOM Integration
This patch integrates MOM into vdsm. For the descprition of how it works, please see Adam's mail: https://fedorahosted.org/pipermail/vdsm-devel/2012-February/000628.html
Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Signed-off-by: Mark Wu wudxw@linux.vnet.ibm.com --- M .gitignore M Makefile.am M vdsm.spec.in M vdsm/API.py M vdsm/Makefile.am M vdsm/clientIF.py M vdsm/config.py.in M vdsm/ksm.py M vdsm/libvirtvm.py A vdsm/mom.conf.in A vdsm/mom.policy A vdsm/momIF.py M vdsm/supervdsmServer.py M vdsm/vm.py 14 files changed, 280 insertions(+), 23 deletions(-)
Approvals: Mark Wu: Verified Doron Fediuck: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/2367 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I61e68f72e9115a913d5bc0f4903b906b0d0cce2f Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Doron Fediuck dfediuck@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org