* [PATCH] Fix grub-probe partition naming on FreeBSD
@ 2010-06-13 10:48 Colin Watson
2010-06-13 15:53 ` Grégoire Sutre
2010-07-01 20:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 3+ messages in thread
From: Colin Watson @ 2010-06-13 10:48 UTC (permalink / raw)
To: grub-devel; +Cc: Axel Beckert, 585068
The following patch is aimed at fixing this Debian bug:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585068
I've tested it on Debian GNU/kFreeBSD and it seems to be producing sensible
output now.
The one glitch is that if you ask it to probe /dev/ad0s1a, it returns
(hd0,msdos1) rather than (hd0,msdos1,bsd1): this is because both /dev/ad0s1
and /dev/ad0s1a have the same start sector, and it just uses the first one
it finds. When I set prefix to (hd0,msdos1)/boot/grub, GRUB can read from
that perfectly well, so can I ignore this glitch on the basis that it
doesn't cause a practical problem?
2010-06-13 Colin Watson <cjwatson@ubuntu.com>
* kern/emu/hostdisk.c: Include <sys/ioctl.h> and <sys/disklabel.h>
on FreeBSD. Define HAVE_DIOCGDINFO on NetBSD and FreeBSD to reduce
the verbosity of later #ifs.
(find_partition_start): Define this function on FreeBSD too.
(device_is_wholedisk) [__FreeBSD__ || __FreeBSD_kernel__]: New
function.
(grub_util_biosdisk_get_grub_dev): Use partition-start-sector logic
on FreeBSD.
=== modified file 'kern/emu/hostdisk.c'
--- kern/emu/hostdisk.c 2010-06-13 00:36:39 +0000
+++ kern/emu/hostdisk.c 2010-06-13 10:27:31 +0000
@@ -102,9 +102,15 @@ struct hd_geometry
# include <libdevmapper.h>
#endif
-#if defined(__NetBSD__)
+#if defined(__NetBSD__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+# define HAVE_DIOCGDINFO
# include <sys/ioctl.h>
# include <sys/disklabel.h> /* struct disklabel */
+#else /* !defined(__NetBSD__) && !defined(__FreeBSD__) && !defined(__FreeBSD_kernel__) */
+# undef HAVE_DIOCGDINFO
+#endif /* defined(__NetBSD__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__) */
+
+#if defined(__NetBSD__)
# ifdef HAVE_GETRAWPARTITION
# include <util.h> /* getrawpartition */
# endif /* HAVE_GETRAWPARTITION */
@@ -329,17 +335,17 @@ device_is_mapped (const char *dev)
}
#endif /* HAVE_DEVICE_MAPPER */
-#if defined(__linux__) || defined(__CYGWIN__) || defined(__NetBSD__)
+#if defined(__linux__) || defined(__CYGWIN__) || defined(HAVE_DIOCGDINFO)
static grub_disk_addr_t
find_partition_start (const char *dev)
{
int fd;
-# if !defined(__NetBSD__)
+# if !defined(HAVE_DIOCGDINFO)
struct hd_geometry hdg;
-# else /* defined(__NetBSD__) */
+# else /* defined(HAVE_DIOCGDINFO) */
struct disklabel label;
int p_index;
-# endif /* !defined(__NetBSD__) */
+# endif /* !defined(HAVE_DIOCGDINFO) */
# ifdef HAVE_DEVICE_MAPPER
if (grub_device_mapper_supported () && device_is_mapped (dev)) {
@@ -413,36 +419,38 @@ devmapper_fail:
if (fd == -1)
{
grub_error (GRUB_ERR_BAD_DEVICE,
-# if !defined(__NetBSD__)
+# if !defined(HAVE_DIOCGDINFO)
"cannot open `%s' while attempting to get disk geometry", dev);
-# else /* defined(__NetBSD__) */
+# else /* defined(HAVE_DIOCGDINFO) */
"cannot open `%s' while attempting to get disk label", dev);
-# endif /* !defined(__NetBSD__) */
+# endif /* !defined(HAVE_DIOCGDINFO) */
return 0;
}
-# if !defined(__NetBSD__)
+# if !defined(HAVE_DIOCGDINFO)
if (ioctl (fd, HDIO_GETGEO, &hdg))
-# else /* defined(__NetBSD__) */
+# else /* defined(HAVE_DIOCGDINFO) */
+# if defined(__NetBSD__)
configure_device_driver (fd);
+# endif /* defined(__NetBSD__) */
if (ioctl (fd, DIOCGDINFO, &label) == -1)
-# endif /* !defined(__NetBSD__) */
+# endif /* !defined(HAVE_DIOCGDINFO) */
{
grub_error (GRUB_ERR_BAD_DEVICE,
-# if !defined(__NetBSD__)
+# if !defined(HAVE_DIOCGDINFO)
"cannot get disk geometry of `%s'", dev);
-# else /* defined(__NetBSD__) */
+# else /* defined(HAVE_DIOCGDINFO) */
"cannot get disk label of `%s'", dev);
-# endif /* !defined(__NetBSD__) */
+# endif /* !defined(HAVE_DIOCGDINFO) */
close (fd);
return 0;
}
close (fd);
-# if !defined(__NetBSD__)
+# if !defined(HAVE_DIOCGDINFO)
return hdg.start;
-# else /* defined(__NetBSD__) */
+# else /* defined(HAVE_DIOCGDINFO) */
p_index = dev[strlen(dev) - 1] - 'a';
if (p_index >= label.d_npartitions)
@@ -452,9 +460,9 @@ devmapper_fail:
return 0;
}
return (grub_disk_addr_t) label.d_partitions[p_index].p_offset;
-# endif /* !defined(__NetBSD__) */
+# endif /* !defined(HAVE_DIOCGDINFO) */
}
-#endif /* __linux__ || __CYGWIN__ */
+#endif /* __linux__ || __CYGWIN__ || HAVE_DIOCGDINFO */
#ifdef __linux__
/* Cache of partition start sectors for each disk. */
@@ -994,8 +1002,7 @@ grub_util_biosdisk_fini (void)
/*
* Note: we do not use the new partition naming scheme as dos_part does not
- * necessarily correspond to an msdos partition. See e.g. the FreeBSD code
- * in function grub_util_biosdisk_get_grub_dev.
+ * necessarily correspond to an msdos partition.
*/
static char *
make_device_name (int drive, int dos_part, int bsd_part)
@@ -1333,6 +1340,27 @@ device_is_wholedisk (const char *os_dev)
}
#endif /* defined(__NetBSD__) */
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+static int
+device_is_wholedisk (const char *os_dev)
+{
+ const char *p;
+
+ if (strncmp (os_dev, "/dev/", 5) != 0)
+ return 0;
+
+ for (p = os_dev + 5; *p; ++p)
+ if (grub_isdigit (*p))
+ {
+ if (strchr (p, 's'))
+ return 0;
+ break;
+ }
+
+ return 1;
+}
+#endif /* defined(__FreeBSD__) || defined(__FreeBSD_kernel__) */
+
static int
find_system_device (const char *os_dev, struct stat *st)
{
@@ -1392,7 +1420,7 @@ grub_util_biosdisk_get_grub_dev (const c
#endif
return make_device_name (drive, -1, -1);
-#if defined(__linux__) || defined(__CYGWIN__) || defined(__NetBSD__)
+#if defined(__linux__) || defined(__CYGWIN__) || defined(HAVE_DIOCGDINFO)
/* Linux counts partitions uniformly, whether a BSD partition or a DOS
partition, so mapping them to GRUB devices is not trivial.
Here, get the start sector of a partition by HDIO_GETGEO, and
@@ -1402,8 +1430,8 @@ grub_util_biosdisk_get_grub_dev (const c
does not count the extended partition and missing primary
partitions. Use same method as on Linux here.
- For NetBSD, proceed as for Linux, except that the start sector is
- obtained from the disk label. */
+ For NetBSD and FreeBSD, proceed as for Linux, except that the start
+ sector is obtained from the disk label. */
{
char *name, *partname;
grub_disk_t disk;
@@ -1431,13 +1459,13 @@ grub_util_biosdisk_get_grub_dev (const c
name = make_device_name (drive, -1, -1);
-# if !defined(__NetBSD__)
+# if !defined(HAVE_DIOCGDINFO)
if (MAJOR (st.st_rdev) == FLOPPY_MAJOR)
return name;
-# else /* defined(__NetBSD__) */
+# else /* defined(HAVE_DIOCGDINFO) */
/* Since os_dev and convert_system_partition_to_system_disk (os_dev) are
* different, we know that os_dev cannot be a floppy device. */
-# endif /* !defined(__NetBSD__) */
+# endif /* !defined(HAVE_DIOCGDINFO) */
start = find_partition_start (os_dev);
if (grub_errno != GRUB_ERR_NONE)
@@ -1506,41 +1534,6 @@ grub_util_biosdisk_get_grub_dev (const c
return make_device_name (drive, dos_part, bsd_part);
}
-#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__APPLE__)
- /* FreeBSD uses "/dev/[a-z]+[0-9]+(s[0-9]+[a-z]?)?". */
- {
- int dos_part = -1;
- int bsd_part = -1;
-
- if (strncmp ("/dev/", os_dev, 5) == 0)
- {
- const char *p;
- char *q;
- long int n;
-
- for (p = os_dev + 5; *p; ++p)
- if (grub_isdigit(*p))
- {
- p = strchr (p, 's'); /* msdos or apple (or ... ?) partition map */
- if (p)
- {
- p++;
- n = strtol (p, &q, 10);
- if (p != q && n != GRUB_LONG_MIN && n != GRUB_LONG_MAX)
- {
- dos_part = (int) n - 1;
-
- if (*q >= 'a' && *q <= 'g')
- bsd_part = *q - 'a';
- }
- }
- break;
- }
- }
-
- return make_device_name (drive, dos_part, bsd_part);
- }
-
#else
# warning "The function `grub_util_biosdisk_get_grub_dev' might not work on your OS correctly."
return make_device_name (drive, -1, -1);
Thanks,
--
Colin Watson [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Fix grub-probe partition naming on FreeBSD
2010-06-13 10:48 [PATCH] Fix grub-probe partition naming on FreeBSD Colin Watson
@ 2010-06-13 15:53 ` Grégoire Sutre
2010-07-01 20:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 3+ messages in thread
From: Grégoire Sutre @ 2010-06-13 15:53 UTC (permalink / raw)
To: The development of GNU GRUB
Hi Colin,
> The following patch is aimed at fixing this Debian bug:
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585068
>
> I've tested it on Debian GNU/kFreeBSD and it seems to be producing sensible
> output now.
In another thread [1], it was observed that offsets are not absolute in
FreeBSD disklabels, whereas they are absolute in NetBSD disklabels.
I believe that find_partition_start is supposed to return absolute
offsets. Does DIOCGDINFO convert the on-disk label into absolute
offsets on FreeBSD?
> The one glitch is that if you ask it to probe /dev/ad0s1a, it returns
> (hd0,msdos1) rather than (hd0,msdos1,bsd1): this is because both /dev/ad0s1
> and /dev/ad0s1a have the same start sector, and it just uses the first one
> it finds. When I set prefix to (hd0,msdos1)/boot/grub, GRUB can read from
> that perfectly well, so can I ignore this glitch on the basis that it
> doesn't cause a practical problem?
We get a similar behavior on NetBSD. As I mentioned in [2], this may
have an impact when the kernel is (a) loaded with the multiboot
protocol, and (b) relies on the MBI boot_device field to find its
root -- which it shouldn't anyway, so it's not a big deal. I am not
aware of other impacts.
I know that hybrid msdos+gpt are not recommended, but, for the record,
I guess that another glitch could happen if /dev/ad0s1X (msdos) and
/dev/ad0p1Y (gpt) are actually the same partition: grub-probe would
return the same answer for both, thus ignoring the user's desire to use
a specific partitioning scheme. I believe that such a partitioning is
supported by FreeBSD, but I may be wrong (please tell me if so), I did
not test this myself.
Grégoire
[1] http://lists.gnu.org/archive/html/grub-devel/2010-05/msg00065.html
[2] http://lists.gnu.org/archive/html/grub-devel/2010-06/msg00075.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix grub-probe partition naming on FreeBSD
2010-06-13 10:48 [PATCH] Fix grub-probe partition naming on FreeBSD Colin Watson
2010-06-13 15:53 ` Grégoire Sutre
@ 2010-07-01 20:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 3+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-07-01 20:57 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 549 bytes --]
On 06/13/2010 12:48 PM, Colin Watson wrote:
> The following patch is aimed at fixing this Debian bug:
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585068
>
>
> +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> +static int
> +device_is_wholedisk (const char *os_dev)
> +{
> + const char *p;
> +
> + if (strncmp (os_dev, "/dev/", 5) != 0)
> + return 0;
> +
> + for (p = os_dev + 5; *p; ++p)
>
Please use sizeof. Other than that please go ahead.
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-07-01 20:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-13 10:48 [PATCH] Fix grub-probe partition naming on FreeBSD Colin Watson
2010-06-13 15:53 ` Grégoire Sutre
2010-07-01 20:57 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.