[parted-devel] [PATCH 1/2] gpt_read: reintroduce restoring the primary/backup GPT when corruption

[Hatayama, Daisuke]

Hi,

Do you have any comments for the following 2-length patch set I posted
last week?

    https://alioth-lists.debian.net/pipermail/parted-devel/2018-November/005328.html
    https://alioth-lists.debian.net/pipermail/parted-devel/2018-November/005331.html

The patch set deals with the issue that there is no feature of
recovery of the primary/backup GPTs when their corruption is detected.

I think this is a REGRESSION issue because there had been the feature
until it was removed in the following commit:

    http://git.savannah.gnu.org/cgit/parted.git/commit/?id=432a33115c50005bbe96a09d55edc7d034715ec8

I found where the corresponding patch was posted in the parted-devel ML archives:

    https://alioth-lists.debian.net/pipermail/parted-devel/2010-February/003403.html

but it appears that there's no discussion that remains there. So, I
have no idea about the background of the change more than the
description of the commit 432a33115c50005bbe96a09d55edc7d034715ec8.

I also found the following commit that made recovery of the backup GPT
not run in the non-interactive mode, similar to what the patch set of mine
is trying to do:

    https://alioth-lists.debian.net/pipermail/parted-devel/2009-February/002610.html

    # git log --grep="suspicious GPT"
    commit d7d9f2c40c6bb3e83a7e6ea3164b4f4eb7440a47
    Author: Petr Uzel <petr.uzel at suse.cz>
    Date:   Fri Feb 13 13:27:55 2009 +0100

	gpt: do not automatically "correct" a suspicious GPT partition table

	Previously, when parted was invoked on a disk with a GPT partition table
	and the backup GPT was not in the last sector of the disk, and even if
	the requested operation was just to print the partition table, parted
	would "repair" this automatically. This behavior is undesirable in the
	following situation:

	dm-raid on top of block device. The dm-raid is partitioned with GPT. If
	the dm-raid starts on the first block of underlying device (AFAIK this is
	the case with FastTrack controllers) and the user runs parted on the
	dm-raid, it will identify the physical device as being partitioned with
	GPT and see the backup GPT table not to be in the last sector of the
	physical device and thus move it to this location (which may lead to
	destruction of dm-raid metadata in case they are located at the end of
	physical device).

	This patch modifies parted's behavior to ignore fixing of backup GPT
	position by default.

So, I guess the policy that the recovery of the GPTs should not be
fixed automatically in the interactive mode has been accepted and
shared among the parted-devel guys, is this right?

If so, I guess (and want to expect) that the patch set of mine is also
be accepted.

Thanks.
HATAYAMA, Daisuke

> -----Original Message-----
> From: parted-devel
> [mailto:parted-devel-bounces+d.hatayama=jp.fujitsu.com at alioth-lists.debian
> .net] On Behalf Of Hatayama, Daisuke
> Sent: Wednesday, November 7, 2018 3:55 PM
> To: 'parted-devel at lists.alioth.debian.org'
> <parted-devel at lists.alioth.debian.org>
> Subject: [parted-devel] [PATCH 1/2] gpt_read: reintroduce restoring the
> primary/backup GPT when corruption
> 
> Currently, no primary/backup GPT is restored when one of them is
> corrupted due to the commit 432a33115c50005bbe96a09d55edc7d034715ec8.
> 
> According to the description of the commit, the purpose of the commit
> is to deal with corruption of pMBR due to another issue during
> clobbering, but there seems no such issue because now
> ped_disk_clobber() avoids pMBR to clobber.
> 
> This is essentially a revert of the commit
> 432a33115c50005bbe96a09d55edc7d034715ec8.
> ---
>  libparted/labels/gpt.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
> index 860f415..914dfe3 100644
> --- a/libparted/labels/gpt.c
> +++ b/libparted/labels/gpt.c
> @@ -1042,6 +1042,7 @@ gpt_read (PedDisk *disk)
>          goto error_free_gpt;
> 
>        gpt = primary_gpt;
> +      write_back = 1;
>      }
>    else /* !primary_gpt && backup_gpt */
>      {
> @@ -1054,6 +1055,7 @@ gpt_read (PedDisk *disk)
>          goto error_free_gpt;
> 
>        gpt = backup_gpt;
> +      write_back = 1;
>      }
>    backup_gpt = NULL;
>    primary_gpt = NULL;
> --
> 1.7.1
> 
> 
>