Nir Soffer has posted comments on this change.

Change subject: spec: Require newer multipath version
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/49113/3//COMMIT_MSG
Commit Message:

Line 7: spec: Require newer multipath version
Line 8: 
Line 9: There is a race between udev and multiapth which is solved in newer
Line 10: multipath versions, this patch requires those versions on RHEL 7.2 and
Line 11: Fedora 21,22 and 23
Are these the version fixing the issue, or just a convenient default (just 
require the latest version in each platform)?
Line 12: 
Line 13: We don't require the package on CentOS, since it is not available yet.
Line 14: 
Line 15: Change-Id: I63a607aa0d8df678666d87eb593e37bb0331dd82


https://gerrit.ovirt.org/#/c/49113/3/vdsm.spec.in
File vdsm.spec.in:

Line 182: %if 0%{?rhel}
Line 183: %if 0%{?centos}
Line 184: Requires: device-mapper-multipath >= 0.4.9-68
Line 185: %else
Line 186: Requires: device-mapper-multipath >= 0.4.9-84.el7
Please remove multipath from this block and put it on its own section.
Line 187: %endif
Line 188: Requires: e2fsprogs
Line 189: Requires: fence-agents-all
Line 190: %if 0%{?centos}


Line 215: %else
Line 216: %if 0%{?fedora} >= 23
Line 217: Requires: device-mapper-multipath >= 0.4.9-80.fc23
Line 218: %endif
Line 219: %endif
Look like you wan to add requirement for fedora 21, 22, and 23, so why do we 
need nested if else?

Fedora 21 is EOL in 2015-12-01, so there is not point to require anything, we 
cannot support it any more in master/3.6.

The old spec was hardly readable, and now it is completely unreadbale.

Please move out multipath requirement so we have one if block per platform.
So we are left wit fedora 22 and 23, lets have one block for each - outside the 
rhel/centos block, and not in the else block.

This is what we need:

   # device-mapper-multipath

    %if 0%{rhel}
    %if 0%{?centos}
    ...
    %else
    ...
    %endif # centos
    %endif  # rhel

    %if 0%{fedora} == 23
    ...
    %endif

    %if 0%{fedora} == 22
    ...
    %endif
Line 220: %endif
Line 221: Requires: e2fsprogs >= 1.41.14
Line 222: %if 0%{?fedora} >= 23
Line 223: Requires: policycoreutils-python-utils


-- 
To view, visit https://gerrit.ovirt.org/49113
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I63a607aa0d8df678666d87eb593e37bb0331dd82
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Amit Aviram <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Idan Shaby <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to