Re: [PATCH 7/9] btrfs: Add support for recovery for a RAID 5 btrfs profiles.
On 17/10/2018 16.14, Daniel Kiper wrote:
> On Thu, Oct 11, 2018 at 08:51:01PM +0200, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli
>>
>> Add support for recovery for a RAID 5 btrfs profile. In addition
>> it is added some code as preparatory work for RAID 6 recovery code.
>>
>> Signed-off-by: Goffredo Baroncelli
>> ---
[...]
>> +
>> + for (failed_devices = 0, i = 0; i < nstripes; i++)
>> +{
>> + struct grub_btrfs_chunk_stripe *stripe;
>> + grub_disk_addr_t paddr;
>> + grub_device_t dev;
>> + grub_err_t err;
>> +
>> + /* after the struct grub_btrfs_chunk_item, there is an array of
>> + struct grub_btrfs_chunk_stripe */
>
> /* Struct grub_btrfs_chunk_stripe lives behind struct grub_btrfs_chunk_item.
> */
What about
/* The struct grub_btrfs_chunk_stripe array lives behind struct
grub_btrfs_chunk_item. */
[...]
>> @@ -921,17 +1061,29 @@ grub_btrfs_read_logical (struct grub_btrfs_data
>> *data, grub_disk_addr_t addr,
>> grub_dprintf ("btrfs", "reading laddr 0x%" PRIxGRUB_UINT64_T "\n",
>>addr);
>>
>> -for (i = 0; i < redundancy; i++)
>> +if (!is_raid56)
>
> Why not "if (is_raid56)"? I asked about that earlier. Please change
> this if and of course code below. It will be much easier to read. And
> you do not need curly brackets for for loop after else.
Frankly speaking I don't see any problem having a if (!...). However I update
the code as your request, hoping to speedup this patch set
[...]
--
gpg @keyserver.linux.it: Goffredo Baroncelli
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 2/2] loader/i386/linux: report UEFI secure boot status to the Linux kernel
Linux kernel from 4.11 has secure_boot member as part of linux_kernel_params.
Currently, GRUB does not populate it, so the kernel reports
"Secure boot could not be determined" on boot. We can populate it in EFI mode,
so the kernel "knows" the status.
Signed-off-by: Ignat Korchagin
---
grub-core/loader/i386/linux.c | 54 ++-
include/grub/i386/linux.h | 14 +--
2 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index 4eab55a2d..7016974a6 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -396,6 +396,57 @@ grub_linux_boot_mmap_fill (grub_uint64_t addr,
grub_uint64_t size,
return 0;
}
+#ifdef GRUB_MACHINE_EFI
+
+/* from
https://github.com/rhboot/shim/blob/b953468e91eac48d2e3817f18cd604e20f39c56b/lib/guid.c#L39
*/
+#define GRUB_EFI_SHIM_LOCK_GUID \
+ { 0x605dab50, 0xe046, 0xe046, { 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b,
0x23 }}
+
+/* mostly taken from
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/firmware/efi/libstub/secureboot.c?h=linux-4.18.y=a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93#n37
+ except for the case, when "SecureBoot" variable is not found, because
+ grub_efi_get_variable does not report EFI_STATUS to the caller */
+static grub_uint8_t
+grub_efi_secureboot_mode (void)
+{
+ grub_efi_guid_t efi_var_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+ grub_size_t efi_var_size = 0;
+ grub_efi_uint32_t attr = 0;
+ grub_uint8_t *secure_boot;
+ grub_uint8_t *setup_mode;
+ grub_uint8_t *moksb_state;
+ grub_uint8_t secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_UNKNOWN;
+
+ secure_boot = grub_efi_get_variable ("SecureBoot", _var_guid,
_var_size);
+ setup_mode = grub_efi_get_variable ("SetupMode", _var_guid,
_var_size);
+ efi_var_guid = GRUB_EFI_SHIM_LOCK_GUID;
+ moksb_state = grub_efi_get_variable_with_attributes ("MokSBState",
_var_guid, _var_size, );
+
+ if (!secure_boot || !setup_mode)
+goto fail;
+
+ if ((*secure_boot == 0) || (*setup_mode == 1))
+secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_DISABLED;
+ else
+secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_ENABLED;
+
+ if (moksb_state)
+{
+ if (!(attr & GRUB_EFI_VARIABLE_RUNTIME_ACCESS) && *moksb_state == 1)
+secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_DISABLED;
+}
+
+fail:
+ if (moksb_state)
+grub_free (moksb_state);
+ if (setup_mode)
+grub_free (setup_mode);
+ if (secure_boot)
+grub_free (secure_boot);
+
+ return secureboot_mode;
+}
+#endif
+
static grub_err_t
grub_linux_boot (void)
{
@@ -574,6 +625,7 @@ grub_linux_boot (void)
grub_efi_uintn_t efi_desc_size;
grub_size_t efi_mmap_target;
grub_efi_uint32_t efi_desc_version;
+ctx.params->secure_boot = grub_efi_secureboot_mode ();
err = grub_efi_finish_boot_services (_mmap_size, efi_mmap_buf, NULL,
_desc_size, _desc_version);
if (err)
@@ -760,7 +812,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
linux_params.code32_start = prot_mode_target + lh.code32_start -
GRUB_LINUX_BZIMAGE_ADDR;
linux_params.kernel_alignment = (1 << align);
- linux_params.ps_mouse = linux_params.padding10 = 0;
+ linux_params.ps_mouse = linux_params.padding11 = 0;
len = sizeof (linux_params) - sizeof (lh);
if (grub_file_read (file, (char *) _params + sizeof (lh), len) != len)
diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
index 60c7c3b5e..4d20abb2e 100644
--- a/include/grub/i386/linux.h
+++ b/include/grub/i386/linux.h
@@ -87,6 +87,12 @@ enum
GRUB_VIDEO_LINUX_TYPE_SIMPLE = 0x70/* Linear framebuffer without any
additional functions. */
};
+/* Possible values for Linux secure_boot kernel parameter */
+#define LINUX_EFI_SECUREBOOT_MODE_UNSET0
+#define LINUX_EFI_SECUREBOOT_MODE_UNKNOWN 1
+#define LINUX_EFI_SECUREBOOT_MODE_DISABLED 2
+#define LINUX_EFI_SECUREBOOT_MODE_ENABLED 3
+
/* For the Linux/i386 boot protocol version 2.10. */
struct linux_i386_kernel_header
{
@@ -270,7 +276,11 @@ struct linux_kernel_params
grub_uint8_t mmap_size; /* 1e8 */
- grub_uint8_t padding9[0x1f1 - 0x1e9];
+ grub_uint8_t padding9[0x1ec - 0x1e9];
+
+ grub_uint8_t secure_boot; /* 1ec */
+
+ grub_uint8_t padding10[0x1f1 - 0x1ed];
grub_uint8_t setup_sects;/* The size of the setup in sectors */
grub_uint16_t root_flags;/* If the root is mounted readonly */
@@ -280,7 +290,7 @@ struct linux_kernel_params
grub_uint16_t vid_mode; /* Video mode control */
grub_uint16_t root_dev; /* Default root device number */
- grub_uint8_t padding10; /* 1fe */
+ grub_uint8_t padding11; /* 1fe */
grub_uint8_t ps_mouse; /* 1ff */
grub_uint16_t jump; /* Jump instruction */
--
2.11.0
[PATCH v2 1/2] efi: add function to read EFI variables with attributes
Signed-off-by: Ignat Korchagin
---
grub-core/kern/efi/efi.c | 13 +++--
include/grub/efi/efi.h | 4
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 708581fcb..5f7d1478f 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -224,7 +224,16 @@ grub_efi_set_variable(const char *var, const
grub_efi_guid_t *guid,
void *
grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
- grub_size_t *datasize_out)
+ grub_size_t *datasize_out)
+{
+ return grub_efi_get_variable_with_attributes (var, guid, datasize_out, NULL);
+}
+
+void *
+grub_efi_get_variable_with_attributes (const char *var,
+ const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out,
+ grub_efi_uint32_t *attributes)
{
grub_efi_status_t status;
grub_efi_uintn_t datasize = 0;
@@ -260,7 +269,7 @@ grub_efi_get_variable (const char *var, const
grub_efi_guid_t *guid,
return NULL;
}
- status = efi_call_5 (r->get_variable, var16, guid, NULL, , data);
+ status = efi_call_5 (r->get_variable, var16, guid, attributes, ,
data);
grub_free (var16);
if (status == GRUB_EFI_SUCCESS)
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index 2c6648d46..727722797 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -77,6 +77,10 @@ grub_err_t EXPORT_FUNC (grub_efi_set_virtual_address_map)
(grub_efi_uintn_t memo
void *EXPORT_FUNC (grub_efi_get_variable) (const char *variable,
const grub_efi_guid_t *guid,
grub_size_t *datasize_out);
+void *EXPORT_FUNC (grub_efi_get_variable_with_attributes) (const char
*variable,
+ const
grub_efi_guid_t *guid,
+ grub_size_t
*datasize_out,
+ grub_efi_uint32_t
*attributes);
grub_err_t
EXPORT_FUNC (grub_efi_set_variable) (const char *var,
const grub_efi_guid_t *guid,
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] loader/i386/linux: report UEFI secure boot status to the Linux kernel
On Tue, Oct 09, 2018 at 04:04:03PM +, Ignat Korchagin wrote:
> Linux kernel from 4.11 has secure_boot member as part of linux_kernel_params.
> Currently, GRUB does not populate it, so the kernel reports
> "Secure boot could not be determined" on boot. We can populate it in EFI mode,
> so the kernel "knows" the status.
>
> Signed-off-by: Ignat Korchagin
> ---
> grub-core/loader/i386/linux.c | 34 +-
> include/grub/i386/linux.h | 12 ++--
> 2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 4eab55a2d..7fc188603 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -396,6 +396,37 @@ grub_linux_boot_mmap_fill (grub_uint64_t addr,
> grub_uint64_t size,
>return 0;
> }
>
> +#ifdef GRUB_MACHINE_EFI
> +static grub_uint8_t
> +grub_efi_secureboot_mode (void)
> +{
> + grub_efi_guid_t efi_var_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
> + grub_size_t efi_var_size = 0;
> + grub_uint8_t *secure_boot;
> + grub_uint8_t *setup_mode;
> + grub_uint8_t secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_UNSET;
> +
> + secure_boot = grub_efi_get_variable ("SecureBoot", _var_guid,
> _var_size);
> + setup_mode = grub_efi_get_variable ("SetupMode", _var_guid,
> _var_size);
> +
> + if (!secure_boot || !setup_mode)
> +goto fail;
> +
> + if ((*secure_boot == 0) || (*setup_mode == 1))
> +secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_DISABLED;
> + else
> +secureboot_mode = LINUX_EFI_SECUREBOOT_MODE_ENABLED;
> +
> +fail:
> + if (setup_mode)
> +grub_free (setup_mode);
> + if (secure_boot)
> +grub_free (secure_boot);
> +
> + return secureboot_mode;
May I ask you to duplicate the logic from
linux/drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot()?
Additionally, please add the comment that it is taken from there.
And it is also worth mentioning the Linux kernel version or commit id.
> +}
> +#endif
> +
> static grub_err_t
> grub_linux_boot (void)
> {
> @@ -574,6 +605,7 @@ grub_linux_boot (void)
> grub_efi_uintn_t efi_desc_size;
> grub_size_t efi_mmap_target;
> grub_efi_uint32_t efi_desc_version;
> +ctx.params->secure_boot = grub_efi_secureboot_mode ();
> err = grub_efi_finish_boot_services (_mmap_size, efi_mmap_buf, NULL,
>_desc_size, _desc_version);
> if (err)
> @@ -760,7 +792,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__
> ((unused)),
>
>linux_params.code32_start = prot_mode_target + lh.code32_start -
> GRUB_LINUX_BZIMAGE_ADDR;
>linux_params.kernel_alignment = (1 << align);
> - linux_params.ps_mouse = linux_params.padding10 = 0;
> + linux_params.ps_mouse = linux_params.padding11 = 0;
>
>len = sizeof (linux_params) - sizeof (lh);
>if (grub_file_read (file, (char *) _params + sizeof (lh), len) !=
> len)
> diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
> index 60c7c3b5e..4493a3fdb 100644
> --- a/include/grub/i386/linux.h
> +++ b/include/grub/i386/linux.h
> @@ -270,7 +270,15 @@ struct linux_kernel_params
>
>grub_uint8_t mmap_size;/* 1e8 */
>
> - grub_uint8_t padding9[0x1f1 - 0x1e9];
> + grub_uint8_t padding9[0x1ec - 0x1e9];
> +
> + grub_uint8_t secure_boot; /* 1ec */
> +#define LINUX_EFI_SECUREBOOT_MODE_UNSET0
> +#define LINUX_EFI_SECUREBOOT_MODE_UNKNOWN 1
> +#define LINUX_EFI_SECUREBOOT_MODE_DISABLED 2
> +#define LINUX_EFI_SECUREBOOT_MODE_ENABLED 3
Please mov this to constants section above.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 9/9] btrfs: Add RAID 6 recovery for a btrfs filesystem.
On Thu, Oct 11, 2018 at 08:51:03PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli
>
> Add the RAID 6 recovery, in order to use a RAID 6 filesystem even if some
> disks (up to two) are missing. This code use the md RAID 6 code already
> present in grub.
>
> Signed-off-by: Goffredo Baroncelli
> Reviewed-by: Daniel Kiper
> ---
> grub-core/fs/btrfs.c | 60 +++-
> 1 file changed, 54 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index d066d54cc..d20ee09e4 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -702,11 +703,36 @@ rebuild_raid5 (char *dest, struct raid56_buffer
> *buffers,
> }
> }
>
> +static grub_err_t
> +raid6_recover_read_buffer (void *data, int disk_nr,
> +grub_uint64_t addr __attribute__ ((unused)),
> +void *dest, grub_size_t size)
> +{
> +struct raid56_buffer *buffers = data;
> +
> +if (!buffers[disk_nr].data_is_valid)
> + return grub_errno = GRUB_ERR_READ_ERROR;
> +
> +grub_memcpy(dest, buffers[disk_nr].buf, size);
> +
> +return grub_errno = GRUB_ERR_NONE;
> +}
> +
> +static void
> +rebuild_raid6 (struct raid56_buffer *buffers, grub_uint64_t nstripes,
> + grub_uint64_t csize, grub_uint64_t parities_pos, void *dest,
> + grub_uint64_t stripen)
> +
> +{
> + grub_raid6_recover_gen (buffers, nstripes, stripen, parities_pos,
> + dest, 0, csize, 0, raid6_recover_read_buffer);
> +}
> +
> static grub_err_t
> raid56_read_retry (struct grub_btrfs_data *data,
> struct grub_btrfs_chunk_item *chunk,
> -grub_uint64_t stripe_offset,
> -grub_uint64_t csize, void *buf)
> +grub_uint64_t stripe_offset, grub_uint64_t stripen,
> +grub_uint64_t csize, void *buf, grub_uint64_t parities_pos)
> {
>struct raid56_buffer *buffers;
>grub_uint64_t nstripes = grub_le_to_cpu16 (chunk->nstripes);
> @@ -779,6 +805,15 @@ raid56_read_retry (struct grub_btrfs_data *data,
>ret = GRUB_ERR_READ_ERROR;
>goto cleanup;
> }
> + else if (failed_devices > 2 && (chunk_type & GRUB_BTRFS_CHUNK_TYPE_RAID6))
> +{
> + grub_dprintf ("btrfs",
> + "not enough disks for raid6: total %" PRIuGRUB_UINT64_T
s/not enough disks for raid6/not enough disks for RAID 6/
You are using "RAID 5" in earlier patch, so, please be consistent
and use "RAID 6" here.
> + ", missing %" PRIuGRUB_UINT64_T "\n",
> + nstripes, failed_devices);
> + ret = GRUB_ERR_READ_ERROR;
> + goto cleanup;
> +}
>else
> grub_dprintf ("btrfs",
> "enough disks for RAID 5 rebuilding: total %"
> @@ -789,7 +824,7 @@ raid56_read_retry (struct grub_btrfs_data *data,
>if (chunk_type & GRUB_BTRFS_CHUNK_TYPE_RAID5)
> rebuild_raid5 (buf, buffers, nstripes, csize);
>else
> -grub_dprintf ("btrfs", "called rebuild_raid6(), NOT IMPLEMENTED\n");
> +rebuild_raid6 (buffers, nstripes, csize, parities_pos, buf, stripen);
>
>ret = GRUB_ERR_NONE;
> cleanup:
> @@ -879,9 +914,11 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data,
> grub_disk_addr_t addr,
> unsigned redundancy = 1;
> unsigned i, j;
> int is_raid56;
> + grub_uint64_t parities_pos = 0;
>
> - is_raid56 = !!(grub_le_to_cpu64 (chunk->type) &
> -GRUB_BTRFS_CHUNK_TYPE_RAID5);
> +is_raid56 = !!(grub_le_to_cpu64 (chunk->type) &
> +(GRUB_BTRFS_CHUNK_TYPE_RAID5 |
> + GRUB_BTRFS_CHUNK_TYPE_RAID6));
>
> if (grub_le_to_cpu64 (chunk->size) <= off)
> {
> @@ -1030,6 +1067,17 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data,
> grub_disk_addr_t addr,
> */
> grub_divmod64 (high + stripen, nstripes, );
>
> + /*
> +* parities_pos is equal to "(high - nparities) % nstripes"
> +* (see the diagram above).
> +* However "high - nparities" might be negative (eg when high
> +* == 0) leading to an incorrect computation.
However, "high - nparities" can be negative, eg. when high == 0,
leading to an incorrect results.
> +* Instead "high + nstripes - nparities" is always positive and
> +* in modulo nstripes is equal to "(high - nparities) % nstripes"
"high + nstripes - nparities" is always positive and modulo
nstripes is equal to "(high - nparities) % nstripes".
If you change above mentioned things you can retain my
Reviewed-by: Daniel Kiper
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 7/9] btrfs: Add support for recovery for a RAID 5 btrfs profiles.
On Thu, Oct 11, 2018 at 08:51:01PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli
>
> Add support for recovery for a RAID 5 btrfs profile. In addition
> it is added some code as preparatory work for RAID 6 recovery code.
>
> Signed-off-by: Goffredo Baroncelli
> ---
> grub-core/fs/btrfs.c | 162 +--
> 1 file changed, 157 insertions(+), 5 deletions(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index 899dc32b7..d066d54cc 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -665,6 +666,141 @@ btrfs_read_from_chunk (struct grub_btrfs_data *data,
> return err;
> }
>
> +struct raid56_buffer {
> + void *buf;
> + int data_is_valid;
> +};
> +
> +static void
> +rebuild_raid5 (char *dest, struct raid56_buffer *buffers,
> +grub_uint64_t nstripes, grub_uint64_t csize)
> +{
> + grub_uint64_t i;
> + int first;
> +
> + for(i = 0; buffers[i].data_is_valid && i < nstripes; i++);
> +
> + if (i == nstripes)
> +{
> + grub_dprintf ("btrfs", "called rebuild_raid5(), but all disks are
> OK\n");
> + return;
> +}
> +
> + grub_dprintf ("btrfs", "rebuilding RAID 5 stripe #%" PRIuGRUB_UINT64_T
> "\n", i);
> +
> + for (i = 0, first = 1; i < nstripes; i++)
> +{
> + if (!buffers[i].data_is_valid)
> + continue;
> +
> + if (first) {
> + grub_memcpy(dest, buffers[i].buf, csize);
> + first = 0;
> + } else
> + grub_crypto_xor (dest, dest, buffers[i].buf, csize);
> +
Please drop this empty line.
> +}
> +}
> +
> +static grub_err_t
> +raid56_read_retry (struct grub_btrfs_data *data,
> +struct grub_btrfs_chunk_item *chunk,
> +grub_uint64_t stripe_offset,
> +grub_uint64_t csize, void *buf)
> +{
> + struct raid56_buffer *buffers;
> + grub_uint64_t nstripes = grub_le_to_cpu16 (chunk->nstripes);
> + grub_uint64_t chunk_type = grub_le_to_cpu64 (chunk->type);
> + grub_err_t ret = GRUB_ERR_OUT_OF_MEMORY;
> + grub_uint64_t i, failed_devices;
> +
> + buffers = grub_zalloc (sizeof(*buffers) * nstripes);
> + if (!buffers)
> +goto cleanup;
> +
> + for (i = 0; i < nstripes; i++)
> +{
> + buffers[i].buf = grub_zalloc (csize);
> + if (!buffers[i].buf)
> + goto cleanup;
> +}
> +
> + for (failed_devices = 0, i = 0; i < nstripes; i++)
> +{
> + struct grub_btrfs_chunk_stripe *stripe;
> + grub_disk_addr_t paddr;
> + grub_device_t dev;
> + grub_err_t err;
> +
> + /* after the struct grub_btrfs_chunk_item, there is an array of
> + struct grub_btrfs_chunk_stripe */
/* Struct grub_btrfs_chunk_stripe lives behind struct grub_btrfs_chunk_item. */
> + stripe = (struct grub_btrfs_chunk_stripe *) (chunk + 1) + i;
> +
> + paddr = grub_le_to_cpu64 (stripe->offset) + stripe_offset;
> + grub_dprintf ("btrfs", "reading paddr %" PRIxGRUB_UINT64_T
> +" from stripe ID %" PRIxGRUB_UINT64_T "\n", paddr,
> +stripe->device_id);
> +
> + dev = find_device (data, stripe->device_id);
> + if (!dev)
> + {
> + buffers[i].data_is_valid = 0;
> + grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T " FAILED (dev ID
> %"
> + PRIxGRUB_UINT64_T ")\n", i, stripe->device_id);
> + failed_devices++;
> + continue;
> + }
> +
> + err = grub_disk_read (dev->disk, paddr >> GRUB_DISK_SECTOR_BITS,
> + paddr & (GRUB_DISK_SECTOR_SIZE - 1),
> + csize, buffers[i].buf);
> + if (err == GRUB_ERR_NONE)
> + {
> + buffers[i].data_is_valid = 1;
> + grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T " Ok (dev ID %"
s/Ok/OK/
> + PRIxGRUB_UINT64_T ")\n", i, stripe->device_id);
> + }
> + else
> + {
> + buffers[i].data_is_valid = 0;
> + grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T
> + " FAILED (dev ID %" PRIxGRUB_UINT64_T ")\n", i,
> + stripe->device_id);
> + failed_devices++;
> + }
> +}
> +
> + if (failed_devices > 1 && (chunk_type & GRUB_BTRFS_CHUNK_TYPE_RAID5))
> +{
> + grub_dprintf ("btrfs",
> + "not enough disks for RAID 5: total %" PRIuGRUB_UINT64_T
> + ", missing %" PRIuGRUB_UINT64_T "\n",
> + nstripes, failed_devices);
> + ret = GRUB_ERR_READ_ERROR;
> + goto cleanup;
> +}
> + else
> +grub_dprintf ("btrfs",
> + "enough disks for RAID 5 rebuilding: total %"
s/enough disks for RAID 5 rebuilding/enough disks for RAID 5/
> + PRIuGRUB_UINT64_T ", missing %" PRIuGRUB_UINT64_T "\n",
> + nstripes, failed_devices);
> +
> + /* if these are enough, try to rebuild the data */
/* We have enough disks. So,
Re: [PATCH 4/9] btrfs: Avoid a rescan for a device which was already not found.
On Thu, Oct 11, 2018 at 08:50:58PM +0200, Goffredo Baroncelli wrote: > From: Goffredo Baroncelli > > Currently read from missing device triggers rescan. However, it is never > recorded that the device is missing. So, each read of a missing device > triggers rescan again and again. This behavior causes a lot of unneeded > rescans leading to huge slowdowns. > > This patch fixes above mentioned issue. Information about missing devices > is stored in the data->devices_attached[] array as NULL value in dev > member. Rescan is triggered only if no information is found for a given > device. This means that only first time read triggers rescan. > > The patch drops premature return. This way data->devices_attached[] is > filled even when a given device is missing. > > Signed-off-by: Goffredo Baroncelli I changed commit message, so, you should add Signed-off-by: Daniel Kiper I simply forgot to tell you about that. Sorry. And below you can add Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.
On Thu, Oct 11, 2018 at 08:50:55PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli
>
> Signed-off-by: Goffredo Baroncelli
> Signed-off-by: Daniel Kiper
One nit pick below...
Otherwise you can add Reviewed-by: Daniel Kiper
> ---
> grub-core/fs/btrfs.c | 73
> 1 file changed, 73 insertions(+)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index be195448d..933a57d3b 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -119,6 +119,8 @@ struct grub_btrfs_chunk_item
> #define GRUB_BTRFS_CHUNK_TYPE_RAID1 0x10
> #define GRUB_BTRFS_CHUNK_TYPE_DUPLICATED0x20
> #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
> +#define GRUB_BTRFS_CHUNK_TYPE_RAID5 0x80
> +#define GRUB_BTRFS_CHUNK_TYPE_RAID6 0x100
>grub_uint8_t dummy2[0xc];
>grub_uint16_t nstripes;
>grub_uint16_t nsubstripes;
> @@ -764,6 +766,77 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data,
> grub_disk_addr_t addr,
> stripe_offset = low + chunk_stripe_length
> * high;
> csize = chunk_stripe_length - low;
> + break;
> + }
> + case GRUB_BTRFS_CHUNK_TYPE_RAID5:
> + case GRUB_BTRFS_CHUNK_TYPE_RAID6:
> + {
> + grub_uint64_t nparities, stripe_nr, high, low;
> +
> + redundancy = 1; /* no redundancy for now */
> +
> + if (grub_le_to_cpu64 (chunk->type) & GRUB_BTRFS_CHUNK_TYPE_RAID5)
> + {
> + grub_dprintf ("btrfs", "RAID5\n");
> + nparities = 1;
> + }
> + else
> + {
> + grub_dprintf ("btrfs", "RAID6\n");
> + nparities = 2;
> + }
> +
> + /*
> +* RAID 6 layout consists of several stripes spread over
> +* the disks, e.g.:
> +*
> +* Disk_0 Disk_1 Disk_2 Disk_3
> +* A0 B0 P0 Q0
> +* Q1 A1 B1 P1
> +* P2 Q2 A2 B2
> +*
> +* Note: placement of the parities depend on row number.
> +*
> +* Pay attention that the btrfs terminology may differ from
> +* terminology used in other RAID implementations, e.g. LVM,
> +* dm or md. The main difference is that btrfs calls contiguous
> +* block of data on a given disk, e.g. A0, stripe instead of
> chunk.
> +*
> +* The variables listed below have following meaning:
> +* - stripe_nr is the stripe number excluding the parities
> +* (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
> +* - high is the row number (0 for A0...Q0, 1 for Q1...P1,
> etc.),
> +* - stripen is the disk number in a row (0 for A0, Q1, P2,
> +* 1 for B0, A1, Q2, etc.),
> +* - off is the logical address to read,
> +* - chunk_stripe_length is the size of a stripe (typically 64
> KiB),
> +* - nstripes is the number of disks in a row,
> +* - low is the offset of the data inside a stripe,
> +* - stripe_offset is the data offset in an array,
> +* - csize is the "potential" data to read; it will be reduced
> +* to size if the latter is smaller,
> +* - nparities is the number of parities (1 for RAID 5, 2 for
> +* RAID 6); used only in RAID 5/6 code.
> +*/
> + stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
> +
> + /*
> +* stripen is computed without the parities
> +* (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
> +*/
> + high = grub_divmod64 (stripe_nr, nstripes - nparities, );
> +
> + /*
> +* The stripes are spread over the disks. Every each row their
> +* positions are shifted by 1 place. So, the real disks number
> +* change. Hence, we have to take current row number modulo
> +* nstripes into account (0 for A0, 1 for A1, 2 for A2, etc.).
s/current row number modulo nstripes into account/into account current row
number modulo nstripes/
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
