Change in vdsm[master]: Avoid calling restorecon on a nonexistent dir.

mooli tayer has uploaded a new change for review.

Change subject: Avoid calling restorecon on a nonexistent dir.
......................................................................

Avoid calling restorecon on a nonexistent dir.

/var/log/core is only created when vdsm service goes up.
If vdsm never ran on the machine we might get:
restorecon:  lstat(/var/log/core) failed:  No such file or directory

Change-Id: Id7246b4947c0cdb52093c5fe0f60a690d72b1236
Signed-off-by: Mooli Tayer 
---
M init/vdsmd_init_common.sh.in
M vdsm.spec.in
2 files changed, 4 insertions(+), 13 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/22211/1

diff --git a/init/vdsmd_init_common.sh.in b/init/vdsmd_init_common.sh.in
index 4b7d079..cc010fe 100644
--- a/init/vdsmd_init_common.sh.in
+++ b/init/vdsmd_init_common.sh.in
@@ -112,14 +112,6 @@
     "@CHOWN_PATH@" @METADATAUSER@:@METADATAGROUP@ "${_DOM_METADATA_BACKUP_DIR}" > /dev/null 2>&1
 }

-_mk_core_path() {
-    local core_path=/var/log/core
-    if ! [ -d "${core_path}" ]; then
-        @MKDIR_P@ "${core_path}" > /dev/null 2>&1
-    fi
-    "@CHMOD_PATH@" a+tw "${core_path}" > /dev/null 2>&1
-}
-
 _mk_run_path() {
     local run_path

@@ -133,7 +125,6 @@

task_mkdirs(){
     _mk_data_center
-    _mk_core_path
     _mk_dom_backup
     _mk_run_path
     "@CHMOD_PATH@" 1777 /dev/shm
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 7ee0218..a63e415 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -590,6 +590,10 @@
 install -Dm 0644 vdsm/limits.conf \
                  %{buildroot}/etc/security/limits.d/99-vdsm.conf

+# This is not commonplace, but we want /var/log/core to be a world-writable
+# dropbox for core dumps
+install -dDm 1777 %{buildroot}%{_localstatedir}/log/core
+
 %if 0%{?with_systemd}
 install -Dm 0755 init/systemd/systemd-vdsmd %{buildroot}/lib/systemd/systemd-vdsmd
 install -Dm 0644 init/systemd/vdsmd.service %{buildroot}%{_unitdir}/vdsmd.service
@@ -615,10 +619,6 @@
 %endif

%if 0%{?rhel}
-# This is not commonplace, but we want /var/log/core to be a world-writable
-# dropbox for core dumps
-install -dDm 1777 %{buildroot}%{_localstatedir}/log/core
-%else
 # Install the configuration sample
 install -Dm 0644 lib/vdsm/vdsm.conf.sample \
                  %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Avoid calling restorecon on a nonexistent dir.
......................................................................

Patch Set 1: Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5991/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5195/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6083/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/112/ : FAILURE

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

mooli tayer has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 1:

(1 comment)

Shouldn't have abandoned:
http://gerrit.ovirt.org/#/c/21290/

this patch will make sure /var/log/core is always created with  rpm.

....................................................
File vdsm.spec.in
Line 617: install -Dm 0644 init/systemd/vdsm-tmpfiles.d.conf \
Line 618:                  %{buildroot}%{_tmpfilesdir}/%{vdsm_name}.conf
Line 619: %endif
Line 620: 
Line 621: %if 0%{?rhel}
This condition should have actually been negated,
but I couldn't find how to do it.
Any help/pointers?
Line 622: # Install the configuration sample
Line 623: install -Dm 0644 lib/vdsm/vdsm.conf.sample \
Line 624:                  %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf
Line 625:

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 2: Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5992/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5196/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6084/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/113/ : FAILURE

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 2:

(1 comment)

You can restore an abandoned change btw.

....................................................
File vdsm.spec.in
Line 617
Line 618
Line 619
Line 620
Line 621
%else removal kills rpm build.

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

Yaniv Bronhaim has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 2:

I don't understand.. why http://gerrit.ovirt.org/#/c/21290/ shouldn't be abandoned ? and why don't you fix this one.. installing the folder is the right way to handle this

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

Yaniv Bronhaim has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 3: Code-Review-1

(1 comment)

....................................................
File vdsm.spec.in
Line 631: %else
Line 632: install -Dm 0644 vdsm/vdsm-libvirt-access.pkla \
Line 633:                  %{buildroot}%{_polkitdir}/10-vdsm-libvirt-access.pkla
Line 634: %endif
Line 635: %endif
it ends here.. remove your endif
Line 636: 
Line 637: %check
Line 638: make check
Line 639:

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

mooli tayer has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 4:

That's what I'm trying to do.

I have verry little knowledge about spec files syntax.

I think this patch set is better

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

Yaniv Bronhaim has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 4:

(1 comment)

....................................................
File vdsm.spec.in
Line 617: install -Dm 0644 init/systemd/vdsm-tmpfiles.d.conf \
Line 618:                  %{buildroot}%{_tmpfilesdir}/%{vdsm_name}.conf
Line 619: %endif
Line 620: 
Line 621: %if 0%{?rhel}
so should be if not..but why this condition exists anyway? remove the if and put also this sample install above
Line 622: # Install the configuration sample
Line 623: install -Dm 0644 lib/vdsm/vdsm.conf.sample \
Line 624:                  %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf
Line 625: %endif

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 5: Code-Review-1

(1 comment)

....................................................
File vdsm.spec.in
Line 626: %if 0%{?fedora} >= 18
Line 627: install -Dm 0644 vdsm/vdsm-libvirt-access.rules \
Line 628:                  %{buildroot}%{_polkitdir}/10-vdsm-libvirt-access.rules
Line 629: %else
Line 630: install -Dm 0644 vdsm/vdsm-libvirt-access.pkla \
after your change these files are suddenly installed on el6. I do not suppose it was intentional, and it is certainly unrelated to the subject of this patch.

Please try to confine your changes to the core dump issue.
Line 631:                  %{buildroot}%{_polkitdir}/10-vdsm-libvirt-access.pkla
Line 632: %endif
Line 633: 
Line 634: %check

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

Yaniv Bronhaim has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 5:

(1 comment)

....................................................
File vdsm.spec.in
Line 596: 
Line 597: # Install the configuration sample
Line 598: install -Dm 0644 lib/vdsm/vdsm.conf.sample \
Line 599:                  %{buildroot}%{_sysconfdir}/vdsm/vdsm.conf
Line 600: 
I like this change, and its totally right, but if you do that you must specify that in the commit message. it can stay in one patch that does 2 things - making core dump and sample vdsm.conf installed both in rhel and other distros  (btw, maybe you should update also debian folder....) and remove the mkdir part from pre-start as it does not required anymore
Line 601: %if 0%{?with_systemd}
Line 602: install -Dm 0755 init/systemd/systemd-vdsmd %{buildroot}/lib/systemd/systemd-vdsmd
Line 603: install -Dm 0644 init/systemd/vdsmd.service %{buildroot}%{_unitdir}/vdsmd.service
Line 604: install -Dm 0644 init/systemd/supervdsmd.service %{buildroot}%{_unitdir}/supervdsmd.service

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

Itamar Heim has posted comments on this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Patch Set 5:

ping

Change in vdsm[master]: Distribute /var/log/core with vdsm rpm.

Itamar Heim has abandoned this change.

Change subject: Distribute /var/log/core with vdsm rpm.
......................................................................

Abandoned

no activity, restore if relevant