From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1XWCfM-0002ox-MH for mharc-grub-devel@gnu.org; Mon, 22 Sep 2014 19:05:08 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38407) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWCfH-0002lm-Jf for grub-devel@gnu.org; Mon, 22 Sep 2014 19:05:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWCfC-0002QO-SJ for grub-devel@gnu.org; Mon, 22 Sep 2014 19:05:03 -0400 Received: from mail-wg0-x231.google.com ([2a00:1450:400c:c00::231]:47896) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWCfC-0002Pb-Hi for grub-devel@gnu.org; Mon, 22 Sep 2014 19:04:58 -0400 Received: by mail-wg0-f49.google.com with SMTP id x12so3528861wgg.20 for ; Mon, 22 Sep 2014 16:04:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type; bh=OqvIkh1pmMJ05YYX0xZ5WhWRzhhS1PH0aWp0XqPpraY=; b=PXK30vqCj0TRfj2enoGnjNJbjvJnMazbD0JvXju263UBMp2F7wi2G0Ia1N5pHDbBLh mfDJxi3oCXxhFqiAK0yK2avm3ZmTgj2BiuSOkt74kFEfyoKRDXpO+VyiwAUqpHtCBrrF XcvxjEjDdSXQ/qIizzTn8SA85PSQsa4ehGApPmpneZva4Y7VSrt3YiJP0j09tJf+zjg9 A6Y4TS2i3rkvVoMIBy3xb2FUx+8yrg7DRMWN/VV6hn0Q/R0PdtwCoMeSTNAGBv5oEKhE 4iVM49izIFfV/MNiDu16JRvOBJfj79pWnDjgxBrgSrtxHJZkXq+nq7mK0VKTet/n32Xl wmpg== X-Received: by 10.180.74.212 with SMTP id w20mr18766558wiv.7.1411427091920; Mon, 22 Sep 2014 16:04:51 -0700 (PDT) Received: from ?IPv6:2a02:1205:501d:9350:863a:4bff:fe50:abc4? ([2a02:1205:501d:9350:863a:4bff:fe50:abc4]) by mx.google.com with ESMTPSA id js2sm13835460wjc.9.2014.09.22.16.04.50 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 22 Sep 2014 16:04:50 -0700 (PDT) Message-ID: <5420AB11.5080002@gmail.com> Date: Tue, 23 Sep 2014 01:04:49 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.0 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: [RFC PATCH] Add new partition type API and convert GPT type checks. References: <1411412346-4003-1-git-send-email-michael.marineau@coreos.com> In-Reply-To: <1411412346-4003-1-git-send-email-michael.marineau@coreos.com> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="K2Kb0lUkCR4b2Ip9RH3VhK8IXO5B77NEL" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c00::231 Cc: Michael Marineau X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Sep 2014 23:05:05 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --K2Kb0lUkCR4b2Ip9RH3VhK8IXO5B77NEL Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 22.09.2014 20:59, Michael Marineau wrote: > This introduces a new type() function for partmaps modeled after the > label() and uuid() functions for filesystems. A likely future extension= > will be support partition labels and uuids as well. This is in > preparation for adding more functionality like a search.part_type > command. >=20 > The msdos partition code is only partially converted for now, I'm not > sure about the preferred way to do this. Re-reading a msdos partition > table is not as easy as re-reading gpt is. One option would be to turn > 'msdostype' into a union or data pointer that partmap modules can stash= > extra info in. That would also allow gpt to avoid re-reading. Being new= > to grub, are there memory usage concerns I should be aware of here? Please provide a usecase. Every new feature needs a usecase. This patch increases core size and will not go in without a damn good usecase and justification why it needs to be done in core. > --- > grub-core/disk/ldm.c | 17 ++---------- > grub-core/kern/partition.c | 23 +++++++++++++++++ > grub-core/partmap/gpt.c | 61 ++++++++++++++++++++++++++++++------= -------- > grub-core/partmap/msdos.c | 9 +++++++ > include/grub/gpt_partition.h | 20 +++------------ > include/grub/partition.h | 11 ++++++++ > util/grub-install.c | 34 ++++-------------------- > util/grub-probe.c | 54 +++++++++++++++---------------------= --- > 8 files changed, 117 insertions(+), 112 deletions(-) >=20 > diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c > index 8075f2a..6dcfbc8 100644 > --- a/grub-core/disk/ldm.c > +++ b/grub-core/disk/ldm.c > @@ -135,31 +135,18 @@ msdos_has_ldm_partition (grub_disk_t dsk) > return has_ldm; > } > =20 > -static const grub_gpt_part_type_t ldm_type =3D GRUB_GPT_PARTITION_TYPE= _LDM; > - > /* Helper for gpt_ldm_sector. */ > static int > gpt_ldm_sector_iter (grub_disk_t disk, const grub_partition_t p, void = *data) > { > grub_disk_addr_t *sector =3D data; > - struct grub_gpt_partentry gptdata; > - grub_partition_t p2; > - > - p2 =3D disk->partition; > - disk->partition =3D p->parent; > - if (grub_disk_read (disk, p->offset, p->index, > - sizeof (gptdata), &gptdata)) > - { > - disk->partition =3D p2; > - return 0; > - } > - disk->partition =3D p2; > =20 > - if (! grub_memcmp (&gptdata.type, &ldm_type, 16)) > + if (grub_partition_is_type (disk, p, "gpt", GRUB_GPT_PARTITION_TYPE_= LDM)) > { > *sector =3D p->start + p->len - 1; > return 1; > } > + > return 0; > } > =20 > diff --git a/grub-core/kern/partition.c b/grub-core/kern/partition.c > index e499147..f27a8bd 100644 > --- a/grub-core/kern/partition.c > +++ b/grub-core/kern/partition.c > @@ -274,3 +274,26 @@ grub_partition_get_name (const grub_partition_t pa= rtition) > grub_memmove (out, ptr + 1, out + needlen - ptr); > return out; > } > + > +int grub_partition_is_type (struct grub_disk *disk, > + const grub_partition_t partition, > + const char *partmap_name, > + const char *partition_type) > +{ > + char *type; > + int r; > + > + if (!disk || !partition || !partition->partmap->type) > + return 0; > + > + if (grub_strcmp (partition->partmap->name, partmap_name) !=3D 0) > + return 0; > + > + if (partition->partmap->type(disk, partition, &type) !=3D 0) > + return 0; > + > + r =3D grub_strcmp (type, partition_type); > + grub_free (type); > + > + return r =3D=3D 0; > +} > diff --git a/grub-core/partmap/gpt.c b/grub-core/partmap/gpt.c > index 38df7b3..6d6c13f 100644 > --- a/grub-core/partmap/gpt.c > +++ b/grub-core/partmap/gpt.c > @@ -33,11 +33,10 @@ static grub_uint8_t grub_gpt_magic[8] =3D > 0x45, 0x46, 0x49, 0x20, 0x50, 0x41, 0x52, 0x54 > }; > =20 > -static const grub_gpt_part_type_t grub_gpt_partition_type_empty =3D GR= UB_GPT_PARTITION_TYPE_EMPTY; > - > -#ifdef GRUB_UTIL > -static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot =3D= GRUB_GPT_PARTITION_TYPE_BIOS_BOOT; > -#endif > +static const grub_gpt_part_type_t grub_gpt_partition_type_empty =3D > + { 0x0, 0x0, 0x0, > + { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } > + }; > =20 > /* 512 << 7 =3D 65536 byte sectors. */ > #define MAX_SECTOR_LOG 7 > @@ -128,6 +127,40 @@ grub_gpt_partition_map_iterate (grub_disk_t disk, > return GRUB_ERR_NONE; > } > =20 > +static grub_err_t > +grub_gpt_partition_type (grub_disk_t disk, const grub_partition_t part= ition, > + char **type) > +{ > + struct grub_gpt_partentry gptentry; > + grub_gpt_part_type_t gpttype; > + grub_partition_t p2; > + grub_err_t err; > + > + p2 =3D disk->partition; > + disk->partition =3D partition->parent; > + err =3D grub_disk_read (disk, partition->offset, partition->index, > + sizeof (gptentry), &gptentry); > + disk->partition =3D p2; > + > + if (err) > + return err; > + > + gpttype.data1 =3D grub_le_to_cpu32 (gptentry.type.data1); > + gpttype.data2 =3D grub_le_to_cpu16 (gptentry.type.data2); > + gpttype.data3 =3D grub_le_to_cpu16 (gptentry.type.data3); > + grub_memcpy (gpttype.data4, gptentry.type.data4, sizeof (gpttype.dat= a4)); > + > + *type =3D grub_xasprintf ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%= 02x%02x", > + gpttype.data1, gpttype.data2, > + gpttype.data3, gpttype.data4[0], > + gpttype.data4[1], gpttype.data4[2], > + gpttype.data4[3], gpttype.data4[4], > + gpttype.data4[5], gpttype.data4[6], > + gpttype.data4[7]); > + > + return GRUB_ERR_NONE; > +} > + > #ifdef GRUB_UTIL > /* Context for gpt_partition_map_embed. */ > struct gpt_partition_map_embed_ctx > @@ -137,25 +170,14 @@ struct gpt_partition_map_embed_ctx > =20 > /* Helper for gpt_partition_map_embed. */ > static int > -find_usable_region (grub_disk_t disk __attribute__ ((unused)), > +find_usable_region (grub_disk_t disk, > const grub_partition_t p, void *data) > { > struct gpt_partition_map_embed_ctx *ctx =3D data; > - struct grub_gpt_partentry gptdata; > - grub_partition_t p2; > - > - p2 =3D disk->partition; > - disk->partition =3D p->parent; > - if (grub_disk_read (disk, p->offset, p->index, > - sizeof (gptdata), &gptdata)) > - { > - disk->partition =3D p2; > - return 0; > - } > - disk->partition =3D p2; > =20 > /* If there's an embed region, it is in a dedicated partition. */ > - if (! grub_memcmp (&gptdata.type, &grub_gpt_partition_type_bios_boot= , 16)) > + if (grub_partition_is_type(disk, p, "gpt", > + GRUB_GPT_PARTITION_TYPE_BIOS_BOOT)) > { > ctx->start =3D p->start; > ctx->len =3D p->len; > @@ -215,6 +237,7 @@ static struct grub_partition_map grub_gpt_partition= _map =3D > { > .name =3D "gpt", > .iterate =3D grub_gpt_partition_map_iterate, > + .type =3D grub_gpt_partition_type, > #ifdef GRUB_UTIL > .embed =3D gpt_partition_map_embed > #endif > diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c > index 1d81a53..68140a7 100644 > --- a/grub-core/partmap/msdos.c > +++ b/grub-core/partmap/msdos.c > @@ -228,6 +228,14 @@ grub_partition_msdos_iterate (grub_disk_t disk, > return grub_errno; > } > =20 > +static grub_err_t > +grub_partition_msdos_type (grub_disk_t disk __attribute__ ((unused)), > + const grub_partition_t partition, char **type) > +{ > + *type =3D grub_xasprintf ("%02x", partition->msdostype); > + return GRUB_ERR_NONE; > +} > + > #ifdef GRUB_UTIL > =20 > #pragma GCC diagnostic ignored "-Wformat-nonliteral" > @@ -415,6 +423,7 @@ static struct grub_partition_map grub_msdos_partiti= on_map =3D > { > .name =3D "msdos", > .iterate =3D grub_partition_msdos_iterate, > + .type =3D grub_partition_msdos_type, > #ifdef GRUB_UTIL > .embed =3D pc_partition_map_embed > #endif > diff --git a/include/grub/gpt_partition.h b/include/grub/gpt_partition.= h > index 1b32f67..566334c 100644 > --- a/include/grub/gpt_partition.h > +++ b/include/grub/gpt_partition.h > @@ -31,24 +31,12 @@ struct grub_gpt_part_type > } __attribute__ ((aligned(8))); > typedef struct grub_gpt_part_type grub_gpt_part_type_t; > =20 > -#define GRUB_GPT_PARTITION_TYPE_EMPTY \ > - { 0x0, 0x0, 0x0, \ > - { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } \ > - } > - > #define GRUB_GPT_PARTITION_TYPE_BIOS_BOOT \ > - { grub_cpu_to_le32_compile_time (0x21686148), \ > - grub_cpu_to_le16_compile_time (0x6449), \ > - grub_cpu_to_le16_compile_time (0x6e6f), \ > - { 0x74, 0x4e, 0x65, 0x65, 0x64, 0x45, 0x46, 0x49 } \ > - } > - > + "21686148-6449-6e6f-744e-656564454649" > #define GRUB_GPT_PARTITION_TYPE_LDM \ > - { grub_cpu_to_le32_compile_time (0x5808C8AAU),\ > - grub_cpu_to_le16_compile_time (0x7E8F), \ > - grub_cpu_to_le16_compile_time (0x42E0), \ > - { 0x85, 0xD2, 0xE1, 0xE9, 0x04, 0x34, 0xCF, 0xB3 } \ > - } > + "5808c8aa-7e8f-42e0-85d2-e1e90434cfb3" > +#define GRUB_GPT_PARTITION_TYPE_PREP \ > + "9e1a2d38-c612-4316-aa26-8b49521e5a8b" > =20 > struct grub_gpt_header > { > diff --git a/include/grub/partition.h b/include/grub/partition.h > index 7adb7ec..3dc46bf 100644 > --- a/include/grub/partition.h > +++ b/include/grub/partition.h > @@ -50,6 +50,13 @@ struct grub_partition_map > /* Call HOOK with each partition, until HOOK returns non-zero. */ > grub_err_t (*iterate) (struct grub_disk *disk, > grub_partition_iterate_hook_t hook, void *hook_data); > + > + /* Return the type of the partition PARTITION in TYPE. > + The type is returned in a grub_malloc'ed buffer and should > + be freed by the caller. */ > + grub_err_t (*type) (struct grub_disk *disk, > + const grub_partition_t partition, char **type); > + > #ifdef GRUB_UTIL > /* Determine sectors available for embedding. */ > grub_err_t (*embed) (struct grub_disk *disk, unsigned int *nsectors,= > @@ -95,6 +102,10 @@ int EXPORT_FUNC(grub_partition_iterate) (struct gru= b_disk *disk, > grub_partition_iterate_hook_t hook, > void *hook_data); > char *EXPORT_FUNC(grub_partition_get_name) (const grub_partition_t par= tition); > +int EXPORT_FUNC(grub_partition_is_type) (struct grub_disk *disk, > + const grub_partition_t partition, > + const char *partmap_name, > + const char *partition_type); > =20 > =20 > extern grub_partition_map_t EXPORT_VAR(grub_partition_map_list); > diff --git a/util/grub-install.c b/util/grub-install.c > index 7d61c32..ee98b70 100644 > --- a/util/grub-install.c > +++ b/util/grub-install.c > @@ -696,36 +696,12 @@ write_to_disk (grub_device_t dev, const char *fn)= > static int > is_prep_partition (grub_device_t dev) > { > - if (!dev->disk) > - return 0; > - if (!dev->disk->partition) > - return 0; > - if (strcmp(dev->disk->partition->partmap->name, "msdos") =3D=3D 0) > - return (dev->disk->partition->msdostype =3D=3D 0x41); > - > - if (strcmp (dev->disk->partition->partmap->name, "gpt") =3D=3D 0) > - { > - struct grub_gpt_partentry gptdata; > - grub_partition_t p =3D dev->disk->partition; > - int ret =3D 0; > - dev->disk->partition =3D dev->disk->partition->parent; > + if (grub_partition_is_type (dev->disk, dev->disk->partition, "msdos"= , "41")) > + return 1; > =20 > - if (grub_disk_read (dev->disk, p->offset, p->index, > - sizeof (gptdata), &gptdata) =3D=3D 0) > - { > - const grub_gpt_part_type_t template =3D { > - grub_cpu_to_le32_compile_time (0x9e1a2d38), > - grub_cpu_to_le16_compile_time (0xc612), > - grub_cpu_to_le16_compile_time (0x4316), > - { 0xaa, 0x26, 0x8b, 0x49, 0x52, 0x1e, 0x5a, 0x8b } > - }; > - > - ret =3D grub_memcmp (&template, &gptdata.type, > - sizeof (template)) =3D=3D 0; > - } > - dev->disk->partition =3D p; > - return ret; > - } > + if (grub_partition_is_type (dev->disk, dev->disk->partition, > + "gpt", GRUB_GPT_PARTITION_TYPE_PREP)) > + return 1; > =20 > return 0; > } > diff --git a/util/grub-probe.c b/util/grub-probe.c > index ecb7b6b..a2505f0 100644 > --- a/util/grub-probe.c > +++ b/util/grub-probe.c > @@ -27,7 +27,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -253,6 +252,25 @@ probe_abstraction (grub_disk_t disk, char delim) > } > =20 > static void > +probe_partition_type(grub_disk_t disk, const char *partmap, char delim= ) > +{ > + char *type; > + > + if (!disk->partition || !disk->partition->partmap->type || > + strcmp(disk->partition->partmap->name, partmap) !=3D 0) > + { > + putchar (delim); > + return; > + } > + > + if (disk->partition->partmap->type(disk, disk->partition, &type)) > + grub_util_error ("%s", grub_errmsg); > + > + printf ("%s%c", type, delim); > + free(type); > +} > + > +static void > probe (const char *path, char **device_names, char delim) > { > char **drives_names =3D NULL; > @@ -653,44 +671,14 @@ probe (const char *path, char **device_names, cha= r delim) > =20 > if (print =3D=3D PRINT_MSDOS_PARTTYPE) > { > - if (dev->disk->partition > - && strcmp(dev->disk->partition->partmap->name, "msdos") =3D=3D = 0) > - printf ("%02x", dev->disk->partition->msdostype); > - > - putchar (delim); > + probe_partition_type(dev->disk, "msdos", delim); > grub_device_close (dev); > continue; > } > =20 > if (print =3D=3D PRINT_GPT_PARTTYPE) > { > - if (dev->disk->partition > - && strcmp (dev->disk->partition->partmap->name, "gpt") =3D=3D 0= ) > - { > - struct grub_gpt_partentry gptdata; > - grub_partition_t p =3D dev->disk->partition; > - dev->disk->partition =3D dev->disk->partition->parent; > - > - if (grub_disk_read (dev->disk, p->offset, p->index, > - sizeof (gptdata), &gptdata) =3D=3D 0= ) > - { > - grub_gpt_part_type_t gpttype; > - gpttype.data1 =3D grub_le_to_cpu32 (gptdata.type.dat= a1); > - gpttype.data2 =3D grub_le_to_cpu16 (gptdata.type.dat= a2); > - gpttype.data3 =3D grub_le_to_cpu16 (gptdata.type.dat= a3); > - grub_memcpy (gpttype.data4, gptdata.type.data4, 8); > - > - grub_printf ("%08x-%04x-%04x-%02x%02x-%02x%02x%02x%0= 2x%02x%02x", > - gpttype.data1, gpttype.data2, > - gpttype.data3, gpttype.data4[0],=20 > - gpttype.data4[1], gpttype.data4[2], > - gpttype.data4[3], gpttype.data4[4], > - gpttype.data4[5], gpttype.data4[6], > - gpttype.data4[7]); > - } > - dev->disk->partition =3D p; > - } > - putchar (delim); > + probe_partition_type(dev->disk, "gpt", delim); > grub_device_close (dev); > continue; > } >=20 --K2Kb0lUkCR4b2Ip9RH3VhK8IXO5B77NEL Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iF4EAREKAAYFAlQgqxEACgkQmBXlbbo5nOtZkgD/eoW6I+AnrF5erIm7Q0o/TFFM haU4hx3nH8z1NLbMWOIA/RuUrR6w/wWFkdM/j6mfbQAPflAnc3zygISf9s9UckEg =77Hq -----END PGP SIGNATURE----- --K2Kb0lUkCR4b2Ip9RH3VhK8IXO5B77NEL--