appliance-tools

Clone
Source Code
GIT

#17 Populate fstab(5) and bootloader configuration for Btrfs subvolumes
Merged 3 years ago by ngompa. Opened 3 years ago by ngompa.
populate-fstab-and-bootloader-config  into  master

file modified
+39 -7 
@@ -76,23 +76,36 @@ 
 

          self.grub2inst_params = []
 

  

      def _get_fstab(self):
 

-         s = ""
 

+         f = ""
 

+         has_btrfs_subvolumes = False
 

          for mp in self.__instloop.mountOrder:
 

              p = None
 

              for p1 in self.__instloop.partitions:
 

+                 if p1['fstype'] == "btrfs" and not p1['mountpoint'].startswith('/'):
 

+                     # This btrfs filesystem uses subvolumes
 

+                     has_btrfs_subvolumes = True
 

                  if p1['mountpoint'] == mp:
 

                      p = p1
 

                      break
 

- 

+             if p['fstype'] == "btrfs" and not p['mountpoint'].startswith('/'):
 

+                 # There's no mountpoint to export to fstab, so skip
 

+                 continue
 

              if not p['UUID'] is None:
 

-                 mountdev = p['UUID']
 

+                 mountdev = "UUID=%s" % p['UUID']
 

              else:
 

                  mountdev = "LABEL=_%s" % p['mountpoint']
 

-             s +=  "%(mountdev)s  %(mountpoint)s %(fstype)s    defaults,noatime 0 0\n" %  {
 

+             f +=  "%(mountdev)s  %(mountpoint)s %(fstype)s    defaults,noatime 0 0\n" %  {
 

                  'mountdev': mountdev,
 

                  'mountpoint': p['mountpoint'],
 

                  'fstype': p['fstype'] }
 

-         return s
 

+         if has_btrfs_subvolumes:
 

+             for s in self.__instloop.subvolumes:
 

+                 f +=  "%(mountdev)s  %(mountpoint)s %(fstype)s    subvol=%(name)s,noatime 0 0\n" %  {
 

+                     'mountdev': "UUID=%s" % s['UUID'],
 

+                     'mountpoint': s['mountpoint'],
 

+                     'fstype': "btrfs",
 

+                     'name': s['name'] }
 

+         return f
 

  

      def _create_mkinitrd_config(self):
 

          #write  to tell which modules to be included in initrd
 
@@ -282,6 +295,7 @@ 
 

          cfg.close()
 

  

      def _get_grub_boot_config(self):
 

+         btrfsboot = False
 

          bootdevnum = None
 

          rootdevnum = None
 

          rootdev = None
 
@@ -298,8 +312,17 @@ 
 

                  else:
 

                      rootdev = "LABEL=_/"
 

  

+         for s in self.__instloop.subvolumes:
 

+             if s['mountpoint'] == "/boot":
 

+                 btrfsboot = True
 

+                 bootdevnum = s['num'] - 1
 

+ 

+             if s['mountpoint'] == "/":
 

+                 rootdevnum = s['num'] - 1
 

+                 rootdev = s['UUID']
 

+ 

          prefix = ""
 

-         if bootdevnum == rootdevnum:
 

+         if bootdevnum == rootdevnum and not btrfsboot:
 

              prefix = "/boot"
 

  

          return (bootdevnum, rootdevnum, rootdev, prefix)
 
@@ -343,6 +366,7 @@ 
 

          cfg.close()
 

  

      def _get_extlinux_boot_config(self):
 

+         btrfsboot = False
 

          bootdevnum = None
 

          rootdevnum = None
 

          rootdev = None
 
@@ -358,8 +382,16 @@ 
 

                      rootdev = p['UUID']
 

                  else:
 

                      rootdev = "LABEL=_/"
 

+         for s in self.__instloop.subvolumes:
 

+             if s['mountpoint'] == "/boot":
 

+                 btrfsboot = True
 

+                 bootdevnum = s['num'] - 1
 

+ 

+             if s['mountpoint'] == "/":
 

+                 rootdevnum = s['num'] - 1
 

+                 rootdev = s['UUID']
 

          prefix = ""
 

-         if bootdevnum == rootdevnum:
 

+         if bootdevnum == rootdevnum and not btrfsboot:
 

              prefix = "/boot"
 

  

          return (bootdevnum, rootdevnum, rootdev, prefix)

file modified
+12 -5 
@@ -66,7 +66,12 @@ 
 

                                  'num': None}) # Partition number
 

  

      def add_subvolume(self, parent, mountpoint, name):
 

-         self.subvolumes.append({'parent': parent, 'mountpoint': mountpoint, 'name': name})
 

+         self.subvolumes.append({'parent': parent, # parent location for subvolume
 

+                                 'mountpoint': mountpoint, # Mount relative to chroot
 

+                                 'name': name, # subvolume name
 

+                                 'device': None, # kpartx device node for partition
 

+                                 'UUID': None, # UUID for partition
 

+                                 'num': None}) # Partition number
 

  

      def __format_disks(self):
 

          logging.debug("Formatting disks")
 
@@ -299,11 +304,14 @@ 
 

          others = []
 

          ordered = []
 

          for s in self.subvolumes:
 

+             s['device'] = subprocess.Popen(["findmnt", "-n", "-o", "SOURCE", "%s%s" % (self.mountdir, s['parent'])], stdout=subprocess.PIPE).communicate()[0].decode("utf-8").strip()
 

+             s['UUID'] = self.__getuuid(s['device'])
 

+             s['num'] = int(s['device'].rsplit('p', 1)[1])
 

              if s['mountpoint'] == '/':
 

                  ordered.append(s)
 

              else:
 

                  others.append(s)
 

-   

+ 

          ordered += others
 

          for s in ordered:
 

              base = "%s%s" % (self.mountdir, s['parent'])
 
@@ -313,8 +321,7 @@ 
 

              subprocess.call(['btrfs', 'subvol', 'create', path])
 

              subprocess.call(['mkdir', '-p', mountpath])
 

              logging.debug("Mounting Btrfs subvolume %s at path %s" % (s['name'], mountpath))
 

-             device = subprocess.Popen(["findmnt", "-n", "-o", "SOURCE", base], stdout=subprocess.PIPE).communicate()[0].decode("utf-8").strip()
 

-             subprocess.call(['mount', '-t', 'btrfs', '-o', 'subvol=%s' % s['name'], device, mountpath])
 

+             subprocess.call(['mount', '-t', 'btrfs', '-o', 'subvol=%s' % s['name'], s['device'], mountpath])
 

  

      def mount(self):
 

          for dev in list(self.disks.keys()):
 
@@ -391,7 +398,7 @@ 
 

          devdata = subprocess.Popen(["/sbin/blkid", partition], stdout=subprocess.PIPE)
 

          devdataout = devdata.communicate()[0].decode("utf-8").split()
 

          for data in devdataout:
 

-             if data.startswith("UUID"):
 

+             if data.startswith("UUID="):
 

                  UUID = data.replace('"', '')
 

                  continue
 

          return UUID

In order for an image to be setup fully on boot, we need to populate
fstab(5) configuration and ensure the bootloader has the right
references for where the Btrfs filesystems are located on the disk.

rebased onto dfbbb52
3 years ago

@dcavalca @pwhalen Please test this and see if this makes a bootable image.

rebased onto 87eaedb
3 years ago 
[davide@localhost ~]$ sudo /usr/bin/appliance-creator -c koji-image-f33-build-50158384-x86_64.ks --logfile appliance.log --cache  koji-appliance -o app-output --format raw --name Fedora-Xfce-armhfp-33-20200825.n.1 --version 33 --release 20200825.n.1
Create subvolume '/var/tmp/imgcreate-1ck_k6rv/install_rootbtrfs.007/root'
Create subvolume '/var/tmp/imgcreate-1ck_k6rv/install_rootbtrfs.007/home'
Traceback (most recent call last):
  File "/usr/bin/appliance-creator", line 193, in <module>
    sys.exit(main())
  File "/usr/bin/appliance-creator", line 153, in main
    creator.mount("NONE", options.cachedir)
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 573, in mount
    self.__write_fstab()
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 458, in __write_fstab
    fstab.write(self._get_fstab())
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 91, in _get_fstab
    if not p['UUID'] is None:
TypeError: 'NoneType' object is not subscriptable
[davide@localhost ~]$

I think we need to assign p = p1 here as well, or the test in the block below can fail because it will still be None

With that fix it builds, but fails right at the end with

Created symlink /etc/systemd/system/multi-user.target.wants/initial-setup.service → /usr/lib/systemd/system/initial-setup.service.
Traceback (most recent call last):
  File "/usr/bin/appliance-creator", line 193, in <module>
    sys.exit(main())
  File "/usr/bin/appliance-creator", line 155, in main
    creator.configure()
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 799, in configure
    self._create_bootconfig()
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 610, in _create_bootconfig
    self._create_grub_config()
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 330, in _create_grub_config
    (bootdevnum, rootdevnum, rootdev, prefix) = self._get_grub_boot_config()
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 320, in _get_grub_boot_config
    rootdevnum = s['num'] - 1
TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'
lazy umount succeeded on /var/tmp/imgcreate-5epmbj4d/install_root/var/cache/dnf
[davide@localhost ~]$

This comparison always fails because:

p['device'] = /dev/loop33
s['device'] = /dev/mapper/loop3p3

which results in s['num'] never getting initialized.
Edited 3 years ago by dcavalca

Hmm, actually, I think I need the parent loop also skipped in this case...

rebased onto 99fd67b
3 years ago

@dcavalca I think I fixed the patch to do the right thing here...

rebased onto 7d43272
3 years ago

rebased onto 7d43272
3 years ago

Testing the latest:

Writing kickstart file.
Writing GRUB Legacy config.
Traceback (most recent call last):
File "/usr/bin/appliance-creator", line 193, in <module>
sys.exit(main())
File "/usr/bin/appliance-creator", line 155, in main
creator.configure()
File "/usr/lib/python3.8/site-packages/imgcreate/creator.py", line 799, in configure
self._create_bootconfig()
File "/usr/lib/python3.8/site-packages/appcreate/appliance.py", line 611, in _create_bootconfig
self._create_grub_config()
File "/usr/lib/python3.8/site-packages/appcreate/appliance.py", line 331, in _create_grub_config
(bootdevnum, rootdevnum, rootdev, prefix) = self._get_grub_boot_config()
File "/usr/lib/python3.8/site-packages/appcreate/appliance.py", line 321, in _get_grub_boot_config
rootdevnum = s['num'] - 1
TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'

rebased onto 8598a73
3 years ago

@dcavalca @pwhalen Can you take a look again?

Fails early with

Creating mount point /var/tmp/imgcreate-u2bjy8sg/install_rootbtrfs.007
Mounting /dev/loop03 at /var/tmp/imgcreate-u2bjy8sg/install_rootbtrfs.007
Traceback (most recent call last):
  File "/usr/bin/appliance-creator", line 193, in <module>
    sys.exit(main())
  File "/usr/bin/appliance-creator", line 153, in main
    creator.mount("NONE", options.cachedir)
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 537, in mount
    self._mount_instroot(base_on)
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 250, in _mount_instroot
    self.__instloop.mount()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 389, in mount
    self.setup_subvolumes()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 309, in setup_subvolumes
    s['num'] = int(s['device'].rsplit('p', 1))
TypeError: int() argument must be a string, a bytes-like object or a number, not 'list'
Unmounting directory /var/tmp/imgcreate-u2bjy8sg/install_root/home
umount: /var/tmp/imgcreate-u2bjy8sg/install_root/home: no mount point specified.
Unmounting directory /var/tmp/imgcreate-u2bjy8sg/install_root/home failed, using lazy umount
Unmounting directory /var/tmp/imgcreate-u2bjy8sg/install_root/home failed, using lazy umount
umount: /var/tmp/imgcreate-u2bjy8sg/install_root/home: no mount point specified.
Unmounting directory /var/tmp/imgcreate-u2bjy8sg/install_root/
umount: /var/tmp/imgcreate-u2bjy8sg/install_root/: not mounted.
Unmounting directory /var/tmp/imgcreate-u2bjy8sg/install_root/ failed, using lazy umount
Unmounting directory /var/tmp/imgcreate-u2bjy8sg/install_root/ failed, using lazy umount
umount: /var/tmp/imgcreate-u2bjy8sg/install_root/: not mounted.
Removing compat symlinks
Unmapping /dev/loop0
Exception ignored in: <function ImageCreator.__del__ at 0x7fdbd9772310>
Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 114, in __del__
    self.cleanup()
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 613, in cleanup
    self.unmount()
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 587, in unmount
    self._unmount_instroot()
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 638, in _unmount_instroot
    self.__instloop.cleanup()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 257, in cleanup
    self.__unmap_partitions()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 225, in __unmap_partitions
    raise MountError("Failed to unmap partitions for '%s'" %
imgcreate.errors.MountError: Failed to unmap partitions for '/dev/loop0'
[davide@localhost ~]$

rebased onto 3856725
3 years ago

@dcavalca @pwhalen So I was dumb and forgot to type in the index thingy, I just pushed an updated patch that has that.

Fails in the same spot still, but differently so:

Creating mount point /var/tmp/imgcreate-4muoovjo/install_rootbtrfs.007
Mounting /dev/loop33 at /var/tmp/imgcreate-4muoovjo/install_rootbtrfs.007
Traceback (most recent call last):
  File "/usr/bin/appliance-creator", line 193, in <module>
    sys.exit(main())
  File "/usr/bin/appliance-creator", line 153, in main
    creator.mount("NONE", options.cachedir)
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 537, in mount
    self._mount_instroot(base_on)
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 250, in _mount_instroot
    self.__instloop.mount()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 389, in mount
    self.setup_subvolumes()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 309, in setup_subvolumes
    s['num'] = int(s['device'].rsplit('p', 1)[0])
ValueError: invalid literal for int() with base 10: '/dev/mapper/loop3'
Unmounting directory /var/tmp/imgcreate-4muoovjo/install_root/home
umount: /var/tmp/imgcreate-4muoovjo/install_root/home: no mount point specified.
Unmounting directory /var/tmp/imgcreate-4muoovjo/install_root/home failed, using lazy umount
Unmounting directory /var/tmp/imgcreate-4muoovjo/install_root/home failed, using lazy umount
umount: /var/tmp/imgcreate-4muoovjo/install_root/home: no mount point specified.
Unmounting directory /var/tmp/imgcreate-4muoovjo/install_root/
umount: /var/tmp/imgcreate-4muoovjo/install_root/: not mounted.
Unmounting directory /var/tmp/imgcreate-4muoovjo/install_root/ failed, using lazy umount
Unmounting directory /var/tmp/imgcreate-4muoovjo/install_root/ failed, using lazy umount
umount: /var/tmp/imgcreate-4muoovjo/install_root/: not mounted.
Removing compat symlinks
Unmapping /dev/loop3
Exception ignored in: <function ImageCreator.__del__ at 0x7f15eeb8c310>
Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 114, in __del__
    self.cleanup()
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 613, in cleanup
    self.unmount()
  File "/usr/lib/python3.9/site-packages/imgcreate/creator.py", line 587, in unmount
    self._unmount_instroot()
  File "/usr/lib/python3.9/site-packages/appcreate/appliance.py", line 638, in _unmount_instroot
    self.__instloop.cleanup()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 257, in cleanup
    self.__unmap_partitions()
  File "/usr/lib/python3.9/site-packages/appcreate/partitionedfs.py", line 225, in __unmap_partitions
    raise MountError("Failed to unmap partitions for '%s'" %
imgcreate.errors.MountError: Failed to unmap partitions for '/dev/loop3'
[davide@localhost ~]$

rebased onto 27119bc
3 years ago

@dcavalca @pwhalen I just fixed it. Whoops! That's me not understanding rsplit()...

Ok it now builds and completes, the partition layout is correct, but I can't seem to be able to get it to spit out a bootloader config when building for x86_64, and I can't build for armhf due to https://pagure.io/fedora-btrfs/project/issue/11

As noted above, builds fine. Attempting to boot failed. Checking the kernel parameters in the extlinux.conf:

"append ro root=UUID_SUB=6063995c-7541-4f1c-8e22-eb52892c7122 rhgb quiet LANG=en_US.UTF-8 rootflags=subvol=root cma=192MB"

Using UUID_SUB rather than UUID, blkid output:
/dev/sdb3: LABEL="_btrfs.007" UUID="ee019b9b-5e48-4574-a85e-17eda0378bc3" UUID_SUB="6063995c-7541-4f1c-8e22-eb52892c7122" BLOCK_SIZE="4096" TYPE="btrfs" PARTUUID="539d5ee5-03"
Edited 3 years ago by pwhalen

rebased onto 4d7c81f
3 years ago

1 new commit added

  • appcreate/partitionedfs: Ensure __getuuid() actually gets the UUID
3 years ago

@pwhalen I believe I've fixed the parameter with this refreshed PR. Could you please try again?

Pull-Request has been merged by ngompa
3 years ago

In todays Rawhide images created with appliance-tools-011.0-1.fc34:

The good:
cat extlinux.conf:
..
append ro root=UUID=8a8b2744-fabb-43fc-bf31-766214ba7afd rhgb quiet LANG=en_US.UTF-8

The bad:
cat etc/fstab
UUID=UUID=8a8b2744-fabb-43fc-bf31-766214ba7afd / ext4 defaults,noatime 0 0
UUID=UUID=78e0388c-8f65-414f-ab33-2919e4714fb8 /boot ext4 defaults,noatime 0 0
UUID=UUID=E803-6385 /boot/efi vfat defaults,noatime 0 0

The above is from Fedora-Minimal-armhfp-Rawhide-20200827.n.0.

Oh man. :sob:

Fixed that in 441205f and released 011.1.
Metadata
None
No Tags
Changes Summary 2
+39 -7
file changed
appcreate/appliance.py
+12 -5
file changed
appcreate/partitionedfs.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.