+def get_value(server, cmd, log_msg, fieldname):
I wouldn't make a separate function out of this. it probably won't be
used except by the eafp_dpool_cap_reserve_val() function.
+ msg = log_msg % fieldname
+ ret, value = run_remote(server, cmd)
+ if ret != 0:
+ logger.error("%s", log_msg, fieldname)
+ return FAIL, value
+ return PASS, value
+
+def eafp_dpool_cap_reserve_val(server, virt, poolname):
The capabilities and reserved values are properties of the ResourcePools
- I'd drop eafp from the name since these values aren't related to the
EAFP association.
Also, I'd move these functions to the logicaldevices.py file. Or, you
could create a resourcepool.py file and add the
verify_common_pool_values() function to it.
The common_util.py is becoming quite large - so, it might be a good idea
to try to place more specific functions like these (that deal with a
specific provider's values) into a file specific for that provider.
+ libvirt_version = virsh_version(server, virt)
+ capacity = reserved = None
+ if libvirt_version >= '0.4.1':
+ # get the value from pool-info
+ log_msg= "Failed to get the '%s' info from pool-info"
+ dp_name, pname = poolname.split("/")
+
+ cmd = "virsh pool-info %s | awk '/Capacity/ { print \$2}'" \
+ % pname
+ status, cap_val = get_value(server, cmd, log_msg, 'Capacity')
+ if status != PASS:
+ return FAIL, capacity, reserved
+ cap_val = float(cap_val)
+ capacity = int(cap_val * 1024 * 1024 * 1024) >> 20
This calculation is odd. You're converting GB into bytes and then from
bytes into megabytes. You can instead convert straight to megabytes:
capacity = int(cap_val * 1024)
+
+ cmd = "virsh pool-info %s | awk '/Allocation/ { print \$2}'"
\
+ % pname
+ status, alloc_val = get_value(server, cmd, log_msg, 'Allocation')
+ if status != PASS:
+ return FAIL, capacity, reserved
+ alloc_val = float(alloc_val)
+ reserved = int(alloc_val * 1024 * 1024 *1024) >> 20
Same here - convert straight to megabytes
+
+ else:
+ # get info from stat -filesystem
+ log_msg = "Stat on the '%s' file failed"
+
+ cmd = "stat -f %s | awk '/size/ {print \$7}'" % disk_file
+ status, f_bsize = get_value(server, cmd, log_msg, disk_file)
+ if status != PASS:
+ return FAIL, capacity, reserved
+
+ cmd = " stat -f %s | awk '/Blocks/ {print \$3}'" %
disk_file
+ status, b_total = get_value(server, cmd, log_msg, disk_file)
+ if status != PASS:
+ return FAIL, capacity, reserved
+ cap_val = (int(f_bsize) * int(b_total))
+ capacity = (int(f_bsize) * int(b_total)) >> 20
+
+ cmd = "stat -f %s | awk '/Blocks/ {print \$5}'" % disk_file
+ status, b_free = get_value(server, cmd, log_msg, disk_file)
+ if status != PASS:
+ return FAIL, capacity, reserved
+ reserved = (cap_val - (int(f_bsize) * int(b_free))) >> 20
+
+ return PASS, capacity, reserved
If you want to break up the size of this function some, you could do
something like:
dpool_cap_reserve_val():
libvirt_version = virsh_version(server, virt)
capacity = reserved = None
if libvirt_version >= '0.4.1':
libvirt_dpool_cap_res()
else:
fs_dpool_cap_res()
Having sub functions for each of these breaks up the work nicely, and
it's possible (although unlikely) that a test would need to call one of
these functions explicitly.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com