[node-patches] Change in ovirt-node[node-3.0]: add wait code after the call of fuction subprocess_closefds.
fabiand at fedoraproject.org
fabiand at fedoraproject.org
Wed Oct 16 10:16:30 UTC 2013
Hello hai bo,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/20225
to review the following change.
Change subject: add wait code after the call of fuction subprocess_closefds.
......................................................................
add wait code after the call of fuction subprocess_closefds.
Now in ovirt-node, we has the fail possible to get the output
the call of subprocess_closefds, because of not waiting for
the complete of the command.
This patch is to avoid the problem.
Change-Id: I3b5a7c41c5e6809d148bda37f3fb1a75ed3fd4dd
Signed-off-by: boh.ricky <boh.ricky at gmail.com>
---
M src/ovirtnode/install.py
M src/ovirtnode/ovirtfunctions.py
M src/ovirtnode/storage.py
3 files changed, 70 insertions(+), 47 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/25/20225/1
diff --git a/src/ovirtnode/install.py b/src/ovirtnode/install.py
index f73fabc..ac8fb53 100755
--- a/src/ovirtnode/install.py
+++ b/src/ovirtnode/install.py
@@ -139,7 +139,8 @@
efi = _functions.subprocess_closefds(efi_cmd, shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- efi_out = efi.stdout.read().strip()
+ efi_out, efi_err = efi.communicate()
+ efi_out = efi_out.strip()
matches = re.search(_functions.PRODUCT_SHORT + r'\s+(HD\(.+?\))', \
efi_out)
if matches and matches.groups():
@@ -185,7 +186,7 @@
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- grub_results = grub_setup.stdout.read()
+ grub_results, grub_err = grub_setup.communicate()
logger.debug(grub_results)
if grub_setup.wait() != 0 or "Error" in grub_results:
logger.error("GRUB setup failed")
@@ -238,7 +239,7 @@
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- grub_results = grub_setup.stdout.read()
+ grub_results, grub_err = grub_setup.communicate()
logger.info(grub_results)
if grub_setup.wait() != 0 or "Error" in grub_results:
logger.error("grub2-install Failed")
@@ -502,7 +503,8 @@
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- self.disk = grub_disk.stdout.read().strip()
+ grub_disk_output, grub_disk_err = grub_disk.communicate()
+ self.disk = grub_disk_output.strip()
if "cciss" in self.disk:
self.disk = self.disk.replace("!", "/")
# flush to sync DM and blockdev, workaround from rhbz#623846#c14
diff --git a/src/ovirtnode/ovirtfunctions.py b/src/ovirtnode/ovirtfunctions.py
index 972c8c3..09dc7e9 100644
--- a/src/ovirtnode/ovirtfunctions.py
+++ b/src/ovirtnode/ovirtfunctions.py
@@ -198,7 +198,7 @@
mem_size_mb = subprocess_closefds(mem_size_cmd, shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- MEM_SIZE_MB = mem_size_mb.stdout.read()
+ MEM_SIZE_MB, err = mem_size_mb.communicate()
MEM_SIZE_MB = int(MEM_SIZE_MB) / 1024
# we multiply the overcommit coefficient by 10 then divide the
# product by 10 to avoid decimals in the result
@@ -367,16 +367,18 @@
def wipe_volume_group(vg):
vg_name_cmd = "vgs -o vg_name,vg_uuid --noheadings 2>/dev/null | grep -w \"" + vg + "\" | awk '{print $1}'"
vg_name = subprocess_closefds(vg_name_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- vg = vg_name.stdout.read().strip()
+ vg, err = vg_name.communicate()
+ vg = vg.strip()
files_cmd = "grep '%s' /proc/mounts|awk '{print $2}'|sort -r" % vg
files = subprocess_closefds(files_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- files_output = files.stdout.read()
+ files_output, err = files.communicate()
logger.debug("Mounts:\n" + files_output)
for file in files_output.split():
system_closefds("umount %s &>/dev/null" % file)
swap_cmd = "grep '%s' /proc/swaps|awk '{print $1}'" % vg
swap = subprocess_closefds(swap_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- swap_output = swap.stdout.read().strip()
+ swap_output, err = swap.communicate()
+ swap_output = swap_output.strip()
for d in swap_output.split():
system_closefds("swapoff %s &>/dev/null" % d)
# Deactivate VG
@@ -399,20 +401,20 @@
# find_srv ovirt tcp
def find_srv(srv, proto):
domain = subprocess_closefds("dnsdomainname 2>/dev/null", shell=True, stdout=PIPE, stderr=STDOUT)
- domain_output = domain.stdout.read()
+ domain_output, err = domain.communicate()
if domain_output == "localdomain":
domain=""
# FIXME dig +search does not seem to work with -t srv
# dnsreply=$(dig +short +search -t srv _$1._$2)
# This is workaround:
search = subprocess_closefds("grep search /etc/resolv.conf", shell=True, stdout=PIPE, stderr=STDOUT)
- search_output = search.stdout.read()
+ search_output, err = search.communicate()
search = search_output.replace("search ","")
domain_search = domain_output + search_output
for d in domain_search.split():
dig_cmd = "dig +short -t srv _%s._%s.%s" % (srv, proto,search)
dig = subprocess_closefds(dig_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- dig_output = dig.stdout.read()
+ dig_output, err = dig.communicate()
dig.poll()
dig_rc = dig.returncode
if dig_rc == 0:
@@ -564,7 +566,7 @@
# bind mount all persisted configs to rootfs
filelist_cmd = "find /config -type f"
filelist = subprocess_closefds(filelist_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- filelist = filelist.stdout.read()
+ filelist, err = filelist.communicate()
for f in filelist.split():
logger.debug("Bind Mounting: " + f)
if os.path.isfile(f) and f != "/config/files":
@@ -629,7 +631,7 @@
logging_services= []
prgs_cmd = "cd /etc/init.d|lsof -Fc +D /var/log|grep ^c|sort -u"
prgs = subprocess_closefds(prgs_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- prgs_output = prgs.stdout.read()
+ prgs_output, err = prgs.communicate()
for prg in prgs_output.split():
svc = prg = prg[1:]
if not svc == "python":
@@ -996,7 +998,9 @@
system('sed --copy -i "\|%s$|d" /config/files' % filename)
if os.path.isdir(filename):
- for child in subprocess_closefds("ls -d '%s'" % filename, shell=True, stdout=PIPE, stderr=STDOUT).stdout.read():
+ ls_cmd = subprocess_closefds("ls -d '%s'" % filename, shell=True, stdout=PIPE, stderr=STDOUT)
+ output, err = ls_cmd.communicate()
+ for child in output:
ovirt_safe_delete_config(child)
system("rm -rf /config'%s'" % filename)
system("rm -rf '%s'" % filename)
@@ -1069,8 +1073,10 @@
e2label_root_cmd = "e2label '%s' Root" % root_update_dev
logger.debug(e2label_rootbackup_cmd)
logger.debug(e2label_root_cmd)
- subprocess_closefds(e2label_rootbackup_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- subprocess_closefds(e2label_root_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ cmd = subprocess_closefds(e2label_rootbackup_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ cmd.communicate()
+ cmd = subprocess_closefds(e2label_root_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ cmd.communicate()
if is_iscsi_install():
boot_update_dev = findfs("BootUpdate")
@@ -1222,13 +1228,14 @@
def get_gateway(ifname):
cmd = "ip route list dev "+ ifname + " | awk ' /^default/ {print $3}'"
result = subprocess_closefds(cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- result = result.stdout.read().strip()
- return result
+ output, err = result.communicate()
+ return output.strip()
def get_ipv6_address(interface):
inet6_lookup_cmd = "ip addr show dev %s | awk '$1==\"inet6\" && $4==\"global\" { print $2 }'" % interface
inet6_lookup = subprocess_closefds(inet6_lookup_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- ipv6_addr = inet6_lookup.stdout.read().strip()
+ ipv6_addr, err = inet6_lookup.communicate()
+ ipv6_addr = ipv6_addr.strip()
try:
ip, netmask = ipv6_addr.split("/")
return (ip,netmask)
@@ -1239,8 +1246,8 @@
def get_ipv6_gateway(ifname):
cmd = "ip route list dev "+ ifname + " | awk ' /^default/ {print $3}'"
result = subprocess_closefds(cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- result = result.stdout.read().strip()
- return result
+ output, err = result.communicate()
+ return output.strip()
def has_ip_address(ifname):
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
@@ -1294,20 +1301,24 @@
def get_dm_device(device):
dev_major_cmd="stat -c '%t' " + "\"/dev/" + device + "\""
dev_minor_cmd="stat -c '%T' " + "\"/dev/" + device + "\""
- major_lookup = subprocess_closefds(dev_major_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- minor_lookup = subprocess_closefds(dev_minor_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- major_lookup = major_lookup.stdout.read().strip()
- minor_lookup = minor_lookup.stdout.read().strip()
+ major_lookup_cmd = subprocess_closefds(dev_major_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ minor_lookup_cmd = subprocess_closefds(dev_minor_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ major_lookup, err = major_lookup_cmd.communicate()
+ major_lookup = major_lookup.strip()
+ minor_lookup, err = minor_lookup_cmd.communicate()
+ minor_lookup = minor_lookup.strip()
dm_cmd = "ls /dev/mapper"
dm_cmd = subprocess_closefds(dm_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
devices = dm_cmd.stdout.read().strip()
for dm in devices.split("\n"):
dm_major_cmd="stat -c '%t' " + "\"/dev/mapper/" + dm + "\""
dm_minor_cmd="stat -c '%T' " + "\"/dev/mapper/" + dm + "\""
- dm_major_lookup = subprocess_closefds(dm_major_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- dm_minor_lookup = subprocess_closefds(dm_minor_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- dm_major_lookup = dm_major_lookup.stdout.read().strip()
- dm_minor_lookup = dm_minor_lookup.stdout.read().strip()
+ dm_major_lookup_cmd = subprocess_closefds(dm_major_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ dm_minor_lookup_cmd = subprocess_closefds(dm_minor_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
+ dm_major_lookup, err = dm_major_lookup_cmd.communicate()
+ dm_major_lookup = dm_major_lookup.strip()
+ dm_minor_lookup, err = dm_minor_lookup_cmd.communicate()
+ dm_minor_lookup = dm_minor_lookup.strip()
if dm_major_lookup == major_lookup and minor_lookup == dm_minor_lookup:
dm = "/dev/mapper/" + dm
return dm
@@ -1320,7 +1331,8 @@
else:
devices_cmd="pvs --separator=: -o pv_name,vg_name --noheadings 2>/dev/null| grep -v '%s' | grep %s | awk -F: {'print $1'}" % (install_dev, vg_name)
devices_cmd = subprocess_closefds(devices_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- devices = devices_cmd.stdout.read().strip()
+ devices, err = devices_cmd.communicate()
+ devices = devices.strip()
if len(devices) > 0:
logger.error("There appears to already be an installation on another device:")
for device in devices.split(":"):
@@ -1341,7 +1353,8 @@
if "/dev/cciss" in dev:
cciss_dev_cmd = "cciss_id " + dev
cciss_dev = subprocess_closefds(cciss_dev_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- dev = "/dev/mapper/" + cciss_dev.stdout.read().strip()
+ output, err = cciss_dev.communicate()
+ dev = "/dev/mapper/" + output.strip()
dm_dev_cmd = "multipath -ll '%s' | egrep dm-[0-9]+" % dev
dm_dev = subprocess_closefds(dm_dev_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
(dm_dev_output, dummy) = dm_dev.communicate()
@@ -1428,7 +1441,8 @@
system("udevadm settle")
blkid_cmd = "/sbin/blkid -c /dev/null -l -o device -t LABEL=\"" + label + "\""
blkid = subprocess_closefds(blkid_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- blkid_output = blkid.stdout.read().strip()
+ blkid_output, err = blkid.communicate()
+ blkid_output = blkid_output.strip()
return blkid_output
def system(command):
@@ -1479,7 +1493,8 @@
def get_cpu_flags():
cpuflags_cmd = "cat /proc/cpuinfo |grep ^flags|tail -n 1"
cpuflags_lookup = subprocess_closefds(cpuflags_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- return cpuflags_lookup.stdout.read().strip()
+ output, err = cpuflags_lookup.communicate()
+ return output.strip()
def kvm_enabled():
try:
@@ -1601,7 +1616,8 @@
hostkey = open(fn_hostkey).read()
hostkey_fp_cmd = "ssh-keygen -l -f '%s'" % fn_hostkey
hostkey_fp_lookup = subprocess_closefds(hostkey_fp_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- fingerprint = hostkey_fp_lookup.stdout.read().strip().split(" ")[1]
+ output, err = hostkey_fp_lookup.communicate()
+ fingerprint = output.strip().split(" ")[1]
return (fingerprint, hostkey)
def get_mac_address(dev):
@@ -1722,8 +1738,8 @@
def nic_link_detected(iface):
link_status_cmd = "ip link set dev {dev} up ; ethtool {dev} |grep \"Link detected\"".format(dev=iface)
link_status = subprocess_closefds(link_status_cmd, shell=True, stdout=PIPE, stderr=STDOUT)
- link_status = link_status.stdout.read()
- return ("yes" in link_status)
+ link_status_output, err = link_status.communicate()
+ return ("yes" in link_status_output)
def is_capslock_on():
"""Returns True if Caps Lock is on.
@@ -1734,8 +1750,11 @@
# CapsLock, so return nothing
return None
cmd = "LC_ALL=C setleds < /dev/%s | awk '/Current flags:/{print $6;}'" % tty
- return "on" == subprocess_closefds(cmd, shell=True, stdout=PIPE, \
- stderr=STDOUT).stdout.read().strip()
+ cmd_rtn == subprocess_closefds(cmd, shell=True, stdout=PIPE, \
+ stderr=STDOUT)
+ output, err = cmd_rtn.communicate()
+ return "on" == output.strip()
+
def rng_status():
bit_value = 0
disable_aes_ni = 0
diff --git a/src/ovirtnode/storage.py b/src/ovirtnode/storage.py
index ae497d9..4f53b71 100644
--- a/src/ovirtnode/storage.py
+++ b/src/ovirtnode/storage.py
@@ -172,10 +172,11 @@
def get_drive_size(self, drive):
logger.debug("Getting Drive Size For: %s" % drive)
size_cmd = "sfdisk -s " + drive + " 2>/dev/null"
- size = _functions.subprocess_closefds(size_cmd, shell=True,
+ size_popen = _functions.subprocess_closefds(size_cmd, shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- size = size.stdout.read().strip()
+ size, size_err = size_popen.communicate()
+ size = size.strip()
size = int(int(size) / 1024)
logger.debug(size)
return size
@@ -266,7 +267,8 @@
device_sys = _functions.subprocess_closefds(device_sys_cmd, shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- device_sys_output = device_sys.stdout.read().strip()
+ device_sys_output, device_sys_err = device_sys.communicate()
+ device_sys_output = device_sys_output.strip()
if not device_sys_output is "":
device = os.path.basename(os.path.dirname(device_sys_output))
return device
@@ -280,7 +282,7 @@
deps = _functions.subprocess_closefds(deps_cmd, shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- deps_output = deps.stdout.read()
+ deps_output, deps_err = deps.communicate()
for dep in deps_output.split():
device = self.get_sd_name(dep)
if device is not None:
@@ -306,7 +308,7 @@
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- byid_list_output = byid_list.stdout.read()
+ byid_list_output, byid_list_err = byid_list.communicate()
for d in byid_list_output.split():
d = os.readlink(d)
d_basename = os.path.basename(d)
@@ -327,7 +329,7 @@
shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- multipath_list_output = multipath_list.stdout.read()
+ multipath_list_output, multipath_list_err = multipath_list.communicate()
for d in multipath_list_output.split():
devices.append("/dev/mapper/%s" % d)
@@ -339,7 +341,7 @@
dm_dev = _functions.subprocess_closefds(dm_dev_cmd, shell=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
- dm_dev_output = dm_dev.stdout.read()
+ dm_dev_output, dm_dev_err = dm_dev.communicate()
devs_to_remove = ("%s %s %s" % (devs_to_remove, sd_devs,
dm_dev_output))
# Remove /dev/sd* devices that are part of a multipath device
@@ -367,9 +369,9 @@
dev_serial = device.get_property("ID_SERIAL")
dev_desc = device.get_property("ID_SCSI_COMPAT")
dev_size_cmd = "sfdisk -s %s 2>/dev/null" % dev_name
- dev_size = _functions.subprocess_closefds(dev_size_cmd, shell=True,
+ dev_size_popen = _functions.subprocess_closefds(dev_size_cmd, shell=True,
stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
- dev_size = dev_size.stdout.read()
+ dev_size, dev_size_err = dev_size_popen.communicate()
size_failed = 0
if not device.get_property("ID_CDROM"):
try:
--
To view, visit http://gerrit.ovirt.org/20225
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3b5a7c41c5e6809d148bda37f3fb1a75ed3fd4dd
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: node-3.0
Gerrit-Owner: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: hai bo <boh.ricky at gmail.com>
More information about the node-patches
mailing list