[PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.

[PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.

[Peter Jones]

When we have 4kB sectors instead of 512b sectors, hd.partition_start and
grub_partition_get_start() won't match - the latter assumes 512-byte
sectors, and the former gives us the correct number based on the
physical media's sector size.  So when we have to compare them, we need
to compensate.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 grub-core/disk/efi/efidisk.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
index 77ab5b0..a905b52 100644
--- a/grub-core/disk/efi/efidisk.c
+++ b/grub-core/disk/efi/efidisk.c
@@ -791,11 +791,13 @@ grub_efidisk_get_device_name (grub_efi_handle_t *handle)
       auto int find_partition (grub_disk_t disk, const grub_partition_t part);
 
       /* Find the identical partition.  */
-      int find_partition (grub_disk_t disk __attribute__ ((unused)),
-			  const grub_partition_t part)
+      int find_partition (grub_disk_t disk, const grub_partition_t part)
 	{
-	  if (grub_partition_get_start (part) == hd.partition_start
-	      && grub_partition_get_len (part) == hd.partition_size)
+	  struct grub_efidisk_data *d = disk->data;
+	  grub_efi_block_io_media_t *m = d->block_io->media;
+
+	  if (grub_partition_get_start (part) / (m->block_size / 512) == hd.partition_start
+	      && grub_partition_get_len (part) / (m->block_size / 512) == hd.partition_size)
 	    {
 	      partition_name = grub_partition_get_name (part);
 	      return 1;
-- 
1.8.1.4

Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.

[Andrey Borzenkov]

В Wed,  3 Apr 2013 11:53:06 -0400
Peter Jones <pjones@redhat.com> пишет:

> When we have 4kB sectors instead of 512b sectors, hd.partition_start and
> grub_partition_get_start() won't match - the latter assumes 512-byte
> sectors, and the former gives us the correct number based on the
> physical media's sector size.  So when we have to compare them, we need
> to compensate.
> 
> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>  grub-core/disk/efi/efidisk.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> index 77ab5b0..a905b52 100644
> --- a/grub-core/disk/efi/efidisk.c
> +++ b/grub-core/disk/efi/efidisk.c
> @@ -791,11 +791,13 @@ grub_efidisk_get_device_name (grub_efi_handle_t *handle)
>        auto int find_partition (grub_disk_t disk, const grub_partition_t part);
>  

You need to rebase to current trunk which does not use nested functions
anymore. Also in current trunk there is second use of
grub_partition_get_start() which looks like it has the same issue.

>        /* Find the identical partition.  */
> -      int find_partition (grub_disk_t disk __attribute__ ((unused)),
> -			  const grub_partition_t part)
> +      int find_partition (grub_disk_t disk, const grub_partition_t part)
>  	{
> -	  if (grub_partition_get_start (part) == hd.partition_start
> -	      && grub_partition_get_len (part) == hd.partition_size)
> +	  struct grub_efidisk_data *d = disk->data;
> +	  grub_efi_block_io_media_t *m = d->block_io->media;
> +
> +	  if (grub_partition_get_start (part) / (m->block_size / 512) == hd.partition_start
> +	      && grub_partition_get_len (part) / (m->block_size / 512) == hd.partition_size)
>  	    {
>  	      partition_name = grub_partition_get_name (part);
>  	      return 1;

Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.

[Peter Jones]

On Wed, Apr 03, 2013 at 11:03:46PM +0400, Andrey Borzenkov wrote:
> В Wed,  3 Apr 2013 11:53:06 -0400
> Peter Jones <pjones@redhat.com> пишет:
> 
> > When we have 4kB sectors instead of 512b sectors, hd.partition_start and
> > grub_partition_get_start() won't match - the latter assumes 512-byte
> > sectors, and the former gives us the correct number based on the
> > physical media's sector size.  So when we have to compare them, we need
> > to compensate.
> > 
> > Signed-off-by: Peter Jones <pjones@redhat.com>
> > ---
> >  grub-core/disk/efi/efidisk.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> > index 77ab5b0..a905b52 100644
> > --- a/grub-core/disk/efi/efidisk.c
> > +++ b/grub-core/disk/efi/efidisk.c
> > @@ -791,11 +791,13 @@ grub_efidisk_get_device_name (grub_efi_handle_t *handle)
> >        auto int find_partition (grub_disk_t disk, const grub_partition_t part);
> >  
> 
> You need to rebase to current trunk which does not use nested functions
> anymore. Also in current trunk there is second use of
> grub_partition_get_start() which looks like it has the same issue.

Oh, indeed, you're right, that has changed.  Although the current tree looks
like it already takes this in to consideration.

So nevermind then :)

Thanks for having a look.
-- 
        Peter

Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.

[Andrey Borzenkov]

В Thu, 4 Apr 2013 13:52:05 -0400
Peter Jones <pjones@redhat.com> пишет:

> On Wed, Apr 03, 2013 at 11:03:46PM +0400, Andrey Borzenkov wrote:
> > В Wed,  3 Apr 2013 11:53:06 -0400
> > Peter Jones <pjones@redhat.com> пишет:
> > 
> > > When we have 4kB sectors instead of 512b sectors, hd.partition_start and
> > > grub_partition_get_start() won't match - the latter assumes 512-byte
> > > sectors, and the former gives us the correct number based on the
> > > physical media's sector size.  So when we have to compare them, we need
> > > to compensate.
> > > 
> > > Signed-off-by: Peter Jones <pjones@redhat.com>
> > > ---
> > >  grub-core/disk/efi/efidisk.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> > > index 77ab5b0..a905b52 100644
> > > --- a/grub-core/disk/efi/efidisk.c
> > > +++ b/grub-core/disk/efi/efidisk.c
> > > @@ -791,11 +791,13 @@ grub_efidisk_get_device_name (grub_efi_handle_t *handle)
> > >        auto int find_partition (grub_disk_t disk, const grub_partition_t part);
> > >  
> > 
> > You need to rebase to current trunk which does not use nested functions
> > anymore. Also in current trunk there is second use of
> > grub_partition_get_start() which looks like it has the same issue.
> 
> Oh, indeed, you're right, that has changed.  Although the current tree looks
> like it already takes this in to consideration.
> 

Hmm ... I do not see it. Your change looks still necessary.

> So nevermind then :)
> 
> Thanks for having a look.

Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1907 bytes --]

On 04.04.2013 20:14, Andrey Borzenkov wrote:

> В Thu, 4 Apr 2013 13:52:05 -0400
> Peter Jones <pjones@redhat.com> пишет:
> 
>> On Wed, Apr 03, 2013 at 11:03:46PM +0400, Andrey Borzenkov wrote:
>>> В Wed,  3 Apr 2013 11:53:06 -0400
>>> Peter Jones <pjones@redhat.com> пишет:
>>>
>>>> When we have 4kB sectors instead of 512b sectors, hd.partition_start and
>>>> grub_partition_get_start() won't match - the latter assumes 512-byte
>>>> sectors, and the former gives us the correct number based on the
>>>> physical media's sector size.  So when we have to compare them, we need
>>>> to compensate.
>>>>
>>>> Signed-off-by: Peter Jones <pjones@redhat.com>
>>>> ---
>>>>  grub-core/disk/efi/efidisk.c | 10 ++++++----
>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
>>>> index 77ab5b0..a905b52 100644
>>>> --- a/grub-core/disk/efi/efidisk.c
>>>> +++ b/grub-core/disk/efi/efidisk.c
>>>> @@ -791,11 +791,13 @@ grub_efidisk_get_device_name (grub_efi_handle_t *handle)
>>>>        auto int find_partition (grub_disk_t disk, const grub_partition_t part);
>>>>  
>>>
>>> You need to rebase to current trunk which does not use nested functions
>>> anymore. Also in current trunk there is second use of
>>> grub_partition_get_start() which looks like it has the same issue.
>>
>> Oh, indeed, you're right, that has changed.  Although the current tree looks
>> like it already takes this in to consideration.
>>
> 
> Hmm ... I do not see it. Your change looks still necessary.
> 

Probably your copy is a bit old. I adjusted and committed his patch.

>> So nevermind then :)
>>
>> Thanks for having a look.
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]