System state with 'space' in name prevent update-grub to create menu entries in grub.cfg

Bug #1903524 reported by Jean-Baptiste Lallement
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
grub2 (Ubuntu)
Fix Released
Medium
Jean-Baptiste Lallement
Focal
Triaged
Medium
Jean-Baptiste Lallement
Groovy
Won't Fix
Medium
Jean-Baptiste Lallement
grubzfs-testsuite (Ubuntu)
Fix Released
Undecided
Unassigned
zsys (Ubuntu)
Fix Released
Medium
Jean-Baptiste Lallement
Focal
Triaged
Medium
Jean-Baptiste Lallement
Groovy
Won't Fix
Medium
Jean-Baptiste Lallement

Bug Description

[Impact]
Some characters are not supported in snapshot names by either zfs, grub or init. As a consequence a system cannot be reverted to a snapshot created with such characters, and the system fails to boot with various errors.

This patch forces the name of snapshot created with zsysctl to the following characters: a-z, A-Z, 0-9, _, -, .

Space is not unsupported by ZFS and would be perfectly correct to create a snapshot with a space in it. However it creates issues in GRUB when it generates the boot command line due to the way it quotes the argument root= by adding a double before root which is then wrongly interpreted by init. It also creates issues in init which uses for loop to read the command line and splits it on spaces, and in the middle of the root argument while it should be a single argument. Fixing space handling in GRUB and init is really intrusive and beyond the scope of this bug so we decided to prevent spaces too in snapshot names.

[Test Case]
1. Boot a system with zsys without the patch

* Positive test *
2. Create a system snapshot with a valid character. Valid characters are "a-z", "A-Z", "0-9", "_", "-", "." Name must not start by . or -
=> Verify that the snapshot is created successfully
3. Reboot to the grub menu and in the history menu select the snapshot you just created.
=> Verify that the system boot to the reverted state of the snapshot

* Negative test 1*
4. Create a snapshot with an invalid character.
=> Verify that the creation fails with an error message listing the invalid character/s

* Negative test 2*
5. Create a snapshot that starts with '-'
=> Verify that the snapshot cannot be created.

* Negative test 3*
5. Create a snapshot names '.' or '..'
=> Verify that the snapshot cannot be created.

[Regression potential]
If filtering is not done properly the patch can either:
1. prevent supported characters to be used in snapshot names but snapshots can still be created and names generated by auto snapshots are supported.
2. allow unsupported characters to be used in snapshot names. In this case we are back to current situation.

The patch is covered by several tests in the included test suite.

[Original Description]
From https://github.com/ubuntu/zsys/issues/169

Describe the bug
I did create few days ago a System state with a 'space' in the name : "pool/ROOT/ubuntu_u7dn3s@Stable Plex"

server:~$ zsysctl show
Name: rpool/ROOT/ubuntu_u7dn3s
ZSys: true
Last Used: current
History:
  - Name: rpool/ROOT/ubuntu_u7dn3s@autozsys_ceqh6a
    Created on: 2020-10-16 06:49:59
  - Name: rpool/ROOT/ubuntu_u7dn3s@autozsys_ahzb7l
    Created on: 2020-10-15 06:40:13
  - Name: rpool/ROOT/ubuntu_u7dn3s@autozsys_a00j2k
    Created on: 2020-10-08 06:46:34
   - Name: rpool/ROOT/ubuntu_u7dn3s@autozsys_b7t6lj
     Created on: 2020-10-07 06:20:00
   - Name: rpool/ROOT/ubuntu_u7dn3s@Stable Plex
     Created on: 2020-10-03 19:18:18
   - Name: rpool/ROOT/ubuntu_u7dn3s@autozsys_a81mbp
     Created on: 2020-10-03 17:58:19
(truncated)...
I found myself in the impossibility to boot the server few days after as the Grub menu had only the UEFI entry in it. No more entries for current kernel or zsys history.

server:~$ cat /boot/grub/grub.cfg
...(truncated)
### END /etc/grub.d/20_linux_xen ###

### BEGIN /etc/grub.d/30_os-prober ###
### END /etc/grub.d/30_os-prober ###

### BEGIN /etc/grub.d/30_uefi-firmware ###
menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
 fwsetup
}
### END /etc/grub.d/30_uefi-firmware ###

### BEGIN /etc/grub.d/40_custom ###
# This file provides an easy way to add custom menu entries. Simply type the
# menu entries you want to add after this comment. Be careful not to change
# the 'exec tail' line above.
### END /etc/grub.d/40_custom ###
(truncated)...
After chrooting into the server, I tried to rebuild the menu with update-grub and got a weird error message on ignoring datasets:

server:~$ update-grub
Sourcing file `/etc/default/grub'
Sourcing file `/etc/default/grub.d/init-select.cfg'
Generating grub configuration file ...
Found linux image: vmlinuz-5.4.0-48-generic in rpool/ROOT/ubuntu_25bi0j
Found initrd image: initrd.img-5.4.0-48-generic in rpool/ROOT/ubuntu_25bi0j
Found linux image: vmlinuz-5.4.0-42-generic in rpool/ROOT/ubuntu_25bi0j
Found initrd image: initrd.img-5.4.0-42-generic in rpool/ROOT/ubuntu_25bi0j
...(truncated)...
Found initrd image: initrd.img-5.4.0-42-generic in rpool/ROOT/ubuntu_u7dn3s@autozsys_a81mbp
Warning: Failed to find a valid directory 'etc' for dataset 'rpool/ROOT/ubuntu_u7dn3s@Stable'. Ignoring
Warning: Ignoring rpool/ROOT/ubuntu_u7dn3s@Stable
filesystem 'Plex' cannot be mounted, unable to open the dataset
Adding boot menu entry for UEFI Firmware Settings
done
As a result the grub.cfg file was letf with only the UEFI entry.

To solve this issue, I had to delete the problematic state and reran update-grub. I then was back to normal.

server:~$ zsysctl state remove 'Stable Plex' --system
ZSys is adding automatic system snapshot to GRUB menu

server:~$ update-grub
...(truncated)...

server:~$ cat /boot/grub/grub.cfg
...(truncated)
 menuentry '* Ubuntu 20.04.1 LTS, with Linux 5.4.0-48-generic (recovery mode)' --class ubuntu --class gnu-linux --class gnu --class os ${menuentry_id_option} 'gnulinux-rpool/ROOT/ubuntu_u7dn3s-5.4.0-48-generic' {
  recordfail
  load_video
  insmod gzio
  if [ "${grub_platform}" = xen ]; then insmod xzio; insmod lzopio; fi
  insmod part_gpt
  insmod zfs
  if [ x$feature_platform_search_hint = xy ]; then
    search --no-floppy --fs-uuid --set=root 9976c549b4d44f87
  else
    search --no-floppy --fs-uuid --set=root 9976c549b4d44f87
  fi
  echo Loading Linux 5.4.0-48-generic ...
  linux /BOOT/ubuntu_u7dn3s@/vmlinuz-5.4.0-48-generic root=ZFS=rpool/ROOT/ubuntu_u7dn3s ro recovery nomodeset dis_ucode_ldr
  echo 'Loading initial ramdisk ...'
  initrd /BOOT/ubuntu_u7dn3s@/initrd.img-5.4.0-48-generic
 }
}
submenu 'History for Ubuntu 20.04.1 LTS' ${menuentry_id_option} 'gnulinux-history-rpool/ROOT/ubuntu_u7dn3s' {
 submenu 'Revert to 10/16/2020 @ 06:49' ${menuentry_id_option} 'gnulinux-history-rpool/ROOT/ubuntu_u7dn3s@autozsys_ceqh6a' {
(truncated)...
To Reproduce

Create a system state with a 'space' in the name
Run update-grub and check the warning message at the end of the process
Check /boot/grub/grub.cfg file for missing Grub menu entries
Expected behavior
A grub.cfg file with all necessary menu entries.
zsysctl should forbid to create state with space in name or zsys update-grub helper should be able to handle states' names with spaces in it.

For ubuntu users, please run and copy the following:

ubuntu-bug zsys --save=/tmp/report
Copy paste below /tmp/report content:
File is too big. Will put it in comment.
Installed versions:

OS: (/etc/os-release)
NAME="Ubuntu"
VERSION="20.04.1 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.1 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal
Zsysd running version: (zsysctl version output)
zsysctl 0.4.7
zsysd 0.4.7
Additional context
It's a Ubuntu Server flavor, booting on ZFS with Zsys and installed with the following procedure: https://openzfs.github.io

Changed in grub2 (Ubuntu):
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in grub2 (Ubuntu Focal):
status: New → Triaged
Changed in grub2 (Ubuntu Groovy):
status: New → Triaged
Changed in grub2 (Ubuntu Focal):
importance: Undecided → Medium
Changed in grub2 (Ubuntu Groovy):
importance: Undecided → Medium
Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

Adding zsys and removing grubzfs-testsuite. We now block invalid characters in zsys from state name when a user creates a system or user snapshot. Space is not really an unsupported character but it causes problems in grub itself and init so we exclude it from the list of valid characters.

Changed in zsys (Ubuntu):
status: New → In Progress
assignee: nobody → Jean-Baptiste Lallement (jibel)
importance: Undecided → Medium
Changed in zsys (Ubuntu Focal):
status: New → Triaged
importance: Undecided → Medium
Changed in zsys (Ubuntu Groovy):
status: New → Triaged
importance: Undecided → Medium
no longer affects: grubzfs-testsuite (Ubuntu Groovy)
no longer affects: grubzfs-testsuite (Ubuntu Focal)
no longer affects: grubzfs-testsuite (Ubuntu)
Changed in grub2 (Ubuntu Focal):
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in grub2 (Ubuntu Groovy):
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in zsys (Ubuntu Focal):
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in zsys (Ubuntu Groovy):
assignee: nobody → Jean-Baptiste Lallement (jibel)
Changed in zsys (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package zsys - 0.5.3

---------------
zsys (0.5.3) hirsute; urgency=medium

  [ Jean-Baptiste Lallement ]
  [ Didier Roche ]
  * Prevent usage of forbidden characters (starting with -, spaces…) for state
    names (LP: #1903524)

 -- Didier Roche <email address hidden> Tue, 17 Nov 2020 10:27:47 +0100

Changed in zsys (Ubuntu):
status: Fix Committed → Fix Released
description: updated
description: updated
description: updated
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package grubzfs-testsuite - 0.4.14

---------------
grubzfs-testsuite (0.4.14) hirsute; urgency=medium

  * Revert "Add testcases to cover names with spaces and dashes", as
    matching grub upload is not in hirsute.

grubzfs-testsuite (0.4.13) hirsute; urgency=medium

  * Switch to upstream branch of go-libzfs with 2.0.x support.

grubzfs-testsuite (0.4.12) hirsute; urgency=medium

  [ Jean-Baptiste Lallement ]
  [ Didier Roche ]
  * Added test cases to cover snapshot names with spaces or dashes
    (LP: #1898177, #1903524)

 -- Dimitri John Ledkov <email address hidden> Fri, 05 Feb 2021 19:18:25 +0000

Changed in grubzfs-testsuite (Ubuntu):
status: New → Fix Released
Changed in grubzfs-testsuite (Ubuntu):
status: Fix Released → Triaged
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package grub2 - 2.04-1ubuntu40

---------------
grub2 (2.04-1ubuntu40) hirsute; urgency=medium

  * Revert: rhboot-f34-tcp-add-window-scaling-support.patch,
    rhboot-f34-support-non-ethernet.patch,
    ubuntu-fixup-rhboot-f34-support-non-ethernet.patch,
    ubuntu-fixup-rhboot-f34-support-non-ethernet-2.patch: these break MAAS
    LXD KVM pod deployments. LP: #1915288

grub2 (2.04-1ubuntu39) hirsute; urgency=medium

  * Cherrypick a bunch of patches:
    - fix crash in http LP: #1915288
    - add bootp6 documentation
    - add support for UEFI boot protocols
    - use UEFI protocols for http & https networking
    - make netboot search for by-mac/by-uuid/by-ip for grub.cfg
    - update documentation for netboot search paths of grub.cfg
  * Make prebuilt netboot image look for MAAS grub.cfg
  * Fix grub-initrd-fallback.service thanks to JawnSmith LP: #1910815

grub2 (2.04-1ubuntu38) hirsute; urgency=medium

  [ Jean-Baptiste Lallement ]
  [ Didier Roche ]
  * Fix warnings during grub menu generation. Thanks wdoekes for the patch
    (LP: #1898177)
    - Fix warnings when bpool doesn't exist.
    - Fix warnings when snapshot name contains dashes.
  * Do not fail to generate grub menu when name of the snapshot contains
    spaces. (LP: #1903524)

 -- Dimitri John Ledkov <email address hidden> Fri, 12 Feb 2021 20:29:16 +0000

Changed in grub2 (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package grubzfs-testsuite - 0.4.15build1

---------------
grubzfs-testsuite (0.4.15build1) hirsute; urgency=medium

  * No change rebuild with fixed ownership.

 -- Dimitri John Ledkov <email address hidden> Tue, 16 Feb 2021 15:15:19 +0000

Changed in grubzfs-testsuite (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote :

The Groovy Gorilla has reached end of life, so this bug will not be fixed for that release

Changed in grub2 (Ubuntu Groovy):
status: Triaged → Won't Fix
Changed in zsys (Ubuntu Groovy):
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.