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

[Hatayama, Daisuke]

> From: Phillip Susi [mailto:phill at thesusis.net]
> Sent: Friday, November 16, 2018 3:27 AM

> 
> On 11/14/2018 9:56 PM, Hatayama, Daisuke wrote:
> > I don't understand what 'if no other modification is made' means.
> 
> I mean if you just print the table vs modifying it in some way.  If you
> modify it then it certainly has to be written to the disk and that will
> write a non corrupt table to both copies.
> 
> > The reason why I think it problem is it's regression. The UEFI spec
> > says whether to perform recovery of the primary/backup GPTs is SHOULD,
> > not MUST so parted command originally didn't have to implement it, but
> > once provided, needs to keep it.
> 
> It will fix it if you cause the partition table to be written, the
> question is do we want to fix it when you are only printing?  On the one
> hand, I'd say we want to go ahead and always USE the backup so we can
> print, but on the other hand, we should not fix the table on the disk
> without the user saying to.  The way the code is now, we can always
> print, but won't fix it when we are just printing.
> 
> With your patch, the table is always written back to disk, even when
> just printing.  We don't want to do that.  Also, in the UNHANDLED case,
> you can no longer print the table, which isn't good.
> 
> >>  You don't expect a parted print operation to modify the disk.
> >
> > Yes, I don't expect parted read operation to modify the disk.
> 
> But that's exactly what you are patching it to do.
> 

This 1/2 patch just reintroduces the recovery of the GPT tables. There is
the 2/2 patch to suppress performing the recovery in script mode:

    [PATCH 2/2] gpt_read: don't restore the primary/backup GPT in script mode
    https://alioth-lists.debian.net/pipermail/parted-devel/2018-November/005331.html

> > But note for in case that UEFI spec says need of user confirmation in
> > advance is SHOULD, not MUST. The need of user confirmation for parted
> > command comes from the fact that print command is read-only.
> 
> The need of confirmation in parted does not come from the UEFI spec, but
> from the fact that parted should not modify the disk ( with any
> partition table ) without being told to.
> 

Yes, I guess I said the same as you.

> > There is no chance for users to make any response in scripting mode
> > because ped_exception_throw() unconditionally returns
> PED_EXCEPTION_UNHANDLED
> > in scripting mode:
> 
> Right, and in the current code, a result of PED_EXCEPTION_UNHANDLED
> still causes the backup table to be used, and if the table is written
> back to the disk, the error will be corrected.  We certainly don't want
> to correct it in a parted print when we get PED_EXCEPTION_UNHANDLED.
> 

The 2/2 patch tries to fix based on this.

> >> I seem to remember there being a patch a while back to have gpt save and
> >> restore the existing pbmr when it writes out instead of generating a new
> >> one each time.
> >
> > Anyway the issue about the clobbering pMBR mentioned in the commit has
> > already fixed, right?
> 
> I think so.
> 

Thanks.
HATAYAMA, Daisuke