[node-patches] Change in ovirt-node[master]: add return value when persist command does not return an err...

liyuran9522 at gmail.com liyuran9522 at gmail.com
Fri Dec 6 02:20:30 UTC 2013


Hello Changming Bai,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/22117

to review the following change.

Change subject: add return value when persist command does not return an error code on failure
......................................................................

add return value when persist command does not return an error code on failure

move the ovirt_store_config code into a new function "ovirt_store_config_retnum"
Have it return different error number consist with the different error case
The "ovirt_store_config" function will invoke the new function "ovirt_store_config_retnum"
The ovirt_store_config will judge the error number from the ovirt_store_config_retnum
and ovirt_store_config will also return True/False value from the judgement error number
In the shell script ovirt-functions.in, invoke the ovirt_store_config_retnum directly

Change-Id: I014c41c9919cc9f8c260d85e0ef1a23ad17021a2
Signed-off-by: Changming Bai <baichm at linux.vnet.ibm.com>
Signed-off-by: lizhihui <zhihuili at linux.vnet.ibm.com>
---
M scripts/ovirt-functions.in
M scripts/ovirt-init-functions.sh.in
A src/ovirtnode/exceptions.py
M src/ovirtnode/ovirtfunctions.py
4 files changed, 101 insertions(+), 67 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/17/22117/1

diff --git a/scripts/ovirt-functions.in b/scripts/ovirt-functions.in
index 5dbc8f4..7f1e322 100644
--- a/scripts/ovirt-functions.in
+++ b/scripts/ovirt-functions.in
@@ -647,73 +647,22 @@
 #   ovirt_store_config /etc/config /etc/config2 ...
 #   copy to /config and bind-mount back
 ovirt_store_config() {
-    rc=0
-    if is_stateless; then
-        # no need to do anything
-        return 0;
+    for p in "$@"; do
+        python <<EOP
+import sys
+from ovirtnode.ovirtfunctions import ovirt_store_config_retnum
+from ovirtnode.exceptions import ExceptionWithValue
+try:
+    ovirt_store_config_retnum("$p")
+    sys.exit(0)
+except ExceptionWithValue as e:
+    sys.exit(e.value)
+EOP
+    rc=$?
+    if [ $rc -eq 1 ] || [ $rc -eq 2 ]; then
+        return $rc
     fi
-    if grep -q " /config ext[234]" /proc/mounts; then
-        for p in "$@"; do
-            local filename=$(readlink -f $p)
-            local persist_it=true
-            if [[ "$filename" =~ ^/[^/]*$ ]]; then
-                # persisting top-level folder makes trouble rhbz#611617
-                printf "Cannot persist system folder: ${filename}\n"
-                persist_it=false
-            elif [ -d $filename ]; then
-                if [ -d /config$filename ]; then
-                    printf "Directory already persisted: ${filename}\n"
-                    printf "You need to unpersist its child directories and/or files and try again.\n"
-                    persist_it=false
-                fi
-            elif [ -f $filename ]; then
-                if [ -f /config$filename ]; then
-                    local md5root=$(md5 $filename)
-                    local md5stored=$(md5 /config$filename)
-                    if [ "$md5root" = "$md5stored" ]; then
-                        printf "File already persisted: ${filename}\n"
-                        persist_it=false
-                    else
-                        # persistent copy needs refresh
-                        umount -n $filename 2> /dev/null || :
-                        rm -f /config$filename
-                    fi
-                fi
-            else
-                printf "Cannot persist: ${filename}\n"
-                persist_it=false
-            fi
-
-            if $persist_it; then
-                # skip if file does not exist
-                if [ ! -e "${filename}" ]; then
-                    printf " Skipping, file '${filename}' does not exist\n"
-                    continue
-                fi
-                # skip if already bind-mounted
-                if ! grep -q " $filename ext[234]" /proc/mounts ; then
-                    mkdir -p /config$(dirname $filename)
-                    cp -a $filename /config$filename \
-                        && mount -n --bind /config$filename $filename
-                    if [ $? -ne 0 ]; then
-                        printf " Failed to persist\n"
-                        rc=1
-                    else
-                        printf " File persisted\n"
-                    fi
-                fi
-                # register in /config/files used by rc.sysinit
-                if ! grep -q "^${filename}$" /config/files 2> /dev/null ; then
-                    printf "${filename}\n" >> /config/files
-                fi
-                printf "\nSuccessfully persisted ${filename}\n"
-            fi
-        done
-        echo
-    else
-        printf "WARNING: persistent config storage not available\n"
-        rc=2
-    fi
+    done
     return $rc
 }
 
diff --git a/scripts/ovirt-init-functions.sh.in b/scripts/ovirt-init-functions.sh.in
index d9218c7..42e217c 100644
--- a/scripts/ovirt-init-functions.sh.in
+++ b/scripts/ovirt-init-functions.sh.in
@@ -1184,7 +1184,11 @@
             autoinstall_failed
         fi
         disable_firstboot
-        ovirt_store_firstboot_config || autoinstall_failed
+        ovirt_store_firstboot_config
+        retval=$?
+        if [ $retval -eq 1 ] || [ $retval -eq 2 ]; then
+            autoinstall_failed
+        fi
 
         # rhbz#920208
         # the sysd-upd-utmp-shutdown.service (see below) is pulling in the var-log.mount unit.
diff --git a/src/ovirtnode/exceptions.py b/src/ovirtnode/exceptions.py
new file mode 100644
index 0000000..016ffc4
--- /dev/null
+++ b/src/ovirtnode/exceptions.py
@@ -0,0 +1,26 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA  02110-1301, USA.  A copy of the GNU General Public License is
+# also available at http://www.gnu.org/copyleft/gpl.html.
+
+"""
+oVirtnode specific exceptions
+"""
+
+class ExceptionWithValue(Exception):
+    def __init__(self, value):
+        self.value = value
+
diff --git a/src/ovirtnode/ovirtfunctions.py b/src/ovirtnode/ovirtfunctions.py
index ace96b5..3ce9cc5 100644
--- a/src/ovirtnode/ovirtfunctions.py
+++ b/src/ovirtnode/ovirtfunctions.py
@@ -38,6 +38,8 @@
 import grp
 import pwd
 from ovirt.node.config import defaults
+from ovirt.node.utils import process,system
+from ovirtnode.exceptions import *
 from ovirt.node.utils import process
 import ovirt.node.utils.system as osystem
 from ovirt.node.utils.console import TransactionProgress, Transaction
@@ -851,6 +853,59 @@
             rc = rc and True
     return rc
 
+def ovirt_store_config_retnum(filename):
+    if is_stateless():
+        return True
+    if not os.path.ismount("/config"):
+        logger.error("/config is not mounted")
+        raise ExceptionWithValue(2)
+    filename = os.path.abspath(filename)
+
+    if os.path.isdir(filename):
+        # ensure that, if this is a directory
+        # that it's not already persisted
+        if os.path.isdir("/config/" + filename):
+            logger.warn("Directory already persisted: " + filename)
+            logger.warn("You need to unpersist its child directories " +
+                        "and/or files and try again.")
+            raise ExceptionWithValue(3)
+    elif os.path.isfile(filename):
+        # if it's a file then make sure it's not already persisted
+        if os.path.isfile("/config/" + filename):
+            cksumroot=cksum(filename)
+            cksumstored=cksum("/config" + filename)
+            if cksumroot == cksumstored:
+                logger.warn("File already persisted: " + filename)
+                raise ExceptionWithValue(4)
+            else:
+                # persistent copy needs refresh
+                if system("umount -n " + filename + " 2> /dev/null"):
+                    system("rm -f /config"+ filename)
+    else:
+        # skip if file does not exist
+        logger.warn("Skipping, file '" + filename + "' does not exist")
+        raise ExceptionWithValue(5)
+
+    # skip if already bind-mounted
+    if not check_bind_mount(filename):
+        dirname = os.path.dirname(filename)
+        system("mkdir -p /config/" + dirname)
+        if system("cp -a " + filename + " /config"+filename):
+            if not system("mount -n --bind /config"+filename+ " " + \
+                          filename):
+                logger.error("Failed to persist: " + filename)
+                raise ExceptionWithValue(1)
+            else:
+                logger.info("File: " + filename + " persisted")
+    # register in /config/files used by rc.sysinit
+    ret = system_closefds("grep -q \"^$" + filename +"$\" " + \
+                          " /config/files 2> /dev/null")
+    if ret > 0:
+        system_closefds("echo "+filename+" >> /config/files")
+        logger.info("Successfully persisted: " + filename)
+
+    return True
+
 def ovirt_store_config_atomic(filename, source=None):
     rc = True
     persist_it = True


-- 
To view, visit http://gerrit.ovirt.org/22117
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I014c41c9919cc9f8c260d85e0ef1a23ad17021a2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: zhihui li <liyuran9522 at gmail.com>
Gerrit-Reviewer: Changming Bai <baichm at linux.vnet.ibm.com>



More information about the node-patches mailing list