Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Skip invalid disk drives (zero sized, no media) when saving layout #3047

Merged
merged 4 commits into from Sep 13, 2023
Merged

Skip invalid disk drives (zero sized, no media) when saving layout #3047

merged 4 commits into from Sep 13, 2023

Conversation

pcahyna
Copy link
Member

@pcahyna pcahyna commented Sep 11, 2023

Pull Request Details:

  • Type: Enhancement

  • Impact: Normal

  • Reference to related issue (URL): fixes ReaR fails while verifying the disklayout when a disk has a 0 size #2958

  • How was this pull request tested?

    • rear savelayout on Fedora 36 with empty USB SD card reader
      I show the relevant log excerpt and the corresponding part of disklayout.conf, if any.
      • original code: 
        No partition label type for 'disk /dev/sdb' (may cause 'rear recover' failure)

        # Disk /dev/sdb
# Format: disk <devname> <size(bytes)> <partition label type>
#disk /dev/sdb 0
      • new code: 
        Ignoring sdb: blockdev: cannot open /dev/sdb: No medium found
        No sdb in disklayout
      • new code without reverting 0a1d634
        same result
      • original code only with 0a1d634 reverted
        This reproduces Empty SD card slot appears as /dev/sdb disk without partition type #2810 
        ERROR: Invalid 'disk /dev/sdb' entry (no partition table type for '/dev/sdb')
Some latest log messages since the last called script 200_partition_layout.sh:
  2023-06-20 19:23:08.249840221 Saving disks and their partitions
Some messages from /var/tmp/rear.ZC1Unp4TNvGyfNj/tmp/rear.savelayout.stdout_stderr since the last called script 200_partition_layout.sh:
  blockdev: cannot open /dev/sdb: No medium found
  Error: Error opening /dev/sdb: No medium found
Use debug mode '-d' for some debug messages or debugscript mode '-D' for full debug messages with 'set -x' output
Aborting due to an error, check /home/pcahyna/rear/rear/var/log/rear/rear-pacaziste.log for details
Terminated
    • rear savelayout on Fedora 36 with empty USB SD card reader and AUTOEXCLUDE_DISKS=n
      • original code: 
        No partition label type for 'disk /dev/sdb' (may cause 'rear recover' failure)
/dev/sdb size 0 is not a positive integer
ERROR: 
====================
BUG in /home/pcahyna/rear/rear/usr/share/rear/layout/save/default/950_verify_disklayout_file.sh line 257:
'Entries in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf are broken ('rear recover' would fail)'
--------------------
Please report it at https://github.com/rear/rear/issues
and include all related parts from /home/pcahyna/rear/rear/var/log/rear/rear-pacaziste.log
preferably the whole debug information via 'rear -D savelayout'
====================
Some latest log messages since the last called script 950_verify_disklayout_file.sh:
  2023-06-20 19:16:28.417175129 Verifying that the entries in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf are correct
  2023-06-20 19:16:28.424548585 Verifying that the 'disk' entries in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf are correct
  2023-06-20 19:16:28.436529921 Verifying that the 'part' entries for /dev/sda in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf are correct
  2023-06-20 19:16:28.446834398 Verifying that the 'part' entries for /dev/sda in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf specify consecutive partitions
  2023-06-20 19:16:28.458051434 Verifying that the 'part' entries for /dev/sdb in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf are correct
  2023-06-20 19:16:28.468396791 Verifying that the 'part' entries for /dev/sdb in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf specify consecutive partitions
  2023-06-20 19:16:28.473731880 Verifying that the 'lvm...' entries in /home/pcahyna/rear/rear/var/lib/rear/layout/disklayout.conf are correct
  2023-06-20 19:16:28.506275876 /dev/sdb size 0 is not a positive integer
Some messages from /var/tmp/rear.dSRYZZLoczSVake/tmp/rear.savelayout.stdout_stderr since the last called script 950_verify_disklayout_file.sh:
  256060514304
  1073741824
  1048576
  254985371648
  1074790400
  0
Use debug mode '-d' for some debug messages or debugscript mode '-D' for full debug messages with 'set -x' output
Aborting due to an error, check /home/pcahyna/rear/rear/var/log/rear/rear-pacaziste.log for details
Terminated

        # Disk /dev/sdb
# Format: disk <devname> <size(bytes)> <partition label type>
disk /dev/sdb 0 
# Partitions on /dev/sdb
        This reproduces ReaR fails while verifying the disklayout when a disk has a 0 size #2958
      • new code: 
        Ignoring sdb: blockdev: cannot open /dev/sdb: No medium found
        No sdb in disklayout
      • new code without reverting 0a1d634
        same result
    • full backup and recovery on RHEL 8 with empty USB SD card reader 
      Running 'layout/save' stage ======================
Creating disk layout
Ignoring sde: blockdev: cannot open /dev/sde: No medium found
Automatically excluding disk /dev/sdb (not used by any mounted filesystem)
Marking component '/dev/sdb' as done in /var/lib/rear/layout/disktodo.conf
Automatically excluding disk /dev/sdc (not used by any mounted filesystem)
Marking component '/dev/sdc' as done in /var/lib/rear/layout/disktodo.conf
Automatically excluding disk /dev/sdd (not used by any mounted filesystem)
Marking component '/dev/sdd' as done in /var/lib/rear/layout/disktodo.conf
Disabling excluded components in /var/lib/rear/layout/disklayout.conf
Disabling component 'disk /dev/sdb' in /var/lib/rear/layout/disklayout.conf
Disabling component 'disk /dev/sdc' in /var/lib/rear/layout/disklayout.conf
Disabling component 'disk /dev/sdd' in /var/lib/rear/layout/disklayout.conf
    • full backup and recovery on RHEL 8 with empty USB SD card reader and AUTOEXCLUDE_DISKS=n 
      Running 'layout/save' stage ======================
Creating disk layout
Ignoring sde: blockdev: cannot open /dev/sde: No medium found
Disabling excluded components in /var/lib/rear/layout/disklayout.conf

  • Brief description of the changes in this pull request:

    • Introduce function is_disk_valid, which performs checks for disk usability beyond validating the device name, and use it when saving the disk layout. In some cases the device name may be valid, but there are no data, typically because it is a drive with removable media and there is no medium in the tray. Happens typically with card (e.g. SD card) readers with empty slot.
      This is a normal occurrence, so do not Error out, only display a message and skip the device.
    • Revert commit 0a1d634 . We now skip disks with no data (like when there is no medium), so incomplete disk entries (without partition type) should not occur anymore. Restore the code that aborted when such disks were encountered.
      Incomplete entries should not be allowed to occur, as they could confuse the layout restoration code. Moreover, the layout restoration wipes all disks in the layout, so if during layout restoration there happens to be a medium in the drive that was empty during layout save, the data on the medium would get overwritten and lost. And if there is not medium, the layout recreation script would fail.
      See the discussion at ReaR fails while verifying the disklayout when a disk has a 0 size #2958 (comment)

pcahyna referenced this pull request Sep 11, 2023
@jsmeix
Update 200_partition_layout.sh
0a1d634 
In layout/save/GNU/Linux/200_partition_layout.sh
do not error out when there is no partition label type value
for a 'disk' entry in disklayout.conf because "rear recover" works
in a special case without partition label type value when there is
only a 'disk' entry but nothing else for this disk exists in disklayout.conf
which can happen when /dev/sdX is an empty SD card slot without medium,
see #2810
@pcahyna pcahyna requested a review from a team September 11, 2023 15:28
@jsmeix jsmeix added enhancement Adaptions and new features cleanup labels Sep 12, 2023
@jsmeix jsmeix added this to the ReaR v2.8 milestone Sep 12, 2023
@jsmeix
Copy link
Member

jsmeix commented Sep 12, 2023

@pcahyna
thank you for this enhancement and
the cleanup of the old clumsy code
and for your explanatory description
of the various cases (in particular
things like the special DASD case)!

@pcahyna
Copy link
Member Author

pcahyna commented Sep 12, 2023

@jsmeix I believe I addressed all review comments in last 3 commits.

@jsmeix
Copy link
Member

jsmeix commented Sep 13, 2023

@pcahyna
thank you for the fine tuning.
All looks perfect to me.
Feel free to merge it as you like.

@pcahyna
Move "parted $devname" after validation of $devname
3ba1f96 
This prevents errors when parted is called on an invalid device.
@pcahyna
Validate disk when saving layout
808b15a 
Introduce function is_disk_valid, which performs checks for disk
usability beyond validating the device name. In some cases the device
name may be valid, but there are no data, typically because it is a
drive with removable media and there is no medium in the tray. Happens
typically with card (e.g. SD card) readers with empty slot.

This is a normal occurrence, so do not Error out, only display a message
and skip the device.
@pcahyna
Revert "Update 200_partition_layout.sh"
cfe34a4 
This reverts commit 0a1d634.

We now skip disks with no data (like when there is no medium), so
incomplete disk entries (without partition type) should not occur
anymore. Restore the code that aborted when such disks were encountered.
Incomplete entries should not be allowed to occur, as they could confuse
the layout restoration code. Moreover, the layout restoration wipes
all disks in the layout, so if during layout restoration there happens
to be a medium in the drive that was empty during layout save, the data
on the medium would get overwritten and lost. And if there is not medium,
the layout recreation script would fail.

See the discussion at rear#2958 (comment)
@pcahyna
Explain what a disk validation error means
db191aa
@pcahyna pcahyna merged commit c08658d into rear:master Sep 13, 2023
15 checks passed
@pcahyna pcahyna mentioned this pull request Sep 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsmeix jsmeix jsmeix approved these changes

Assignees

@pcahyna pcahyna

Labels
cleanup enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Milestone
ReaR v2.8
Development

Successfully merging this pull request may close these issues.

ReaR fails while verifying the disklayout when a disk has a 0 size
2 participants
@pcahyna @jsmeix