hai bo has uploaded a new change for review. Change subject: Add wait code after the call of subprocess_closefds. ......................................................................
Add wait code after the call of subprocess_closefds. Now in ovirt-node, if reading the cmd output directly after the call of subprocess_closefds, it maybe failed. This patch is to avoid the problem. Change-Id: I3b5a7c41c5e6809d148bda37f3fb1a75ed3fd4dd Signed-off-by: boh.ricky <[email protected]> --- M src/ovirtnode/install.py M src/ovirtnode/ovirtfunctions.py M src/ovirtnode/storage.py 3 files changed, 72 insertions(+), 48 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/71/18671/1 diff --git a/src/ovirtnode/install.py b/src/ovirtnode/install.py index c0ad30c..33d11fa 100755 --- a/src/ovirtnode/install.py +++ b/src/ovirtnode/install.py @@ -137,7 +137,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(): @@ -183,7 +184,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") @@ -236,7 +237,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") @@ -496,7 +497,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 7b0d41e..70ccefc 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 @@ -1775,7 +1794,8 @@ shell=True, \ stdout=subprocess.PIPE, \ stderr=subprocess.STDOUT) - efi_out = efi_mgr.stdout.read().strip() + efi_out, err = efi_mgr.communicate() + efi_out = efi_out.strip() logger.debug(efi_mgr_cmd) logger.debug(efi_out) for line in efi_out.splitlines(): diff --git a/src/ovirtnode/storage.py b/src/ovirtnode/storage.py index 4022170..52853af 100644 --- a/src/ovirtnode/storage.py +++ b/src/ovirtnode/storage.py @@ -173,10 +173,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 @@ -267,7 +268,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 @@ -281,7 +283,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: @@ -307,7 +309,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) @@ -328,7 +330,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) @@ -340,7 +342,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 @@ -368,9 +370,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/18671 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3b5a7c41c5e6809d148bda37f3fb1a75ed3fd4dd Gerrit-PatchSet: 1 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: hai bo <[email protected]> _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
