From 29e739ae7c0651f8f77c60846bfbe2b6c91baa29 Mon Sep 17 00:00:00 2001
From: Pavel Cahyna <pcahyna@redhat.com>
Date: Sat, 31 Dec 2022 17:40:39 +0100
Subject: [PATCH] Protect against colons in pvdisplay output

LVM can be configured to show device names under /dev/disk/by-path
in command output. These names often contain colons that separate fields
like channel and target (for example /dev/disk/by-path/pci-*-scsi-0:0:1:0-*,
similarly the pci-* part, which contains colon-separated PCI bus and
device numbers). Since the "pvdisplay -c" output also uses colons as
field separators and does not escape embedded colons in any way,
embedded colons break parsing of this output.

As a fix, use the pipe character '|' as the field separator in pvdisplay
output. (This would break if a PV device has a '|' in its name, but this
is very much less likely than having a ':' .)

Also, configure explicitly what fields to output - "pvdisplay -c"
prints many fields, but I have not found documentation about what fields
is it using exactly, so one had to guess what the output means. Using
"pvdisplay -C" and selecting the fields explicitly is much clearer.

This also changes the PV size field to match documentation, the comment
says that size is in bytes, but it actually was not in bytes. As nothing
is actually using the PV size field, this inconsistency has not caused
any problem in practice, and no code needs adjusting for the change.
---
 .../layout/save/GNU/Linux/220_lvm_layout.sh   | 24 ++++++++++++-------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/usr/share/rear/layout/save/GNU/Linux/220_lvm_layout.sh b/usr/share/rear/layout/save/GNU/Linux/220_lvm_layout.sh
index e01dbf465..7400c586e 100644
--- a/usr/share/rear/layout/save/GNU/Linux/220_lvm_layout.sh
+++ b/usr/share/rear/layout/save/GNU/Linux/220_lvm_layout.sh
@@ -70,14 +70,20 @@ local lvs_exit_code
     # Get physical_device configuration.
     # Format: lvmdev <volume_group> <device> [<uuid>] [<size(bytes)>]
     header_printed="no"
-    # Example output of "lvm pvdisplay -c":
-    #   /dev/sda1:system:41940992:-1:8:8:-1:4096:5119:2:5117:7wwpcO-KmNN-qsTE-7sp7-JBJS-vBdC-Zyt1W7
+    # Set pvdisplay separator to '|' to prevent issues with a colon in the path under /dev/disk/by-path
+    # that contains a ':' in the SCSI slot name.
+    # Example output of "lvm pvdisplay -C --separator '|' --noheadings --nosuffix --units=b -o pv_name,vg_name,pv_size,pv_uuid"
+    # on a system where LVM is configured to show the /dev/disk/by-path device names instead of the usual
+    # /dev/sda etc. (by using a setting like
+    # filter = [ "r|/dev/disk/by-path/.*-usb-|", "a|/dev/disk/by-path/pci-.*-nvme-|", "a|/dev/disk/by-path/pci-.*-scsi-|", "a|/dev/disk/by-path/pci-.*-ata-|", "a|/dev/disk/by-path/pci-.*-sas-|", "a|loop|", "r|.*|" ]
+    # in /etc/lvm/lvm.conf):
+    #   /dev/disk/by-path/pci-0000:03:00.0-scsi-0:0:1:0-part1|system|107340627968|7wwpcO-KmNN-qsTE-7sp7-JBJS-vBdC-Zyt1W7
     # There are two leading blanks in the output (at least on SLES12-SP4 with LVM 2.02.180).
-    lvm pvdisplay -c | while read line ; do
+    lvm pvdisplay -C --separator '|' --noheadings --nosuffix --units=b -o pv_name,vg_name,pv_size,pv_uuid | while read line ; do
 
-        # With the above example pdev=/dev/sda1
+        # With the above example pdev=/dev/disk/by-path/pci-0000:03:00.0-scsi-0:0:1:0-part1
         # (the "echo $line" makes the leading blanks disappear)
-        pdev=$( echo $line | cut -d ":" -f "1" )
+        pdev=$( echo $line | cut -d "|" -f "1" )
 
         # Skip lines that are not describing physical devices
         # i.e. lines where pdev does not start with a leading / character:
@@ -91,11 +97,11 @@ local lvs_exit_code
         fi
 
         # With the above example vgrp=system
-        vgrp=$( echo $line | cut -d ":" -f "2" )
-        # With the above example size=41940992
-        size=$( echo $line | cut -d ":" -f "3" )
+        vgrp=$( echo $line | cut -d "|" -f "2" )
+        # With the above example size=107340627968
+        size=$( echo $line | cut -d "|" -f "3" )
         # With the above example uuid=7wwpcO-KmNN-qsTE-7sp7-JBJS-vBdC-Zyt1W7
-        uuid=$( echo $line | cut -d ":" -f "12" )
+        uuid=$( echo $line | cut -d "|" -f "4" )
 
         # Translate pdev through diskbyid_mappings file:
         pdev=$( get_device_mapping $pdev )