grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.
@ 2013-04-03 15:53 Peter Jones
  2013-04-03 19:03 ` Andrey Borzenkov
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Jones @ 2013-04-03 15:53 UTC (permalink / raw)
  To: grub-devel

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.
  2013-04-03 15:53 [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks Peter Jones
@ 2013-04-03 19:03 ` Andrey Borzenkov
  2013-04-04 17:52   ` Peter Jones
  0 siblings, 1 reply; 5+ messages in thread
From: Andrey Borzenkov @ 2013-04-03 19:03 UTC (permalink / raw)
  To: grub-devel

В 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;



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.
  2013-04-03 19:03 ` Andrey Borzenkov
@ 2013-04-04 17:52   ` Peter Jones
  2013-04-04 18:14     ` Andrey Borzenkov
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Jones @ 2013-04-04 17:52 UTC (permalink / raw)
  To: The development of GNU GRUB

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.
  2013-04-04 17:52   ` Peter Jones
@ 2013-04-04 18:14     ` Andrey Borzenkov
  2013-04-05  7:09       ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 5+ messages in thread
From: Andrey Borzenkov @ 2013-04-04 18:14 UTC (permalink / raw)
  To: grub-devel

В 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.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks.
  2013-04-04 18:14     ` Andrey Borzenkov
@ 2013-04-05  7:09       ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 5+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-04-05  7:09 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-04-05  7:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-03 15:53 [PATCH] Make grub_efidisk_get_device_name() work on 4K native disks Peter Jones
2013-04-03 19:03 ` Andrey Borzenkov
2013-04-04 17:52   ` Peter Jones
2013-04-04 18:14     ` Andrey Borzenkov
2013-04-05  7:09       ` Vladimir 'φ-coder/phcoder' Serbinenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).