[PATCH 0/3] Warn users about NetworkManager instability
NetworkManager might have some instability when managing the bridges created by libvirt. There some bugs against NM about it and there is no much we can do more than warn the users about it. So, this patchset checks if NM is running and, if so, will display a message to users. Jose Ricardo Ziviani (3): Add the env parameter to run_command. Implement function to check if NM is running. Add a warn about NM running in the system. src/kimchi/model/config.py | 1 + src/kimchi/model/featuretests.py | 28 ++++++++++++++++++++++++++++ src/kimchi/utils.py | 6 ++++-- tests/test_rest.py | 3 ++- ui/js/src/kimchi.network.js | 5 +++++ ui/pages/i18n.json.tmpl | 1 + 6 files changed, 41 insertions(+), 3 deletions(-) -- 1.9.1
- The called may want to pass a custom environment variable to run_command and now this is possible with this new parameter. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- src/kimchi/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index d71338a..2017cbe 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -165,11 +165,12 @@ def check_url_path(path): return True -def run_command(cmd, timeout=None): +def run_command(cmd, timeout=None, env=None): """ cmd is a sequence of command arguments. timeout is a float number in seconds. timeout default value is None, means command run without timeout. + env is the environment variable passed to the command. """ # subprocess.kill() can leave descendants running # and halting the execution. Using psutil to @@ -193,7 +194,8 @@ def run_command(cmd, timeout=None): try: proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + stderr=subprocess.PIPE, + env=env) if timeout is not None: timer = Timer(timeout, kill_proc, [proc, timeout_flag]) timer.setDaemon(True) -- 1.9.1
The patch looks nice, but could you provide us a scenario where this patch is needed? In which situation are you trying to run a command and you need to specify a different set of environment variables other than the default ones (i.e. the env inherited by the main process)? I couldn't figure this out by reading the patch's description. On Fri, Apr 17, 2015 at 4:29 PM Jose Ricardo Ziviani < joserz@linux.vnet.ibm.com> wrote:
- The called may want to pass a custom environment variable to run_command and now this is possible with this new parameter.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- src/kimchi/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index d71338a..2017cbe 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -165,11 +165,12 @@ def check_url_path(path): return True
-def run_command(cmd, timeout=None): +def run_command(cmd, timeout=None, env=None): """ cmd is a sequence of command arguments. timeout is a float number in seconds. timeout default value is None, means command run without timeout. + env is the environment variable passed to the command. """ # subprocess.kill() can leave descendants running # and halting the execution. Using psutil to @@ -193,7 +194,8 @@ def run_command(cmd, timeout=None):
try: proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + stderr=subprocess.PIPE, + env=env) if timeout is not None: timer = Timer(timeout, kill_proc, [proc, timeout_flag]) timer.setDaemon(True) -- 1.9.1
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
On 27-04-2015 11:48, Crístian Viana wrote:
The patch looks nice, but could you provide us a scenario where this patch is needed? In which situation are you trying to run a command and you need to specify a different set of environment variables other than the default ones (i.e. the env inherited by the main process)? I couldn't figure this out by reading the patch's description.
On Fri, Apr 17, 2015 at 4:29 PM Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com <mailto:joserz@linux.vnet.ibm.com>> wrote:
- The called may want to pass a custom environment variable to run_command and now this is possible with this new parameter.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com <mailto:joserz@linux.vnet.ibm.com>> --- src/kimchi/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index d71338a..2017cbe 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -165,11 +165,12 @@ def check_url_path(path): return True
-def run_command(cmd, timeout=None): +def run_command(cmd, timeout=None, env=None): """ cmd is a sequence of command arguments. timeout is a float number in seconds. timeout default value is None, means command run without timeout. + env is the environment variable passed to the command. """ # subprocess.kill() can leave descendants running # and halting the execution. Using psutil to @@ -193,7 +194,8 @@ def run_command(cmd, timeout=None):
try: proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + stderr=subprocess.PIPE, + env=env) if timeout is not None: timer = Timer(timeout, kill_proc, [proc, timeout_flag]) timer.setDaemon(True) -- 1.9.1
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org <mailto:Kimchi-devel@ovirt.org> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
Hello Crístian! Sorry, I removed this commit from V2 on because it turned out to be not necessary. However, in this particular case, I was looking for a string ("running") printed in stdout by nmcli. Passing LANG=C to environment I make sure that I'll receive the string in the program's primary language, not a possible translated version. -- Jose Ricardo Ziviani ----------------------------- Software Engineer Linux Technology Center - IBM
- NetworkManager does not (fully) support bridges yet and there is no portable/bullet-proof way to solve any possible issue that may happen to libvirt bridge creation due to NM problems. - This commit adds a new capability to inform the frontend whether the NM is running or not, so the frontend could inform the user about it. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- src/kimchi/model/config.py | 1 + src/kimchi/model/featuretests.py | 28 ++++++++++++++++++++++++++++ tests/test_rest.py | 3 ++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py index 1c43360..a417c9b 100644 --- a/src/kimchi/model/config.py +++ b/src/kimchi/model/config.py @@ -140,6 +140,7 @@ class CapabilitiesModel(object): 'federation': kconfig.get("server", "federation"), 'auth': kconfig.get("authentication", "method"), 'kernel_vfio': self.kernel_vfio, + 'nm_running': FeatureTests.is_nm_running(), } diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 4aac8ed..64f2c01 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -223,3 +223,31 @@ class FeatureTests(object): kimchi_log.warning("Unable to load Kernal module vfio-pci.") return False return True + + @staticmethod + def is_nm_running(): + '''Tries to determine whether NetworkManager is running.''' + + # force the language to original + env = {'LANG': 'C'} + + nmcli = ['nmcli', + '-t', + '--fields', + 'running', + 'general', + 'status'] + + # there are different ways to know if NM is running and it + # depends on the version, so if one command fails we try the + # next one. Otherwise we assume NM is not running. + out, _, rc = run_command(nmcli, timeout=1.0, env=env) + if rc != 0: + # change option from 'general' to 'nm' + nmcli[4] = 'nm' + out, _, rc = run_command(nmcli, timeout=1.0, env=env) + + if rc == 0 and out.rstrip('\n') == 'running': + return True + + return False diff --git a/tests/test_rest.py b/tests/test_rest.py index 16ff41d..603e531 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1098,7 +1098,8 @@ class RestTests(unittest.TestCase): keys = [u'libvirt_stream_protocols', u'qemu_stream', u'qemu_spice', u'screenshot', u'system_report_tool', u'update_tool', - u'repo_mngt_tool', u'federation', u'kernel_vfio', u'auth'] + u'repo_mngt_tool', u'federation', u'kernel_vfio', u'auth', + u'nm_running'] self.assertEquals(sorted(keys), sorted(conf.keys())) def test_peers(self): -- 1.9.1
On 04/18/2015 03:29 AM, Jose Ricardo Ziviani wrote:
- NetworkManager does not (fully) support bridges yet and there is no portable/bullet-proof way to solve any possible issue that may happen to libvirt bridge creation due to NM problems.
- This commit adds a new capability to inform the frontend whether the NM is running or not, so the frontend could inform the user about it.
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- src/kimchi/model/config.py | 1 + src/kimchi/model/featuretests.py | 28 ++++++++++++++++++++++++++++ tests/test_rest.py | 3 ++- 3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py index 1c43360..a417c9b 100644 --- a/src/kimchi/model/config.py +++ b/src/kimchi/model/config.py @@ -140,6 +140,7 @@ class CapabilitiesModel(object): 'federation': kconfig.get("server", "federation"), 'auth': kconfig.get("authentication", "method"), 'kernel_vfio': self.kernel_vfio, + 'nm_running': FeatureTests.is_nm_running(), }
diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 4aac8ed..64f2c01 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -223,3 +223,31 @@ class FeatureTests(object): kimchi_log.warning("Unable to load Kernal module vfio-pci.") return False return True + + @staticmethod + def is_nm_running(): + '''Tries to determine whether NetworkManager is running.''' + + # force the language to original + env = {'LANG': 'C'} Not sure why do we do this? + + nmcli = ['nmcli', + '-t', + '--fields', + 'running', + 'general', + 'status'] + + # there are different ways to know if NM is running and it + # depends on the version, so if one command fails we try the + # next one. Otherwise we assume NM is not running. + out, _, rc = run_command(nmcli, timeout=1.0, env=env) + if rc != 0: + # change option from 'general' to 'nm' + nmcli[4] = 'nm' + out, _, rc = run_command(nmcli, timeout=1.0, env=env) I'm thinking about whether we can use a cmd which does not differ from version to version, such as "nmcli dev list" + + if rc == 0 and out.rstrip('\n') == 'running': + return True + + return False diff --git a/tests/test_rest.py b/tests/test_rest.py index 16ff41d..603e531 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1098,7 +1098,8 @@ class RestTests(unittest.TestCase):
keys = [u'libvirt_stream_protocols', u'qemu_stream', u'qemu_spice', u'screenshot', u'system_report_tool', u'update_tool', - u'repo_mngt_tool', u'federation', u'kernel_vfio', u'auth'] + u'repo_mngt_tool', u'federation', u'kernel_vfio', u'auth', + u'nm_running'] self.assertEquals(sorted(keys), sorted(conf.keys()))
def test_peers(self):
On 20-04-2015 04:22, Royce Lv wrote:
nmcli dev list
Hello Royce! Unfortunately this command is not 'cross distro' as well. It doesn't work on RHEL7.1: # nmcli device list Usage: nmcli device { COMMAND | help } ... Error: 'dev' command 'list' is not valid. 'nmcli dev status' seems to work on both Ubuntu/RHEL but I'll try on OpenSuse/Fedora as well. Thank you -- Jose Ricardo Ziviani ----------------------------- Software Engineer Linux Technology Center - IBM
On 20-04-2015 09:33, Jose Ricardo Ziviani wrote:
'nmcli dev status' seems to work on both Ubuntu/RHEL but I'll try on OpenSuse/Fedora as well.
It does work on Fedora 21: vianac@kitkat kimchi $ cat /etc/redhat-release Fedora release 21 (Twenty One) vianac@kitkat kimchi $ nmcli device status (process:9061): libnm-glib-WARNING **: Error in get_property: Method "Get" with signature "ss" on interface "org.freedesktop.DBus.Properties" doesn't exist DEVICE TYPE STATE CONNECTION docker0 bridge connected docker0 virbr0 bridge connected virbr0 virbr1 bridge connected virbr1 [...] lo loopback unmanaged --
- If NetworkManager is running when user is trying to create a bridge, it will warn about problems that could happen, asking the user to turn NM service off while dealing with bridge creation. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- ui/js/src/kimchi.network.js | 5 +++++ ui/pages/i18n.json.tmpl | 1 + 2 files changed, 6 insertions(+) diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js index 90fb62b..be140e8 100644 --- a/ui/js/src/kimchi.network.js +++ b/ui/js/src/kimchi.network.js @@ -323,6 +323,11 @@ kimchi.setDefaultNetworkType = function(isInterfaceAvail) { kimchi.enableBridgeOptions(false); $("#networkBriDisabledLabel").show(); } else { + kimchi.getCapabilities(function(result) { + if (result && result.nm_running) { + kimchi.message.warn(i18n['KCHNET6001W']); + } + }); $("#bridgeOptions").slideDown(100); $("#networkVlanID").toggle(false); $("#labelNetworkVlanID").toggle(false); diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index a7f9daf..5e7dc4f 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -153,6 +153,7 @@ "KCHNET6002M": "$_("This action will interrupt network connectivity for any virtual machine that depend on this network.")", "KCHNET6003M": "$_("Create a network")", "KCHNET6004M": "$_("This network is not persistent. Instead of stop, this action will permanently delete it. Would you like to continue?")", + "KCHNET6001W": "$_("The bridged VLAN tag may not work well with NetworkManager enabled. You can disable it or add NM_CONTROLLER=NO to the network file configuration.")", "KCHPOOL6001M": "$_("This will permanently delete the storage pool. Would you like to continue?")", "KCHPOOL6002M": "$_("This storage pool is empty.")", -- 1.9.1
On 18/04/15 05:29, Jose Ricardo Ziviani wrote:
NetworkManager might have some instability when managing the bridges created by libvirt. There some bugs against NM about it and there is no much we can do more than warn the users about it. So, this patchset checks if NM is running and, if so, will display a message to users.
++ to the concept. This did my laptop in a few weeks ago after a few iterations of the test suite creating & deleting bridges. However: "You can disable it or add NM_CONTROLLER=NO to the network file configuration." Is going to be distro specific, possibly best to just say "You should consider disabling it."
On 18-04-2015 06:06, Julien Goodwin wrote:
On 18/04/15 05:29, Jose Ricardo Ziviani wrote:
NetworkManager might have some instability when managing the bridges created by libvirt. There some bugs against NM about it and there is no much we can do more than warn the users about it. So, this patchset checks if NM is running and, if so, will display a message to users.
++ to the concept.
This did my laptop in a few weeks ago after a few iterations of the test suite creating & deleting bridges.
However: "You can disable it or add NM_CONTROLLER=NO to the network file configuration."
Is going to be distro specific, possibly best to just say "You should consider disabling it."
Thank you Julien, I'll change that sentence. -- Jose Ricardo Ziviani ----------------------------- Software Engineer Linux Technology Center - IBM
participants (5)
-
Crístian Deives -
Crístian Viana
-
Jose Ricardo Ziviani -
Julien Goodwin -
Royce Lv