[project-kimchi][PATCH 0/3] NFS prevalidation

[lvroy...@gmail.com]

From: Royce Lv <lvr...@linux.vnet.ibm.com>

This patchset solves following problem:
1. libvirt uses local directory when nfs target path is not accessible,
this patch denies this unaccessible path instead.
2. libvirt storage subsystem hang for a long period, with a quick checking,
storage pool operation will not hang if pre-checked.
3. Future access control check will do in prevalidation,
this prevents qemu manipulate storage pool of no read access to make early warning.
Royce Lv (3):
utils: Add a helper function to parse cmd result
NFS prevalidation: try nfs path mount in feature test
Integrate nfs path check before create nfs pool

src/kimchi/featuretests.py | 32 ++++++++++++++++++++++++++++++++
src/kimchi/model.py | 5 +++++
src/kimchi/utils.py | 7 +++++++
3 files changed, 44 insertions(+)

--
1.8.1.2

[lvroy...@gmail.com]

From: Royce Lv <lvr...@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.pass_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'}]

Signed-off-by: Royce Lv <lvr...@linux.vnet.ibm.com>
---
src/kimchi/utils.py | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index f7eda93..1890a28 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -84,3 +84,10 @@ def import_class(class_path):

def import_module(module_name):
return __import__(module_name, fromlist=[''])
+
+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

[lvroy...@gmail.com]

From: Royce Lv <lvr...@linux.vnet.ibm.com>

To prevent future mount of nfs path will hang,
we will try nfs path with a quick mount,
if this test fails, report warning to user.

Signed-off-by: Royce Lv <lvr...@linux.vnet.ibm.com>
---

src/kimchi/featuretests.py | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/src/kimchi/featuretests.py b/src/kimchi/featuretests.py
index 4cabdc9..36036e5 100644
--- a/src/kimchi/featuretests.py
+++ b/src/kimchi/featuretests.py
@@ -23,10 +23,12 @@
import libvirt
import os
import subprocess
+import tempfile
import threading


from kimchi import config
+from kimchi.utils import parse_cmd_output


ISO_STREAM_XML = """
@@ -111,3 +113,33 @@ class FeatureTests(object):
return False

return True
+
+ @staticmethod
+ def check_nfs_export(export_path):
+ res = False
+ outputs = None
+ mnt_point = tempfile.mkdtemp(dir='/tmp')
+ cmd = ["mount", "-o", 'soft,timeo=30,retrans=3,retry=0',
+ export_path, mnt_point]
+ proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE)
+ thread = threading.Thread(target = proc.communicate)
+ thread.start()
+ thread.join(9)
+
+ 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)

[lvroy...@gmail.com]

From: Royce Lv <lvr...@linux.vnet.ibm.com>

Before creating nfs pool, add a 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 <lvr...@linux.vnet.ibm.com>
---

src/kimchi/model.py | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index 2ecbc9c..a761488 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -991,6 +991,11 @@ class Model(object):

if params['type'] == 'kimchi-iso':
task_id = self._do_deep_scan(params)
+ if params['type'] == 'netfs':
+ export_path = params['nfsserver'] + ':' + params['nfspath']
+ if not FeatureTests.check_nfs_export(export_path):
+ raise InvalidParameter(
+ "Export path %s may block during nfs mount" % export_path)
xml = _get_pool_xml(**params)
except KeyError, key:
raise MissingParameter(key)
--
1.8.1.2

[Ramon Medeiros]

can you get the Popen result and use readlines, to get a array with the
lines, instead of striping it?

> + res.append(dict(zip(output_items, line.split())))
> +
> + return res


--

Ramon Nunes Medeiros
Software Engineer - Linux Technology Center Brazil
IBM Systems & Technology Group
Phone : +55 19 2132 7878
ram...@br.ibm.com

[Aline Manera]

On 12/17/2013 10:10 AM, lvroy...@gmail.com wrote:

You should put it in utils.py as it isn't a feature test
Independent of the result of this function the NFS pool feature will
continue enable to user.

> +
> + @staticmethod
> + def check_nfs_export(export_path):
> + res = False
> + outputs = None
> + mnt_point = tempfile.mkdtemp(dir='/tmp')
> + cmd = ["mount", "-o", 'soft,timeo=30,retrans=3,retry=0',
> + export_path, mnt_point]
> + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
> + stderr=subprocess.PIPE)
> + thread = threading.Thread(target = proc.communicate)
> + thread.start()
> + thread.join(9)
> +
> + if thread.is_alive():
> + proc.kill()
> + thread.join()
> +
> + with open("/proc/mounts" , "rb") as f:
> + outputs = f.read()

Here you could use f.readlines() as Ramon suggested

[Shu Ming]

Thanks for addressing this issue and I get a few questions about this patch:
I) nfs server can be be reachable can be caused by different problems,
ie. network, server &etc. For network proI blems, it may come back to
life after down for a few seconds. So it is hard to predict when the nfs
server will be down.
II) I think feature test is a bit broken now and we get many junk
messages in the screen when the Kimchi server is started. Really, We
don't want more junk message in the screen.

[Royce Lv]

I realized that put in featuretests.py will cause some confusion that
this is tested when kimchi started. In fact this will be validated just
before storage pool created so the lease will be very small between
create pool and validation. Will fix in next release.

[Royce Lv]

Hi Ramon,

Thanks for your review, I think for file read result it is OK, but seems
for popen.communicate just pull out the raw output , and only stdout
provide readlines interface, which official doc does not recommend
because of deadlock problem.
(http://docs.python.org/3.3/library/subprocess.html#subprocess.Popen.communicate).

[Royce Lv]

ACK

>
>> +
>> + @staticmethod
>> + def check_nfs_export(export_path):
>> + res = False
>> + outputs = None
>> + mnt_point = tempfile.mkdtemp(dir='/tmp')
>> + cmd = ["mount", "-o", 'soft,timeo=30,retrans=3,retry=0',
>> + export_path, mnt_point]
>> + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
>> + stderr=subprocess.PIPE)
>> + thread = threading.Thread(target = proc.communicate)
>> + thread.start()
>> + thread.join(9)
>> +
>> + if thread.is_alive():
>> + proc.kill()
>> + thread.join()
>> +
>> + with open("/proc/mounts" , "rb") as f:
>> + outputs = f.read()
>
> Here you could use f.readlines() as Ramon suggested

I wrote parse function for Popen.communicate result parse and file read
result. Popen.communicate result cannot apply readlines as I explained
in feed back to Ramon.
As http://docs.python.org/3.3/library/io.html suggests, The line
terminator is always b'\n' for binary files; literally it is same as my
implementation.

[Sheldon]

Reviewed-by: ShaoHe Feng <sha...@linux.vnet.ibm.com>
       

I think Ramon want:

+        with open("/proc/mounts" , "rb") as f:
+            res = []
+            for output in f.readlines():
+                res.append(dict(zip(['dev_path', 'mnt_point', 'type'], output.split()))
+            return res

maybe you want a common function to parse both the output from stdout and files.

blkid = subprocess.Popen(["cat", "/proc/mounts"],

+ res.append(dict(zip(output_items, line.split())))
+
+ return res

 

Sheldon Feng(冯少合)
IBM Linux Technology Center

[Sheldon]

On 12/18/2013 02:40 PM, Sheldon wrote:

Reviewed-by: ShaoHe Feng <sha...@linux.vnet.ibm.com>

sorry, this Reviewed-by is added automatically by my thunderbird plugin.

       

On 12/18/2013 02:31 PM, Royce Lv wrote:

Sheldon Feng(锟斤拷锟劫猴拷)
IBM Linux Technology Center

--
project-kimchi mailing list <project...@googlegroups.com>
https://groups.google.com/forum/#!forum/project-kimchi
---
You received this message because you are subscribed to the Google Groups "project-kimchi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-kimch...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

 

Sheldon Feng(锟斤拷锟劫猴拷)
IBM Linux Technology Center

[Royce Lv]

On 2013锟斤拷12锟斤拷18锟斤拷 14:40, Sheldon wrote:

Reviewed-by: ShaoHe Feng <sha...@linux.vnet.ibm.com>
       

On 12/18/2013 02:31 PM, Royce Lv wrote:

True, I want a helper function that all command can use to get a structural result.
So that in the future if we want to get iscsi session, or other things , we can reuse this function.

+ res.append(dict(zip(output_items, line.split())))
+
+ return res

 

Sheldon Feng(锟斤拷锟劫猴拷)
IBM Linux Technology Center

[Ramon Medeiros]

yes, this was my wish!

maybe you want a common function to parse both the output from stdout and files.

blkid = subprocess.Popen(["cat", "/proc/mounts"],

True, I want a helper function that all command can use to get a structural result.
So that in the future if we want to get iscsi session, or other things , we can reuse this function.

ok

+ res.append(dict(zip(output_items, line.split())))
+
+ return res

 

Sheldon Feng(锟斤拷锟劫猴拷)
IBM Linux Technology Center

--
project-kimchi mailing list <project...@googlegroups.com>
https://groups.google.com/forum/#!forum/project-kimchi
---
You received this message because you are subscribed to the Google Groups "project-kimchi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-kimch...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
project-kimchi mailing list <project...@googlegroups.com>
https://groups.google.com/forum/#!forum/project-kimchi
---
You received this message because you are subscribed to the Google Groups "project-kimchi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-kimch...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.