[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH] Add new partition type API and convert GPT type checks.
From: |
Vladimir 'φ-coder/phcoder' Serbinenko |
Subject: |
Re: [RFC PATCH] Add new partition type API and convert GPT type checks. |
Date: |
Tue, 23 Sep 2014 01:04:49 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.0 |
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.
>
> 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(-)
>
> 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;
> }
>
> -static const grub_gpt_part_type_t ldm_type = 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 = data;
> - struct grub_gpt_partentry gptdata;
> - grub_partition_t p2;
> -
> - p2 = disk->partition;
> - disk->partition = p->parent;
> - if (grub_disk_read (disk, p->offset, p->index,
> - sizeof (gptdata), &gptdata))
> - {
> - disk->partition = p2;
> - return 0;
> - }
> - disk->partition = p2;
>
> - if (! grub_memcmp (&gptdata.type, &ldm_type, 16))
> + if (grub_partition_is_type (disk, p, "gpt", GRUB_GPT_PARTITION_TYPE_LDM))
> {
> *sector = p->start + p->len - 1;
> return 1;
> }
> +
> return 0;
> }
>
> 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
> partition)
> 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) != 0)
> + return 0;
> +
> + if (partition->partmap->type(disk, partition, &type) != 0)
> + return 0;
> +
> + r = grub_strcmp (type, partition_type);
> + grub_free (type);
> +
> + return r == 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] =
> 0x45, 0x46, 0x49, 0x20, 0x50, 0x41, 0x52, 0x54
> };
>
> -static const grub_gpt_part_type_t grub_gpt_partition_type_empty =
> GRUB_GPT_PARTITION_TYPE_EMPTY;
> -
> -#ifdef GRUB_UTIL
> -static const grub_gpt_part_type_t grub_gpt_partition_type_bios_boot =
> GRUB_GPT_PARTITION_TYPE_BIOS_BOOT;
> -#endif
> +static const grub_gpt_part_type_t grub_gpt_partition_type_empty =
> + { 0x0, 0x0, 0x0,
> + { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 }
> + };
>
> /* 512 << 7 = 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;
> }
>
> +static grub_err_t
> +grub_gpt_partition_type (grub_disk_t disk, const grub_partition_t partition,
> + char **type)
> +{
> + struct grub_gpt_partentry gptentry;
> + grub_gpt_part_type_t gpttype;
> + grub_partition_t p2;
> + grub_err_t err;
> +
> + p2 = disk->partition;
> + disk->partition = partition->parent;
> + err = grub_disk_read (disk, partition->offset, partition->index,
> + sizeof (gptentry), &gptentry);
> + disk->partition = p2;
> +
> + if (err)
> + return err;
> +
> + gpttype.data1 = grub_le_to_cpu32 (gptentry.type.data1);
> + gpttype.data2 = grub_le_to_cpu16 (gptentry.type.data2);
> + gpttype.data3 = grub_le_to_cpu16 (gptentry.type.data3);
> + grub_memcpy (gpttype.data4, gptentry.type.data4, sizeof (gpttype.data4));
> +
> + *type = 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
>
> /* 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 = data;
> - struct grub_gpt_partentry gptdata;
> - grub_partition_t p2;
> -
> - p2 = disk->partition;
> - disk->partition = p->parent;
> - if (grub_disk_read (disk, p->offset, p->index,
> - sizeof (gptdata), &gptdata))
> - {
> - disk->partition = p2;
> - return 0;
> - }
> - disk->partition = p2;
>
> /* 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 = p->start;
> ctx->len = p->len;
> @@ -215,6 +237,7 @@ static struct grub_partition_map grub_gpt_partition_map =
> {
> .name = "gpt",
> .iterate = grub_gpt_partition_map_iterate,
> + .type = grub_gpt_partition_type,
> #ifdef GRUB_UTIL
> .embed = 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;
> }
>
> +static grub_err_t
> +grub_partition_msdos_type (grub_disk_t disk __attribute__ ((unused)),
> + const grub_partition_t partition, char **type)
> +{
> + *type = grub_xasprintf ("%02x", partition->msdostype);
> + return GRUB_ERR_NONE;
> +}
> +
> #ifdef GRUB_UTIL
>
> #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> @@ -415,6 +423,7 @@ static struct grub_partition_map grub_msdos_partition_map
> =
> {
> .name = "msdos",
> .iterate = grub_partition_msdos_iterate,
> + .type = grub_partition_msdos_type,
> #ifdef GRUB_UTIL
> .embed = 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;
>
> -#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"
>
> 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 grub_disk
> *disk,
> grub_partition_iterate_hook_t hook,
> void *hook_data);
> char *EXPORT_FUNC(grub_partition_get_name) (const grub_partition_t
> partition);
> +int EXPORT_FUNC(grub_partition_is_type) (struct grub_disk *disk,
> + const grub_partition_t partition,
> + const char *partmap_name,
> + const char *partition_type);
>
>
> 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") == 0)
> - return (dev->disk->partition->msdostype == 0x41);
> -
> - if (strcmp (dev->disk->partition->partmap->name, "gpt") == 0)
> - {
> - struct grub_gpt_partentry gptdata;
> - grub_partition_t p = dev->disk->partition;
> - int ret = 0;
> - dev->disk->partition = dev->disk->partition->parent;
> + if (grub_partition_is_type (dev->disk, dev->disk->partition, "msdos",
> "41"))
> + return 1;
>
> - if (grub_disk_read (dev->disk, p->offset, p->index,
> - sizeof (gptdata), &gptdata) == 0)
> - {
> - const grub_gpt_part_type_t template = {
> - 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 = grub_memcmp (&template, &gptdata.type,
> - sizeof (template)) == 0;
> - }
> - dev->disk->partition = p;
> - return ret;
> - }
> + if (grub_partition_is_type (dev->disk, dev->disk->partition,
> + "gpt", GRUB_GPT_PARTITION_TYPE_PREP))
> + return 1;
>
> 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 <grub/fs.h>
> #include <grub/partition.h>
> #include <grub/msdos_partition.h>
> -#include <grub/gpt_partition.h>
> #include <grub/emu/hostdisk.h>
> #include <grub/emu/getroot.h>
> #include <grub/term.h>
> @@ -253,6 +252,25 @@ probe_abstraction (grub_disk_t disk, char delim)
> }
>
> 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) != 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 = NULL;
> @@ -653,44 +671,14 @@ probe (const char *path, char **device_names, char
> delim)
>
> if (print == PRINT_MSDOS_PARTTYPE)
> {
> - if (dev->disk->partition
> - && strcmp(dev->disk->partition->partmap->name, "msdos") == 0)
> - printf ("%02x", dev->disk->partition->msdostype);
> -
> - putchar (delim);
> + probe_partition_type(dev->disk, "msdos", delim);
> grub_device_close (dev);
> continue;
> }
>
> if (print == PRINT_GPT_PARTTYPE)
> {
> - if (dev->disk->partition
> - && strcmp (dev->disk->partition->partmap->name, "gpt") == 0)
> - {
> - struct grub_gpt_partentry gptdata;
> - grub_partition_t p = dev->disk->partition;
> - dev->disk->partition = dev->disk->partition->parent;
> -
> - if (grub_disk_read (dev->disk, p->offset, p->index,
> - sizeof (gptdata), &gptdata) == 0)
> - {
> - grub_gpt_part_type_t gpttype;
> - gpttype.data1 = grub_le_to_cpu32 (gptdata.type.data1);
> - gpttype.data2 = grub_le_to_cpu16 (gptdata.type.data2);
> - gpttype.data3 = grub_le_to_cpu16 (gptdata.type.data3);
> - grub_memcpy (gpttype.data4, gptdata.type.data4, 8);
> -
> - grub_printf
> ("%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]);
> - }
> - dev->disk->partition = p;
> - }
> - putchar (delim);
> + probe_partition_type(dev->disk, "gpt", delim);
> grub_device_close (dev);
> continue;
> }
>
signature.asc
Description: OpenPGP digital signature
Re: [RFC PATCH] Add new partition type API and convert GPT type checks.,
Vladimir 'φ-coder/phcoder' Serbinenko <=