1:17 a.m.
Problem:
In PartitionModel.lookup(), it calls disks.get_partitions_names() to
verify the given partition is eligible to be used in a new logical
storage pool as follow.
if name not in disks.get_partitions_names():
raise NotFoundError(...)
When the front-end issues GET request to host/partitions, the back-end
would call get_partitions_names() for each partition. If there are many
disks and partitions in the host, get_partitions_names() takes a long
time to finish, and calling get_partitions_names() for each partition
would make the GET request timeout.
In all, the original control flow is as follow.
GET host/partitions
Partitions._get_resources()
PartitionsModel.get_list()
disks.get_partitions_names()
for each partition name:
Partition.lookup()
PartitionModel.lookup()
verify partition name in disks.get_partitions_names()
disks.get_partition_details()
The solution is to split the block device verification code out of
disks.get_partitions_names(), and put it in a new function
disks._is_available(). Now get_partitions_names() just returns
all the block devices, and disks.get_partition_details() can perform
the eligibility check by calling disks._is_available(). The original
get_partitions_names() checks eligibility for all the detected device,
so calling it for each partition is too slow. The new _is_available()
function just checks the given block device.
The New control flow is as follow.
GET host/partitions
Partitions._get_resources()
PartitionsModel.get_list()
disks.get_partitions_names()
for each partition name:
Partition.lookup()
PartitionModel.lookup()
disks.get_partition_details()
check if the block device _is_available()
v1:
Make use of the resource and model arguments to have Partitions
collection controller set a flag, so Partition model can inspect the
flag and skip the check accordingly.
v2:
v1 solution is too hack, re-write the patch by refactoring the
underlying disks model. Split the batch verification into small peices
amortized by each concrete block device.
Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>
---
src/kimchi/control/host.py | 6 +++++-
src/kimchi/disks.py | 37 +++++++++++++++++++++++--------------
src/kimchi/model/host.py | 3 ---
3 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py
index ad34919..ebf1bed 100644
--- a/src/kimchi/control/host.py
+++ b/src/kimchi/control/host.py
@@ -21,7 +21,7 @@ import cherrypy
from kimchi.control.base import Collection, Resource, SimpleCollection
from kimchi.control.utils import UrlSubNode, validate_method
-from kimchi.exception import OperationFailed
+from kimchi.exception import OperationFailed, NotFoundError
from kimchi.template import render
@@ -80,6 +80,7 @@ class Partitions(Collection):
# sorted by their path
def _get_resources(self, flag_filter):
res_list = super(Partitions, self)._get_resources(flag_filter)
+ res_list = filter(lambda x: x.info['available'], res_list)
res_list.sort(key=lambda x: x.info['path'])
return res_list
@@ -90,6 +91,9 @@ class Partition(Resource):
@property
def data(self):
+ if not self.info['available']:
+ raise NotFoundError("KCHPART0001E", {'name':
self.info['name']})
+
return self.info
diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py
index 842fd48..a13c753 100644
--- a/src/kimchi/disks.py
+++ b/src/kimchi/disks.py
@@ -142,7 +142,23 @@ def _get_vgname(devNodePath):
return re.findall(r"LVM2_VG_NAME='([^\']*)'", out)[0]
-def get_partitions_names():
+def _is_available(name, devtype, fstype, mountpoint, majmin):
+ devNodePath = _get_dev_node_path(majmin)
+ # Only list unmounted and unformated and leaf and (partition or disk)
+ # leaf means a partition, a disk has no partition, or a disk not held
+ # by any multipath device. Physical volume belongs to no volume group
+ # is also listed. Extended partitions should not be listed.
+ if (devtype in ['part', 'disk', 'mpath'] and
+ fstype in ['', 'LVM2_member'] and
+ mountpoint == "" and
+ _get_vgname(devNodePath) == "" and
+ _is_dev_leaf(devNodePath) and
+ not _is_dev_extended_partition(devtype, devNodePath)):
+ return True
+ return False
+
+
+def get_partitions_names(check=False):
names = set()
keys = ["NAME", "TYPE", "FSTYPE",
"MOUNTPOINT", "MAJ:MIN"]
# output is on format key="value",
@@ -151,26 +167,17 @@ def get_partitions_names():
# split()[0] to avoid the second part of the name, after the
# whiteline
name = dev['name'].split()[0]
- devNodePath = _get_dev_node_path(dev['maj:min'])
- # Only list unmounted and unformated and leaf and (partition or disk)
- # leaf means a partition, a disk has no partition, or a disk not held
- # by any multipath device. Physical volume belongs to no volume group
- # is also listed. Extended partitions should not be listed.
- if not (dev['type'] in ['part', 'disk', 'mpath']
and
- dev['fstype'] in ['', 'LVM2_member'] and
- dev['mountpoint'] == "" and
- _get_vgname(devNodePath) == "" and
- _is_dev_leaf(devNodePath) and
- not _is_dev_extended_partition(dev['type'], devNodePath)):
+ if check and not _is_available(name, dev['type'], dev['fstype'],
+ dev['mountpoint'],
dev['maj:min']):
continue
-
names.add(name)
return list(names)
def get_partition_details(name):
- dev_path = _get_dev_node_path(_get_dev_major_min(name))
+ majmin = _get_dev_major_min(name)
+ dev_path = _get_dev_node_path(majmin)
keys = ["TYPE", "FSTYPE", "SIZE",
"MOUNTPOINT"]
try:
@@ -180,6 +187,8 @@ def get_partition_details(name):
"Error getting partition info for %s: %s", name, e)
return {}
+ dev['available'] = _is_available(name, dev['type'],
dev['fstype'],
+ dev['mountpoint'], majmin)
if dev['mountpoint']:
# Sometimes the mountpoint comes with [SWAP] or other
# info which is not an actual mount point. Filtering it
diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py
index 47b0aa1..e9ac487 100644
--- a/src/kimchi/model/host.py
+++ b/src/kimchi/model/host.py
@@ -245,9 +245,6 @@ class PartitionModel(object):
pass
def lookup(self, name):
- if name not in disks.get_partitions_names():
- raise NotFoundError("KCHPART0001E", {'name': name})
-
return disks.get_partition_details(name)
--
1.9.0