From owner-freebsd-geom@FreeBSD.ORG Mon Mar 10 05:27:13 2008 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2FB961065671 for ; Mon, 10 Mar 2008 05:27:13 +0000 (UTC) (envelope-from rick@kiwi-computer.com) Received: from kiwi-computer.com (keira.kiwi-computer.com [63.224.10.3]) by mx1.freebsd.org (Postfix) with SMTP id A3D638FC23 for ; Mon, 10 Mar 2008 05:27:12 +0000 (UTC) (envelope-from rick@kiwi-computer.com) Received: (qmail 50050 invoked by uid 2001); 10 Mar 2008 05:27:11 -0000 Date: Sun, 9 Mar 2008 23:27:11 -0600 From: "Rick C. Petty" To: freebsd-geom@freebsd.org Message-ID: <20080310052711.GA49676@keira.kiwi-computer.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="7AUc2qLy4jB3hD7Z" Content-Disposition: inline User-Agent: Mutt/1.4.2.3i Subject: [patch] geom_vinum platform fixes X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: rick-freebsd@kiwi-computer.com List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Mar 2008 05:27:13 -0000 --7AUc2qLy4jB3hD7Z Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Attached is a patch against RELENG_6 (but applies cleanly against RELENG_7). It contains a fix to the geom_vinum platform brokenness explained in my earlier email. Please review the patch and provide any feedback. I will try to find a committer to review/commit this and any subsequent changes early this week. Here is a quick overview of the patch. I've split the reading and writing of the on-disk vinum header into separate functions. The header is read by gv_drive_taste() and written by gv_save_config() and gv_rm_drive(). For reads, there are three possible on-disk formats it handles: the legacy i386 and legacy amd64 formats and the current (forward-going) format. The legacy formats are distinguished by the algorithm in gv_test_legacy_header_type(), which tests for known zeros, etc. Both legacy formats are stored in little-endian, and this code should work on all architectures regardless of endianness. I use a separate in-memory structure to simplify the conversions. For writes, the patch only supports the new forward-going on-disk format. This format is stored in big-endian (network order, also it's easier to read with hexdump) and uses a new magic. This new magic value contains a version number which I've started at 1, in case future enhancements are made to the on-disk structure. Note that devices with old formats are auto-upgraded when the configuration is written, which does not happen unless "gvinum saveconfig" is used or the gvinum configuration is changed for any reason. This lets admins keep using the old format. All new devices added to gvinum will get the new format. I've tested this patch on 6.3-stable (i386) and 7.0-stable (i386, amd64) systems, using vinum configurations created by amd64 & i386 formats and by the new format. Other testers are welcome! -- Rick C. Petty --7AUc2qLy4jB3hD7Z Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="gvinum-6.diff" --- sys/geom/vinum/geom_vinum.h.orig 2005-12-23 21:21:36.000000000 -0600 +++ sys/geom/vinum/geom_vinum.h 2008-03-09 13:50:58.000000000 -0500 @@ -32,6 +32,8 @@ #define ERRBUFSIZ 1024 /* geom_vinum_drive.c */ +struct gv_hdr *gv_read_header(struct g_consumer *cp, int *error); +int gv_write_header(struct g_consumer *cp, struct gv_hdr* hdr); void gv_config_new_drive(struct gv_drive *); void gv_drive_modify(struct gv_drive *); void gv_save_config_all(struct gv_softc *); --- sys/geom/vinum/geom_vinum_drive.c.orig 2005-12-23 21:21:36.000000000 -0600 +++ sys/geom/vinum/geom_vinum_drive.c 2008-03-09 23:12:04.000000000 -0500 @@ -29,8 +29,9 @@ __FBSDID("$FreeBSD: src/sys/geom/vinum/g #include #include -#include #include +#include +#include #include #include #include @@ -50,6 +51,169 @@ __FBSDID("$FreeBSD: src/sys/geom/vinum/g static void gv_drive_dead(void *, int); static void gv_drive_worker(void *); +int gv_test_legacy_header_type(uint8_t *); + +/* + * Here are the "offset (size)" for the various struct gv_hdr fields, + * for the legacy i386, legacy amd64, and current (cpu & endian agnostic) + * versions of the on-disk format of the vinum header structure: + * + * i386 amd64 current field + * -------- -------- -------- ----- + * 0 ( 8) 0 ( 8) 0 ( 8) magic + * 8 ( 4) 8 ( 8) 8 ( 4) config_length + * 12 (32) 16 (32) 16 (32) label.sysname + * 44 (32) 48 (32) 48 (32) label.name + * 76 ( 4) 80 ( 8) 80 ( 8) label.date_of_birth.tv_sec + * 80 ( 4) 88 ( 8) 88 ( 8) label.date_of_birth.tv_usec + * 84 ( 4) 96 ( 8) 96 ( 8) label.last_update.tv_sec + * 88 ( 4) 104 ( 8) 104 ( 8) label.last_update.tv_usec + * 92 ( 8) 112 ( 8) 112 ( 8) label.drive_size + * ======== ======== ======== + * 100 120 120 total size + * + * NOTE: i386 and amd64 formats are stored as little-endian; the current + * format uses big-endian (network order). + */ + +/* + * returns: + * 0 = legacy i386 on-disk format + * 1 = legacy amd64 on-disk format + */ +int +gv_test_legacy_header_type(uint8_t *hdr) +{ + uint32_t* i32; + int i; + + /* if non-empty hostname overlaps amd64 config_length */ + i32 = (uint32_t *)(hdr + 12); + if (*i32 != 0) + return 0; + /* check for non-empty hostname */ + if (hdr[16] != 0) + return 1; + /* check bytes past i386 structure */ + for (i = 100; i < 120; i++) + if (hdr[i] != 0) + return 1; + /* check for overlapping timestamp */ + i32 = (uint32_t *)(hdr + 84); + if (*i32 == 0) + return 1; + return 0; +} + +struct gv_hdr * +gv_read_header(struct g_consumer *cp, int *error) +{ + uint8_t *d_hdr; + struct gv_hdr *m_hdr; + int off; + +#define GV_GET32(endian) \ + endian##32toh(*((uint32_t *)&d_hdr[off])); \ + off += 4 +#define GV_GET64(endian) \ + endian##64toh(*((uint64_t *)&d_hdr[off])); \ + off += 8 + + d_hdr = g_read_data(cp, GV_HDR_OFFSET, GV_HDR_LEN, error); + if (d_hdr == NULL) + return NULL; + m_hdr = g_malloc(GV_HDR_LEN, M_WAITOK | M_ZERO); + off = 0; + m_hdr->magic = GV_GET64(be); + if (m_hdr->magic == GV_MAGIC) { + m_hdr->config_length = GV_GET32(be); + off = 16; + bcopy(d_hdr + off, m_hdr->label.sysname, GV_HOSTNAME_LEN); + off += GV_HOSTNAME_LEN; + bcopy(d_hdr + off, m_hdr->label.name, GV_MAXDRIVENAME); + off += GV_MAXDRIVENAME; + m_hdr->label.date_of_birth.tv_sec = GV_GET64(be); + m_hdr->label.date_of_birth.tv_usec = GV_GET64(be); + m_hdr->label.last_update.tv_sec = GV_GET64(be); + m_hdr->label.last_update.tv_usec = GV_GET64(be); + m_hdr->label.drive_size = GV_GET64(be); + } else if (m_hdr->magic != GV_OLD_MAGIC) { + if (error != NULL) + *error = 0; + g_free(d_hdr); + g_free(m_hdr); + return NULL; + } else if (!gv_test_legacy_header_type(d_hdr)) { + m_hdr->magic = GV_MAGIC; + /* legacy i386 on-disk header */ + m_hdr->config_length = GV_GET32(le); + bcopy(d_hdr + off, m_hdr->label.sysname, GV_HOSTNAME_LEN); + off += GV_HOSTNAME_LEN; + bcopy(d_hdr + off, m_hdr->label.name, GV_MAXDRIVENAME); + off += GV_MAXDRIVENAME; + m_hdr->label.date_of_birth.tv_sec = GV_GET32(le); + m_hdr->label.date_of_birth.tv_usec = GV_GET32(le); + m_hdr->label.last_update.tv_sec = GV_GET32(le); + m_hdr->label.last_update.tv_usec = GV_GET32(le); + m_hdr->label.drive_size = GV_GET64(le); + } else { + m_hdr->magic = GV_MAGIC; + /* legacy amd64 on-disk header */ + m_hdr->config_length = GV_GET64(le); + bcopy(d_hdr + 16, m_hdr->label.sysname, GV_HOSTNAME_LEN); + off += GV_HOSTNAME_LEN; + bcopy(d_hdr + 48, m_hdr->label.name, GV_MAXDRIVENAME); + off += GV_MAXDRIVENAME; + m_hdr->label.date_of_birth.tv_sec = GV_GET64(le); + m_hdr->label.date_of_birth.tv_usec = GV_GET64(le); + m_hdr->label.last_update.tv_sec = GV_GET64(le); + m_hdr->label.last_update.tv_usec = GV_GET64(le); + m_hdr->label.drive_size = GV_GET64(le); + } + + g_free(d_hdr); + return m_hdr; +} + +int +gv_write_header(struct g_consumer *cp, struct gv_hdr *m_hdr) +{ + uint8_t *d_hdr; + int off, ret; + +#define GV_SET32BE(field) \ + do { \ + *((uint32_t *)&d_hdr[off]) = htobe32(field); \ + off += 4; \ + } while (0) +#define GV_SET64BE(field) \ + do { \ + *((uint64_t *)&d_hdr[off]) = htobe64(field); \ + off += 8; \ + } while (0) + + KASSERT(m_hdr != NULL, ("gv_write_header: null m_hdr")); + d_hdr = g_malloc(GV_HDR_LEN, M_WAITOK | M_ZERO); + off = 0; + GV_SET64BE(m_hdr->magic); + GV_SET32BE(m_hdr->config_length); + off = 16; + bcopy(m_hdr->label.sysname, d_hdr + off, GV_HOSTNAME_LEN); + off += GV_HOSTNAME_LEN; + bcopy(m_hdr->label.name, d_hdr + off, GV_MAXDRIVENAME); + off += GV_MAXDRIVENAME; + GV_SET64BE(m_hdr->label.date_of_birth.tv_sec); + GV_SET64BE(m_hdr->label.date_of_birth.tv_usec); + GV_SET64BE(m_hdr->label.last_update.tv_sec); + GV_SET64BE(m_hdr->label.last_update.tv_usec); + GV_SET64BE(m_hdr->label.drive_size); + + ret = g_write_data(cp, GV_HDR_OFFSET, d_hdr, GV_HDR_LEN); + g_free(d_hdr); + return ret; +} + + void gv_config_new_drive(struct gv_drive *d) { @@ -156,7 +320,7 @@ gv_save_config(struct g_consumer *cp, st g_topology_unlock(); do { - error = g_write_data(cp2, GV_HDR_OFFSET, vhdr, GV_HDR_LEN); + error = gv_write_header(cp2, vhdr); if (error) { printf("GEOM_VINUM: writing vhdr failed on drive %s, " "errno %d", d->name, error); @@ -452,7 +616,7 @@ gv_drive_taste(struct g_class *mp, struc /* Now check if the provided slice is a valid vinum drive. */ do { - vhdr = g_read_data(cp, GV_HDR_OFFSET, pp->sectorsize, NULL); + vhdr = gv_read_header(cp, NULL); if (vhdr == NULL) break; if (vhdr->magic != GV_MAGIC) { --- sys/geom/vinum/geom_vinum_rm.c.orig 2005-12-23 21:21:36.000000000 -0600 +++ sys/geom/vinum/geom_vinum_rm.c 2008-03-09 13:41:42.000000000 -0500 @@ -304,7 +304,7 @@ gv_rm_drive(struct gv_softc *sc, struct /* Clear the Vinum Magic. */ d->hdr->magic = GV_NOMAGIC; g_topology_unlock(); - err = g_write_data(cp, GV_HDR_OFFSET, d->hdr, GV_HDR_LEN); + err = gv_write_header(cp, d->hdr); if (err) { printf("GEOM_VINUM: gv_rm_drive: couldn't write header to '%s'" ", errno: %d\n", cp->provider->name, err); --- sys/geom/vinum/geom_vinum_var.h.orig 2005-08-19 03:48:04.000000000 -0500 +++ sys/geom/vinum/geom_vinum_var.h 2008-03-09 22:22:56.000000000 -0500 @@ -136,10 +136,13 @@ struct gv_label { /* The 'header' of each valid vinum drive. */ struct gv_hdr { uint64_t magic; -#define GV_MAGIC 22322600044678729LL -#define GV_NOMAGIC 22322600044678990LL +#define GV_OLD_MAGIC 0x494E2056494E4F00LL +#define GV_OLD_NOMAGIC 0x4E4F2056494E4F00LL +#define GV_MAGIC 0x56494E554D2D3100LL +#define GV_NOMAGIC 0x56494E554D2D2D00LL +#define GV_VERSION(hdrp) ((((gv)->magic >> 8) & 127) - 48) - int config_length; + uint64_t config_length; struct gv_label label; }; --7AUc2qLy4jB3hD7Z--