Re: [PATCH] fix grub-setup on kfreebsd by adding 0x10 to the sysctl ("kern.geom.debugflags") flags

[Felix Zielcke <fzielcke@z-51.de>]

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

Am Montag, den 13.04.2009, 16:49 +0200 schrieb Robert Millan:

Thanks for reviewing.

> On Mon, Apr 13, 2009 at 04:26:19PM +0200, Felix Zielcke wrote:
> > +#ifdef __FreeBSD_kernel__
> 
> Unfortunately FreeBSD doesn't define __FreeBSD_kernel__, so we need
> to check for both macros.

Oh right.

> > +#if defined __FreeBSD_kernel__
> > +  int sysctl_flags, sysctl_oldflags;
> > +  size_t sysctl_size = sizeof (sysctl_flags);
> 
> You can add a `const' on this one.
> 
> > +  sysctl_flags = sysctl_oldflags |= 0x10;
> 
> Shouldn't this be "sysctl_oldflags | 0x10" ?

Yes.
> Btw you can avoid the two subsequent calls if sysctl_flags == sysctl_oldflags.

Right and actually it can be even avoided if 0x10 is already set.

Is it correct that I used now `[__FreeBSD__]: Likewise.' in the
Changelog?
The GCS unfortunately doestn't tell anything about the case that a
change applies to 2 or more marcros and I couldn't find an example in
the existing Changelog for this.

-- 
Felix Zielcke

[-- Attachment #2: kfreebsd.patch.2 --]
[-- Type: text/plain, Size: 1865 bytes --]

2009-04-13  Felix Zielcke  <fzielcke@z-51.de>

	util/hostdisk.c [__FreeBSD_kernel]: Include <sys/param.h> and
	<sys/sysctl.h>.
	[__FreeBSD__]: Likewise.
	(open_device) [__FreeBSD_kernel_]: Use sysctlgetbyname() to add 0x10 to
	`kern.geom.debugflags' if it's not already set, before opening the
	device and reset them afterwards.
	[__FreeBSD__]: Likewise.

Index: util/hostdisk.c
===================================================================
--- util/hostdisk.c	(revision 2094)
+++ util/hostdisk.c	(working copy)
@@ -88,6 +88,8 @@ struct hd_geometry
 
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
 # include <sys/disk.h> /* DIOCGMEDIASIZE */
+# include <sys/param.h>
+# include <sys/sysctl.h>
 #endif
 
 struct
@@ -340,7 +342,24 @@ open_device (const grub_disk_t disk, gru
       sector -= disk->partition->start;
   }
 #else /* ! __linux__ */
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+  int sysctl_flags, sysctl_oldflags;
+  const size_t sysctl_size = sizeof (sysctl_flags);
+
+  if (sysctlbyname ("kern.geom.debugflags", &sysctl_oldflags, &sysctl_size, NULL, 0))
+    grub_util_error ("cannot get current flags of sysctl kern.geom.debugflags");
+  sysctl_flags = sysctl_oldflags | 0x10;
+  if (! sysctl_oldflags & 0x10 && sysctlbyname ("kern.geom.debugflags", NULL , 0, &sysctl_flags, sysctl_size))
+    grub_util_error ("cannot set flags of sysctl kern.geom.debugflags");
+#endif
+
   fd = open (map[disk->id].device, flags);
+
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+  if (! sysctl_oldflags & 0x10 && sysctlbyname ("kern.geom.debugflags", NULL , 0, &sysctl_oldflags, sysctl_size))
+    grub_util_error ("cannot set flags back to the old value for sysctl kern.geom.debugflags");
+#endif
+
   if (fd < 0)
     {
       grub_error (GRUB_ERR_BAD_DEVICE, "cannot open `%s' in open_device()", map[disk->id].device);