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",
> > >+ if not "_type_initialized" in dir(cls):
> > >+ exec_cmd("modprobe %s %s" % ("openvswitch",
> > >"_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
> 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
> lsmod | grep openvswitch # returned nothing before 'ovs-vsctl'
> 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
> 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