Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2009-07-23 Thread Vladimir 'phcoder' Serbinenko
On Thu, Jul 23, 2009 at 2:04 AM, Pavel Roskinpro...@gnu.org wrote:
 On Thu, 2009-07-23 at 00:18 +0200, Vladimir 'phcoder' Serbinenko wrote:

 There is strictly no need to do this restructuration. The real bug is
 different fix would be setting dev-disk-partition to 0 before
 calling grub_disk_read and restoring it afterwards. This part of code
 is changed anyway with my nested partition patch and I was hoping it
 could be applied quickly. Could you test nestpart branch of my
 repository?

 I'm not comfortable to approve a 60k long patch that reorganizes the
 code and fixed numerous bugs in the same time.  I'm sure bugs can be
 fixed separately.

It fixed bugs because it replaced some parts of code with new
constructions. So basically wht you ask is to do some work and then
abandon, it additionally spending time rediffing patches. Similar
thing happens when you ask to split patches but then all split go in
without any discussion. Sorry, but this is just useless work
 --
 Regards,
 Pavel Roskin


 ___
 Grub-devel mailing list
 Grub-devel@gnu.org
 http://lists.gnu.org/mailman/listinfo/grub-devel




-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2009-07-23 Thread Jean-Pierre Flori
On Thu, Jul 23, 2009 at 2:04 AM, Pavel Roskinaddr...@hidden wrote:
 On Thu, 2009-07-23 at 00:18 +0200, Vladimir 'phcoder' Serbinenko wrote:

 There is strictly no need to do this restructuration. The real bug is
 different fix would be setting dev-disk-partition to 0 before
 calling grub_disk_read and restoring it afterwards. This part of code
 is changed anyway with my nested partition patch and I was hoping it
 could be applied quickly. Could you test nestpart branch of my
 repository?
With your version exerything seems to work fine.
Thanks a lot.

 I'm not comfortable to approve a 60k long patch that reorganizes the
 code and fixed numerous bugs in the same time.  I'm sure bugs can be
 fixed separately.

It fixed bugs because it replaced some parts of code with new
constructions. So basically wht you ask is to do some work and then
abandon, it additionally spending time rediffing patches. Similar
thing happens when you ask to split patches but then all split go in
without any discussion. Sorry, but this is just useless work
 --
 Regards,
 Pavel Roskin


 ___
 Grub-devel mailing list
 addr...@hidden
 http://lists.gnu.org/mailman/listinfo/grub-devel




--
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

-- 
Jean-Pierre Flori


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2009-07-22 Thread Jean-Pierre Flori
As already mentioned in this mailing list (
http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00192.html)nhttp://lists.gnu.org/archive/html/grub-devel/2008-07/msg00192.html%29nI
can't chainload Syslinux from Grub2 on a disk with a pc partition
table
(Syslinux is installed on a primary partition).
I just get Boot error (from Syslinux i guess).
However it does work from Grub Legacy.
I also couldn't chainload Syslinux from Grub2 if the disk used a GPT
partition table.
The patch provided in the previous thread and adapted to current Debian
unstable Grub2 package (1.96+20090721-3, patch attached) fixes the problem.
Have any decision been made to include that patch in a future release (or a
better one just providing enough information for Syslinux to boot as it
was mentioned that this one copies more than necessary) ?

Thanks.

-- 
Jean-Pierre Flori
diff -ru grub2-1.96+20090721/include/grub/pc_partition.h grub2+patch/include/grub/pc_partition.h
--- grub2-1.96+20090721/include/grub/pc_partition.h	2009-05-09 13:04:08.0 +0200
+++ grub2+patch/include/grub/pc_partition.h	2009-07-22 10:49:34.557536456 +0200
@@ -184,6 +184,9 @@
 
   /* The offset of the extended partition.  */
   unsigned long ext_offset;
+
+  /* Partition entry. */
+  struct grub_pc_partition_entry pc_part_entry;
 };
 
 static inline int
diff -ru grub2-1.96+20090721/loader/i386/pc/chainloader.c grub2+patch/loader/i386/pc/chainloader.c
--- grub2-1.96+20090721/loader/i386/pc/chainloader.c	2009-06-11 18:13:39.0 +0200
+++ grub2+patch/loader/i386/pc/chainloader.c	2009-07-22 10:51:36.170516258 +0200
@@ -32,6 +32,7 @@
 #include grub/dl.h
 #include grub/command.h
 #include grub/machine/biosnum.h
+#include grub/pc_partition.h
 
 static grub_dl_t my_mod;
 static int boot_drive;
@@ -94,10 +95,11 @@
   dev = grub_device_open (0);
   if (dev  dev-disk  dev-disk-partition)
 {
-  grub_disk_read (dev-disk, dev-disk-partition-offset, 446, 64,
-		  (void *) GRUB_MEMORY_MACHINE_PART_TABLE_ADDR);
-  part_addr = (void *) (GRUB_MEMORY_MACHINE_PART_TABLE_ADDR
-			+ (dev-disk-partition-index  4));
+  struct grub_pc_partition *pcdata = dev-disk-partition-data;
+  grub_memcpy((void *) GRUB_MEMORY_MACHINE_PART_TABLE_ADDR,
+			  (void *) pcdata-pc_part_entry,
+			  sizeof(struct grub_pc_partition_entry));
+  part_addr = (void *) (GRUB_MEMORY_MACHINE_PART_TABLE_ADDR);
 }
 
   if (dev)
diff -ru grub2-1.96+20090721/partmap/pc.c grub2+patch/partmap/pc.c
--- grub2-1.96+20090721/partmap/pc.c	2009-06-10 23:04:23.0 +0200
+++ grub2+patch/partmap/pc.c	2009-07-22 10:53:34.679495828 +0200
@@ -131,6 +131,8 @@
 	  pcdata.bsd_part = -1;
 	  pcdata.dos_type = e-type;
 	  pcdata.bsd_type = -1;
+	  grub_memcpy(pcdata.pc_part_entry, e, sizeof(struct grub_pc_partition_entry));
+	  pcdata.pc_part_entry.start = grub_cpu_to_le32(p.offset + grub_le_to_cpu32 (e-start));
 
 	  grub_dprintf (partition,
 			partition %d: flag 0x%x, type 0x%x, start 0x%llx, len 0x%llx\n,
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2009-07-22 Thread Vladimir 'phcoder' Serbinenko
On Thu, Jul 23, 2009 at 12:11 AM, Jean-Pierre Florijpfl...@gmail.com wrote:
 As already mentioned in this mailing list
 (http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00192.html)n I
 can't chainload Syslinux from Grub2 on a disk with a pc partition table
 (Syslinux is installed on a primary partition).
 I just get Boot error (from Syslinux i guess).
 However it does work from Grub Legacy.
 I also couldn't chainload Syslinux from Grub2 if the disk used a GPT
 partition table.
 The patch provided in the previous thread and adapted to current Debian
 unstable Grub2 package (1.96+20090721-3, patch attached) fixes the problem.
 Have any decision been made to include that patch in a future release (or a
 better one just providing enough information for Syslinux to boot as it
 was mentioned that this one copies more than necessary) ?

There is strictly no need to do this restructuration. The real bug is
different fix would be setting dev-disk-partition to 0 before
calling grub_disk_read and restoring it afterwards. This part of code
is changed anyway with my nested partition patch and I was hoping it
could be applied quickly. Could you test nestpart branch of my
repository?
 Thanks.

 --
 Jean-Pierre Flori

 ___
 Grub-devel mailing list
 Grub-devel@gnu.org
 http://lists.gnu.org/mailman/listinfo/grub-devel





-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2009-07-22 Thread Pavel Roskin
On Thu, 2009-07-23 at 00:18 +0200, Vladimir 'phcoder' Serbinenko wrote:

 There is strictly no need to do this restructuration. The real bug is
 different fix would be setting dev-disk-partition to 0 before
 calling grub_disk_read and restoring it afterwards. This part of code
 is changed anyway with my nested partition patch and I was hoping it
 could be applied quickly. Could you test nestpart branch of my
 repository?

I'm not comfortable to approve a 60k long patch that reorganizes the
code and fixed numerous bugs in the same time.  I'm sure bugs can be
fixed separately.

-- 
Regards,
Pavel Roskin


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2008-07-10 Thread Lucas Gadani
On Sun, Jul 6, 2008 at 5:47 PM, Lucas Gadani [EMAIL PROTECTED] wrote:
 On Sun, Jul 6, 2008 at 18:41, Robert Millan [EMAIL PROTECTED] wrote:

 Looks like it increases pc.mod code to store information that is only useful
 to the chainloader.  Is there a reason for not having the chainloader
 retrieve it?  (note that for disk access we have a cache)

 We would need to duplicate the code that iterates through the logical
 partitions in the chainloader or we can change
 pc_partition_map_iterate hook function to also receive
 grub_pc_partition (we can't use the pc_partition_map_iterate cause the
 hook function only receives a grub_disk_t and grub_partition_t, but
 not the grub_pc_partition that's necessary to reconstruct the
 partition info).

What would be the best patch to get accepted into grub? I'm willing to
code the patch, but first I'd like to know the best way to get
comitted to grub.

-- 
Lucas


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2008-07-10 Thread Bean
On Thu, Jul 10, 2008 at 10:08 PM, Lucas Gadani [EMAIL PROTECTED] wrote:
 On Sun, Jul 6, 2008 at 5:47 PM, Lucas Gadani [EMAIL PROTECTED] wrote:
 On Sun, Jul 6, 2008 at 18:41, Robert Millan [EMAIL PROTECTED] wrote:

 Looks like it increases pc.mod code to store information that is only useful
 to the chainloader.  Is there a reason for not having the chainloader
 retrieve it?  (note that for disk access we have a cache)

 We would need to duplicate the code that iterates through the logical
 partitions in the chainloader or we can change
 pc_partition_map_iterate hook function to also receive
 grub_pc_partition (we can't use the pc_partition_map_iterate cause the
 hook function only receives a grub_disk_t and grub_partition_t, but
 not the grub_pc_partition that's necessary to reconstruct the
 partition info).

 What would be the best patch to get accepted into grub? I'm willing to
 code the patch, but first I'd like to know the best way to get
 comitted to grub.

Hi,

I don't know how many fields of partition table is actually used by
syslinux. But from your code, I get the sense that only the start
sector is used. If that's the case, there is no need to save the
partition entry, we can just set the value in chainloader.

-- 
Bean


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2008-07-06 Thread Robert Millan
On Mon, Jun 30, 2008 at 02:28:24AM +, Lucas Gadani wrote:
 Hello,
 
 This is my first patch for GRUB, and I haven't found any coding
 styles/formatting, nor developer documentation, so, if there's
 something wrong, please, forgive me and point me to the right
 direction.
 
 Current implementation of the chainloading code does not work, cause
 it reads the partition table from the first sector of the root device,
 and the root device does not contain partition information. I've
 changed the way partition table are loaded, keeping a copy of the
 partition entry in grub_partition_t structs data field, and just
 loading this information in the correct address when chainloading.
 I've also changed the logical partitions start address to point to the
 actual disk sector, instead of pointing to the sector relative to the
 beginning of the extended partition, so we can now boot into logical
 partitions (for example, syslinux doesn't boot if the partition table
 doesn't map to the correct disk sector).
 
 Any comments are appreciated.

Looks like it increases pc.mod code to store information that is only useful
to the chainloader.  Is there a reason for not having the chainloader
retrieve it?  (note that for disk access we have a cache)

-- 
Robert Millan

GPLv2 I know my rights; I want my phone call!
DRM What good is a phone call… if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2008-07-06 Thread Lucas Gadani
On Sun, Jul 6, 2008 at 18:41, Robert Millan [EMAIL PROTECTED] wrote:

 Looks like it increases pc.mod code to store information that is only useful
 to the chainloader.  Is there a reason for not having the chainloader
 retrieve it?  (note that for disk access we have a cache)

We would need to duplicate the code that iterates through the logical
partitions in the chainloader or we can change
pc_partition_map_iterate hook function to also receive
grub_pc_partition (we can't use the pc_partition_map_iterate cause the
hook function only receives a grub_disk_t and grub_partition_t, but
not the grub_pc_partition that's necessary to reconstruct the
partition info).

-- 
Lucas


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix chainloding + Chainloading into logical partitions

2008-06-30 Thread Fulvio Scapin
2008/6/30 Lucas Gadani [EMAIL PROTECTED]:

 Hello,

 This is my first patch for GRUB, and I haven't found any coding
 styles/formatting, nor developer documentation, so, if there's
 something wrong, please, forgive me and point me to the right
 direction.

 Current implementation of the chainloading code does not work, cause
 it reads the partition table from the first sector of the root device,
 and the root device does not contain partition information. I've
 changed the way partition table are loaded, keeping a copy of the
 partition entry in grub_partition_t structs data field, and just
 loading this information in the correct address when chainloading.
 I've also changed the logical partitions start address to point to the
 actual disk sector, instead of pointing to the sector relative to the
 beginning of the extended partition, so we can now boot into logical
 partitions (for example, syslinux doesn't boot if the partition table
 doesn't map to the correct disk sector).

 Any comments are appreciated.

 --
 Lucas

Thank you so much for this. I've been having problem for weeks trying to
boot some syslinux-bootable partitions on a usb stick. I knew of the problem
but lacked the necessary knowledge to contribute myself. I've just begun to
try the patched grub, but until now performs as expected.
Something that might be related: I've created a stick with a GPT label, and
I get partitions to boot using syslinux, or rather, they all boot until the
fourth. The fifth and the following ones don't, even when I clone the
contents of another partition which does boot.
I suspect a similar problem to the one you've addressed.
Thanks again for the effort.
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel