On Thu, Sep 15, 2016 at 07:16:44PM +0300, Ido Schimmel wrote:
> On Thu, Sep 15, 2016 at 05:50:38PM +0200, Ondrej Lichtner wrote:
> > On Thu, Sep 15, 2016 at 10:42:14AM +0300, idosch(a)idosch.org wrote:
> > > From: Ido Schimmel <idosch(a)mellanox.com>
> > > + def enable_lldpad(self):
> > > + exec_cmd("service lldpad start")
> > > + return True
> > > +
> > > + def disable_lldpad(self):
> > > + exec_cmd("service lldpad stop")
> > > + return True
> > > +
> >
> > Will this also work correctly on systemd systems? I'm not sure if the
> > sysv<->systemd translation scripts are still available/installed by
> > default.
> > Maybe calling either "sytemctl start" or "service start"
based on the
> > current system would make more sense.
>
> Well, I ran the recipes on a systemd system (Fedora), so yes. Also,
> Fedora wiki says this:
>
> "Note on 'service' and 'chkconfig' commands
> The 'service' and 'chkconfig' commands will mostly continue to work
as
> expected in the systemd world, this guide is how to use the native
> systemctl replacements."
>
https://fedoraproject.org/wiki/SysVinit_to_Systemd_Cheatsheet
>
> I guess it's fine.
Ah, well I'm a bit concerned about the "_mostly_ continue to work" part
:), who knows if Fedora won't decide to to just scrap this in the next
release... Also, even though we don't use them, what about other distros
that are pure systemd without the "service" aliases?
>
> Regarding to why I chose to use 'service' instead of 'systemctl',
then
> it's because I already saw other LNST parts are using it. See
> recipes/regression_tests/phase1/3_vlans.py for example
>
> Whereas:
>
> $ git grep -n 'systemctl' | wc -l
> 0
The difference here is that it's part of a recipe that could be seen as
slightly specific for our uses, while you're adding a feature directly
to LNST core. The use of "service" in our recipes might be an oversight
and could be fixed as well, I'll discuss with the rest of the team.
Also, now that I think about it... how about instead of doing these
{enable, disable}_lldpad Slave Methods, you'd do a general {enable,
disable}_service method that takes the service name as a parameter, and
it could also work for both SysV and systemd?
+1 for generic enable_service(service)
>-Ondrej
>_______________________________________________
>LNST-developers mailing list
>lnst-developers(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahosted.org