#1106 Issue `closed`: Extended partition not detected with util-linux 2.26-rc1¶

Labels: bug, fixed / solved / done

gozora opened issue at 2016-12-05 21:05:¶

Relax-and-Recover (rear) Issue Template¶

Please fill in the following items before submitting a new issue (quick response is not guaranteed with free support):

rear version (/usr/sbin/rear -V):1.19
OS version (cat /etc/rear/os.conf or lsb_release -a): Ubuntu 16.04.1 LTS
rear configuration files (cat /etc/rear/site.conf or cat /etc/rear/local.conf): N/A
Brief description of the issue:
Since util-linux 2.26-rc1 sfdisk -c is deprecated in favour of --part-type which might make code in usr/share/rear/layout/save/GNU/Linux/200_partition_layout.sh misbehave.

c. f.

commit 8eab3194ce1737a167812d5e84d83b0dfc253fac
Author: Karel Zak <kzak@redhat.com>
Date:   Mon Sep 15 12:37:52 2014 +0200

    sfdisk: add --parttype

    The patch also makes --{id,change-id,print-id} deprecated in favour
    of --parttype. The original --id is too generic option name and the
    --print-id and --change-id are unnecessary and inconsistent with
    another sfdisk options (e.g. we don't have --change-bootable)

New output from sfdisk -c /dev/sda 3 looks as follows:

sfdisk: --id is deprecated in favour of --part-type
 5

Warning message "sfdisk: --id is deprecated in favour of --part-type" is not that bad (as it was returned on stderr), I however had trouble with returned partition type prefixed with space (' ') which made following condition to evaluate incorrectly:

if [[ "$partition_id" = 5 || "$partition_id" = f || "$partition_id" = 85 ]]; then
   sed -i /^$partition_nr\ /s/\ primary\ /\ extended\ / $TMP_DIR/partitions
fi

As a result extended partition was not detected correctly which resulted to failed partitioning during rear recover

Work-around, if any
I've temporarily fixed code as follows:

if [[ "$partition_id" = 5 || "$partition_id" = f || "$partition_id" = 85 || "$partition_id" = " 5" ]]; then
   sed -i /^$partition_nr\ /s/\ primary\ /\ extended\ / $TMP_DIR/partitions
fi

But for some reason I don't like it that much ...

jsmeix commented at 2016-12-06 08:21:¶

In general:
I never had a detailed look at the partitioning code
but from my little experience with it (mainly when
I worked on that SUSE sepecific 'gpt_sync_mbr' stuff)
I think the whole partitioning code is fragile.
When something does not work during "rear mkbackup"
it blindly proceeds (no 'set -e') and one gets wrong
data in disklayout.conf (e.g. missing entries) and
then it fails at a random place during "rear recover".

In particular regarding 'sfdisk':
On my openSUSE Leap 42.1 system "sfdisk --version"
shows "sfdisk from util-linux 2.25" and "man sfdisk" reads:

sfdisk doesn't understand the GUID Partition Table
(GPT) format and it is not designed for large partitions.
In these cases use the more advanced GNU parted(8).

If sfdisk from util-linux 2.26 does also not support GPT
I think we should try to get rid of sfdisk in ReaR because
tools that do not support GPT and large partitions
will cause and endless sequence of more and more
problems in ReaR in the future - until we get rid of
such tools.

I don't know if
https://github.com/rear/rear/issues/1105
is directly related but at least it is about huge disks that
requires tools that support GPT and large partitions.

gozora commented at 2016-12-06 08:50:¶

Hello @jsmeix ,
Actually this bug can hit you ONLY if you are using extended partitions, so I really don't think that many users will be affected by it.

#1105 is IMHO not related to this.

V.

jsmeix commented at 2016-12-06 08:55:¶

Regarding a quick workaround that makes it work for now
until the general sfdisk issue is cleaned up:

What about making the output safe against intermixed
unwanted characters and - by the way - fix the deprecated '&8'
usage, cf. https://github.com/rear/rear/issues/887

declare partition_id=$( sfdisk -c $device $partition_nr 2>/dev/null | tr -c -d '[:alnum:]' )

Another by the way:
I wonder why there is 'declare' here?
I do not understand why it is not simply

partition_id=$( ... )

jsmeix commented at 2016-12-06 09:04:¶

I meant that in general as long as ReaR uses partitioning
tools that do not support GPT and large partitions
in general users would be hit by this or that shortcomings.

This particular issue is only about extended partitions
which - as far as I know - do not exists for GPT
(or are at least not supported by GPT).

gozora commented at 2016-12-06 09:11:¶

I was thinking about using tr as well it is certainly nicer comparing to what I've used.

Another by the way:
I wonder why there is 'declare' here?
I do not understand why it is not simply

Exactly my thoughts ...

jsmeix commented at 2016-12-06 09:23:¶

@gozora
because you can test what fix actually makes it work,
I will basically blindly accept your pull request.

gozora commented at 2016-12-06 09:25:¶

@jsmeix , sure I'll prepare it.

jsmeix commented at 2016-12-06 09:36:¶

Regarding a general cleanup of "sfdisk" I submitted
https://github.com/rear/rear/issues/1107

jsmeix commented at 2016-12-07 10:59:¶

With
https://github.com/rear/rear/issues/1109
merged plus
https://github.com/rear/rear/commit/56bb14793cf655d67e10a53baf3d00ff54df7e7f
I consider this issue to be fixed.

[Export of Github issue for rear/rear.]

#1106 Issue closed: Extended partition not detected with util-linux 2.26-rc1¶