----- Original Message -----
From: "Jan Tluka" <jtluka(a)redhat.com>
To: "Ondrej Lichtner" <olichtne(a)redhat.com>
Cc: "Christos Sfakianakis" <csfakian(a)redhat.com>,
lnst-developers(a)lists.fedorahosted.org
Sent: Wednesday, October 31, 2018 2:21:21 PM
Subject: Re: [PATCH-next 10/10] lnst.Devices: edit OvsBridgeDevice
Wed, Oct 31, 2018 at 12:43:09PM CET, olichtne(a)redhat.com wrote:
>On Wed, Oct 31, 2018 at 07:07:55AM -0400, Christos Sfakianakis wrote:
>> ----- Original Message -----
>> > From: "Jan Tluka" <jtluka(a)redhat.com>
>> > To: csfakian(a)redhat.com
>> > Cc: lnst-developers(a)lists.fedorahosted.org
>> > Sent: Friday, October 26, 2018 12:03:26 PM
>> > Subject: Re: [PATCH-next 10/10] lnst.Devices: edit OvsBridgeDevice
>> >
>> > Fri, Oct 26, 2018 at 10:41:16AM CEST, csfakian(a)redhat.com wrote:
>> > >From: Christos Sfakianakis <csfakian(a)redhat.com>
>> > >
>> > >Avoid termination in case cls._type_initialized or cls._moduleparams
>> > >are undefined.
>> > >
>> > >Signed-off-by: Christos Sfakianakis <csfakian(a)redhat.com>
>> > >---
>> > > lnst/Devices/OvsBridgeDevice.py | 4 ++--
>> > > 1 file changed, 2 insertions(+), 2 deletions(-)
>> > >
>> > >diff --git a/lnst/Devices/OvsBridgeDevice.py
>> > >b/lnst/Devices/OvsBridgeDevice.py
>> > >index d54cf94..57527d2 100644
>> > >--- a/lnst/Devices/OvsBridgeDevice.py
>> > >+++ b/lnst/Devices/OvsBridgeDevice.py
>> > >@@ -25,8 +25,8 @@ class OvsBridgeDevice(SoftDevice):
>> > >
>> > > @classmethod
>> > > def _type_init(cls):
>> > >- if not cls._type_initialized:
>> > >- exec_cmd("modprobe %s %s" %
("openvswitch",
>> > >cls._moduleparams))
>> > >+ if not "_type_initialized" in dir(cls):
>> > >+ exec_cmd("modprobe %s %s" %
("openvswitch", getattr(cls,
>> > >"_moduleparams", "")))
>> > >
>> > > if not check_process_running("ovsdb-server"):
>> > > exec_cmd("mkdir -p /var/run/openvswitch/")
>> >
>> > Few comments on this one (sorry I did not on v1).
>> >
>> > The commit's description should be something more verbose that
"edit
>> > OvsBridgeDevice".
>> Ok, will be more specific next time
>> >
>> > I checked the openvswitch module and it does not support any module
>> > parameters:
>> > modinfo openvswitch | grep -i parm
>> >
>> > Is it ok to remove this "_moduleparams" thing completely?
>> >
>> Your point here seems to be correct. I ran
>> VirtualOvsBridgeVlanInGuestRecipe and "_moduleparams"
>> is empty string at this point (which agrees with your comment). Since it
>> is only used
>> as parameter in the 'modprobe' command in the '_type_init()'
method, it
>> can be disregarded.
>> > Also wondering if the openvswitch module is loaded automagically when
>> > ovs-vsctl is called? Then we could probably remove _type_init()
>> > completely.
>> >
>> It seems that 'ovs-vsctl' script does not suffice load the module or
>> prepare the dependency files. When
>> I ran the same recipe as above, with the module not loaded initially, the
>> was failure as below:
>>
>> lsmod | grep openvswitch # returned nothing before
>> 'ovs-vsctl' command
>> ...
>> Executing: "ovs-vsctl add-br t_ovsbr0" # _create method entered, this
is
>> the first time 'ovs-vsctl' is called
>> ovs-vsctl: unix:/var/run/openvswitch/db.sock: database connection failed
>> (No such file or directory)
>>
>> So it seems this part of the _type_init() method is required for the
>> module to work properly.
>> Adding Ondrej in case I am missing something.
>>
>> - Christos
>
>ovs related modules are loaded automagically when the
>openvswitch system service is started. When support for ovs was first
>introduced this didn't exist so we had to launch ovs manually - create
>the db, load the modules and run the ovsdb-server and ovs-vswitchd. Now
>however this is all handled by the systemd service.
>
>As to what to do, OvsBridgeDevice class is the only device class left
>that still uses a type_init method, it's been removed from all the other
>types. I'm thinking that when I get to actually reimplement it, it might
>end up also being removed.
>
>On the other hand IF it stays, it should probably just attempt to start
>the openvswitch service and ignore any modules etc... This can be done
>now and maybe removed later if we decide that...
>
>@Christos, if this makes sense and Jan agrees you can probably do this
>option.
ACK.
-Jan
Hello, I just sent the change. I didn't include a command to stop
openvswitch in the "destroy()" method (to deactivate the module and return to
the
system state at the beginning of the recipe) because, under the current design, this
method
is never called for an OvsBridgeDevice instance. So I only added a single
systemd command.
-Christos
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
https://getfedora.org/code-of-conduct.html
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
>
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...
>