Alon Bar-Lev has posted comments on this change. Change subject: packaging: refactored firewalld support ......................................................................
Patch Set 2: (3 inline comments) Looks good! .................................................... File Makefile Line 273: #FirewallD Line 274: install -m 644 packaging/fedora/setup/ovirt-nfs.xml $(DESTDIR)$(DATA_DIR)/firewalld/ovirt-nfs.xml Line 275: install -m 644 packaging/fedora/setup/ovirt-aio.xml $(DESTDIR)$(DATA_DIR)/firewalld/ovirt-aio.xml Line 276: install -m 644 packaging/fedora/setup/ovirt-http.xml.in $(DESTDIR)$(DATA_DIR)/firewalld/ovirt-http.xml.in Line 277: install -m 644 packaging/fedora/setup/ovirt-https.xml.in $(DESTDIR)$(DATA_DIR)/firewalld/ovirt-https.xml.in copy all? Line 278: Line 279: install -m 644 packaging/fedora/setup/nfs.sysconfig $(DESTDIR)$(DATA_DIR)/conf Line 280: install -m 644 packaging/fedora/setup/ovirt-engine-proxy.conf.in $(DESTDIR)$(DATA_DIR)/conf Line 281: install -m 644 packaging/fedora/setup/ovirt-engine-root-redirect.conf.in $(DESTDIR)$(DATA_DIR)/conf .................................................... File packaging/fedora/setup/plugins/all_in_one_100.py Line 228: '#migration', Line 229: '-A INPUT -p tcp -m state --state NEW -m multiport --dports 49152:49216 -j ACCEPT' Line 230: ] Line 231: Line 232: if basedefs.CONST_CONFIG_EXTRA_FIREWALLD_RULES not in controller.CONF: I think this should be done in setup when building sequence. Line 233: controller.CONF[basedefs.CONST_CONFIG_EXTRA_FIREWALLD_RULES] = [] Line 234: controller.CONF[basedefs.CONST_CONFIG_EXTRA_FIREWALLD_RULES].append('ovirt-aio') Line 235: Line 236: def waitForJbossUp(): .................................................... File packaging/fedora/spec/ovirt-engine.spec.in Line 604: # Firewalld configuration Line 605: %dir %{engine_data}/firewalld Line 606: %{engine_data}/firewalld/ovirt-nfs.xml Line 607: %{engine_data}/firewalld/ovirt-http.xml.in Line 608: %{engine_data}/firewalld/ovirt-https.xml.in I think we can add the entire firewalld directory, no? Oh... aio... So... what about adding subdir such as base and aio? I just hate this tool forces us to name each file explicitly, so every change in build changes packaging. Line 609: Line 610: # Man pages Line 611: %{_mandir}/man8/engine-setup.* Line 612: %{_mandir}/man8/engine-upgrade.* -- To view, visit http://gerrit.ovirt.org/13638 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If6a60e1667182f926f399e22bbfbb939e7171cb0 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Alex Lourie <alou...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches