Alon Bar-Lev has posted comments on this change.
Change subject: packaging: refactored firewalld support
......................................................................
Patch Set 5: (4 inline comments)
....................................................
File Makefile
Line 235: @install -dm 755 $(DESTDIR)$(DATA_DIR)/db-backups
Line 236: @install -dm 755 $(DESTDIR)$(DATA_DIR)/ovirt-isos
Line 237: @install -dm 755 $(DESTDIR)$(DATA_DIR)/scripts/plugins
Line 238: @install -dm 755 $(DESTDIR)$(DATA_DIR)/scripts/dbutils
Line 239: @install -dm 755 $(DESTDIR)$(DATA_DIR)/firewalld
I don't think this is required... install should install recursively.
Line 240: @install -dm 755 $(DESTDIR)$(DATA_DIR)/firewalld/base
Line 241: @install -dm 755 $(DESTDIR)$(MAN_DIR)/man8
Line 242: @install -dm 755 $(DESTDIR)$(PYTHON_DIR)/sos/plugins
Line 243: @install -dm 755 $(DESTDIR)$(PKG_SYSCONF_DIR)/engine-config
Line 320: install -m 750 backend/manager/tools/dbutils/fkvalidator.sh
$(DESTDIR)$(DATA_DIR)/scripts/dbutils
Line 321: install -m 640 backend/manager/tools/dbutils/fkvalidator_sp.sql
$(DESTDIR)$(DATA_DIR)/scripts/dbutils
Line 322:
Line 323: install_aio_plugin:
Line 324: install -dm 755 $(DESTDIR)$(DATA_DIR)/firewalld/aio
I would have moved it to the place actually required, just before you install
the service.
Line 325: install -m 644 packaging/fedora/setup/plugins/all_in_one_100.py
$(DESTDIR)$(DATA_DIR)/scripts/plugins
Line 326: install -m 644 packaging/resources/firewalld/aio/ovirt-aio.xml
$(DESTDIR)$(DATA_DIR)/firewalld/aio/ovirt-aio.xml
Line 327:
Line 328: install_sec:
....................................................
File packaging/fedora/spec/ovirt-engine.spec.in
Line 601: # Task Cleaner
Line 602: %{engine_data}/scripts/dbutils
Line 603:
Line 604: # Firewalld configuration
Line 605: %dir %{engine_data}/firewalld
Are you sure the above is required?
Line 606: %{engine_data}/firewalld/base
Line 607:
Line 608: # Man pages
Line 609: %{_mandir}/man8/engine-setup.*
....................................................
File packaging/resources/firewalld/base/ovirt-http.xml.in
Line 1: <?xml version="1.0" encoding="utf-8"?>
Line 2: <service>
Line 3: <short>ovirt-http</short>
Line 4: <description>oVirt configured http service</description>
Line 5: <port protocol="tcp" port="@PORT@"/>
I know I am commenting the otopi implementation... but maybe it is better to
use HTTP_PORT and HTTPS_PORT so same dictionary can be used to process both
template... but this is totally minor.
--
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: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
