linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] block: online resize of disk partitions
@ 2012-02-13 19:30 Vivek Goyal
  2012-02-13 19:30 ` [PATCH 1/2] block: add partition resize function to blkpg ioctl Vivek Goyal
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Vivek Goyal @ 2012-02-13 19:30 UTC (permalink / raw)
  To: linux-kernel, axboe, dm-devel, kzak; +Cc: vgoyal, psusi, psusi, maxim.patlasov

Hi,

Following patch adds support for online resizing of a partition. This patch
is based on previously posted patches by Phillip Susi.

There are two patches. Out of which one is kernel patch and other one is
util-linux patch to add support of a user space utility "resizepart" to
allow resizing the partition.

This ioctl only resizes the partition size in kenrel and does not change
the size on disk. A user needs to make sure that corresponding changes
are made to disk data structures also using fdisk(or partx), if changes
are to be retained across reboot.

I made some changes according to the feedback received last time. Following
are changes since the version Phillip posted.

- RESIZE ioctl ignores the partition "start" and does not expect user to
  specify one. Caller needs to just specify "device", "partition number" and
  "size" of new partition. 

- Got rid of part_nr_sects_write_begin/part_nr_sects_write_end functions
  and replaced these with single part_nr_sects_write().

- Some sequence counter related changes are simply lifted from i_size_write().

- Initialized part->nr_sects_seq using seqcount_init().

Phillip, do let me know if I should put your signed-off-by also in the
patch.

Any feedback is welcome.

I did following test.

- Create a partition of 10MB on a disk using fdisk.
- Add this partition to a volume group
- Use fdisk to increase the partition size to 20MB. (First delete the
  partition and then create a new one of 20MB size).
- Use resizepart to extend partition size in kernel.
	resizepart /dev/sdc 1 40960
- Do pvresize on partition so that physical volume can be incrased in
  size online.
	pvresize /dev/sda1

pvresize does recognize the new size. Also lsblk and /proc/partitions
report the new size of partition.

Thanks
Vivek

[PATCH 1/2] block: add partition resize function to blkpg ioctl
[PATCH 2/2] resizepart: Utility to resize a partition

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] block: add partition resize function to blkpg ioctl
  2012-02-13 19:30 [PATCH 0/2] block: online resize of disk partitions Vivek Goyal
@ 2012-02-13 19:30 ` Vivek Goyal
  2012-02-13 19:30 ` [PATCH 2/2] resizepart: Utility to resize a partition Vivek Goyal
  2012-02-13 21:34 ` [PATCH 0/2] block: online resize of disk partitions Phillip Susi
  2 siblings, 0 replies; 9+ messages in thread
From: Vivek Goyal @ 2012-02-13 19:30 UTC (permalink / raw)
  To: linux-kernel, axboe, dm-devel, kzak; +Cc: vgoyal, psusi, psusi, maxim.patlasov

Add a new operation code ( BLKPG_RESIZE_PARTITION ) to the
BLKPG ioctl that allows altering the size of an existing
partition, even if it is currently in use.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 block/genhd.c             |   20 ++++++++++++----
 block/ioctl.c             |   51 +++++++++++++++++++++++++++++++++++++--
 block/partition-generic.c |    4 ++-
 include/linux/blkpg.h     |    1 +
 include/linux/genhd.h     |   57 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 124 insertions(+), 9 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 23b4f70..935e09b 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -153,7 +153,7 @@ struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
 		part = rcu_dereference(ptbl->part[piter->idx]);
 		if (!part)
 			continue;
-		if (!part->nr_sects &&
+		if (!part_nr_sects_read(part) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY) &&
 		    !(piter->flags & DISK_PITER_INCL_EMPTY_PART0 &&
 		      piter->idx == 0))
@@ -190,7 +190,7 @@ EXPORT_SYMBOL_GPL(disk_part_iter_exit);
 static inline int sector_in_part(struct hd_struct *part, sector_t sector)
 {
 	return part->start_sect <= sector &&
-		sector < part->start_sect + part->nr_sects;
+		sector < part->start_sect + part_nr_sects_read(part);
 }
 
 /**
@@ -765,8 +765,8 @@ void __init printk_all_partitions(void)
 
 			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
-			       (unsigned long long)part->nr_sects >> 1,
-			       disk_name(disk, part->partno, name_buf), uuid);
+			       (unsigned long long)part_nr_sects_read(part) >> 1
+			       , disk_name(disk, part->partno, name_buf), uuid);
 			if (is_part0) {
 				if (disk->driverfs_dev != NULL &&
 				    disk->driverfs_dev->driver != NULL)
@@ -857,7 +857,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 	while ((part = disk_part_iter_next(&piter)))
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part_devt(part)), MINOR(part_devt(part)),
-			   (unsigned long long)part->nr_sects >> 1,
+			   (unsigned long long)part_nr_sects_read(part) >> 1,
 			   disk_name(sgp, part->partno, buf));
 	disk_part_iter_exit(&piter);
 
@@ -1263,6 +1263,16 @@ struct gendisk *alloc_disk_node(int minors, int node_id)
 		}
 		disk->part_tbl->part[0] = &disk->part0;
 
+		/*
+		 * set_capacity() and get_capacity() currently don't use
+		 * seqcounter to read/update the part0->nr_sects. Still init
+		 * the counter as we can read the sectors in IO submission
+		 * patch using seqence counters.
+		 *
+		 * TODO: Ideally set_capacity() and get_capacity() should be
+		 * converted to make use of bd_mutex and sequence counters.
+		 */
+		seqcount_init(&disk->part0.nr_sects_seq);
 		hd_ref_init(&disk->part0);
 
 		disk->minors = minors;
diff --git a/block/ioctl.c b/block/ioctl.c
index ba15b2d..57d99b2 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -13,7 +13,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 {
 	struct block_device *bdevp;
 	struct gendisk *disk;
-	struct hd_struct *part;
+	struct hd_struct *part, *lpart;
 	struct blkpg_ioctl_arg a;
 	struct blkpg_partition p;
 	struct disk_part_iter piter;
@@ -36,8 +36,8 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 		case BLKPG_ADD_PARTITION:
 			start = p.start >> 9;
 			length = p.length >> 9;
-			/* check for fit in a hd_struct */ 
-			if (sizeof(sector_t) == sizeof(long) && 
+			/* check for fit in a hd_struct */
+			if (sizeof(sector_t) == sizeof(long) &&
 			    sizeof(long long) > sizeof(long)) {
 				long pstart = start, plength = length;
 				if (pstart != start || plength != length
@@ -92,6 +92,51 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
 			bdput(bdevp);
 
 			return 0;
+		case BLKPG_RESIZE_PARTITION:
+			/* new length of partition in bytes */
+			length = p.length >> 9;
+			/* check for fit in a hd_struct */
+			if (sizeof(sector_t) == sizeof(long) &&
+			    sizeof(long long) > sizeof(long)) {
+				long plength = length;
+				if (plength != length || plength < 0)
+					return -EINVAL;
+			}
+			part = disk_get_part(disk, partno);
+			if (!part)
+				return -ENXIO;
+			bdevp = bdget(part_devt(part));
+			if (!bdevp) {
+				disk_put_part(part);
+				return -ENOMEM;
+			}
+			mutex_lock(&bdevp->bd_mutex);
+			mutex_lock_nested(&bdev->bd_mutex, 1);
+			start = part->start_sect;
+
+			/* overlap? */
+			disk_part_iter_init(&piter, disk,
+					    DISK_PITER_INCL_EMPTY);
+			while ((lpart = disk_part_iter_next(&piter))) {
+				if (lpart->partno != partno &&
+				   !(start + length <= lpart->start_sect ||
+				   start >= lpart->start_sect + lpart->nr_sects)
+				   ) {
+					disk_part_iter_exit(&piter);
+					mutex_unlock(&bdevp->bd_mutex);
+					mutex_unlock(&bdev->bd_mutex);
+					disk_put_part(part);
+					return -EBUSY;
+				}
+			}
+			disk_part_iter_exit(&piter);
+			part_nr_sects_write(part, (sector_t)length);
+			i_size_write(bdevp->bd_inode, p.length);
+			mutex_unlock(&bdevp->bd_mutex);
+			mutex_unlock(&bdev->bd_mutex);
+			bdput(bdevp);
+			disk_put_part(part);
+			return 0;
 		default:
 			return -EINVAL;
 	}
diff --git a/block/partition-generic.c b/block/partition-generic.c
index d06ec1c..363a6f6 100644
--- a/block/partition-generic.c
+++ b/block/partition-generic.c
@@ -84,7 +84,7 @@ ssize_t part_size_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
 	struct hd_struct *p = dev_to_part(dev);
-	return sprintf(buf, "%llu\n",(unsigned long long)p->nr_sects);
+	return sprintf(buf, "%llu\n",(unsigned long long)part_nr_sects_read(p));
 }
 
 static ssize_t part_ro_show(struct device *dev,
@@ -294,6 +294,8 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno,
 		err = -ENOMEM;
 		goto out_free;
 	}
+
+	seqcount_init(&p->nr_sects_seq);
 	pdev = part_to_dev(p);
 
 	p->start_sect = start;
diff --git a/include/linux/blkpg.h b/include/linux/blkpg.h
index faf8a45..a851944 100644
--- a/include/linux/blkpg.h
+++ b/include/linux/blkpg.h
@@ -40,6 +40,7 @@ struct blkpg_ioctl_arg {
 /* The subfunctions (for the op field) */
 #define BLKPG_ADD_PARTITION	1
 #define BLKPG_DEL_PARTITION	2
+#define BLKPG_RESIZE_PARTITION	3
 
 /* Sizes of name fields. Unused at present. */
 #define BLKPG_DEVNAMELTH	64
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index fe23ee7..0def3ef 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -98,7 +98,13 @@ struct partition_meta_info {
 
 struct hd_struct {
 	sector_t start_sect;
+	/*
+	 * nr_sects is protected by sequence counter. One might extend a
+	 * partition while IO is happening to it and update of nr_sects
+	 * can be non-atomic on 32bit machines with 64bit sector_t.
+	 */
 	sector_t nr_sects;
+	seqcount_t nr_sects_seq;
 	sector_t alignment_offset;
 	unsigned int discard_alignment;
 	struct device __dev;
@@ -653,6 +659,57 @@ static inline void hd_struct_put(struct hd_struct *part)
 		__delete_partition(part);
 }
 
+/*
+ * Any access of part->nr_sects which is not protected by partition
+ * bd_mutex or gendisk bdev bd_mutex, should be done using this
+ * accessor function.
+ *
+ * Code written along the lines of i_size_read() and i_size_write().
+ * CONFIG_PREEMPT case optimizes the case of UP kernel with preemption
+ * on.
+ */
+static inline sector_t part_nr_sects_read(struct hd_struct *part)
+{
+#if BITS_PER_LONG==32 && defined(CONFIG_LBDAF) && defined(CONFIG_SMP)
+	sector_t nr_sects;
+	unsigned seq;
+	do {
+		seq = read_seqcount_begin(&part->nr_sects_seq);
+		nr_sects = part->nr_sects;
+	} while (read_seqcount_retry(&part->nr_sects_seq, seq));
+	return nr_sects;
+#elif BITS_PER_LONG==32 && defined(CONFIG_PREEMPT)
+	sector_t nr_sects;
+
+	preempt_disable();
+	nr_sects = part->nr_sects;
+	preempt_enable();
+	return nr_sects;
+#else
+	return part->nr_sects;
+#endif
+}
+
+/*
+ * Should be called with mutex lock held (typically bd_mutex) of partition
+ * to provide mutual exlusion among writers otherwise seqcount might be
+ * left in wrong state leaving the readers spinning infinitely.
+ */
+static inline void part_nr_sects_write(struct hd_struct *part, sector_t size)
+{
+#if BITS_PER_LONG==32 && defined(CONFIG_LBDAF) && defined(CONFIG_SMP)
+	write_seqcount_begin(&part->nr_sects_seq);
+	part->nr_sects = size;
+	write_seqcount_end(&part->nr_sects_seq);
+#elif BITS_PER_LONG==32 && defined(CONFIG_LBDAF) && defined(CONFIG_PREEMPT)
+	preempt_disable();
+	part->nr_sects = size;
+	preempt_enable();
+#else
+	part->nr_sects = size;
+#endif
+}
+
 #else /* CONFIG_BLOCK */
 
 static inline void printk_all_partitions(void) { }
-- 
1.7.6.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] resizepart: Utility to resize a partition
  2012-02-13 19:30 [PATCH 0/2] block: online resize of disk partitions Vivek Goyal
  2012-02-13 19:30 ` [PATCH 1/2] block: add partition resize function to blkpg ioctl Vivek Goyal
@ 2012-02-13 19:30 ` Vivek Goyal
  2012-02-13 21:34 ` [PATCH 0/2] block: online resize of disk partitions Phillip Susi
  2 siblings, 0 replies; 9+ messages in thread
From: Vivek Goyal @ 2012-02-13 19:30 UTC (permalink / raw)
  To: linux-kernel, axboe, dm-devel, kzak; +Cc: vgoyal, psusi, psusi, maxim.patlasov

A simple user space utility to resize an existing partition.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 partx/Makefile.am  |    4 ++--
 partx/partx.h      |   19 +++++++++++++++++++
 partx/resizepart.8 |   38 ++++++++++++++++++++++++++++++++++++++
 partx/resizepart.c |   30 ++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 partx/resizepart.8
 create mode 100644 partx/resizepart.c

diff --git a/partx/Makefile.am b/partx/Makefile.am
index 080bc47..1f4dbf5 100644
--- a/partx/Makefile.am
+++ b/partx/Makefile.am
@@ -1,7 +1,7 @@
 include $(top_srcdir)/config/include-Makefile.am
 
-usrsbin_exec_PROGRAMS = addpart delpart
-dist_man_MANS = addpart.8 delpart.8
+usrsbin_exec_PROGRAMS = addpart delpart resizepart
+dist_man_MANS = addpart.8 delpart.8 resizepart.8
 
 usrsbin_exec_PROGRAMS += partx
 partx_SOURCES = partx.c partx.h \
diff --git a/partx/partx.h b/partx/partx.h
index b40fa8f..7a509f3 100644
--- a/partx/partx.h
+++ b/partx/partx.h
@@ -41,4 +41,23 @@ static inline int partx_add_partition(int fd, int partno,
 	return ioctl(fd, BLKPG, &a);
 }
 
+static inline int partx_resize_partition(int fd, int partno, unsigned long size)
+{
+	struct blkpg_ioctl_arg a;
+	struct blkpg_partition p;
+
+	p.pno = partno;
+	/* start is unused for resize operation. It can't be changed */
+	p.start = 0;
+	p.length = size << 9;
+	p.devname[0] = 0;
+	p.volname[0] = 0;
+	a.op = BLKPG_RESIZE_PARTITION;
+	a.flags = 0;
+	a.datalen = sizeof(p);
+	a.data = &p;
+
+	return ioctl(fd, BLKPG, &a);
+}
+
 #endif /*  UTIL_LINUX_PARTX_H */
diff --git a/partx/resizepart.8 b/partx/resizepart.8
new file mode 100644
index 0000000..0b47e81
--- /dev/null
+++ b/partx/resizepart.8
@@ -0,0 +1,38 @@
+.\" resizepart.8 --
+.\" Copyright 2012 Vivek Goyal <vgoyal@redhat.com>
+.\" Copyright 2012 Red Hat, Inc.
+.\" May be distributed under the GNU General Public License
+.TH RESIZEPART 8 "February 2012" "util-linux" "System Administration"
+.SH NAME
+resizepart \-
+simple wrapper around the "resize partition" ioctl
+.SH SYNOPSIS
+.B resizepart
+.I device partition length
+.SH DESCRIPTION
+.B resizepart
+is a program that informs the Linux kernel of new partition size.
+
+This command doesn't manipulate partitions on hard drive.
+
+.SH PARAMETERS
+.TP
+.I device
+Specify the disk device.
+.TP
+.I partition
+Specify the partition number.
+.TP
+.I length
+Specify the length of the partition (in 512-byte sectors).
+
+.SH SEE ALSO
+.BR addpart (8),
+.BR delpart (8),
+.BR fdisk (8),
+.BR parted (8),
+.BR partprobe (8),
+.BR partx (8)
+.SH AVAILABILITY
+The addpart command is part of the util-linux package and is available from
+ftp://ftp.kernel.org/pub/linux/utils/util-linux/.
diff --git a/partx/resizepart.c b/partx/resizepart.c
new file mode 100644
index 0000000..e230fcd
--- /dev/null
+++ b/partx/resizepart.c
@@ -0,0 +1,30 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+
+#include "partx.h"
+
+int
+main(int argc, char **argv)
+{
+	int fd;
+
+	if (argc != 4) {
+		fprintf(stderr,
+			"usage: %s diskdevice partitionnr length\n",
+			argv[0]);
+		exit(1);
+	}
+	if ((fd = open(argv[1], O_RDONLY)) < 0) {
+		perror(argv[1]);
+		exit(1);
+	}
+
+	if (partx_resize_partition(fd, atoi(argv[2]),
+				atoll(argv[3]))) {
+		perror("BLKPG");
+		exit(1);
+	}
+
+	return 0;
+}
-- 
1.7.6.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] block: online resize of disk partitions
  2012-02-13 19:30 [PATCH 0/2] block: online resize of disk partitions Vivek Goyal
  2012-02-13 19:30 ` [PATCH 1/2] block: add partition resize function to blkpg ioctl Vivek Goyal
  2012-02-13 19:30 ` [PATCH 2/2] resizepart: Utility to resize a partition Vivek Goyal
@ 2012-02-13 21:34 ` Phillip Susi
  2012-02-13 21:50   ` Vivek Goyal
  2 siblings, 1 reply; 9+ messages in thread
From: Phillip Susi @ 2012-02-13 21:34 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, axboe, dm-devel, Karel Zak, maxim.patlasov

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/13/2012 2:30 PM, Vivek Goyal wrote:
> Hi,
> 
> Following patch adds support for online resizing of a partition.
> This patch is based on previously posted patches by Phillip Susi.
> 
> There are two patches. Out of which one is kernel patch and other
> one is util-linux patch to add support of a user space utility
> "resizepart" to allow resizing the partition.
> 
> This ioctl only resizes the partition size in kenrel and does not
> change the size on disk. A user needs to make sure that
> corresponding changes are made to disk data structures also using
> fdisk(or partx), if changes are to be retained across reboot.
> 
> I made some changes according to the feedback received last time.
> Following are changes since the version Phillip posted.
> 
> - RESIZE ioctl ignores the partition "start" and does not expect
> user to specify one. Caller needs to just specify "device",
> "partition number" and "size" of new partition.

I would prefer that the start argument not be left undefined.  I
checked it to make sure it was unchanged as a simple sanity check to
guard against user space being goofy, as well as to allow for the
possibility of extending the interface in the future to allow changing
the start as well as the length.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPOYH+AAoJEJrBOlT6nu75rvwIALjEMrUZPEvwYzH9MI9wHmGg
lZzeBTSalqEOWL/hM0Ge6o1Of1adjeEyB7riXi7Fbb4Z8U55oBkAyKcRD/fdm0EA
8SyfMnGnMqZjh6VthIZMw43HbNwHojXeu2ri4ygeUORMIrX8tmZLAePw6iuOWcX6
qxOZsJOz3CdoGy2yc362AuIQACKH6IfFimP81NH5uUA01DIawWpes3EuN+KcxOQP
hlIeixVPzlNrXuDY/m35xlXNaXcTUqTYFJ30vvVn0/MHylAbfCXKx6P19jvPHTt9
j0OyBNVnsb/BCKsbSW89lfHoVPMeYC6Kz2u8d1AX5byCxcYcznqclA6S93/UUms=
=ZABC
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] block: online resize of disk partitions
  2012-02-13 21:34 ` [PATCH 0/2] block: online resize of disk partitions Phillip Susi
@ 2012-02-13 21:50   ` Vivek Goyal
  2012-02-13 21:56     ` Vivek Goyal
  2012-02-13 22:18     ` Phillip Susi
  0 siblings, 2 replies; 9+ messages in thread
From: Vivek Goyal @ 2012-02-13 21:50 UTC (permalink / raw)
  To: Phillip Susi; +Cc: linux-kernel, axboe, dm-devel, Karel Zak, maxim.patlasov

On Mon, Feb 13, 2012 at 04:34:54PM -0500, Phillip Susi wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 2/13/2012 2:30 PM, Vivek Goyal wrote:
> > Hi,
> > 
> > Following patch adds support for online resizing of a partition.
> > This patch is based on previously posted patches by Phillip Susi.
> > 
> > There are two patches. Out of which one is kernel patch and other
> > one is util-linux patch to add support of a user space utility
> > "resizepart" to allow resizing the partition.
> > 
> > This ioctl only resizes the partition size in kenrel and does not
> > change the size on disk. A user needs to make sure that
> > corresponding changes are made to disk data structures also using
> > fdisk(or partx), if changes are to be retained across reboot.
> > 
> > I made some changes according to the feedback received last time.
> > Following are changes since the version Phillip posted.
> > 
> > - RESIZE ioctl ignores the partition "start" and does not expect
> > user to specify one. Caller needs to just specify "device",
> > "partition number" and "size" of new partition.
> 
> I would prefer that the start argument not be left undefined.  I
> checked it to make sure it was unchanged as a simple sanity check to
> guard against user space being goofy,

I am not sure that it is really helping. I looked at pvresize, lvresize
and resize2fs user interfaces and a user has to just specify the new
size of physical volume, logical volume, filesystem respectively. So
it makes sense to keep partition resize interface inline with above
tools.

So if a user anyway does not specify the start of sector, then tools
shall have to first read it and then call the ioctl. If anyway tools
are specifying, there is no much scope of screwing up the things?

Also in delete partition ioctl, we just ask the user the partition
number being deleted. We don't ask for start and size of partition
in an effort to verify that you are deleting the right partition.

> as well as to allow for the
> possibility of extending the interface in the future to allow changing
> the start as well as the length.

Even if we allow changing start at some point of time, then IOCTL can
remain the same and just the implementation will change in a backward
compatible manner. Old tools still will continue to work as they have
always been, and new ones can start passing "start" too.

So I really did not find passing and checking "start" of partition
very appealing.

Thanks
Vivek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] block: online resize of disk partitions
  2012-02-13 21:50   ` Vivek Goyal
@ 2012-02-13 21:56     ` Vivek Goyal
  2012-02-13 22:18     ` Phillip Susi
  1 sibling, 0 replies; 9+ messages in thread
From: Vivek Goyal @ 2012-02-13 21:56 UTC (permalink / raw)
  To: Phillip Susi; +Cc: linux-kernel, axboe, dm-devel, Karel Zak, maxim.patlasov

On Mon, Feb 13, 2012 at 04:50:39PM -0500, Vivek Goyal wrote:

[..]
> > as well as to allow for the
> > possibility of extending the interface in the future to allow changing
> > the start as well as the length.
> 
> Even if we allow changing start at some point of time, then IOCTL can
> remain the same and just the implementation will change in a backward
> compatible manner. Old tools still will continue to work as they have
> always been, and new ones can start passing "start" too.

Let me thake that back. Changes will not be backward compatible as
if old tools run on new kernel specifying start as 0, then change
will fail on newer kernels which might allow changing start too.

Do you really expect that changing start will really be useful? If yes,
then it does make a case for not leaving "start" undefined.

Thanks
Vivek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] block: online resize of disk partitions
  2012-02-13 21:50   ` Vivek Goyal
  2012-02-13 21:56     ` Vivek Goyal
@ 2012-02-13 22:18     ` Phillip Susi
  2012-02-13 22:24       ` Vivek Goyal
  1 sibling, 1 reply; 9+ messages in thread
From: Phillip Susi @ 2012-02-13 22:18 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, axboe, dm-devel, Karel Zak, maxim.patlasov

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/13/2012 4:50 PM, Vivek Goyal wrote:
> So if a user anyway does not specify the start of sector, then 
> tools shall have to first read it and then call the ioctl. If 
> anyway tools are specifying, there is no much scope of screwing up 
> the things?

Tools certainly should know where the partition starts.  I already
have patched parted and partx to pass the current size, which they
had trivially available.  Making sure of that is a good sanity check
to guard against, for instance, resizing the wrong partition.

> Even if we allow changing start at some point of time, then IOCTL 
> can remain the same and just the implementation will change in a 
> backward compatible manner. Old tools still will continue to work 
> as they have always been, and new ones can start passing "start" 
> too.
> 
> So I really did not find passing and checking "start" of partition
>  very appealing.

That is exactly why passing and checking start is required.  If it is
entirely ignored, then the interface can not start using it in the
future in a backward compatible way, because user mode tools will have
grown used to passing in any kind of garbage, so if the kernel starts
using it to alter the start position of the partition, older tools
would randomly and accidentally be shifting the the start of
partitions they just mean to change the length of.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPOYw5AAoJEJrBOlT6nu75CV4H/2vyZwujRqb+mqTAfQ+/XJE7
E850b+MOFZEQEoKQy+XAeu15pmm/TIa8NTk83kBeoQb30P2sU814+rJjbMKpZ9x/
xc8DZVC+DdnLLJ8a9aQw5feV0nmy102QrL0CWS+Wi6vS3DLlLCQQ+nv7AZ1jPV0Z
NPzqtmI/zBopf/F2IrL+5DLD9TAi7mf+Yv5IBb7w2JQSiU9LoBo9TTMJhPAhaqtC
ihqSJ0Q/t0DG3WacK+8fwh6Jyta1Bx+YsoUzYijtsaH6GQ1zbe+3c44T1JMI+Zvq
30M8V7y1BsLhp+lfZSUlC5YvPpu7gCJ1lg2fpMz56q2DENURj3L1HkdFwPwfLcw=
=TTAk
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] block: online resize of disk partitions
  2012-02-13 22:18     ` Phillip Susi
@ 2012-02-13 22:24       ` Vivek Goyal
  2012-02-14  1:24         ` Phillip Susi
  0 siblings, 1 reply; 9+ messages in thread
From: Vivek Goyal @ 2012-02-13 22:24 UTC (permalink / raw)
  To: Phillip Susi; +Cc: linux-kernel, axboe, dm-devel, Karel Zak, maxim.patlasov

On Mon, Feb 13, 2012 at 05:18:33PM -0500, Phillip Susi wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 2/13/2012 4:50 PM, Vivek Goyal wrote:
> > So if a user anyway does not specify the start of sector, then 
> > tools shall have to first read it and then call the ioctl. If 
> > anyway tools are specifying, there is no much scope of screwing up 
> > the things?
> 
> Tools certainly should know where the partition starts.  I already
> have patched parted and partx to pass the current size, which they
> had trivially available.  Making sure of that is a good sanity check
> to guard against, for instance, resizing the wrong partition.

Ok, so user still specifies just the new size of partition and tool
passes in both the "start" and new "size" to the ioctl?

> 
> > Even if we allow changing start at some point of time, then IOCTL 
> > can remain the same and just the implementation will change in a 
> > backward compatible manner. Old tools still will continue to work 
> > as they have always been, and new ones can start passing "start" 
> > too.
> > 
> > So I really did not find passing and checking "start" of partition
> >  very appealing.
> 
> That is exactly why passing and checking start is required.  If it is
> entirely ignored, then the interface can not start using it in the
> future in a backward compatible way, because user mode tools will have
> grown used to passing in any kind of garbage, so if the kernel starts
> using it to alter the start position of the partition, older tools
> would randomly and accidentally be shifting the the start of
> partitions they just mean to change the length of.

Ok, I think keeping the ioctl backward compatible in light of any
future changes makes sense. I will change the patches to not ignore
the partition "start" and repost.

Thanks for the comments.

Vivek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/2] block: online resize of disk partitions
  2012-02-13 22:24       ` Vivek Goyal
@ 2012-02-14  1:24         ` Phillip Susi
  0 siblings, 0 replies; 9+ messages in thread
From: Phillip Susi @ 2012-02-14  1:24 UTC (permalink / raw)
  To: Vivek Goyal; +Cc: linux-kernel, axboe, dm-devel, Karel Zak, maxim.patlasov

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/13/2012 05:24 PM, Vivek Goyal wrote:
> Ok, so user still specifies just the new size of partition and tool
> passes in both the "start" and new "size" to the ioctl?

Bingo.  Eventually I'm kicking around the idea of having the ioctl allow
changing the start and modifying gparted to be able to use that on a
mounted partition if the fs supports it ( I think btrfs could be made
to without too much difficulty ).  In the mean time, libparted enforces
the start remaining unchanged and parted, gparted, and kpartx automatically
use the existing start position.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPObfXAAoJEJrBOlT6nu75IKwH/jKcbW6L/j/v7hF0rXHql7Tg
/E17+L37YvtwNA3aiZvDmj6FWablT2Dmvi/zcPITJ0FOWtZ+HKQ6+1odqPWRkOl/
4gQrcUyYn/OK/RqyqCM9O6AwpiKZCvAn9LmDc0BCKDptbKshXe+L2GmgOQbiFSdk
2rcbncXI9Sz1pt8nCP8M3gNdYIl6Ln2u0PxwohMoIVGFDhxETdzm8YfX9MfH4Ay6
FU+5chGJUEiXQgjDw5k+vgX85Xc4pC7IRR6vqAZ5I69RxWB4DBteiR0znBNBZG1Z
Mg8cAJTku2n4fZojCAXv35i3gRa3bgwyHv0suaSc2BGKYEeAu41ncAC3q77Arp4=
=Qxgd
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-02-14  1:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-13 19:30 [PATCH 0/2] block: online resize of disk partitions Vivek Goyal
2012-02-13 19:30 ` [PATCH 1/2] block: add partition resize function to blkpg ioctl Vivek Goyal
2012-02-13 19:30 ` [PATCH 2/2] resizepart: Utility to resize a partition Vivek Goyal
2012-02-13 21:34 ` [PATCH 0/2] block: online resize of disk partitions Phillip Susi
2012-02-13 21:50   ` Vivek Goyal
2012-02-13 21:56     ` Vivek Goyal
2012-02-13 22:18     ` Phillip Susi
2012-02-13 22:24       ` Vivek Goyal
2012-02-14  1:24         ` Phillip Susi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).