Merge pull request #976 from aschnell/master

make more use of new SystemCmd interface

doc/blocksizes.md

@@ -165,7 +165,7 @@ nr_this_dev:            0
 first_bucket:           1
 /dev/sdf blkdiscard beginning...done
 > bcache attach /dev/sdf /dev/sdc
-Failed to attache to cset 423801be-5553-4205-9650-c8d12eb7897d
+Failed to attach to cset 423801be-5553-4205-9650-c8d12eb7897d
 > dmesg | tail -1
 [ 5084.307221] bcache: bch_cached_dev_attach() Couldn't attach sdc: block size less than set's block size
 ```
@@ -272,7 +272,7 @@ Name            Type            State                   Bname           AttachTo
 /dev/sde        0 (cache)       active                  N/A             N/A
 /dev/sdc        3 (cache)       active                  N/A             N/A
 > bcache attach 921ea429-2483-4382-ab8f-290511ed0d55 /dev/sdd
-Failed to attache to cset 921ea429-2483-4382-ab8f-290511ed0d55
+Failed to attach to cset 921ea429-2483-4382-ab8f-290511ed0d55
 > bcache attach 921ea429-2483-4382-ab8f-290511ed0d55 /dev/sdf
 ```
 

doc/devicegraphs.md

@@ -33,7 +33,7 @@ libstorage-ng uses three special devicegraphs:
   Only very few operations (only the immediate_* functions) are allowed on the
   system devicegraph.
 
-  The intension in the future is to update the system devicegraph during
+  The intention in the future is to update the system devicegraph during
   commit and copy it to probed devicegraph after the commit.
 
 

doc/meetings/yast-storage-requirements.md

@@ -8,7 +8,7 @@ This document was created as a summary of a meeting held in Nuernberg in Februar
 
 * AI Lukas: ML: [email protected]
 * AI PM: Clarify requirements on FAT partitions
-* AI lmb, Holgi: Claryfy requirements on filesystems (see below)
+* AI lmb, Holgi: Clarify requirements on filesystems (see below)
 * AI Architecture maintainers: Clarify which configurations of RAID are bootable, possibly under which conditions
 * AI: Verify support of resizing logical volumes for LVM
 * AI: Clarify why we do not support encryption of root filesystem, which tools would possibly need to be fixed
@@ -51,7 +51,7 @@ ways to manage the disk space provided by the disks
        * mdadm RAID: https://en.wikipedia.org/wiki/Mdadm
        * dmraid (Device Mapper RAID): https://en.wikipedia.org/wiki/Device_mapper ?
 
-BIOS-managed raid is configured in BIOS (and Linux system detects it), Linux softwre RAID must be configured in Linux
+BIOS-managed raid is configured in BIOS (and Linux system detects it), Linux software RAID must be configured in Linux
 
 RAID levels:
 
@@ -80,7 +80,7 @@ Raid and boot partitions
 ### Volume Managers
 
    * LVM
-   * Btrfs (volume manager features): not yet supported, needs FATE with usecases and technical information
+   * Btrfs (volume manager features): not yet supported, needs FATE with use-cases and technical information
    * EVMS: legacy; no longer supported, drop, not even for upgrade (see https://bugzilla.suse.com/show_bug.cgi?id=848821#c18)
 
 Supported actions (LVM only):
@@ -106,7 +106,7 @@ Not supported (use low-level tools instead):
 
 ### Encryption
 
-   * full disk encryptionn (LVM only: encrypted PVs)
+   * full disk encryption (LVM only: encrypted PVs)
    * per partition (not root file system) with device mapper
    * per partition also for the root filesystem (currently not supported, AI: find out why it is not possible and fix it)
    * using LUKS (Linux Unified Key Setup)
@@ -185,7 +185,7 @@ File Systems and operations to be supported by new libstorage
 | ext2       | X      | X    | X      |                                                         |
 | ext3       | X      | X    | X      |                                                         |
 | ext4       | X      | X    | X      |                                                         |
-| Btrfs      | X      | X    | X      | primary root parition, avoid for data                   |
+| Btrfs      | X      | X    | X      | primary root partition, avoid for data                  |
 | XFS        | X      | X    | X      | preferred for data                                      |
 | JFS        | X      | X    | X      | Drop                                                    |
 | ReiserFS   |        | X    | X      | Support existing partitions, not allowed to create new  |
@@ -226,16 +226,16 @@ Local file system can be located in a disk partition, in whole disk, on an MD ar
                * latest (7/2015): 48 bit LBA; 144 PB (144000 TB) limit
            * there have always been limits, they always seemed way out of reach, and we always reached them before we thought it possible.
            * BIOS can only access first 8 disks
-       * Stage1 and Stage2 of grub2 should always be on the same disk (means including the partition where stage2 will be insalled into and the config)
-       * BIOS can only support 8 harddisks, so bootloader needs to be on the first 8 ones
+       * Stage1 and Stage2 of grub2 should always be on the same disk (means including the partition where stage2 will be installed into and the config)
+       * BIOS can only support 8 hard-disks, so bootloader needs to be on the first 8 ones
 
    * UEFI (Unified Extensible Firmware Interface):
        * typically used with GPT disk label
        * requires EFI system partition (with FAT file system)
 
    * PPC (Power PC)
        * can use MS-DOS or GPT disk label
-       * requires PReP partition in KVM or LPAR, on LPAR less than 8MB, idealy size of grub2 stage1
+       * requires PReP partition in KVM or LPAR, on LPAR less than 8MB, ideally size of grub2 stage1
        * no special partition required for OPAL/PowerNV/Bare
        * ***TO DO: Get more information from the PPC arch maintainers***
 
@@ -269,7 +269,7 @@ Local file system can be located in a disk partition, in whole disk, on an MD ar
        * parted will use the smallest available number when creating a new partition
        * bootflags are not required (maybe important for legacy BIOS, ignored UEFI)
        * **UEFI**
-           * EFI system partiton, vfat formated, might require specific UUID, limited to 32GB (vfat)
+           * EFI system partition, vfat formatted, might require specific UUID, limited to 32GB (vfat)
        * **legacy BIOS**
            * protective MBR (should have active flag)
            * hybrid MBR -> we do not want that any longer
@@ -278,7 +278,7 @@ Local file system can be located in a disk partition, in whole disk, on an MD ar
    * DASD (s/390 specific)
        * Up to 4 partitions
        * Separate zipl partition needed
-       * First sector is special, only writeable with appropriate tools
+       * First sector is special, only writable with appropriate tools
        * Needs to write the blocklist onto zipl partition, kernel/initrd must reside there (needs to be large enough to hold two copies of kernel and initrd)
        * AI: Provide details about specific tools for first sector writing, reasoning for two copies of kernel (why two?) for DASD
 

storage/Devices/MdImpl.cc

@@ -1368,7 +1368,7 @@ namespace storage
 	SystemCmd cmd(cmd_args, SystemCmd::DoThrow);
 
 	// Thanks to udev "md-raid-assembly.rules" running "parted <disk>
-	// print" readds the device to the md if the signature is still
+	// print" re-adds the device to the md if the signature is still
 	// valid. Thus remove the signature.
 	blk_device->get_impl().wipe_device();
     }

storage/Devices/MsdosImpl.cc

@@ -109,7 +109,7 @@ namespace storage
 	// https://en.wikipedia.org/wiki/BIOS_boot_partition. Normally the
 	// space for the MBR gap is unused anyway due to partition alignment
 	// but for disks with an alignment offset it can be required to
-	// explicitely reserve it.
+	// explicitly reserve it.
 
 	unsigned long long at_front = max(1ULL, region.to_blocks(minimal_mbr_gap));
 

storage/Devices/PartitionImpl.cc

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) [2014-2015] Novell, Inc.
- * Copyright (c) [2016-2023] SUSE LLC
+ * Copyright (c) [2016-2024] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -947,7 +947,7 @@ namespace storage
 	    boost::replace_all(t, "'", "\\'");
 	    boost::replace_all(t, "\"", "\\\"");
 
-	    return "\"'" + t + "'\"";
+	    return "'" + t + "'";
 	}
 
     }
@@ -963,19 +963,19 @@ namespace storage
 
 	do_create_pre_hack(tmps);
 
-	string cmd_line = PARTED_BIN " --script ";
+	SystemCmd::Args cmd_args = { PARTED_BIN, "--script" };
 
 	if (PartedVersion::supports_wipe_signatures())
-	    cmd_line += "--wipesignatures ";
+	    cmd_args << "--wipesignatures";
 
-	cmd_line += quote(partitionable->get_name()) + " unit s mkpart ";
+	cmd_args << partitionable->get_name() << "unit" << "s" << "mkpart";
 
 	if (is_msdos(partition_table))
-	    cmd_line += toString(get_type()) + " ";
+	    cmd_args << toString(get_type());
 
 	if (is_gpt(partition_table))
 	    // pass empty string as partition name
-	    cmd_line += quote_label("") + " ";
+	    cmd_args << quote_label("");
 
 	if (get_type() != PartitionType::EXTENDED)
 	{
@@ -989,32 +989,32 @@ namespace storage
 	    switch (get_id())
 	    {
 		case ID_SWAP:
-		    cmd_line += "linux-swap ";
+		    cmd_args << "linux-swap";
 		    break;
 
 		case ID_DOS32:
-		    cmd_line += "fat32 ";
+		    cmd_args << "fat32";
 		    break;
 
 		case ID_NTFS:
 		case ID_WINDOWS_BASIC_DATA:
-		    cmd_line += "ntfs ";
+		    cmd_args << "ntfs";
 		    break;
 
 		default:
-		    cmd_line += "ext2 ";
+		    cmd_args << "ext2";
 		    break;
 	    }
 	}
 
 	unsigned long long factor = parted_sector_adjustment_factor();
 
-	cmd_line += to_string(get_region().get_start() * factor) + " " +
-	    to_string((get_region().get_end() - tmps.size()) * factor + (factor - 1));
+	cmd_args << to_string(get_region().get_start() * factor)
+		 << to_string((get_region().get_end() - tmps.size()) * factor + (factor - 1));
 
 	udev_settle();
 
-	SystemCmd cmd(cmd_line, SystemCmd::DoThrow);
+	SystemCmd cmd(cmd_args, SystemCmd::DoThrow);
 
 	do_create_post_hack(tmps);
 
@@ -1095,26 +1095,27 @@ namespace storage
 
 	for (unsigned int i = 0; i < tmps.size(); ++i)
 	{
-	    string cmd_line = PARTED_BIN " --script ";
+	    SystemCmd::Args cmd_args = { PARTED_BIN, "--script" };
 
 	    if (PartedVersion::supports_wipe_signatures())
-		cmd_line += "--wipesignatures ";
+		cmd_args << "--wipesignatures";
 
-	    cmd_line += quote(partitionable->get_name()) + " unit s mkpart ";
+	    cmd_args << partitionable->get_name() << "unit" << "s" << "mkpart";
 
 	    if (is_msdos(partition_table))
-		cmd_line += "primary ";
+		cmd_args << "primary";
 
 	    if (is_gpt(partition_table))
-		cmd_line += "'\"\"' ";
+		// pass empty string as partition name
+		cmd_args << quote_label("");
 
-	    cmd_line += "ext2 ";
+	    cmd_args << "ext2";
 
-	    cmd_line += to_string(get_region().get_end() - i) + " " + to_string(get_region().get_end() - i);
+	    cmd_args << to_string(get_region().get_end() - i) << to_string(get_region().get_end() - i);
 
 	    udev_settle();
 
-	    SystemCmd cmd(cmd_line, SystemCmd::DoThrow);
+	    SystemCmd cmd(cmd_args, SystemCmd::DoThrow);
 	}
 
 	y2mil("do_create_pre_number_hack end");
@@ -1321,12 +1322,10 @@ namespace storage
     {
 	const Partitionable* partitionable = get_partitionable();
 
-	string cmd_line = PARTED_BIN " --script " + quote(partitionable->get_name()) + " name " +
-	    to_string(get_number()) + " ";
+	SystemCmd::Args cmd_args = { PARTED_BIN, "--script", partitionable->get_name(), "name",
+	    to_string(get_number()), quote_label(label) };
 
-	cmd_line += quote_label(label);
-
-	SystemCmd cmd(cmd_line, SystemCmd::DoThrow);
+	SystemCmd cmd(cmd_args, SystemCmd::DoThrow);
     }
 
 

storage/SystemInfo/Arch.cc

@@ -38,8 +38,6 @@ namespace storage
     // TODO overall not nice
 
     Arch::Arch(bool do_probe)
-	: arch("unknown"), ppc_mac(false), ppc_pegasos(false), ppc_power_nv(false),
-	  efiboot(false), page_size(0)
     {
 	if (do_probe)
 	    probe();

storage/SystemInfo/Arch.h

@@ -68,12 +68,12 @@ namespace storage
 
 	void probe();
 
-	std::string arch;
-	bool ppc_mac;
-	bool ppc_pegasos;
-	bool ppc_power_nv;
-	bool efiboot;
-	unsigned int page_size;
+	std::string arch = "unknown";
+	bool ppc_mac = false;
+	bool ppc_pegasos = false;
+	bool ppc_power_nv = false;
+	bool efiboot = false;
+	unsigned int page_size = 0;
 
     };
 

storage/SystemInfo/CmdCryptsetup.cc

@@ -37,7 +37,7 @@ namespace storage
 
 
     CmdCryptsetupStatus::CmdCryptsetupStatus(const string& name)
-	: name(name), encryption_type(EncryptionType::UNKNOWN)
+	: name(name)
     {
 	SystemCmd cmd({ CRYPTSETUP_BIN, "status", name }, SystemCmd::DoThrow);
 

storage/SystemInfo/CmdCryptsetup.h

@@ -54,7 +54,7 @@ namespace storage
 
 	const string name;
 
-	EncryptionType encryption_type;
+	EncryptionType encryption_type = EncryptionType::UNKNOWN;
 
     };
 

storage/SystemInfo/CmdDasdview.cc

@@ -32,7 +32,7 @@ namespace storage
 {
 
     Dasdview::Dasdview(const string& device)
-	: device(device), bus_id(), type(DasdType::UNKNOWN), format(DasdFormat::NONE)
+	: device(device)
     {
 	SystemCmd cmd({ DASDVIEW_BIN, "--extended", device }, SystemCmd::DoThrow);
 

storage/SystemInfo/CmdDasdview.h

@@ -55,8 +55,8 @@ namespace storage
 
 	string bus_id;
 
-	DasdType type;
-	DasdFormat format;
+	DasdType type = DasdType::UNKNOWN;
+	DasdFormat format = DasdFormat::NONE;
 
     };
 

storage/SystemInfo/CmdNvme.h

@@ -70,7 +70,7 @@ namespace storage
 	struct Path
 	{
 	    string name;
-	    Transport transport;
+	    Transport transport = Transport::UNKNOWN;
 	};
 
 	struct Subsystem

storage/SystemInfo/CmdUdevadm.cc

@@ -51,7 +51,9 @@ namespace storage
 	// events (fixed in SUSE versions). So always run 'udevadm settle'.
 	udevadm.settle();
 
-	SystemCmd cmd({ UDEVADM_BIN, "info", file }, SystemCmd::DoThrow);
+	SystemCmd::Options options({ UDEVADM_BIN, "info", file }, SystemCmd::DoThrow);
+	options.env.push_back("SYSTEMD_COLORS=false");
+	SystemCmd cmd(options);
 
 	parse(cmd.stdout());
     }

testsuite/dependencies/partition-tables/repair1-mockup.xml

@@ -15,7 +15,7 @@
       <!-- stdout missing -->
     </Command>
     <Command>
-      <name>/usr/sbin/parted --script --wipesignatures '/dev/sdb' unit s mkpart "''" ext2 2048 2147485695</name>
+      <name>/usr/sbin/parted --script --wipesignatures /dev/sdb unit s mkpart '' ext2 2048 2147485695</name>
       <!-- stdout missing -->
     </Command>
     <Command>

testsuite/dependencies/partitions/dasd1-mockup.xml

@@ -11,13 +11,13 @@
       <name>/usr/sbin/parted --script /dev/vda unit s print</name>
     </Command>
     <Command>
-      <name>/usr/sbin/parted --script --wipesignatures '/dev/dasda' unit s mkpart ext2 192 409727</name>
+      <name>/usr/sbin/parted --script --wipesignatures /dev/dasda unit s mkpart ext2 192 409727</name>
     </Command>
     <Command>
       <name>/usr/sbin/blkdiscard --verbose /dev/dasda1</name>
     </Command>
     <Command>
-      <name>/usr/sbin/parted --script --wipesignatures '/dev/vda' unit s mkpart ext2 24 51215</name>
+      <name>/usr/sbin/parted --script --wipesignatures /dev/vda unit s mkpart ext2 24 51215</name>
     </Command>
     <Command>
       <name>/usr/sbin/blkdiscard --verbose /dev/vda1</name>