lvm: Improve error handling when parsing lines

Turns out that sudo may modify command output. One example seen with EL
7.6 is this /etc/pam.d/system-auth configuration:

    session     required      pam_lastlog.so showfailed

With this configuration lvm command output includes unexpected "Last
login" line:

    # sudo -n vgs --noheadings --separator "|" --units b
    Last login: Sat Dec 29 13:56:06 IST 2018
      fedora_root|2|4|0|wz--n-|255525388288B|260046848B

This breaks vdsm lvm command line parsing, and can break any other
command run by sudo in unexpected ways, so we cannot support such
configuration. However, the failure was logged in an unhelpful way:

      File "/usr/lib/python2.7/site-packages/vdsm/storage/lvm.py", line 389, in _reloadvgs
        pv_name = fields[pvNameIdx]
    IndexError: list index out of range

To make it easier to debug such issues in the field, verify the number
of fields when parsing lvm commands output lines, and raise a detailed
exception showing the raw offending line.

With this patch the error would be logged as:

    InvalidOutputLine: Invalid pvs command ouptut line: 'Last login: Sat Dec 29 14:42:39 IST 2018'

Change-Id: I2acadf84cebaac6aed2f2739f371f2cf5c53d36d
Bug-Url: https://bugzilla.redhat.com/1662449
Signed-off-by: Nir Soffer <[email protected]>

lib/vdsm/storage/lvm.py

@@ -41,6 +41,8 @@
 from subprocess import list2cmdline
 
 from vdsm import constants
+from vdsm.common import errors
+
 from vdsm.storage import devicemapper
 from vdsm.storage import exception as se
 from vdsm.storage import misc
@@ -53,9 +55,14 @@
 
 PV_FIELDS = ("uuid,name,size,vg_name,vg_uuid,pe_start,pe_count,"
              "pe_alloc_count,mda_count,dev_size,mda_used_count")
+PV_FIELDS_LEN = len(PV_FIELDS.split(","))
+
 VG_FIELDS = ("uuid,name,attr,size,free,extent_size,extent_count,free_count,"
              "tags,vg_mda_size,vg_mda_free,lv_count,pv_count,pv_name")
+VG_FIELDS_LEN = len(VG_FIELDS.split(","))
+
 LV_FIELDS = "uuid,name,vg_name,attr,size,seg_start_pe,devices,tags"
+LV_FIELDS_LEN = len(LV_FIELDS.split(","))
 
 VG_ATTR_BITS = ("permission", "resizeable", "exported",
                 "partial", "allocation", "clustered")
@@ -73,6 +80,14 @@
 Stub = namedtuple("Stub", "name, stale")
 
 
+class InvalidOutputLine(errors.Base):
+    msg = "Invalid {self.command} command ouptut line: {self.line!r}"
+
+    def __init__(self, command, line):
+        self.command = command
+        self.line = line
+
+
 class Unreadable(Stub):
     __slots__ = ()
 
@@ -330,6 +345,9 @@ def _reloadpvs(self, pvName=None):
             updatedPVs = {}
             for line in out:
                 fields = [field.strip() for field in line.split(SEPARATOR)]
+                if len(fields) != PV_FIELDS_LEN:
+                    raise InvalidOutputLine("pvs", line)
+
                 pv = makePV(*fields)
                 if pv.name == UNKNOWN:
                     log.error("Missing pv: %s in vg: %s", pv.uuid, pv.vg_name)
@@ -390,6 +408,9 @@ def _reloadvgs(self, vgName=None):
             vgsFields = {}
             for line in out:
                 fields = [field.strip() for field in line.split(SEPARATOR)]
+                if len(fields) != VG_FIELDS_LEN:
+                    raise InvalidOutputLine("vgs", line)
+
                 uuid = fields[VG._fields.index("uuid")]
                 pvNameIdx = VG._fields.index("pv_name")
                 pv_name = fields[pvNameIdx]
@@ -448,6 +469,9 @@ def _reloadlvs(self, vgName, lvNames=None):
             updatedLVs = {}
             for line in out:
                 fields = [field.strip() for field in line.split(SEPARATOR)]
+                if len(fields) != LV_FIELDS_LEN:
+                    raise InvalidOutputLine("lvs", line)
+
                 lv = makeLV(*fields)
                 # For LV we are only interested in its first extent
                 if lv.seg_start_pe == "0":
@@ -482,6 +506,9 @@ def _reloadAllLvs(self):
             updatedLVs = set()
             for line in out:
                 fields = [field.strip() for field in line.split(SEPARATOR)]
+                if len(fields) != LV_FIELDS_LEN:
+                    raise InvalidOutputLine("lvs", line)
+
                 lv = makeLV(*fields)
                 # For LV we are only interested in its first extent
                 if lv.seg_start_pe == "0":