Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-9526

Fix issues with systemd variables in lustre.spec.in

Details

    • Bug
    • Resolution: Fixed
    • Major
    • Lustre 2.10.0
    • None
    • None
    • 3
    • 9223372036854775807

    Description

      LU-9439 (change 26925) introduced some issues in the spec file that should be cleaned up before 2.10.

      First of all, the patch added:

      %bcond_with    systemd
      

      That is right and good. But then the spec files goes on to almost completely avoid using that, and that part is bad.

      There is already a perfect example of how to unset a bcond when another setting overrides the bcond early in the file:

      %if %{without servers}
          # --without servers overrides --with {ldiskfs|zfs}
          # so undefine the internal variables set by bcond_*
          %undefine with_ldiskfs
          %undefine with_zfs
      %endif
      

      If you want to set a bcond, do the opposite. So setting the internal variable for the systemd bcond would look like this:

      #define with_systemd
      

      That's it! Now you can use the %{with systemd} and %{without systemd} macros instead of introducing a second systemd variable (_systemd).

      So the main change needed is to eliminate the _systemd macro.

      This block can be removed entirely:

      # Generic enable switch for systemd
      %if %{with systemd}
      %define _systemd 1
      %endif
      
      

      Then change each instance of:

      %define _systemd 1
      

      into:

      %define with_systemd
      

      Change each instance of:

      %if 0%{?_systemd}
      

      into:

      %if %{with systemd}
      

      Change:

      %if 0%{!?_systemd:1}
      

      into:

      %if %{without systemd}
      

      And finally, instead of introducing a third systemd macro here:

      %if 0%{?_systemd}
          %define systemd --with-systemdsystemunitdir=%{_unitdir}
      %else
          %define systemd --with-systemdsystemunitdir=no
      %endif
      

      You can just inline the options in eval_configure like so:

                %{?with_systemd:--with-systemdsystemunitdir=%{_unitdir}} \
                %{!?with_systemd:--with-systemdsystemunitdir=no} \
      

      I would think that when systemd is not detected, I would have thought that "--with-systemdsystemunitdir=no" would already be the default. Perhaps that line can be dropped?

      Attachments

        Activity

          [LU-9526] Fix issues with systemd variables in lustre.spec.in
          pjones Peter Jones added a comment -

          Landed for 2.10

          pjones Peter Jones added a comment - Landed for 2.10

          Oleg Drokin (oleg.drokin@intel.com) merged in patch https://review.whamcloud.com/27215/
          Subject: LU-9526 spec: Improve systemd compat in spec file
          Project: fs/lustre-release
          Branch: master
          Current Patch Set:
          Commit: 933cfab484343742cbcbf46e234d7c80b5f66160

          gerrit Gerrit Updater added a comment - Oleg Drokin (oleg.drokin@intel.com) merged in patch https://review.whamcloud.com/27215/ Subject: LU-9526 spec: Improve systemd compat in spec file Project: fs/lustre-release Branch: master Current Patch Set: Commit: 933cfab484343742cbcbf46e234d7c80b5f66160

          Giuseppe Di Natale (dinatale2@llnl.gov) uploaded a new patch: https://review.whamcloud.com/27215
          Subject: LU-9526 spec: Improve systemd compat in spec file
          Project: fs/lustre-release
          Branch: master
          Current Patch Set: 1
          Commit: f5440e4e3f1ec0c3a003f8dd044e1577aa42e263

          gerrit Gerrit Updater added a comment - Giuseppe Di Natale (dinatale2@llnl.gov) uploaded a new patch: https://review.whamcloud.com/27215 Subject: LU-9526 spec: Improve systemd compat in spec file Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: f5440e4e3f1ec0c3a003f8dd044e1577aa42e263

          Actually, looks like the lnet systemd addition didn't get reverted, my mistake. Will introduce a new patch under this ticket.

          dinatale2 Giuseppe Di Natale (Inactive) added a comment - Actually, looks like the lnet systemd addition didn't get reverted, my mistake. Will introduce a new patch under this ticket.

          https://review.whamcloud.com/#/c/27214/ in case anyone is interested in reviewing.

          dinatale2 Giuseppe Di Natale (Inactive) added a comment - https://review.whamcloud.com/#/c/27214/ in case anyone is interested in reviewing.

          These will be taken care of in LU-9526 since the patches got reverted.

          dinatale2 Giuseppe Di Natale (Inactive) added a comment - These will be taken care of in LU-9526 since the patches got reverted.

          People

            dinatale2 Giuseppe Di Natale (Inactive)
            morrone Christopher Morrone (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: