Re: [PATCH 2/2] Add legacy_boot flag for GPT parititons

[Jim Meyering]

Brian C. Lane wrote:
> Add support for the Legacy BIOS Bootable flag in the GPT
> Attribute field. This is used by software like syslinux to
> determine which partition to boot when in BIOS mode.
>
> See Page 105, Table 19 Bit 2 of the UEFI Spec 2.3 book.
>
> * doc/C/parted.8: document legacy_boot
> * doc/parted.texi: document legacy_boot
> * include/parted/disk.h: Add PED_PARTITION_LEGACY_BOOT flag
> * libparted/disk.c (ped_partition_flag_get_name): Add legacy_boot flag
> * libparted/labels/gpt.c (_parse_part_entry,
>   _partition_generate_part_entry, gpt_partition_new,
>   gpt_partition_set_flag, gpt_partition_get_flag,
>   gpt_partition_is_flag_available): Add legacy_boot flag support
...
> +  if (gpt_part_data->legacy_boot)
> +    pte->Attributes.LegacyBIOSBootable= 1;

Hi Brian,

Thanks!  I've pushed those two, with slightly different commit logs
and a single code formatting tweak: s/e=/e =/.

It would have been nice to include a test, but I've written one
for you that also tests any existing (and to-be-added) gpt flags.

>From 7fc2cf02e1e6a4b02d989db68511224f1bc341c5 Mon Sep 17 00:00:00 2001
From: Brian C. Lane <address@hidden>
Date: Fri, 25 Feb 2011 14:09:18 -0800
Subject: [PATCH 1/6] gpt: add partition attribute bits: NoBlockIOProtocol, 
LegacyBIOSBootable

Add NoBlockIOProtocol and LegacyBIOSBootable flags to the GPT partition
Attributes entry.
---
 libparted/labels/gpt.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index fad352f..46e6077 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -166,12 +166,16 @@ struct __attribute__ ((packed)) 
_GuidPartitionEntryAttributes_t
 {
 #ifdef __GNUC__                        /* XXX narrow this down to !TinyCC */
   uint64_t RequiredToFunction:1;
-  uint64_t Reserved:47;
+  uint64_t NoBlockIOProtocol:1;
+  uint64_t LegacyBIOSBootable:1;
+  uint64_t Reserved:45;
   uint64_t GuidSpecific:16;
 #else
 #       warning "Using crippled partition entry type"
   uint32_t RequiredToFunction:1;
-  uint32_t Reserved:32;
+  uint32_t NoBlockIOProtocol:1;
+  uint32_t LegacyBIOSBootable:1;
+  uint32_t Reserved:30;
   uint32_t LOST:5;
   uint32_t GuidSpecific:16;
 #endif
--
1.7.4.1.21.g4cc62


>From 3dd52c822a40f9787765f21f7cbd6f1c37b42f7e Mon Sep 17 00:00:00 2001
From: Brian C. Lane <address@hidden>
Date: Fri, 25 Feb 2011 14:09:19 -0800
Subject: [PATCH 2/6] gpt: add legacy_boot partition flag

Add support for the Legacy BIOS Bootable flag in the GPT
Attribute field.  This is used by software like syslinux to
determine which partition to boot when in BIOS mode.

See Page 105, Table 19 Bit 2 of the UEFI Spec 2.3 book.

* doc/C/parted.8: Document legacy_boot.
* doc/parted.texi: Document legacy_boot.
* include/parted/disk.h: Add PED_PARTITION_LEGACY_BOOT flag.
* libparted/disk.c (ped_partition_flag_get_name): Add legacy_boot flag.
* libparted/labels/gpt.c (_parse_part_entry):
(_partition_generate_part_entry, gpt_partition_new):
(gpt_partition_set_flag, gpt_partition_get_flag):
(gpt_partition_is_flag_available): Add legacy_boot flag support.
---
 doc/C/parted.8         |    2 +-
 doc/parted.texi        |    4 ++++
 include/parted/disk.h  |    5 +++--
 libparted/disk.c       |    2 ++
 libparted/labels/gpt.c |   15 ++++++++++++++-
 5 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/doc/C/parted.8 b/doc/C/parted.8
index 2a8992e..f4b940c 100644
--- a/doc/C/parted.8
+++ b/doc/C/parted.8
@@ -132,7 +132,7 @@ or an LVM logical volume if necessary.
 .B set \fIpartition\fP \fIflag\fP \fIstate\fP
 Change the state of the \fIflag\fP on \fIpartition\fP to \fIstate\fP.
 Supported flags are: "boot", "root", "swap", "hidden", "raid", "lvm", "lba",
-and "palo".
+"legacy_boot" and "palo".
 \fIstate\fP should be either "on" or "off".
 .TP
 .B unit \fIunit\fP
diff --git a/doc/parted.texi b/doc/parted.texi
index 1111efd..1a57c07 100644
--- a/doc/parted.texi
+++ b/doc/parted.texi
@@ -1049,6 +1049,10 @@ depending on what disk label you are using:
 (GPT) - Enable this to record that the selected partition is a
 GRUB BIOS partition.

address@hidden legacy_boot
+(GPT) - this flag is used to tell special purpose software that the GPT
+partition may be bootable.
+
 @item boot
 (Mac, MS-DOS, PC98) - should be enabled if you want to boot off the
 partition.  The semantics vary between disk labels.  For MS-DOS disk
diff --git a/include/parted/disk.h b/include/parted/disk.h
index 3a1450c..dd461fb 100644
--- a/include/parted/disk.h
+++ b/include/parted/disk.h
@@ -69,10 +69,11 @@ enum _PedPartitionFlag {
         PED_PARTITION_MSFT_RESERVED=11,
         PED_PARTITION_BIOS_GRUB=12,
         PED_PARTITION_APPLE_TV_RECOVERY=13,
-        PED_PARTITION_DIAG=14
+        PED_PARTITION_DIAG=14,
+        PED_PARTITION_LEGACY_BOOT=15
 };
 #define PED_PARTITION_FIRST_FLAG        PED_PARTITION_BOOT
-#define PED_PARTITION_LAST_FLAG         PED_PARTITION_DIAG
+#define PED_PARTITION_LAST_FLAG         PED_PARTITION_LEGACY_BOOT

 enum _PedDiskTypeFeature {
         PED_DISK_TYPE_EXTENDED=1,       /**< supports extended partitions */
diff --git a/libparted/disk.c b/libparted/disk.c
index 376571e..f6f795f 100644
--- a/libparted/disk.c
+++ b/libparted/disk.c
@@ -2442,6 +2442,8 @@ ped_partition_flag_get_name (PedPartitionFlag flag)
                 return N_("atvrecv");
         case PED_PARTITION_DIAG:
                 return N_("diag");
+        case PED_PARTITION_LEGACY_BOOT:
+                return N_("legacy_boot");

        default:
                ped_exception_throw (
diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c
index 46e6077..1b9fcfa 100644
--- a/libparted/labels/gpt.c
+++ b/libparted/labels/gpt.c
@@ -4,7 +4,7 @@
     original version by Matt Domsch <address@hidden>
     Disclaimed into the Public Domain

-    Portions Copyright (C) 2001-2003, 2005-2010 Free Software Foundation, Inc.
+    Portions Copyright (C) 2001-2003, 2005-2011 Free Software Foundation, Inc.

     EFI GUID Partition Table handling
     Per Intel EFI Specification v1.02
@@ -281,6 +281,7 @@ typedef struct _GPTPartitionData
   int msftres;
   int atvrecv;
   int msftrecv;
+  int legacy_boot;
 } GPTPartitionData;

 static PedDiskType gpt_disk_type;
@@ -786,10 +787,13 @@ _parse_part_entry (PedDisk *disk, GuidPartitionEntry_t 
*pte)
     = gpt_part_data->boot = gpt_part_data->hp_service
     = gpt_part_data->hidden = gpt_part_data->msftres
     = gpt_part_data->msftrecv
+    = gpt_part_data->legacy_boot
     = gpt_part_data->bios_grub = gpt_part_data->atvrecv = 0;

   if (pte->Attributes.RequiredToFunction & 0x1)
     gpt_part_data->hidden = 1;
+  if (pte->Attributes.LegacyBIOSBootable & 0x1)
+    gpt_part_data->legacy_boot = 1;

   if (!guid_cmp (gpt_part_data->type, PARTITION_SYSTEM_GUID))
     gpt_part_data->boot = 1;
@@ -1166,6 +1170,8 @@ _partition_generate_part_entry (PedPartition *part, 
GuidPartitionEntry_t *pte)

   if (gpt_part_data->hidden)
     pte->Attributes.RequiredToFunction = 1;
+  if (gpt_part_data->legacy_boot)
+    pte->Attributes.LegacyBIOSBootable = 1;

   for (i = 0; i < 72 / sizeof (efi_char16_t); i++)
     pte->PartitionName[i]
@@ -1308,6 +1314,7 @@ gpt_partition_new (const PedDisk *disk,
   gpt_part_data->msftres = 0;
   gpt_part_data->msftrecv = 0;
   gpt_part_data->atvrecv = 0;
+  gpt_part_data->legacy_boot = 0;
   uuid_generate ((unsigned char *) &gpt_part_data->uuid);
   swap_uuid_and_efi_guid ((unsigned char *) (&gpt_part_data->uuid));
   memset (gpt_part_data->name, 0, sizeof gpt_part_data->name);
@@ -1595,6 +1602,9 @@ gpt_partition_set_flag (PedPartition *part, 
PedPartitionFlag flag, int state)
     case PED_PARTITION_HIDDEN:
       gpt_part_data->hidden = state;
       return 1;
+    case PED_PARTITION_LEGACY_BOOT:
+      gpt_part_data->legacy_boot = state;
+      return 1;
     case PED_PARTITION_SWAP:
     case PED_PARTITION_ROOT:
     case PED_PARTITION_LBA:
@@ -1631,6 +1641,8 @@ gpt_partition_get_flag (const PedPartition *part, 
PedPartitionFlag flag)
       return gpt_part_data->atvrecv;
     case PED_PARTITION_HIDDEN:
       return gpt_part_data->hidden;
+    case PED_PARTITION_LEGACY_BOOT:
+      return gpt_part_data->legacy_boot;
     case PED_PARTITION_SWAP:
     case PED_PARTITION_LBA:
     case PED_PARTITION_ROOT:
@@ -1655,6 +1667,7 @@ gpt_partition_is_flag_available (const PedPartition *part,
     case PED_PARTITION_DIAG:
     case PED_PARTITION_APPLE_TV_RECOVERY:
     case PED_PARTITION_HIDDEN:
+    case PED_PARTITION_LEGACY_BOOT:
       return 1;
     case PED_PARTITION_SWAP:
     case PED_PARTITION_ROOT:
--
1.7.4.1.21.g4cc62


>From 4d21f837efd5a5b843031569a634930e601329bb Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 26 Feb 2011 07:15:05 +0100
Subject: [PATCH 3/6] tests: remove useless commands

* tests/t3310-flags.sh: Remove useless exp-manipulating commands.
---
 tests/t3310-flags.sh |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/tests/t3310-flags.sh b/tests/t3310-flags.sh
index 4354ccf..bd3317b 100644
--- a/tests/t3310-flags.sh
+++ b/tests/t3310-flags.sh
@@ -38,7 +38,6 @@ for mode in on_only on_and_off ; do
     # Exclude the supplemental flags.
     # These are not boolean, like the others.
     case $flag in boot|lba|hidden) continue;; esac
-    echo $flag > exp || fail=1

     # Turn on each flag, one at a time.
     parted -m -s $dev set 1 $flag on u s print > raw 2> err || fail=1
@@ -48,7 +47,6 @@ for mode in on_only on_and_off ; do

     if test $mode = on_and_off; then
       # Turn it off
-      : > exp
       parted -m -s $dev set 1 $flag off u s print > raw 2> err || fail=1
       perl -nle '/^1:2048s:4095s:2048s:::.*;$/ and print $1' raw > out
       compare err /dev/null || fail=1
--
1.7.4.1.21.g4cc62


>From e9e3ad6328c58512d6a3ede592327e89f6146b82 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 26 Feb 2011 07:24:13 +0100
Subject: [PATCH 4/6] tests: prepare t3310-flags to check gpt boolean flags, too

* tests/t3310-flags.sh: Put a loop around the body of the test,
and factor out the partition table name.
---
 tests/t3310-flags.sh |   68 ++++++++++++++++++++++++++++----------------------
 1 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/tests/t3310-flags.sh b/tests/t3310-flags.sh
index bd3317b..b4f944d 100644
--- a/tests/t3310-flags.sh
+++ b/tests/t3310-flags.sh
@@ -20,37 +20,45 @@
 ss=$sector_size_
 dev=dev-file

-# Extract all msdos flag names from the texinfo documentation.
-msdos_flags=$(sed -n '/address@hidden set/,/address@hidden/p' 
"$abs_top_srcdir/doc/parted.texi" \
-  | perl -00 -ne '/address@hidden (\w+).*MS-DOS/s and print lc($1), "\n"')
-
-n_sectors=5000
-dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || fail=1
-
-parted -s $dev mklabel msdos \
-  mkpart pri ext2 $((1*2048))s $((2*2048-1))s \
-    > out 2> err || fail=1
-compare out /dev/null || fail=1
-
-for mode in on_only on_and_off ; do
-  for flag in $msdos_flags; do
-
-    # Exclude the supplemental flags.
-    # These are not boolean, like the others.
-    case $flag in boot|lba|hidden) continue;; esac
-
-    # Turn on each flag, one at a time.
-    parted -m -s $dev set 1 $flag on u s print > raw 2> err || fail=1
-    perl -nle '/^1:2048s:4095s:2048s:::(\w+);$/ and print $1' raw > out
-    compare out exp || fail=1
-    compare err /dev/null || fail=1
-
-    if test $mode = on_and_off; then
-      # Turn it off
-      parted -m -s $dev set 1 $flag off u s print > raw 2> err || fail=1
-      perl -nle '/^1:2048s:4095s:2048s:::.*;$/ and print $1' raw > out
+for table_type in msdos; do
+
+  # Extract flag names of type $table_type from the texinfo documentation.
+  case $table_type in
+      msdos) search_term=MS-DOS;;
+  esac
+  flags=$(sed -n '/address@hidden set/,/address@hidden/p' \
+                    "$abs_top_srcdir/doc/parted.texi" \
+                | perl -00 -ne \
+                    '/address@hidden (\w+).*'"$search_term"'/s and print 
lc($1), "\n"')
+
+  n_sectors=5000
+  dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || fail=1
+
+  parted -s $dev mklabel $table_type \
+    mkpart pri ext2 $((1*2048))s $((2*2048-1))s \
+      > out 2> err || fail=1
+  compare out /dev/null || fail=1
+
+  for mode in on_only on_and_off ; do
+    for flag in $flags; do
+
+      # Exclude the supplemental flags.
+      # These are not boolean, like the others.
+      case $flag in boot|lba|hidden) continue;; esac
+
+      # Turn on each flag, one at a time.
+      parted -m -s $dev set 1 $flag on u s print > raw 2> err || fail=1
+      perl -nle '/^1:2048s:4095s:2048s:::(\w+);$/ and print $1' raw > out
+      compare out exp || fail=1
       compare err /dev/null || fail=1
-    fi
+
+      if test $mode = on_and_off; then
+        # Turn it off
+        parted -m -s $dev set 1 $flag off u s print > raw 2> err || fail=1
+        perl -nle '/^1:2048s:4095s:2048s:::.*;$/ and print $1' raw > out
+        compare err /dev/null || fail=1
+      fi
+    done
   done
 done

--
1.7.4.1.21.g4cc62


>From e7e85f647a99a82b325a26f6590e3c7720a40107 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 26 Feb 2011 07:55:10 +0100
Subject: [PATCH 5/6] tests: t3310-flags: check all gpt boolean flags, too

* tests/t3310-flags.sh: Check gpt as well as msdos flags.
Parametrize mkpart usage.
Accommodate gpt-induced difference in parted's print output
GPT partitions have a name, while msdos ones do not.
This also fixes the "on_and_off" perl flag extraction regexp,
which before was using $1 with no corresponding () group.
---
 tests/t3310-flags.sh |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/tests/t3310-flags.sh b/tests/t3310-flags.sh
index b4f944d..350485c 100644
--- a/tests/t3310-flags.sh
+++ b/tests/t3310-flags.sh
@@ -20,11 +20,17 @@
 ss=$sector_size_
 dev=dev-file

-for table_type in msdos; do
+extract_flags()
+{
+  perl -nle '/^1:2048s:4095s:2048s::(?:P1)?:(.+);$/ and print $1' "$@"
+}
+
+for table_type in msdos gpt; do

   # Extract flag names of type $table_type from the texinfo documentation.
   case $table_type in
-      msdos) search_term=MS-DOS;;
+      msdos) search_term=MS-DOS; pri_or_name=pri;;
+      gpt)   search_term=GPT;    pri_or_name=P1;;
   esac
   flags=$(sed -n '/address@hidden set/,/address@hidden/p' \
                     "$abs_top_srcdir/doc/parted.texi" \
@@ -35,7 +41,7 @@ for table_type in msdos; do
   dd if=/dev/null of=$dev bs=$ss seek=$n_sectors || fail=1

   parted -s $dev mklabel $table_type \
-    mkpart pri ext2 $((1*2048))s $((2*2048-1))s \
+    mkpart $pri_or_name ext2 $((1*2048))s $((2*2048-1))s \
       > out 2> err || fail=1
   compare out /dev/null || fail=1

@@ -48,14 +54,17 @@ for table_type in msdos; do

       # Turn on each flag, one at a time.
       parted -m -s $dev set 1 $flag on u s print > raw 2> err || fail=1
-      perl -nle '/^1:2048s:4095s:2048s:::(\w+);$/ and print $1' raw > out
-      compare out exp || fail=1
+      extract_flags raw > out
+      grep -F "$flag" out \
+        || { warn_ "$ME: flag not turned on: $(cat out)"; fail=1; }
       compare err /dev/null || fail=1

       if test $mode = on_and_off; then
         # Turn it off
         parted -m -s $dev set 1 $flag off u s print > raw 2> err || fail=1
-        perl -nle '/^1:2048s:4095s:2048s:::.*;$/ and print $1' raw > out
+        extract_flags raw > out
+        grep -F "$flag" out \
+          && { warn_ "$ME: flag not turned off: $(cat out)"; fail=1; }
         compare err /dev/null || fail=1
       fi
     done
--
1.7.4.1.21.g4cc62


>From dff6b96b551bec36dfe1c4783184755da72b2901 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 26 Feb 2011 08:34:02 +0100
Subject: [PATCH 6/6] maint: two files lacked a newline at EOF: add one to each

* include/Makefile.am: Add newline at end of file.
* libparted/fs/jfs/Makefile.am: Likewise.
---
 include/Makefile.am          |    2 +-
 libparted/fs/jfs/Makefile.am |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/Makefile.am b/include/Makefile.am
index 25715ce..c5579ae 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -1,3 +1,3 @@
 SUBDIRS = parted

-MAINTAINERCLEANFILES = Makefile.in
\ No newline at end of file
+MAINTAINERCLEANFILES = Makefile.in
diff --git a/libparted/fs/jfs/Makefile.am b/libparted/fs/jfs/Makefile.am
index 0218667..9e316a0 100644
--- a/libparted/fs/jfs/Makefile.am
+++ b/libparted/fs/jfs/Makefile.am
@@ -5,4 +5,4 @@ libjfs_la_SOURCES       = jfs.c jfs_superblock.h jfs_types.h

 INCLUDES = $(partedincludedir) $(INTLINCS)

-MAINTAINERCLEANFILES = Makefile.in
\ No newline at end of file
+MAINTAINERCLEANFILES = Makefile.in
--
1.7.4.1.21.g4cc62