Date: Sun, 12 Jul 2009 03:12:19 +0400 From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: Marcel Moolenaar <xcllnt@mac.com> Cc: freebsd-current@freebsd.org, John Marshall <john.marshall@riverwillow.com.au> Subject: Re: 8.0-BETA1 bsdlabel broken? Message-ID: <OLkQkjh4JtvZ8Q7GFHMAD2ALVfs@UL%2BEw68dotyeVWwcX17wEARTb1s> In-Reply-To: <LfBOtJAxBxAex2vUkLqeg/IZi/0@UL%2BEw68dotyeVWwcX17wEARTb1s> References: <fX%2BVI6m2svXk4wDqOGQ3HIesgO8@jmKTY7juey8QgiyMw1P6k9Lb4sg> <20090710071023.GB32316@rwpc12.mby.riverwillow.net.au> <20090710112631.GE32316@rwpc12.mby.riverwillow.net.au> <3a142e750907100433y307f9b2bya1dc54953bdf5de2@mail.gmail.com> <0B1F6799-2FAC-4C01-A978-42E247979CAB@mac.com> <1z5niluEh3OBPNSdMbOMyoEwzX4@CWODRlDR5RMqbkBfR0/UzHcfNhE> <267A655F-13A6-4D79-A933-3A78854AC5FD@mac.com> <TiR5kGIN59NiPk3Q5HjMiImYooQ@MbxgtlHN37ICHkxRO9kSrKasfoA> <A70794BC-F018-4F0A-9AF9-56E28D2B4845@mac.com> <LfBOtJAxBxAex2vUkLqeg/IZi/0@UL%2BEw68dotyeVWwcX17wEARTb1s>
next in thread | previous in thread | raw e-mail | index | archive | help
--t0UkRYy7tHLRMCai
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Sun, Jul 12, 2009 at 01:16:28AM +0400, Eygene Ryabinkin wrote:
> Sat, Jul 11, 2009 at 01:39:03PM -0700, Marcel Moolenaar wrote:
> > Yes, that's the idea. I would add a safety check though:
> > even though the 'c' partition is defined and reserved to
> > mean the whole disk, there's nothing preventing creative
> > souls from using the 'c' partition for something else. I
> > would add a check to make sure that the offset of the 'c'
> > partition is less than or equal to any of the offsets in
> > the partition table before subtracting.
>
> ...and the end of partition 'c' lies not below the end any other
> partition on this disk (so two checks will ensure that all other
> partitions are strictly enclosed within 'c') -- creative souls can make
> 'c' to come physically first (but not from the start of the slice or
> even at the start of the slice; the degree of creativeness differs
> by-case). If there will be no such check, then, technically, there
> will be no problems -- we will add the same offset later when we'll be
> writing the label, but semantically this will be better. Am I missing
> something?
OK, changed the patch to make the described checks. And to fix the error
with the previous patch -- it will set the offset for writes to zero, so
it is better to use this patch variant for doing label writes and never
use the previous one for label modifications. Though it was doing the
proper thing for reads.
But anyway, bsdlabel won't let you write the label (old, patched with
bad patch, patched with the current patch), since it wants to do it via
the BSD class and our slicer is PART. Another patch will be submitted ;))
--
Eygene
_ ___ _.--. #
\`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard
/ ' ` , __.--' # to read the on-line manual
)/' _/ \ `-_, / # while single-stepping the kernel.
`-'" `"\_ ,_.-;_.-\_ ', fsc/as #
_.-'_./ {_.' ; / # -- FreeBSD Developers handbook
{_.-``-' {_/ #
--t0UkRYy7tHLRMCai
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="obtain-slice-offset-from-disklabel.diff"
Content-Transfer-Encoding: quoted-printable
=46rom b9f68a246d79b2d4b8150cf83d8d978118f0a27e Mon Sep 17 00:00:00 2001
=46rom: Eygene Ryabinkin <rea-fbsd@codelabs.ru>
Date: Sat, 11 Jul 2009 14:53:11 +0400
Subject: [PATCH] bsdlabel: obtain slice offset from the disklabel itself
Don't use offset from MBR that is obtained by the geom(4): not any
system has MBR and so on ;)) Partition 'c' holds offset, so it is
used -- this corresponds to the current kernel behaviour.
Extra sanity checks are made to be sure that 'c' really encloses
all other partitions.
Signed-off-by: Eygene Ryabinkin <rea-fbsd@codelabs.ru>
---
sbin/bsdlabel/bsdlabel.c | 69 +++++++++++++++++++++++++++++++++---------=
---
1 files changed, 50 insertions(+), 19 deletions(-)
diff --git a/sbin/bsdlabel/bsdlabel.c b/sbin/bsdlabel/bsdlabel.c
index 1cb9995..37db8b5 100644
--- a/sbin/bsdlabel/bsdlabel.c
+++ b/sbin/bsdlabel/bsdlabel.c
@@ -93,6 +93,7 @@ static int getasciipartspec(char *, struct disklabel *, i=
nt, int);
static int checklabel(struct disklabel *);
static void usage(void);
static struct disklabel *getvirginlabel(void);
+static int deduce_slice_offset(struct disklabel *, off_t *);
=20
#define DEFEDITOR _PATH_VI
#define DEFPARTITIONS 8
@@ -118,7 +119,8 @@ static int installboot; /* non-zero if we should instal=
l a boot program */
static int allfields; /* present all fields in edit */
static char const *xxboot; /* primary boot */
=20
-static off_t mbroffset;
+static off_t sliceoffset;
+
#ifndef LABELSECTOR
#define LABELSECTOR -1
#endif
@@ -344,6 +346,42 @@ makelabel(const char *type, struct disklabel *lp)
bzero(lp->d_packname, sizeof(lp->d_packname));
}
=20
+/*
+ * Determine if the offset of the 'c' partition can be used
+ * as the start of the slice (for example, to convert absolute
+ * partition offsets to the relative ones).
+ *
+ * Check is simple:
+ * - partition 'c' must enclose all other partitions with non-zero
+ * sizes.
+ *
+ * Return values:
+ * - 0 if everything is OK;
+ * - one-based number of the first partition that has problems.
+ */
+static int
+deduce_slice_offset(struct disklabel *lbl, off_t *offset)
+{
+ int i;
+ off_t c_offset =3D lbl->d_partitions[RAW_PART].p_offset;
+ off_t c_size =3D lbl->d_partitions[RAW_PART].p_size;
+ off_t p_offset =3D 0;
+ off_t p_size =3D 0;
+
+ for (i =3D 0; i < lbl->d_npartitions; i++) {
+ p_size =3D lbl->d_partitions[i].p_size;
+ if (!p_size)
+ continue;
+ p_offset =3D lbl->d_partitions[i].p_offset;
+ if (p_offset < c_offset ||
+ p_offset + p_size > c_offset + c_size)
+ return i + 1;
+ }
+
+ *offset =3D c_offset;
+ return 0;
+}
+
static void
readboot(void)
{
@@ -401,9 +439,10 @@ writelabel(void)
lp->d_checksum =3D dkcksum(lp);
if (installboot)
readboot();
+
for (i =3D 0; i < lab.d_npartitions; i++)
if (lab.d_partitions[i].p_size)
- lab.d_partitions[i].p_offset +=3D mbroffset;
+ lab.d_partitions[i].p_offset +=3D sliceoffset;
bsd_disklabel_le_enc(bootarea + labeloffset + labelsoffset * secsize,
lp);
if (alphacksum) {
@@ -481,8 +520,6 @@ readlabel(int flag)
{
int f, i;
int error;
- struct gctl_req *grq;
- char const *errstr;
=20
f =3D open(specname, O_RDONLY);
if (f < 0)
@@ -510,22 +547,16 @@ readlabel(int flag)
=20
if (is_file)
return(0);
- grq =3D gctl_get_handle();
- gctl_ro_param(grq, "verb", -1, "read mbroffset");
- gctl_ro_param(grq, "class", -1, "BSD");
- gctl_ro_param(grq, "geom", -1, pname);
- gctl_rw_param(grq, "mbroffset", sizeof(mbroffset), &mbroffset);
- errstr =3D gctl_issue(grq);
- if (errstr !=3D NULL) {
- mbroffset =3D 0;
- gctl_free(grq);
- return (error);
+
+ sliceoffset =3D 0;
+ i =3D deduce_slice_offset(&lab, &sliceoffset);
+ if (i !=3D 0) {
+ warn("partition '%c' isn't fully enclosed by the partition 'c'",
+ (char)('a' + i - 1));
}
- mbroffset /=3D lab.d_secsize;
- if (lab.d_partitions[RAW_PART].p_offset =3D=3D mbroffset)
- for (i =3D 0; i < lab.d_npartitions; i++)
- if (lab.d_partitions[i].p_size)
- lab.d_partitions[i].p_offset -=3D mbroffset;
+ for (i =3D 0; i < lab.d_npartitions; i++)
+ if (lab.d_partitions[i].p_size)
+ lab.d_partitions[i].p_offset -=3D sliceoffset;
return (error);
}
=20
--=20
1.6.3.1
--t0UkRYy7tHLRMCai--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?OLkQkjh4JtvZ8Q7GFHMAD2ALVfs>