[PATCHv3 0/2] nfs prevalidation

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Add nfs prevalidation to prevent mount hang for long time for unreachable export path. Royce Lv (2): utils: Add nfs prevalication Integrate nfs path check before create nfs pool src/kimchi/model.py | 37 ++++++++++++++++++++++++++++++++++--- src/kimchi/utils.py | 8 ++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) -- 1.8.1.2

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Abstract a helper function to parse cmd result. Usage: (1)get cmd result with subprocess.call or subprocess.Popen (2) call pass_cmd_output to get formated outputs Example: blkid = subprocess.Popen(["cat", "/proc/mounts"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) outs = blkid.communicate()[0] output_items= ['path', 'mnt_point', 'type', 'option'] utils.parse_cmd_output(outs, output_items) Sample output: [{'path': '/dev/sda8', 'type': 'ext4', 'option': 'rw,relatime,data=ordered', 'mnt_point': '/home'}, {'path': 'localhost:/home/royce/isorepo', 'type': 'nfs4', 'option': 'rw...addr=127.0.0.1', 'mnt_point': '/mnt'}] To prevent future mount of nfs path will hang, we will try nfs path with a quick mount, if this check fails, report warning to user. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/utils.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..781525b 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -96,3 +96,11 @@ def check_url_path(path): return False return True + + +def parse_cmd_output(output, output_items): + res = [] + for line in output.split("\n"): + res.append(dict(zip(output_items, line.split()))) + + return res -- 1.8.1.2

on 2014/01/13 18:40, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Abstract a helper function to parse cmd result. Usage: (1)get cmd result with subprocess.call or subprocess.Popen (2) call pass_cmd_output to get formated outputs Example: blkid = subprocess.Popen(["cat", "/proc/mounts"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) outs = blkid.communicate()[0] output_items= ['path', 'mnt_point', 'type', 'option'] utils.parse_cmd_output(outs, output_items) Sample output: [{'path': '/dev/sda8', 'type': 'ext4', 'option': 'rw,relatime,data=ordered', 'mnt_point': '/home'}, {'path': 'localhost:/home/royce/isorepo', 'type': 'nfs4', 'option': 'rw...addr=127.0.0.1', 'mnt_point': '/mnt'}]
To prevent future mount of nfs path will hang, we will try nfs path with a quick mount, if this check fails, report warning to user.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/utils.py | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py index af245c6..781525b 100644 --- a/src/kimchi/utils.py +++ b/src/kimchi/utils.py @@ -96,3 +96,11 @@ def check_url_path(path): return False
return True + + +def parse_cmd_output(output, output_items): + res = [] + for line in output.split("\n"): + res.append(dict(zip(output_items, line.split()))) + + return res
This function looks useful. I think it deserves a better name. May I suggest def parse_table_text(output, keys): ... ? -- Thanks and best regards! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

From: Royce Lv <lvroyce@linux.vnet.ibm.com> Before creating nfs pool, reload the prevalidation for the path and see if it is able to complish a quick mount. If not, deny this storage pool from being created. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..189b2d3 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -70,7 +70,7 @@ from kimchi.isoinfo import IsoImage from kimchi.objectstore import ObjectStore from kimchi.scan import Scanner from kimchi.screenshot import VMScreenshot -from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log +from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log, parse_cmd_output from kimchi.vmtemplate import VMTemplate @@ -1557,8 +1557,39 @@ class NetfsPoolDef(StoragePoolDef): self.path = '/var/lib/kimchi/nfs_mount/' + self.poolArgs['name'] def prepare(self, conn): - # TODO: Verify the NFS export can be actually mounted. - pass + # FIXME: When run_cmd helper function is ready + # this needs to be re-write based on context manager and helper. + res = False + outputs = None + if not os.path.isdir(self.path): + os.mkdir(self.path) + export_path = "%s:%s" % ( + self.poolArgs['source']['host'], self.poolArgs['source']['path']) + cmd = ["mount", "-o", 'soft,timeo=100,retrans=3,retry=0', + export_path, self.path] + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + thread = threading.Thread(target = proc.communicate) + thread.start() + thread.join(30) + + if thread.is_alive(): + proc.kill() + thread.join() + + with open("/proc/mounts" , "rb") as f: + outputs = f.read() + output_items = ['dev_path', 'mnt_point', 'type'] + mounts = parse_cmd_output(outputs, output_items) + for item in mounts: + if 'dev_path' in item and item['dev_path'] == export_path: + res = True + cmd = ["umount", "-f", export_path] + subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + if not res: + raise InvalidParameter( + "Export path %s may block during nfs mount" % export_path) @property def xml(self): -- 1.8.1.2

on 2014/01/13 18:40, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Before creating nfs pool, reload the prevalidation for the path and see if it is able to complish a quick mount. If not, deny this storage pool from being created.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..189b2d3 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -70,7 +70,7 @@ from kimchi.isoinfo import IsoImage from kimchi.objectstore import ObjectStore from kimchi.scan import Scanner from kimchi.screenshot import VMScreenshot -from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log +from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log, parse_cmd_output from kimchi.vmtemplate import VMTemplate
@@ -1557,8 +1557,39 @@ class NetfsPoolDef(StoragePoolDef): self.path = '/var/lib/kimchi/nfs_mount/' + self.poolArgs['name']
def prepare(self, conn): - # TODO: Verify the NFS export can be actually mounted. - pass + # FIXME: When run_cmd helper function is ready + # this needs to be re-write based on context manager and helper. + res = False + outputs = None
I would suggest you rename 'res' to 'mountOK' or 'mounted', and move "res = False" to near where you use it, just above "for item in mounts:". I also suggest you rename 'outputs' to 'rawMounts' and move "outputs = None" to just above "with open(...)". After all, I think you can just delete this "outputs = None" because as long as it succeeds reading "/proc/mounts", "outputs" get a value. When it fails to open and read "/proc/mounts", it raises an exception and the code afterwards playing with outputs are not executed. So a default value for 'outputs' is not necessary. If you want to define a default value for "outputs" anyway, it can be an empty string but not "None". The reason is that when "outputs" is "None", "parse_cmd_output" raises exception "AttributeError" because "None" has no "split" method, on the other hand, when it's an empty string, "parse_cmd_output" does nothing and the latter code raises "InvalidParameter". I think in this case an "InvalidParameter" is more meaningful than "AttributeError".
+ if not os.path.isdir(self.path): + os.mkdir(self.path) + export_path = "%s:%s" % ( + self.poolArgs['source']['host'], self.poolArgs['source']['path']) + cmd = ["mount", "-o", 'soft,timeo=100,retrans=3,retry=0', + export_path, self.path] + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + thread = threading.Thread(target = proc.communicate) + thread.start() + thread.join(30) + + if thread.is_alive(): + proc.kill() + thread.join() + + with open("/proc/mounts" , "rb") as f: + outputs = f.read() + output_items = ['dev_path', 'mnt_point', 'type'] + mounts = parse_cmd_output(outputs, output_items) + for item in mounts: + if 'dev_path' in item and item['dev_path'] == export_path: + res = True + cmd = ["umount", "-f", export_path] + subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + if not res: + raise InvalidParameter( + "Export path %s may block during nfs mount" % export_path)
@property def xml(self):
-- Thanks and best regards! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

On 2014年01月14日 13:21, Zhou Zheng Sheng wrote:
on 2014/01/13 18:40, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Before creating nfs pool, reload the prevalidation for the path and see if it is able to complish a quick mount. If not, deny this storage pool from being created.
Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model.py | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 671af02..189b2d3 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -70,7 +70,7 @@ from kimchi.isoinfo import IsoImage from kimchi.objectstore import ObjectStore from kimchi.scan import Scanner from kimchi.screenshot import VMScreenshot -from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log +from kimchi.utils import get_enabled_plugins, is_digit, kimchi_log, parse_cmd_output from kimchi.vmtemplate import VMTemplate
@@ -1557,8 +1557,39 @@ class NetfsPoolDef(StoragePoolDef): self.path = '/var/lib/kimchi/nfs_mount/' + self.poolArgs['name']
def prepare(self, conn): - # TODO: Verify the NFS export can be actually mounted. - pass + # FIXME: When run_cmd helper function is ready + # this needs to be re-write based on context manager and helper. + res = False + outputs = None I would suggest you rename 'res' to 'mountOK' or 'mounted', and move "res = False" to near where you use it, just above "for item in mounts:". ACK
I also suggest you rename 'outputs' to 'rawMounts' and move "outputs = None" to just above "with open(...)". After all, I think you can just delete this "outputs = None" because as long as it succeeds reading "/proc/mounts", "outputs" get a value. When it fails to open and read "/proc/mounts", it raises an exception and the code afterwards playing with outputs are not executed. So a default value for 'outputs' is not necessary. ACK
If you want to define a default value for "outputs" anyway, it can be an empty string but not "None". The reason is that when "outputs" is "None", "parse_cmd_output" raises exception "AttributeError" because "None" has no "split" method, on the other hand, when it's an empty string, "parse_cmd_output" does nothing and the latter code raises "InvalidParameter". I think in this case an "InvalidParameter" is more meaningful than "AttributeError".
+ if not os.path.isdir(self.path): + os.mkdir(self.path) + export_path = "%s:%s" % ( + self.poolArgs['source']['host'], self.poolArgs['source']['path']) + cmd = ["mount", "-o", 'soft,timeo=100,retrans=3,retry=0', + export_path, self.path] + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + thread = threading.Thread(target = proc.communicate) + thread.start() + thread.join(30) + + if thread.is_alive(): + proc.kill() + thread.join() + + with open("/proc/mounts" , "rb") as f: + outputs = f.read() + output_items = ['dev_path', 'mnt_point', 'type'] + mounts = parse_cmd_output(outputs, output_items) + for item in mounts: + if 'dev_path' in item and item['dev_path'] == export_path: + res = True + cmd = ["umount", "-f", export_path] + subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + if not res: + raise InvalidParameter( + "Export path %s may block during nfs mount" % export_path)
@property def xml(self):
participants (3)
-
lvroyce@linux.vnet.ibm.com
-
Royce Lv
-
Zhou Zheng Sheng