-
|
Hi all, we have used Avocado VT for years now and never with OpenVSwitch which seems obligatory in here: mandatory_programs = {'qemu': basic_program_requirements + ['gcc'],
'spice': basic_program_requirements + ['gcc'],
'libvirt': basic_program_requirements,
'openvswitch': basic_program_requirements,Nevertheless, bootstrapping Avocado VT never failed for us and all we got was a rather annoying error message every few lines like def __init_openvswitch():
if (not __ovs.check()):
raise Exception("Check of OpenVSwitch failed.")
except Exception as e:
LOG.error("Host does not support OpenVSwitch: %s", e)
__ovs = None
return __ovswhich never affected any of our VT tests. I was thinking of proposing a pull request and trying to drop or reduce the scope of the message but at present I am not sure if we are not missing out something with this dependency we don't use. Do you think it is indeed a mandatory dependency that should work and be used in all cases? |
Beta Was this translation helpful? Give feedback.
Replies: 2 comments 10 replies
-
|
@luckyh I have separated this possibly optional import in a discussion to try and figure out if it could be included in a pull request to reduce its verbosity too. Do you happen to have an opinion whether the verbosity here could also be reduced? |
Beta Was this translation helpful? Give feedback.
-
|
I see, thanks for the hints! So the stack trace is the following: and is due to code in the env_process preprocessing hooks that destroy vms. Perhaps destroying a qemu vm is not supposed to lead to OpenVSwitch being checked for existence on each call? |
Beta Was this translation helpful? Give feedback.
-
|
Hi @luckyh is this stracktrace still any useful to you? |
Beta Was this translation helpful? Give feedback.
-
|
@pevogam thank you for providing this useful info! It's just hard for me to find some time working on this issue during the past busy months. Let's see if I can grab some time to dive deeper into it on Dec, thanks! |
Beta Was this translation helpful? Give feedback.
-
|
Alright, let's see if we can make progress with this before the Avocado LTS release 👍 |
Beta Was this translation helpful? Give feedback.
-
|
Hi @pevogam I'm not sure if the following would just be a partial fix to this issue but worth a try. diff --git a/virttest/qemu_vm.py b/virttest/qemu_vm.py
index b58836be..fbc76b39 100644
--- a/virttest/qemu_vm.py
+++ b/virttest/qemu_vm.py
@@ -3700,7 +3700,7 @@ class VM(virt_vm.BaseVM):
if nic.nettype == 'macvtap':
tap = utils_net.Macvtap(nic.ifname)
tap.delete()
- elif nic.ifname:
+ elif nic.ifname and nic.nettype in ('bridge', 'network'):
port_mapping[nic.ifname] = nic
if port_mapping:And if we still can see the issue in the same context I guess something like this would be required. diff --git a/virttest/utils_net.py b/virttest/utils_net.py
index e77b888f..c66760d0 100644
--- a/virttest/utils_net.py
+++ b/virttest/utils_net.py
@@ -1989,6 +1989,10 @@ def find_bridge_manager(br_name, ovs=None):
return None
+def iface_has_master(ifname):
+ ...
+
+
def find_current_bridge(iface_name, ovs=None):
"""
Finds bridge which contains interface iface_name.
@@ -2000,6 +2004,8 @@ def find_current_bridge(iface_name, ovs=None):
master = __bridge
bridge = master.port_to_br(iface_name)
if bridge is None:
+ if not iface_has_master(iface_name):
+ return(None, None)
if ovs is None:
ovs = __init_openvswitch()
if ovs is not None: |
Beta Was this translation helpful? Give feedback.
-
|
I tried both changes but still see the error, now from def find_bridge_manager(br_name, ovs=None):
"""
Finds bridge which contain interface iface_name.
:param br_name: Name of interface.
:return: (br_manager) which contain bridge or None.
"""
# find ifname in standard linux bridge.
if br_name in __bridge.list_br():
return __bridge
if ovs is None:
ovs = __init_openvswitch()All of this makes me wonder if any of the above is even the right approach here since it seems most bridge management functions rely on optional
+ if ovs is None and REQUIRE_OVS:
ovs = __init_openvswitch()What do you think? |
Beta Was this translation helpful? Give feedback.
-
In fact, these issues were caused by a flaw of the model design in the vt qemu network control plane: instead of letting callers specify the driver/manager of the bridge devices it always executes an automatic detect, and as the result We could fix the issues by changing the model design, however, the refactoring to be required will be complex and not go too fast I guess. On the other hand, IMO either a regular fix or a workaround against the current design would sound rational for us, that is,
Or
I don't have a strong preference to the two options therefore I'd like to hear from you, thanks. |
Beta Was this translation helpful? Give feedback.
-
|
Overall I think that adding additional checks for the OVS driver is only going to exacerbate the situation where each method is setting up OVS by itself even further by adding more repeated logic and reinforcing the design so I would likely rather stick with de-escalating the logging level to DEBUG or INFO level and suggest the design gets improved from its current state to not have to log such lines for users not using these drivers in the future. Does that sound good? |
Beta Was this translation helpful? Give feedback.
-
|
Ok, I just issued #3821 per our discussion, thanks! |
Beta Was this translation helpful? Give feedback.
-
|
Alright, closing this as the suggested pull request was merged, I do hope we can improve on the overall design in the future though. |
Beta Was this translation helpful? Give feedback.
Alright, closing this as the suggested pull request was merged, I do hope we can improve on the overall design in the future though.