Rework partition offset/size rounding logic, removing it from

getuint64() which struggled to know what to do. Add it to the
places rounding is actually wanted by creating and using
alignpartition() which takes parameters to determine what is rounded,
to what boundaries and in what direction.

Simplifies and clarifies logic.

Various minor verbiage tweaks and shuffling of output under
expert mode to improve output during alignments.

Been in snaps for a couple of weeks.

much back & forth with and ok otto@

1 files changed, 155 insertions, 133 deletions

diff --git a/sbin/disklabel/editor.c b/sbin/disklabel/editor.c
index e3b4aed2ca8..19762518a87 100644
--- a/sbin/disklabel/editor.c
+++ b/sbin/disklabel/editor.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: editor.c,v 1.334 2018/04/27 20:28:25 krw Exp $	*/
+/*	$OpenBSD: editor.c,v 1.335 2018/07/01 12:59:33 krw Exp $	*/
 
 /*
  * Copyright (c) 1997-2000 Todd C. Miller <Todd.Miller@courtesan.com>
@@ -48,6 +48,12 @@
 #define	DO_CONVERSIONS	0x00000001
 #define	DO_ROUNDING	0x00000002
 
+/* flags for alignpartition() */
+#define	ROUND_OFFSET_UP		0x00000001
+#define	ROUND_OFFSET_DOWN	0x00000002
+#define	ROUND_SIZE_UP		0x00000004
+#define	ROUND_SIZE_DOWN		0x00000008
+
 /* Special return values for getnumber and getuint64() */
 #define	CMD_ABORTED	(ULLONG_MAX - 1)
 #define	CMD_BADVALUE	(ULLONG_MAX)
@@ -132,8 +138,8 @@ void	editor_help(void);
 void	editor_modify(struct disklabel *, char *);
 void	editor_name(struct disklabel *, char *);
 char	*getstring(const char *, const char *, const char *);
-u_int64_t getuint64(struct disklabel *, char *, char *, u_int64_t, u_int64_t,
-    u_int64_t, int);
+u_int64_t getuint64(struct disklabel *, char *, char *, u_int64_t,
+    u_int64_t, int *);
 u_int64_t getnumber(char *, char *, u_int32_t, u_int32_t);
 int	has_overlap(struct disklabel *);
 int	partition_cmp(const void *, const void *);
@@ -164,6 +170,7 @@ int	apply_unit(double, u_char, u_int64_t *);
 int	parse_sizespec(const char *, double *, char **);
 int	parse_sizerange(char *, u_int64_t *, u_int64_t *);
 int	parse_pct(char *, int *);
+int	alignpartition(struct disklabel *, int, u_int64_t, u_int64_t, int);
 
 static u_int64_t starting_sector;
 static u_int64_t ending_sector;
@@ -268,10 +275,12 @@ editor(int f)
 
 		case 'A':
 			if (ioctl(f, DIOCGPDINFO, &newlab) == 0) {
+				int oquiet = quiet, oexpert = expert;
 				aflag = 1;
-				++quiet;
+				quiet = expert = 0;
 				editor_allocspace(&newlab);
-				--quiet;
+				quiet = oquiet;
+				expert = oexpert;
 			} else
 				newlab = lastlabel;
 			break;
@@ -699,7 +708,7 @@ editor_resize(struct disklabel *lp, char *p)
 	struct disklabel label;
 	struct partition *pp, *prev;
 	u_int64_t secs, sz, off;
-	int partno, i;
+	int partno, i, flags;
 
 	label = *lp;
 
@@ -726,9 +735,10 @@ editor_resize(struct disklabel *lp, char *p)
 		fputs("Cannot resize spoofed partition\n", stderr);
 		return;
 	}
+	flags = DO_CONVERSIONS;
 	secs = getuint64(lp, "[+|-]new size (with unit)",
 	    "new size or amount to grow (+) or shrink (-) partition including "
-	    "unit", sz, sz + editor_countfree(lp), 0, DO_CONVERSIONS);
+	    "unit", sz, sz + editor_countfree(lp), &flags);
 
 	if (secs == CMD_ABORTED)
 		return;
@@ -941,7 +951,7 @@ editor_name(struct disklabel *lp, char *p)
 void
 editor_modify(struct disklabel *lp, char *p)
 {
-	struct partition origpart, *pp;
+	struct partition opp, *pp;
 	int partno;
 
 	/* Change which partition? */
@@ -963,7 +973,7 @@ editor_modify(struct disklabel *lp, char *p)
 		return;
 	}
 
-	origpart = *pp;
+	opp = *pp;
 
 	if (get_offset(lp, partno) == 0 &&
 	    get_size(lp, partno) == 0   &&
@@ -975,7 +985,7 @@ editor_modify(struct disklabel *lp, char *p)
 		return;
 
 	/* Bailed out at some point, so undo any changes. */
-	*pp = origpart;
+	*pp = opp;
 }
 
 /*
@@ -1142,7 +1152,7 @@ getnumber(char *prompt, char *helpstring, u_int32_t oval, u_int32_t maxval)
  */
 u_int64_t
 getuint64(struct disklabel *lp, char *prompt, char *helpstring,
-    u_int64_t oval, u_int64_t maxval, u_int64_t offset, int flags)
+    u_int64_t oval, u_int64_t maxval, int *flags)
 {
 	char buf[BUFSIZ], *endptr, *p, operator = '\0';
 	u_int64_t rval = oval;
@@ -1167,7 +1177,7 @@ getuint64(struct disklabel *lp, char *prompt, char *helpstring,
 	} else {
 		/* deal with units */
 		if (buf[0] != '\0' && n > 0) {
-			if ((flags & DO_CONVERSIONS)) {
+			if (flags != NULL && (*flags & DO_CONVERSIONS)) {
 				switch (tolower((unsigned char)buf[n-1])) {
 
 				case 'c':
@@ -1262,31 +1272,15 @@ getuint64(struct disklabel *lp, char *prompt, char *helpstring,
 			}
 		}
 	}
-	if ((flags & DO_ROUNDING) && rval != CMD_BADVALUE) {
-		/* Round to nearest cylinder unless given in sectors */
-		if (
+
+	if (flags != NULL) {
+		if (mult != 1)
+			*flags |= DO_ROUNDING;
 #ifdef SUN_CYLCHECK
-		    ((lp->d_flags & D_VENDOR) || mult != 1) &&
-#else
-		    mult != 1 &&
+		if (lp->d_flags & D_VENDOR)
+			*flags |= DO_ROUNDING;
 #endif
-		    (rval + offset) % lp->d_secpercyl != 0) {
-			u_int64_t nextcyl, lastcyl;
-			u_int32_t secpercyl = lp->d_secpercyl;
-
-			/* Round to start of next cylinder <= maxval */
-			nextcyl = ((offset + rval + secpercyl - 1) / secpercyl)
-			    * secpercyl;
-			lastcyl = ((maxval + offset) / secpercyl) * secpercyl;
-			if (nextcyl > lastcyl)
-				nextcyl = lastcyl;
-			rval = nextcyl - offset;
-			if (!quiet)
-				printf("Rounding %s to cylinder (%d sectors)"
-				    ": %llu\n", prompt, lp->d_secpercyl, rval);
-		}
 	}
-
 	return (rval);
 
 invalid:
@@ -1468,7 +1462,7 @@ edit_parms(struct disklabel *lp)
 		    (u_int64_t)lp->d_ncylinders * lp->d_secpercyl);
 		ui = getuint64(lp, "total sectors",
 		    "The total number of sectors on the disk.",
-		    nsec, nsec, 0, 0);
+		    nsec, nsec, NULL);
 		if (ui == CMD_ABORTED) {
 			*lp = oldlabel;		/* undo damage */
 			return;
@@ -1617,7 +1611,7 @@ set_bounds(struct disklabel *lp)
 	for (;;) {
 		ui = getuint64(lp, "Starting sector",
 		    "The start of the OpenBSD portion of the disk.",
-		    starting_sector, DL_GETDSIZE(lp), 0, 0);
+		    starting_sector, DL_GETDSIZE(lp), NULL);
 		if (ui == CMD_ABORTED)
 			return;
 		else if (ui == CMD_BADVALUE)
@@ -1635,7 +1629,7 @@ set_bounds(struct disklabel *lp)
 		ui = getuint64(lp, "Size ('*' for entire disk)",
 		    "The size of the OpenBSD portion of the disk ('*' for the "
 		    "entire disk).", ending_sector - starting_sector,
-		    DL_GETDSIZE(lp) - start_temp, 0, 0);
+		    DL_GETDSIZE(lp) - start_temp, NULL);
 		if (ui == CMD_ABORTED)
 			return;
 		else if (ui == CMD_BADVALUE)
@@ -1904,89 +1898,74 @@ mpfree(char **mp)
 int
 get_offset(struct disklabel *lp, int partno)
 {
-	struct diskchunk *chunks;
-	struct partition *pp = &lp->d_partitions[partno];
-	u_int64_t ui, maxsize;
-	int i, fstype;
+	struct partition opp, *pp = &lp->d_partitions[partno];
+	u_int64_t ui, offsetalign;
+	int flags;
 
+	flags = DO_CONVERSIONS;
 	ui = getuint64(lp, "offset",
 	    "Starting sector for this partition.",
 	    DL_GETPOFFSET(pp),
-	    DL_GETPOFFSET(pp), 0, DO_CONVERSIONS |
-	    (pp->p_fstype == FS_BSDFFS ? DO_ROUNDING : 0));
-
-	if (ui == CMD_ABORTED)
-		;
-	else if (ui == CMD_BADVALUE)
-		;
-	else if (ui < starting_sector || ui >= ending_sector)
-		fprintf(stderr, "The offset must be >= %llu and < %llu, "
-		    "the limits of the OpenBSD portion\n"
-		    "of the disk. The 'b' command can change these limits.\n",
-		    starting_sector, ending_sector);
+	    DL_GETPOFFSET(pp), &flags);
+
+	if (ui == CMD_ABORTED || ui == CMD_BADVALUE)
+		return (1);
 #ifdef SUN_AAT0
-	else if (partno == 0 && ui != 0)
+	if (partno == 0 && ui != 0) {
 		fprintf(stderr, "This architecture requires that "
 		    "partition 'a' start at sector 0.\n");
+		return (1);
+	}
 #endif
-	else {
-		fstype = pp->p_fstype;
-		pp->p_fstype = FS_UNUSED;
-		chunks = free_chunks(lp);
-		pp->p_fstype = fstype;
-		for (i = 0; chunks[i].start != 0 || chunks[i].stop != 0; i++) {
-			if (ui < chunks[i].start || ui >= chunks[i].stop)
-				continue;
-			DL_SETPOFFSET(pp, ui);
-			maxsize = chunks[i].stop - DL_GETPOFFSET(pp);
-			if (DL_GETPSIZE(pp) > maxsize)
-				DL_SETPSIZE(pp, maxsize);
-			return (0);
-		}
-		fputs("The offset must be in a free area.\n", stderr);
+	opp = *pp;
+	DL_SETPOFFSET(pp, ui);
+	offsetalign = 1;
+	if ((flags & DO_ROUNDING) != 0 && pp->p_fstype == FS_BSDFFS)
+		offsetalign = lp->d_secpercyl;
+
+	if (alignpartition(lp, partno, offsetalign, 1, ROUND_OFFSET_UP) == 1) {
+		*pp = opp;
+		return (1);
 	}
 
-	/* Partition offset was not set. */
-	return (1);
+	if (expert == 1 && quiet == 0 && ui != DL_GETPOFFSET(pp))
+		printf("offset rounded to sector %llu\n", DL_GETPOFFSET(pp));
+
+	return (0);
 }
 
 int
 get_size(struct disklabel *lp, int partno)
 {
-	struct partition *pp = &lp->d_partitions[partno];
-	u_int64_t maxsize, ui;
+	struct partition opp, *pp = &lp->d_partitions[partno];
+	u_int64_t maxsize, ui, sizealign;
+	int flags;
 
 	maxsize = max_partition_size(lp, partno);
-
+	flags = DO_CONVERSIONS;
 	ui = getuint64(lp, "size", "Size of the partition. "
 	    "You may also say +/- amount for a relative change.",
-	    DL_GETPSIZE(pp), maxsize, DL_GETPOFFSET(pp),
-	    DO_CONVERSIONS | ((pp->p_fstype == FS_BSDFFS ||
-	    pp->p_fstype == FS_SWAP) ?  DO_ROUNDING : 0));
-
-	if (ui == CMD_ABORTED)
-		;
-	else if (ui == CMD_BADVALUE)
-		;
-	else if (ui == 0)
-		fputs("The size must be > 0 sectors\n", stderr);
-	else if (ui + DL_GETPOFFSET(pp) > ending_sector)
-		fprintf(stderr, "The size can't be more than "
-		    "%llu sectors, or the partition would\n"
-		    "extend beyond the last sector (%llu) of the "
-		    "OpenBSD portion of\nthe disk. "
-		    "The 'b' command can change this limit.\n",
-		    ending_sector - DL_GETPOFFSET(pp), ending_sector);
-	else if (ui > maxsize)
-		fprintf(stderr,"Sorry, there are only %llu sectors left\n",
-		    maxsize);
-	else {
-		DL_SETPSIZE(pp, ui);
-		return (0);
+	    DL_GETPSIZE(pp), maxsize, &flags);
+
+	if (ui == CMD_ABORTED || ui == CMD_BADVALUE)
+		return (1);
+
+	opp = *pp;
+	DL_SETPSIZE(pp, ui);
+	sizealign = 1;
+	if ((flags & DO_ROUNDING) != 0 && (pp->p_fstype == FS_SWAP ||
+	    pp->p_fstype == FS_BSDFFS))
+		sizealign = lp->d_secpercyl;
+
+	if (alignpartition(lp, partno, 1, sizealign, ROUND_SIZE_UP) == 1) {
+		*pp = opp;
+		return (1);
 	}
 
-	/* Partition size was not set. */
-	return (1);
+	if (expert == 1 && quiet == 0 && ui != DL_GETPSIZE(pp))
+		printf("size rounded to %llu sectors\n", DL_GETPSIZE(pp));
+
+	return (0);
 }
 
 int
@@ -2001,7 +1980,7 @@ get_cpg(struct disklabel *lp, int partno)
 	if (pp->p_cpg == 0)
 		pp->p_cpg = 1;
 
-	if (!expert)
+	if (expert == 0)
 		return (0);
 
 	for (;;) {
@@ -2073,11 +2052,9 @@ int
 get_bsize(struct disklabel *lp, int partno)
 {
 	u_int64_t ui, frag, fsize;
-	struct partition *pp = &lp->d_partitions[partno];
-#ifndef SUN_CYLCHECK
-	u_int64_t adj, bsize, orig_offset, orig_size;
+	struct partition opp, *pp = &lp->d_partitions[partno];
+	u_int64_t offsetalign, sizealign;
 	char *p;
-#endif
 
 	if (pp->p_fstype != FS_BSDFFS)
 		return (0);
@@ -2086,7 +2063,8 @@ get_bsize(struct disklabel *lp, int partno)
 	if (pp->p_fragblock == 0)
 		return (1);
 
-	if (!expert)
+	opp = *pp;
+	if (expert == 0)
 		goto align;
 
 	fsize = DISKLABELV1_FFS_FSIZE(pp->p_fragblock);
@@ -2117,42 +2095,36 @@ get_bsize(struct disklabel *lp, int partno)
 	frag = ui / fsize;
 	pp->p_fragblock = DISKLABELV1_FFS_FRAGBLOCK(fsize, frag);
 
-#ifndef SUN_CYLCHECK
+#ifdef SUN_CYLCHECK
+	return (0);
+#endif
 	p = getstring("Align partition to block size",
 	    "Round the partition offset and size to multiples of bsize?", "y");
-
 	if (*p == 'n' || *p == 'N')
 		return (0);
-#endif
 
 align:
-
-#ifndef SUN_CYLCHECK
-	orig_size = DL_GETPSIZE(pp);
-	orig_offset = DL_GETPOFFSET(pp);
-
-	bsize = (DISKLABELV1_FFS_FRAG(pp->p_fragblock) *
+#ifdef SUN_CYLCHECK
+	return (0);
+#endif
+	sizealign = (DISKLABELV1_FFS_FRAG(pp->p_fragblock) *
 	    DISKLABELV1_FFS_FSIZE(pp->p_fragblock)) / lp->d_secsize;
-	if (DL_GETPOFFSET(pp) != starting_sector) {
-		/* Can't change offset of first partition. */
-		adj = bsize - (DL_GETPOFFSET(pp) % bsize);
-		if (adj != 0 && adj != bsize) {
-			DL_SETPOFFSET(pp, DL_GETPOFFSET(pp) + adj);
-			DL_SETPSIZE(pp, DL_GETPSIZE(pp) - adj);
-		}
+	offsetalign = 1;
+	if (DL_GETPOFFSET(pp) != starting_sector)
+		offsetalign = sizealign;
+
+	if (alignpartition(lp, partno, offsetalign, sizealign, ROUND_SIZE_DOWN)
+	    == 1) {
+		*pp = opp;
+		return (1);
 	}
-	/* Always align end. */
-	adj = (DL_GETPOFFSET(pp) + DL_GETPSIZE(pp)) % bsize;
-	if (adj > 0)
-		DL_SETPSIZE(pp, DL_GETPSIZE(pp) - adj);
-
-	if (orig_offset != DL_GETPOFFSET(pp) && !quiet)
-		printf("Rounding offset to bsize (%llu sectors): %llu\n",
-		    bsize, DL_GETPOFFSET(pp));
-	if (orig_size != DL_GETPSIZE(pp) && !quiet)
-		printf("Rounding size to bsize (%llu sectors): %llu\n",
-		    bsize, DL_GETPSIZE(pp));
-#endif
+
+	if (expert == 1 && quiet == 0 &&
+	    DL_GETPOFFSET(&opp) != DL_GETPOFFSET(pp))
+		printf("offset rounded to sector %llu\n", DL_GETPOFFSET(pp));
+	if (expert == 1 && quiet == 0 && DL_GETPSIZE(&opp) != DL_GETPSIZE(pp))
+		printf("size rounded to %llu sectors\n", DL_GETPSIZE(pp));
+
 	return (0);
 }
 
@@ -2556,3 +2528,53 @@ parse_pct(char *buf, int *n)
 	free(buf);
 	return (0);
 }
+
+int
+alignpartition(struct disklabel *lp, int partno, u_int64_t startalign,
+    u_int64_t stopalign, int flags)
+{
+	struct partition *pp = &lp->d_partitions[partno];
+	struct diskchunk *chunks;
+	u_int64_t start, stop, maxstop;
+	unsigned int i;
+	u_int8_t fstype;
+
+	start = DL_GETPOFFSET(pp);
+	if ((flags & ROUND_OFFSET_UP) == ROUND_OFFSET_UP)
+		start = ((start + startalign - 1) / startalign) * startalign;
+	else if ((flags & ROUND_OFFSET_DOWN) == ROUND_OFFSET_DOWN)
+		start = (start / startalign) * startalign;
+
+	/* Find the chunk that contains 'start'. */
+	fstype = pp->p_fstype;
+	pp->p_fstype = FS_UNUSED;
+	chunks = free_chunks(lp);
+	pp->p_fstype = fstype;
+	for (i = 0; chunks[i].start != 0 || chunks[i].stop != 0; i++) {
+		if (start >= chunks[i].start && start < chunks[i].stop)
+			break;
+	}
+	if (chunks[i].stop == 0) {
+		fprintf(stderr, "offset %llu is outside the OpenBSD bounds "
+		    "or inside another partition\n", start);
+		return (1);
+	}
+	maxstop = (chunks[i].stop / stopalign) * stopalign;
+
+	/* Calculate the new 'stop' sector, the sector after the partition. */
+	stop = DL_GETPOFFSET(pp) + DL_GETPSIZE(pp);
+	if ((flags & ROUND_SIZE_UP) == ROUND_SIZE_UP)
+		stop = ((stop + stopalign - 1) / stopalign) * stopalign;
+	else if ((flags & ROUND_SIZE_DOWN) == ROUND_SIZE_DOWN)
+		stop = (stop / stopalign) * stopalign;
+
+	if (stop  > maxstop)
+		stop = maxstop;
+
+	if (start != DL_GETPOFFSET(pp))
+		DL_SETPOFFSET(pp, start);
+	if (stop > start && stop != DL_GETPOFFSET(pp) + DL_GETPSIZE(pp))
+		DL_SETPSIZE(pp, stop - start);
+
+	return (0);
+}