Douglas Schilling Landgraf has uploaded a new change for review.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
cleanup: remove persist/unpersist calls for legacy node
This patch removes all related to unpersist and persist calls exclusive for oVirt Node Legacy. oVirt Node Next doesn't require such approach.
Change-Id: I1cca0cfa0e2254a5a8af6b3591ffe47689410415 Signed-off-by: Douglas Schilling Landgraf dougsland@redhat.com --- M lib/vdsm/network/configurators/ifcfg.py M lib/vdsm/tool/configfile.py M lib/vdsm/tool/configurators/certificates.py M lib/vdsm/tool/configurators/libvirt.py M lib/vdsm/tool/configurators/multipath.py M lib/vdsm/tool/configurators/passwd.py M lib/vdsm/tool/register.py M lib/vdsm/tool/upgrade.py M lib/vdsm/tool/validate_ovirt_certs.py.in M lib/vdsm/utils.py M tests/toolTests.py M vdsm.spec.in M vdsm/Makefile.am D vdsm/ovirt_functions.sh M vdsm/vdsm-store-net-config.in M vdsm_hooks/Makefile.am M vdsm_hooks/fcoe/fcoe_before_network_setup.py D vdsm_hooks/persist-vdsm-hooks.in D vdsm_hooks/unpersist-vdsm-hook 19 files changed, 9 insertions(+), 219 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/63715/1
diff --git a/lib/vdsm/network/configurators/ifcfg.py b/lib/vdsm/network/configurators/ifcfg.py index 2302ae1..0555034 100644 --- a/lib/vdsm/network/configurators/ifcfg.py +++ b/lib/vdsm/network/configurators/ifcfg.py @@ -55,9 +55,6 @@ from vdsm.network.netinfo.cache import ifaceUsed from vdsm.network.netlink import waitfor
-if utils.isOvirtNode(): - from ovirt.node.utils import fs as node_fs - from . import Configurator, getEthtoolOpts from ..errors import ConfigNetworkError, ERR_FAILED_IFUP from ..models import Nic, Bridge @@ -297,10 +294,7 @@ @staticmethod def _removeFile(filename): """Remove file (directly or using oVirt node's library)""" - if utils.isOvirtNode(): - node_fs.Config().delete(filename) # unpersists and shreds the file - else: - utils.rmFile(filename) + utils.rmFile(filename) logging.debug("Removed file %s", filename)
def createLibvirtNetwork(self, network, bridged=True, iface=None, @@ -512,10 +506,6 @@ except: logging.debug('ignoring restorecon error in case ' 'SElinux is disabled', exc_info=True) - - # make sure that ifcfg files are always persisted by the node - if self.unifiedPersistence and utils.isOvirtNode(): - node_fs.Config().persist(fileName)
def _createConfFile(self, conf, name, ipv4, ipv6, mtu, nameservers, **kwargs): diff --git a/lib/vdsm/tool/configfile.py b/lib/vdsm/tool/configfile.py index 0c49b19..bad0759 100644 --- a/lib/vdsm/tool/configfile.py +++ b/lib/vdsm/tool/configfile.py @@ -175,9 +175,7 @@ if self._entries: self._writeEntries(f, oldentries)
- utils.unpersist(self._filename) os.rename(tname, self._filename) - utils.persist(self._filename)
if self._oldmod != os.stat(self._filename).st_mode: os.chmod(self._filename, self._oldmod) diff --git a/lib/vdsm/tool/configurators/certificates.py b/lib/vdsm/tool/configurators/certificates.py index 0462b4d..edc96b3 100644 --- a/lib/vdsm/tool/configurators/certificates.py +++ b/lib/vdsm/tool/configurators/certificates.py @@ -23,10 +23,8 @@ from vdsm.config import config
from . import YES, NO -from vdsm.tool.validate_ovirt_certs import validate_ovirt_certs from vdsm.constants import P_VDSM_EXEC, SYSCONF_PATH from vdsm.commands import execCmd -from vdsm.utils import isOvirtNode
PKI_DIR = os.path.join(SYSCONF_PATH, 'pki/vdsm') CA_FILE = os.path.join(PKI_DIR, 'certs/cacert.pem') @@ -59,8 +57,6 @@
def configure(): _exec_vdsm_gencerts() - if isOvirtNode(): - validate_ovirt_certs()
def isconfigured(): diff --git a/lib/vdsm/tool/configurators/libvirt.py b/lib/vdsm/tool/configurators/libvirt.py index 40d4958..94c5da2 100644 --- a/lib/vdsm/tool/configurators/libvirt.py +++ b/lib/vdsm/tool/configurators/libvirt.py @@ -27,12 +27,8 @@ from . import InvalidRun, NO, MAYBE from . certificates import CA_FILE, CERT_FILE, KEY_FILE from vdsm.tool.configfile import ConfigFile, ParserWrapper -from vdsm.tool.validate_ovirt_certs import validate_ovirt_certs from vdsm import utils from vdsm import constants - -if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config as NodeCfg
requires = frozenset(('certificates',)) @@ -45,12 +41,6 @@
def configure(): - if utils.isOvirtNode(): - if not os.path.exists(constants.P_VDSM_CERT): - raise InvalidRun( - "vdsm: Missing certificate, vdsm not registered") - validate_ovirt_certs() - # Remove a previous configuration (if present) removeConf()
@@ -193,14 +183,11 @@ """ delete a file if it exists. """ - if utils.isOvirtNode(): - NodeCfg().delete(content['path']) - else: - try: - os.unlink(content['path']) - except OSError as e: - if e.errno != errno.ENOENT: - raise + try: + os.unlink(content['path']) + except OSError as e: + if e.errno != errno.ENOENT: + raise
def _removeSection(path): diff --git a/lib/vdsm/tool/configurators/multipath.py b/lib/vdsm/tool/configurators/multipath.py index 00ec558..a402cda 100644 --- a/lib/vdsm/tool/configurators/multipath.py +++ b/lib/vdsm/tool/configurators/multipath.py @@ -112,7 +112,6 @@ backup = _CONF_FILE + '.' + time.strftime("%Y%m%d%H%M") shutil.copyfile(_CONF_FILE, backup) sys.stdout.write("Backup previous multipath.conf to %r\n" % backup) - utils.persist(backup)
with tempfile.NamedTemporaryFile( mode="wb", @@ -125,15 +124,10 @@ if selinux.is_selinux_enabled(): selinux.restorecon(f.name) os.chmod(f.name, 0o644) - # On ovirt node multipath.conf is a bind mount and rename will fail - # if we do not unpersist first, making this non-atomic. - utils.unpersist(_CONF_FILE) os.rename(f.name, _CONF_FILE) except: os.unlink(f.name) raise - - utils.persist(_CONF_FILE)
# Flush all unused multipath device maps commands.execCmd([constants.EXT_MULTIPATH, "-F"]) diff --git a/lib/vdsm/tool/configurators/passwd.py b/lib/vdsm/tool/configurators/passwd.py index 62830df..b75228e 100644 --- a/lib/vdsm/tool/configurators/passwd.py +++ b/lib/vdsm/tool/configurators/passwd.py @@ -51,10 +51,6 @@ rc, _, err = commands.execCmd(script, data=libvirt_password()) if rc != 0: raise RuntimeError("Set password failed: %s" % (err,)) - if utils.isOvirtNode(): - # It seems that all /etc/libvirt folder is persisted in node, - # but better to persist the db file explicitly - utils.persist(_LIBVIRT_SASLDB)
def removeConf(): diff --git a/lib/vdsm/tool/register.py b/lib/vdsm/tool/register.py index d0ae207..ea74bec 100644 --- a/lib/vdsm/tool/register.py +++ b/lib/vdsm/tool/register.py @@ -266,18 +266,11 @@ __VDSM_ID = "/etc/vdsm/vdsm.id"
if self.vdsm_uuid and os.path.exists(__VDSM_ID): - if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config - Config().unpersist(__VDSM_ID) os.unlink(__VDSM_ID)
if not os.path.exists(__VDSM_ID): with open(__VDSM_ID, 'w') as f: f.write(self.uuid) - - if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config - Config().persist(__VDSM_ID)
self.logger.info("Host UUID: {u}".format(u=self.uuid))
@@ -294,9 +287,6 @@ if not os.path.exists(self.ca_dir): os.makedirs(self.ca_dir, 0o755) self._silent_restorecon(self.ca_dir) - if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config - Config().persist(self.ca_dir)
res = self._execute_http_request(self.url_CA, cert_validation=False) @@ -328,10 +318,6 @@ self.logger.info("Calculated fingerprint: {f}".format( f=self.fprint))
- if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config - Config().persist(self.ca_engine) - def download_ssh(self): """ Download ssh authorized keys and save it in the node @@ -345,9 +331,6 @@ if not os.path.exists(_auth_keys_dir): os.makedirs(_auth_keys_dir, 0o700) self._silent_restorecon(_auth_keys_dir) - if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config - Config().persist(_auth_keys_dir) os.chown(_auth_keys_dir, _uid, _uid)
res = self._execute_http_request(self.url_ssh_key) @@ -368,9 +351,6 @@ os.chown(_auth_keys, _uid, _uid)
os.unlink(f.name) - if utils.isOvirtNode(): - from ovirt.node.utils.fs import Config - Config().persist(_auth_keys)
def execute_registration(self): """ diff --git a/lib/vdsm/tool/upgrade.py b/lib/vdsm/tool/upgrade.py index 4bc904c..291ef13 100644 --- a/lib/vdsm/tool/upgrade.py +++ b/lib/vdsm/tool/upgrade.py @@ -46,7 +46,6 @@ except (OSError, IOError): _get_upgrade_log().exception("Failed to seal upgrade %s", upgrade.name) else: - utils.persist(seal_file) _get_upgrade_log().debug("Upgrade %s successfully performed", upgrade.name)
diff --git a/lib/vdsm/tool/validate_ovirt_certs.py.in b/lib/vdsm/tool/validate_ovirt_certs.py.in index 53726d0..1e6a370 100644 --- a/lib/vdsm/tool/validate_ovirt_certs.py.in +++ b/lib/vdsm/tool/validate_ovirt_certs.py.in @@ -25,11 +25,6 @@ from . import expose, ExtraArgsError from ..commands import execCmd
-try: - from ovirtnode import ovirtfunctions -except ImportError: - pass -
EX_OPENSSL = '@OPENSSL_PATH@'
@@ -60,8 +55,6 @@ if not is_our_cafile(PATH_CACERT): for f in os.listdir(PATH_CERT): if is_our_cafile(PATH_CERT + f): - ovirtfunctions.ovirt_safe_delete_config(PATH_CACERT) shutil.copy(PATH_CERT + f, PATH_CACERT) os.chown(PATH_CACERT, uid, gid) - ovirtfunctions.ovirt_store_config(PATH_CACERT) break diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 0ff1a3c..5401654 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -597,12 +597,6 @@ zombiereaper.autoReapPID(self._proc.pid)
-@memoized -def isOvirtNode(): - return (os.path.exists('/etc/rhev-hypervisor-release') or - bool(glob.glob('/etc/ovirt-node-*-release'))) - - def anyFnmatch(name, patterns): """Returns True if any element in the patterns iterable fnmatches name.""" return any(fnmatch(name, pattern) for pattern in patterns) diff --git a/tests/toolTests.py b/tests/toolTests.py index e14bb53..ab741e2 100644 --- a/tests/toolTests.py +++ b/tests/toolTests.py @@ -334,11 +334,6 @@ libvirt, 'FILES', FakeFiles - ), - ( - utils, - 'isOvirtNode', - lambda: False ) ])
diff --git a/vdsm.spec.in b/vdsm.spec.in index b9ec875..4d5caf1 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -957,9 +957,6 @@ %{_sysconfdir}/libvirt/hooks/qemu %{_libexecdir}/%{vdsm_name}/curl-img-wrap %{_libexecdir}/%{vdsm_name}/fc-scan -%{_libexecdir}/%{vdsm_name}/persist-vdsm-hooks -%{_libexecdir}/%{vdsm_name}/unpersist-vdsm-hook -%{_libexecdir}/%{vdsm_name}/ovirt_functions.sh %{_libexecdir}/%{vdsm_name}/vdsm-gencerts.sh %{_libexecdir}/%{vdsm_name}/vdsmd_init_common.sh %{_libexecdir}/%{vdsm_name}/vm_migrate_hook.py* diff --git a/vdsm/Makefile.am b/vdsm/Makefile.am index 8b535b6..fd38601 100644 --- a/vdsm/Makefile.am +++ b/vdsm/Makefile.am @@ -32,9 +32,6 @@ sitecustomize.py \ $(NULL)
-dist_vdsmexec_SCRIPTS = \ - ovirt_functions.sh - nodist_vdsmexec_SCRIPTS = \ vdsm-gencerts.sh \ $(NULL) diff --git a/vdsm/ovirt_functions.sh b/vdsm/ovirt_functions.sh deleted file mode 100644 index 12f6864..0000000 --- a/vdsm/ovirt_functions.sh +++ /dev/null @@ -1,21 +0,0 @@ -#!/bin/sh -# -# Copyright 2006-2010 Red Hat, Inc. and/or its affiliates. -# -# Licensed to you under the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. See the files README and -# LICENSE_GPL_v2 which accompany this distribution. -# - -isOvirtNode() { - [ "$(echo /etc/ovirt-node-*-release)" != "/etc/ovirt-node-*-release" ] || \ - [ -f /etc/rhev-hypervisor-release ] -} - -# execute a function if called as a script, e.g. -# vdsm-bash-functions isOvirtNode - -if [ "$(basename -- "$0")" = "vdsm-bash-functions" ]; then - "$@" -fi diff --git a/vdsm/vdsm-store-net-config.in b/vdsm/vdsm-store-net-config.in index 76822ee..3292f84 100755 --- a/vdsm/vdsm-store-net-config.in +++ b/vdsm/vdsm-store-net-config.in @@ -16,33 +16,10 @@
PERSISTENCE=$1
-ifcfg_node_persist() { - for f in "$NET_CONF_BACK_DIR"/*; - do - [ ! -f "$f" ] && continue - bf=`basename "$f"` - if [ -f "$NET_CONF_DIR/$bf" ]; - then - ovirt_store_config "$NET_CONF_DIR/$bf" - else - ovirt_safe_delete_config "$NET_CONF_DIR/$bf" - fi - rm "$NET_CONF_BACK_DIR/$bf" - done -} - ifcfg_nonnode_persist() { # Remove the backed up configuration files thus marking the ones under # /etc/sysconfig as "safe". rm -rf "$NET_CONF_BACK_DIR"/* -} - -unified_node_persist() { - unified_nonnode_persist - - # oVirt node ovirt_store_config puts the dir in persistent storage and - # bind mounts it in the original place. So that's all we really need to do. - ovirt_store_config "$PERS_CONF_PATH" }
unified_nonnode_persist() { @@ -67,21 +44,8 @@ }
-if isOvirtNode -then - # for node, persist the changed configuration files - - . /usr/libexec/ovirt-functions - - if [ "$PERSISTENCE" == "unified" ]; then - unified_node_persist - else - ifcfg_node_persist - fi +if [ "$PERSISTENCE" == "unified" ]; then + unified_nonnode_persist else - if [ "$PERSISTENCE" == "unified" ]; then - unified_nonnode_persist - else - ifcfg_nonnode_persist - fi + ifcfg_nonnode_persist fi diff --git a/vdsm_hooks/Makefile.am b/vdsm_hooks/Makefile.am index 8cbfca1..04728ad 100644 --- a/vdsm_hooks/Makefile.am +++ b/vdsm_hooks/Makefile.am @@ -64,18 +64,9 @@ $(NULL) endif
-dist_vdsmexec_SCRIPTS = \ - unpersist-vdsm-hook - -nodist_vdsmexec_SCRIPTS = \ - persist-vdsm-hooks - CLEANFILES = \ config.log \ $(nodist_vdsmexec_SCRIPTS) - -EXTRA_DIST = \ - persist-vdsm-hooks.in
VDSMHOOKS = \ before_device_create \ diff --git a/vdsm_hooks/fcoe/fcoe_before_network_setup.py b/vdsm_hooks/fcoe/fcoe_before_network_setup.py index c507c07..e786254 100755 --- a/vdsm_hooks/fcoe/fcoe_before_network_setup.py +++ b/vdsm_hooks/fcoe/fcoe_before_network_setup.py @@ -86,7 +86,6 @@ outfile.write(CONFFILE_HEADER + "\n") for name, value in six.iteritems(config): outfile.write('%s="%s"\n' % (name, value)) - utils.persist(filename)
def _unconfigure(interface): @@ -95,7 +94,6 @@ """ filename = _get_config_name(interface) if os.path.exists(filename): - utils.unpersist(filename) utils.rmFile(filename)
diff --git a/vdsm_hooks/persist-vdsm-hooks.in b/vdsm_hooks/persist-vdsm-hooks.in deleted file mode 100644 index e7423fb..0000000 --- a/vdsm_hooks/persist-vdsm-hooks.in +++ /dev/null @@ -1,26 +0,0 @@ -#!/bin/bash -# -# Copyright 2009-2010 Red Hat, Inc. All rights reserved. -# Use is subject to license terms. -# -# Description: Persist all VDSM hooks -# - -. @LIBEXECDIR@/ovirt_functions.sh - -HOOKS_DIR=/usr/libexec/vdsm/hooks - -if isOvirtNode -then - . /usr/libexec/ovirt-functions - for hook in "$HOOKS_DIR"/*/* - do - if [[ -f "$hook" ]] - then - ovirt_store_config "$hook" - fi - done -else - echo "Host is not RHEV-Hypervisor" - exit 1 -fi diff --git a/vdsm_hooks/unpersist-vdsm-hook b/vdsm_hooks/unpersist-vdsm-hook deleted file mode 100755 index af5af27..0000000 --- a/vdsm_hooks/unpersist-vdsm-hook +++ /dev/null @@ -1,32 +0,0 @@ -#!/bin/bash -# -# Copyright 2009-2010 Red Hat, Inc. All rights reserved. -# Use is subject to license terms. -# -# Description: Unpersist VDSM Hook -# Input: VDSM Hook file full path -# - -HOOKS_DIR=/usr/libexec/vdsm/hooks -HOOK_TO_UNPERSIST="$1" - -if [[ -z "$HOOK_TO_UNPERSIST" ]]; then - echo "usage: $0 hook_file" - exit 1 -fi - -if [[ -f /etc/rhev-hypervisor-release ]] -then - . /usr/libexec/ovirt-functions - if [[ "$HOOK_TO_UNPERSIST" =~ ^"$HOOKS_DIR" ]] - then - # try to unpersist files only - if [[ -f "$HOOK_TO_UNPERSIST" ]] - then - ovirt_safe_delete_config "$HOOK_TO_UNPERSIST" - fi - fi -else - echo "Host is not RHEV-Hypervisor" - exit 1 -fi
gerrit-hooks has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Douglas Schilling Landgraf has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 1: Verified+1
make rpm works in branch master, clicking verified.
gerrit-hooks has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 2:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
gerrit-hooks has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 3:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
gerrit-hooks has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4:
Douglass, this his very big and risky patch. "make rpm works, clicking verified" is not a valid way to verify it.
I suggest to separate to smaller patches that are easier to verify by the different teams.
Also, in lot of places, we could not write correct code using atomic operations because of the persist/unpersist requirement. Can we use now atomic write (write temp file, rename) on node?
Nir Soffer has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4: Code-Review-1
Dan Kenigsberg has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4: Code-Review-1
Thanks for this much needed cleanup.
At the very least, you can split network-related changes to separate patches. myakove is usually happy to give a tier2 run to a 4.0 backport of such a patch.
I don't think that you should fix atomicity in this cleanup patchset. At most, a reviewer can ask you to add a FIXME where such a fix is required.
Nir Soffer has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4:
I agree with Dan, we don't expect to improve the current code, just remove the usage of persist/unpersist, and leave a comment where we need to improve the old code.
Douglas Schilling Landgraf has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4:
Sure, no problem Nir. I can split the patch and try to test all of it. I was kind of less worried since it's master branch and doesn't look a hurt patch.
From Dan Kenigsberg danken@redhat.com:
Dan Kenigsberg has posted comments on this change.
Change subject: cleanup: remove persist/unpersist calls for legacy node ......................................................................
Patch Set 4:
could you find the time to split this up, Douglas?
vdsm-patches@lists.fedorahosted.org