Change in vdsm[master]: Change vdsmrepo to something non-rhev specific

[Ryan Harper]

Ryan Harper has uploaded a new change for review.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Change vdsmrepo to something non-rhev specific

Instead of /rhevm use something more distro agnostic for storage repo path.
Model this after libvirt (/var/lib/libvirt/images).

Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Signed-off-by: Ryan Harper 
---
M configure.ac
M vdsm.spec.in
2 files changed, 2 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/86/3286/1
--
To view, visit http://gerrit.ovirt.org/3286
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper

Adam Litke has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

I would prefer /var/lib/vdsm/repository instead of /var/lib/vdsm/images since the dir is for metadata and doesn't actually store the images themselves.

....................................................
File configure.ac
Line 76: AC_SUBST([vdsmrepo], ['${sharedstatedir}/vdsm/images'])
How about 'repository' instead of 'images'?  'images' might become confusing because that word already refers to a completely different concept in vdsm.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 1:

argh. welcome to backport hell. population: me.

but what's right is right.. and yes, "images" is too specific

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

In the past, we had a discussion about this replacement, please see the comments here: http://gerrit.ovirt.org/#change,449

Also, note that if we decide to create a directory like "/ovirt" or any other, vdsm must have access to write/create. In that case, the source vdsm/vdsm.rwtab should be updated as well. Otherwise, oVirt Node will complain about it.

Finally, if you have tested your patch, please use the field Verified in "Review" button.

....................................................
Commit Message
Line 9: Instead of /rhevm use something more distro agnostic for storage repo path.
/rhev

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf

Saggi Mizrahi has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: I would prefer that you didn't submit this

This breaks backwards compatibly, add upgrade path to install\reconfigure\vdsm startup

Also don't push this until this change is added to the selinux policy package in fedora (at least)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

Saggi Mizrahi has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: Do not submit

changed to -2 so that it isn't removed by new patchset updates, I'll remove my reservation when selinux support is deemed unimportant by another maintainer or is implemented.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

Saggi, as long as the patch is a one line change to configure.ac, downstream can survive this issue.

The compatible change to selinux in Fedora is indeed an issue that needs to be fixed - after we agree here about the Fedora repo location.

....................................................
File vdsm.spec.in
Line 451: %dir %attr(0775, root, root) %{_sharedstatedir}/%{vdsm_name}/repository
is this intentional?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Ryan Harper]

Ryan Harper has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm.spec.in
Line 451: %dir %attr(0775, root, root) %{_sharedstatedir}/%{vdsm_name}/repository
Yes, if vdsmrepo at the top level is something like /var/lib/vdsm/  the /var/lib is really _sharedstatedir in configure variables.  If someone were to build the rpm with different value for _sharedstatedir (or even configure vdsm with something different than /var/lib for _sharedstatedir then the rpm compile breaks.

This allows rebuilding the rpm with different _sharedstate value (which is passed to the %configure macro and used in the %files section.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm.spec.in
Line 451: %dir %attr(0775, root, root) %{_sharedstatedir}/%{vdsm_name}/repository
but your new default value for @vdsmrepo@ already include a reference to sharedstatedir. that should be enough.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Ryan Harper]

Ryan Harper has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm.spec.in
Line 451: %dir %attr(0775, root, root) %{_sharedstatedir}/%{vdsm_name}/repository
Ah, now I remember.  Because rpm can't expand the Makefile variables.

RPM build errors:
    File must begin with "/": ${sharedstatedir}/vdsm/repository
make: *** [rpm] Error 1

Maybe some makefile magic can help...

for now, in the makefile, we have:

vdsmrepo = ${sharedstatedir}/vdsm/repository

and then

PATHSUBST = sed \
    -e "s,[@]BACKUPDIR[@],$(vdsmbackupdir),g" \
    -e "s,[@]CONFDIR[@],$(vdsmconfdir),g" \
    -e "s,[@]HOOKSDIR[@],$(vdsmhooksdir),g" \
    -e "s,[@]LIBEXECDIR[@],$(vdsmexecdir),g" \
    -e "s,[@]POOLSDIR[@],$(vdsmpoolsdir),g" \
    -e "s,[@]TRUSTSTORE[@],$(vdsmtsdir),g" \
    -e "s,[@]VDSMDIR[@],$(vdsmdir),g" \
    -e "s,[@]VDSMLIBDIR[@],$(vdsmlibdir),g" \
    -e "s,[@]VDSMLOGDIR[@],$(vdsmlogdir),g" \
    -e "s,[@]VDSMREPO[@],$(vdsmrepo),g" \
    -e "s,[@]VDSMRUNDIR[@],$(vdsmrundir),g"

make failthfully replaces @VDSMREPO@ wtih the exact string in the specfile;  I'm not sure we can bridge this gap since sharedstatedir is a property of the environment during install; not build (specfile generation time).

In any case, this is why I switched the %files section.  Definitely should have included the background in the commit message.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm.spec.in
Line 451: %dir %attr(0775, root, root) %{_sharedstatedir}/%{vdsm_name}/repository
If you deem that usage of rpm's %{_sharedstatedir} within Makefile as so important, please find a way to pass that value from the .spec to the Makefile.

However, please do not break the hard-earned possiblity to change vdsmrepo via a single change to configure.ac.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: I would prefer that you didn't submit this

agreed with Dan's last comment.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: No score

Build Started http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/88/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.info/job/vdsm_unit_tests_by_patch/88/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Ryan Harper]

Ryan Harper has abandoned this change.

Change subject: Change vdsmrepo to something non-rhev specific
......................................................................

Patch Set 2: Abandoned

Too old

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I080a161f6338d341c986e0aaa2270a90be10891f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ryan Harper 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Ryan Harper 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server