-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce the libvirt logging verbosity (not needed for qemu vms) #3721
Conversation
(I don't quite get why the signed-off static check failed since I have signed the commit but will push again after some changes are requested if any) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @pevogam , I'd really like this proposal, thanks! However, just have a small suggestion regarding the code changes.
95730bf
to
1c8ce34
Compare
@luckyh I followed your request but oddly ended up with |
We do this by utilzing lazy imports where imported modules will only be read when used and not within any modules of shared qemu and livbirt vm usage. Signed-off-by: Plamen Dimitrov <[email protected]>
1c8ce34
to
df6175e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi all, there are currently two approvals for this PR so I guess we can merge it. Regarding verbosity of OpenVSwitch I guess we will simply debug a problem in #3720 (reply in thread). |
Yeah, sure. |
Thanks to all. |
Hi @luckyh, unfortunately this doesn't seem to work in all cases since using
In addition I have to say that I need a very large patching diff to make this work for more imports (and haven't even collected all of them until I encountered this) instead of my simple one-liner proposal from the original pull request. diff --git a/virttest/cpu.py b/virttest/cpu.py
index 89a97f06d..465249d59 100644
--- a/virttest/cpu.py
+++ b/virttest/cpu.py
@@ -30,14 +30,18 @@ from avocado.utils.software_manager import manager
from avocado.utils import process
from avocado.utils import path
from avocado.core import exceptions
-from virttest import virsh
from virttest import utils_misc
-from virttest import libvirt_xml
-from virttest.libvirt_xml.xcepts import LibvirtXMLNotFoundError
from virttest import libvirt_version
from virttest import data_dir
+# lazy imports for dependencies that are not needed in all modes of use
+# (as the cpu module is generic to qemu vm use only, libvirt is optional)
+from virttest._wrappers import lazy_import
+virsh = lazy_import("virttest.virsh")
+libvirt_xml = lazy_import("virttest.libvirt_xml")
+
+
LOG = logging.getLogger('avocado.' + __name__)
ARCH = platform.machine()
@@ -122,7 +126,7 @@ def get_cpu_xmldata(vm, options=""):
cpu_xmldata['mtype'] = vm_xml.os.machine
try:
cpu_xmldata['current_vcpu'] = int(vm_xml.current_vcpu)
- except LibvirtXMLNotFoundError:
+ except libvirt_xml.xcepts.LibvirtXMLNotFoundError:
LOG.debug("current vcpu value not present in xml, set as max value")
cpu_xmldata['current_vcpu'] = int(vm_xml.vcpu)
cpu_xmldata['vcpu'] = int(vm_xml.vcpu)
@@ -196,7 +200,7 @@ def affinity_from_xml(vm):
try:
vmxml = libvirt_xml.VMXML.new_from_dumpxml(vm.name)
xml_affinity_list = vmxml['cputune'].vcpupins
- except LibvirtXMLNotFoundError:
+ except libvirt_xml.xcepts.LibvirtXMLNotFoundError:
LOG.debug("No <cputune> element find in domain xml")
return xml_affinity
# Store xml_affinity_list to a dict
@@ -628,7 +632,7 @@ def guest_numa_check(vm, exp_vcpu):
vmxml = libvirt_xml.VMXML.new_from_dumpxml(vm.name)
try:
node_num_xml = len(vmxml.cpu.numa_cell)
- except (TypeError, LibvirtXMLNotFoundError):
+ except (TypeError, libvirt_xml.xcepts.LibvirtXMLNotFoundError):
# handle if no numa cell in guest xml, bydefault node 0
node_num_xml = 1
node_num_guest = int(vm_cpu_info["NUMA node(s)"])
@@ -638,15 +642,15 @@ def guest_numa_check(vm, exp_vcpu):
try:
node_cpu_xml = vmxml.cpu.numa_cell[node]['cpus']
node_cpu_xml = cpus_parser(node_cpu_xml)
- except (TypeError, LibvirtXMLNotFoundError):
+ except (TypeError, libvirt_xml.xcepts.LibvirtXMLNotFoundError):
try:
node_cpu_xml = vmxml.current_vcpu
- except LibvirtXMLNotFoundError:
+ except libvirt_xml.xcepts.LibvirtXMLNotFoundError:
node_cpu_xml = vmxml.vcpu
node_cpu_xml = list(range(int(node_cpu_xml)))
try:
node_mem_xml = vmxml.cpu.numa_cell[node]['memory']
- except (TypeError, LibvirtXMLNotFoundError):
+ except (TypeError, libvirt_xml.xcepts.LibvirtXMLNotFoundError):
node_mem_xml = vmxml.memory
node_mem_guest = int(vm.get_totalmem_sys(node=node))
node_cpu_xml_copy = node_cpu_xml[:]
diff --git a/virttest/env_process.py b/virttest/env_process.py
index ebd20228d..ffbf2ed6d 100644
--- a/virttest/env_process.py
+++ b/virttest/env_process.py
@@ -37,7 +37,6 @@ from virttest import qemu_storage
from virttest import data_dir
from virttest import utils_net
from virttest import nfs
-from virttest import libvirt_vm
from virttest import utils_test
from virttest import utils_iptables
from virttest import utils_package
@@ -57,6 +56,7 @@ from virttest.test_setup.networking import NetworkProxies
from virttest._wrappers import lazy_import
utils_libvirtd = lazy_import("virttest.utils_libvirtd")
virsh = lazy_import("virttest.virsh")
+libvirt_vm = lazy_import("virttest.libvirt_vm")
try:
@@ -1500,6 +1500,7 @@ def postprocess(test, params, env):
try:
postprocess_vm_on_hook(test, params, env) # pylint: disable=E1102
except Exception as details:
+ raise details
err += "\nPostprocessing living vm hook: %s" % str(details).replace('\\n', '\n ')
LOG.error(details)
diff --git a/virttest/libvirt_xml/base.py b/virttest/libvirt_xml/base.py
index 3845c8d15..2108ef41d 100644
--- a/virttest/libvirt_xml/base.py
+++ b/virttest/libvirt_xml/base.py
@@ -2,7 +2,7 @@ import logging
from avocado.utils import process
-from .. import propcan, xml_utils, virsh
+from .. import propcan, xml_utils
from ..libvirt_xml import xcepts
from .._wrappers import import_module
diff --git a/virttest/migration.py b/virttest/migration.py
index 9fd9ed19f..7243e470a 100644
--- a/virttest/migration.py
+++ b/virttest/migration.py
@@ -12,7 +12,6 @@ from avocado.core import exceptions
from virttest import libvirt_version
from virttest import remote
-from virttest import virsh
from virttest import utils_disk
from virttest import utils_misc
from virttest import test_setup
@@ -22,6 +21,11 @@ from virttest import utils_test
from virttest.utils_test import libvirt
+# lazy imports for dependencies that are not needed in all modes of use
+from virttest._wrappers import lazy_import
+virsh = lazy_import("virttest.virsh")
+
+
LOG = logging.getLogger('avocado.' + __name__)
diff --git a/virttest/utils_test/__init__.py b/virttest/utils_test/__init__.py
index ac540abd7..eb8a649fc 100755
--- a/virttest/utils_test/__init__.py
+++ b/virttest/utils_test/__init__.py
@@ -70,8 +70,12 @@ from virttest._wrappers import import_module
#
# pylint: disable=unused-import
from virttest.utils_test import qemu
-from virttest.utils_test import libvirt
-from virttest.utils_test import libguestfs
+
+# lazy imports for dependencies that are not needed in all modes of use
+from virttest._wrappers import lazy_import
+libvirt = lazy_import("virttest.utils_test.libvirt")
+libguestfs = lazy_import("virttest.utils_test.libguestfs")
+
# This is so that other tests won't break when importing the names
# 'ping' and 'raw_ping' from this namespace |
Hi @pevogam, I haven't dived into the details of how python manipulates the module importing, but it seems that the issue was related to the routine the diff --git a/virttest/_wrappers.py b/virttest/_wrappers.py
index 7ca031f8..bb2fbfcb 100644
--- a/virttest/_wrappers.py
+++ b/virttest/_wrappers.py
@@ -79,6 +79,8 @@ def lazy_import(name):
:param name: Name of the module that is going to be imported
:type name: String
"""
+ if name in sys.modules:
+ return sys.modules[name]
spec = importlib.util.find_spec(name)
if spec is None:
raise ImportError(f"Could not import module {name}") |
Hi @luckyh, ok this indeed fixes the issue. I guess we will proceed with the cleanup then despite the large diff above and I will create a pull request with it next. |
Feel free to check #3783. |
We do this by importing these modules conditionally within the modules of shared qemu and livbirt vm usage.