tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: disklabel(8) allows addressing non-existent space



Hi,

There was an error in the rounding, please use the attached patch for main.c
instead of the original one.


> > the following patches are my supposal for:
> 
> >  * Adding a batch mode (option -s)
> 
> I don't necessarily object, but does the -i interactive mode require a
> tty?  Otherwise isn't it similar in functionality to -s ?  We also have
> the -R option to apply a label from a file (I personally used the
> latter often).
Handling -i from shell is just difficult to handle (input redirection from
file, error handling, etc.). The option -s is mostly consistent with fdisk
one's. Reading the label from a file is also difficult to handle (print
disklabel to file, *sed magic*, reading disklabel). There was afaik as well no
support for size modifiers as in interactive mode (though that could be
added, of course).

> Just as a note, since this domain interests you, disklabel can only
> handle partition sizes restricted by 32-bit, while gpt(8) can handle
> larger sizes and is likely to become more widely used in the future.  I
> personally never used it, but it's also possible that you'd find that
> gpt(8) also requires user-friendliness improvements...
For gpt(8), I'm still not really sure about the state (see
http://mail-index.netbsd.org/tech-userlevel/2011/10/16/msg005573.html).


Regards, Julian
--- sbin/disklabel/main.c
+++ sbin/disklabel/main.c
@@ -108,10 +108,12 @@
 #else
 #include <sys/ioctl.h>
 #include <sys/disklabel.h>
 #include <sys/disklabel_acorn.h>
 #include <sys/bootblock.h>
+#include <sys/sysctl.h>
+#include <sys/param.h>
 #include <util.h>
 #include <disktab.h>
 #endif /* HAVE_NBTOOL_CONFIG_H */
 
 #include "pathnames.h"
@@ -155,10 +157,11 @@
 static int     Iflag;          /* Read/write direct, but default if absent */
 static int     lflag;          /* List all known file system types and exit */
 static int     mflag;          /* Expect disk to contain an MBR */
 static int verbose;
 static int read_all;           /* set if op = READ && Aflag */
+int    ptn_alignment = 1;      /* Partition alignment. extern in extern.h */
 
 static int write_label(int);
 static int readlabel_direct(int);
 static void writelabel_direct(int);
 static int update_label(int, u_int, u_int);
@@ -265,15 +268,18 @@
 main(int argc, char *argv[])
 {
        FILE    *t;
        int      ch, f, error;
        char    *dkname;
+       char    *partstr = NULL;
+       char    *labelstr = NULL;
+       char    *alignstr = NULL;
        struct stat sb;
        int      writable;
        enum {
                UNSPEC, EDIT, READ, RESTORE, SETWRITABLE, SETREADONLY,
-               WRITE, INTERACT, DELETE
+               WRITE, INTERACT, DELETE, BATCH
        } op = UNSPEC, old_op;
 
        mflag = GETLABELUSESMBR();
        if (mflag < 0) {
                warn("getlabelusesmbr() failed");
@@ -283,11 +289,11 @@
        /* We must avoid doing any ioctl requests */
        Fflag = rflag = 1;
 #endif
 
        error = 0;
-       while ((ch = getopt(argc, argv, "ACDFINRWb:ef:ilmrs:tvw")) != -1) {
+       while ((ch = getopt(argc, argv, "ACDFINRWa:ef:s:p:uilmrtvw")) != -1) {
                old_op = op;
                switch (ch) {
                case 'A':       /* Action all labels */
                        Aflag = 1;
                        rflag = 1;
@@ -314,10 +320,15 @@
                        op = SETREADONLY;
                        break;
                case 'W':       /* Allow writes to label sector */
                        op = SETWRITABLE;
                        break;
+               case 'a':       /* Set partition alignment */
+                       alignstr = strdup(optarg);
+                       if (!alignstr)
+                               err(1, "Could not allocate string.");
+                       break;
                case 'e':       /* Edit label with $EDITOR */
                        op = EDIT;
                        break;
                case 'f':       /* Name of disktab file */
                        if (setdisktab(optarg) == -1)
@@ -330,10 +341,15 @@
                        lflag = 1;
                        break;
                case 'm':       /* Expect disk to have an MBR */
                        mflag ^= 1;
                        break;
+               case 'p':       /* Change disklabel values */
+                       labelstr = strdup(optarg);
+                       if (!labelstr)
+                               err(1, "Could not allocate string.");
+                       break;
                case 'r':       /* Read/write label directly from disk */
                        rflag = 1;
                        break;
                case 't':       /* Format output as a disktab entry */
                        tflag = 1;
@@ -341,10 +357,18 @@
                case 'v':       /* verbose/diag output */
                        verbose++;
                        break;
                case 'w':       /* Write label based on disktab entry */
                        op = WRITE;
+                       break;
+               case 's':       /* Preset partition values. */
+                       partstr = strdup(optarg);
+                       if (!partstr)
+                               err(1, "Could not allocate string.");
+                       break;
+               case 'u':       /* Update label without further interaction. */
+                       op = BATCH;
                        break;
                case '?':
                default:
                        usage();
                }
@@ -381,22 +405,41 @@
                if (argc != 1)
                        usage();
                Dflag = 2;
                writelabel_direct(f);
                break;
+       
+       case BATCH:
+               if (argc != 1)
+                       usage();
+               readlabel(f);
+               set_ptn_alignment(&lab, alignstr);
+               free(alignstr);
+               parse_partstr(&lab, &partstr);
+               parse_labelstr(&lab, &labelstr);
+               batchedit(&lab, f);
+               break;
 
        case EDIT:
                if (argc != 1)
                        usage();
                readlabel(f);
+               set_ptn_alignment(&lab, alignstr);
+               free(alignstr);
+               parse_partstr(&lab, &partstr);
+               parse_labelstr(&lab, &labelstr);
                error = edit(f);
                break;
 
        case INTERACT:
                if (argc != 1)
                        usage();
                readlabel(f);
+               set_ptn_alignment(&lab, alignstr);
+               free(alignstr);
+               parse_partstr(&lab, &partstr);
+               parse_labelstr(&lab, &labelstr);
                /*
                 * XXX: Fill some default values so checklabel does not fail
                 */
                if (lab.d_bbsize == 0)
                        lab.d_bbsize = BBSIZE;
@@ -414,10 +457,14 @@
                        /* Label got printed in the bowels of readlabel */
                        break;
                if (tflag)
                        makedisktab(stdout, &lab);
                else {
+                       set_ptn_alignment(&lab, alignstr);
+                       free(alignstr);
+                       parse_partstr(&lab, &partstr);
+                       parse_labelstr(&lab, &labelstr);
                        showinfo(stdout, &lab, specname);
                        showpartitions(stdout, &lab, Cflag);
                }
                error = checklabel(&lab);
                if (error)
@@ -1906,6 +1953,128 @@
                        free(list);
                }
        }
 
        return ret;
+}
+
+/* Convert a string to a uint16_t. Returns -1 on error, 0 on success. */
+int
+strtouint16(char *intstr, uint16_t *val16)
+{
+       long int lval;
+       char *endstr;
+
+       lval = strtoll(intstr, &endstr, 10);
+       if (*endstr != '\0')
+               return -1;
+       if (lval > UINT16_MAX)
+               return -1;
+
+       *val16 = (uint16_t) lval;
+
+       return 0;
+}
+
+/* Convert a string to a uint32_t. Returns -1 on error, 0 on success. */
+int
+strtouint32(char *intstr, uint32_t *val32)
+{
+       long int lval;
+       char *endstr;
+
+       lval = strtoll(intstr, &endstr, 10);
+       if (*endstr != '\0')
+               return -1;
+       if (lval > UINT32_MAX)
+               return -1;
+
+       *val32 = (uint32_t) lval;
+
+       return 0;
+}
+
+/* Convert a string with modifier to int, return negative value on error.
+ * If rounding is 0, do not round. If rounding is 1, round. If rounding is 2, 
+ * round down, if rounding is 3, round up. */
+intmax_t
+modstrtoint(struct disklabel *lp, char *modstr, int rounding)
+{
+       char *cp;
+       intmax_t ret;
+       int len, part;
+       size_t partlen;
+
+       ret = strtoll(modstr, &cp, 10);
+       len = strcspn(cp, " \t\n");
+       if (modstr == cp)
+               return -1;
+       if (!strncasecmp(cp, "sector", len))
+               {}
+       else if (!strncasecmp(cp, "tb", len)) /* Might be too long for int. */
+               ret *= (intmax_t)(1024 * 1024 * 1024 / lp->d_secsize * 1024);
+       else if (!strncasecmp(cp, "gb", len))
+               ret *= 1024 * 1024 * 1024 / lp->d_secsize;
+       else if (!strncasecmp(cp, "mb", len))
+               ret *= 1024 * 1024 / lp->d_secsize;
+       else if (!strncasecmp(cp, "kb", len))
+               ret *= 1024 / lp->d_secsize;
+       else if (!strncasecmp(cp, "cylinder", len))
+               ret *= lp->d_secpercyl;
+       else if (!strncasecmp(cp, "%", len)) {
+               if (ret < 0 || ret > 100)
+                       err(1, "Percentage must be in [0, 100], is %d.", ret);
+               /* lp->d_secperunit is the best approximation for the right 
value we can 
+                * get. We cannot determine the mbr partition size for sure if 
there is 
+                * one, so simply compute percentages of the whole disk. */
+               // XXX: We could also use some magic involving checks of 
partition c, 
+               // etc., to be sure to take percentages of the mbr.
+               ret *= lp->d_secperunit / lp->d_secsize / 100;
+               ret *= lp->d_secsize;
+               ret += ptn_alignment;
+       } else
+               return -1;
+
+       switch (rounding) {
+               case 0: /* Nothing at all. */
+                       break;
+               case 1: /* Round. */
+                       ret = ((ret / ptn_alignment) + (ret % ptn_alignment ? 1 
: 0)) * ptn_alignment;
+                       break;
+               case 2: /* Round down. */
+                       ret = (ret / ptn_alignment) * ptn_alignment;
+                       break;
+       }
+
+       return ret;
+}
+
+/* The alignment stuff is mostly copied from fdisk(8). */
+void
+set_ptn_alignment(struct disklabel *lp, char *alignstr)
+{
+       intmax_t i;
+
+       /* Default to using 'traditional' cylinder alignment */
+       ptn_alignment = lp->d_secpercyl;
+
+       if (alignstr != NULL) {
+               i = modstrtoint(lp, alignstr, 0);
+               if (i != -1)
+                       ptn_alignment = (unsigned int)i;
+               else
+                       errx(1, "Invalid alignment specifier.");
+       } else {
+               i = lp->d_partitions[0].p_offset + lp->d_partitions[0].p_size;
+               if (i != 0) {
+                       if (!(i & 2047)) {
+                               /* Should nearly always extend to 1MB */
+                               ptn_alignment = 2048;
+                       }
+               } else {
+                       /* Use 1MB alignment for large disks */
+                       if (lp->d_secperunit > 2048 * 1024 * 128)
+                               ptn_alignment = 2048;
+               }
+       }
+
+       return;
 }

Attachment: signature.asc
Description: PGP signature



Home | Main Index | Thread Index | Old Index