Change in vdsm[master]: Enable balloon and use the MoM multiple policies feature

wudxw at linux.vnet.ibm.com wudxw at linux.vnet.ibm.com
Tue Jul 16 03:46:23 UTC 2013


Mark Wu has posted comments on this change.

Change subject: Enable balloon and use the MoM multiple policies feature
......................................................................


Patch Set 4: (4 inline comments)

all are minor issues.  I am sorry that I didn't fully review it before.

....................................................
File vdsm/Makefile.am
Line 115: 	mom.conf.in \
Line 116: 	mom.d/03-ksm.policy \
Line 117: 	mom.d/02-balloon.policy \
Line 118: 	mom.d/01-parameters.policy \
Line 119: 	mom.d/00-python.policy \
it needs be sorted in ascending order.  you could just select the items in vim and type  'sort'
Line 120: 	sourceRoute.sh \
Line 121: 	sudoers.vdsm.in \
Line 122: 	supervdsmd.init.in \
Line 123: 	supervdsmd.service.in \


Line 231: 		$(DESTDIR)$(vdsmconfdir)/mom.d/01-parameters.policy
Line 232: 	$(INSTALL_DATA) mom.d/02-balloon.policy \
Line 233: 		$(DESTDIR)$(vdsmconfdir)/mom.d/02-balloon.policy
Line 234: 	$(INSTALL_DATA) mom.d/03-ksm.policy \
Line 235: 		$(DESTDIR)$(vdsmconfdir)/mom.d/03-ksm.policy
it could be cleaner to use wildcard to install policy files:
    $(INSTALL_DATA) mom.d/*.policy \
        $(DESTDIR)$(vdsmconfdir)/mom.d
Line 236: 	$(INSTALL_DATA) mom.conf \
Line 237: 		$(DESTDIR)$(vdsmconfdir)/mom.conf
Line 238: 
Line 239: uninstall-data-mom:


....................................................
File vdsm.spec.in
Line 826: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/logger.conf
Line 827: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/svdsm.logger.conf
Line 828: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.conf
Line 829: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/00-python.policy
Line 830: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/01-parameters.policy
I have a question about the update policy. Do we expect the new policy be applied automatically after the vdsm package is installed?  We don't expect customer change the policy, right?  So it could be better to remove 'noreplace'. But I don't strongly insist on it
Line 831: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/02-balloon.policy
Line 832: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/03-ksm.policy
Line 833: %config(noreplace) %{_sysconfdir}/logrotate.d/vdsm
Line 834: %config(noreplace) %{_sysconfdir}/rwtab.d/vdsm


Line 828: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.conf
Line 829: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/00-python.policy
Line 830: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/01-parameters.policy
Line 831: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/02-balloon.policy
Line 832: %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/03-ksm.policy
you could use wildcard too:
%config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.d/*.policy
then we don't need change update the spec file when we add new policy in future
Line 833: %config(noreplace) %{_sysconfdir}/logrotate.d/vdsm
Line 834: %config(noreplace) %{_sysconfdir}/rwtab.d/vdsm
Line 835: %config(noreplace) %{_sysconfdir}/sysctl.d/vdsm
Line 836: %if 0%{?with_systemd}


-- 
To view, visit http://gerrit.ovirt.org/16728
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3fa80990bdbcd88f8ec83f5c664047c75c2e7a73
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list