[PATCH] dynamic addition of virtual disks on PPC64 iSeries

[PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 4374 bytes --]

Hi Andrew, Linus,

This patch allows us to dynamically add virtual disks to an iSeries
partition. It works like this: after you have created the virtual disk
file on OS/400 and attached it to the Linux partition, you need to read
/sys/bus/vio/drivers/viodasd/probe.  This will do the probe and list any
new disks discovered.

This was the nicest way I could think of doing this as the interface to
the hypervisor is polled ...

Please apply.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN 2.6.6-bk7/drivers/block/viodasd.c 2.6.6-bk7.dyndasd/drivers/block/viodasd.c
--- 2.6.6-bk7/drivers/block/viodasd.c	2004-05-21 16:24:00.000000000 +1000
+++ 2.6.6-bk7.dyndasd/drivers/block/viodasd.c	2004-05-20 20:36:43.000000000 +1000
@@ -40,8 +40,12 @@
 #include <linux/string.h>
 #include <linux/dma-mapping.h>
 #include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
 
 #include <asm/uaccess.h>
+#include <asm/vio.h>
+#include <asm/page.h>		/* for PAGE_SIZE */
 #include <asm/iSeries/HvTypes.h>
 #include <asm/iSeries/HvLpEvent.h>
 #include <asm/iSeries/HvLpConfig.h>
@@ -456,7 +460,7 @@
  * Probe a single disk and fill in the viodasd_device structure
  * for it.
  */
-static void probe_disk(struct viodasd_device *d)
+static int probe_disk(struct viodasd_device *d)
 {
 	HvLpEvent_Rc hvrc;
 	struct viodasd_waitevent we;
@@ -480,14 +484,14 @@
 			0, 0, 0);
 	if (hvrc != 0) {
 		printk(VIOD_KERN_WARNING "bad rc on HV open %d\n", (int)hvrc);
-		return;
+		return 0;
 	}
 
 	wait_for_completion(&we.com);
 
 	if (we.rc != 0) {
 		if (flags != 0)
-			return;
+			return 0;
 		/* try again with read only flag set */
 		flags = vioblockflags_ro;
 		goto retry;
@@ -517,22 +521,15 @@
 	if (hvrc != 0) {
 		printk(VIOD_KERN_WARNING
 		       "bad rc sending event to OS/400 %d\n", (int)hvrc);
-		return;
+		return 0;
 	}
-	printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
-			"CHS=%d/%d/%d sector size %d%s\n",
-			dev_no, (unsigned long)(d->size >> 9),
-			(unsigned long)(d->size >> 20),
-			(int)d->cylinders, (int)d->tracks,
-			(int)d->sectors, (int)d->bytes_per_sector,
-			d->read_only ? " (RO)" : "");
 	/* create the request queue for the disk */
 	spin_lock_init(&d->q_lock);
 	q = blk_init_queue(do_viodasd_request, &d->q_lock);
 	if (q == NULL) {
 		printk(VIOD_KERN_WARNING "cannot allocate queue for disk %d\n",
 				dev_no);
-		return;
+		return 0;
 	}
 	g = alloc_disk(1 << PARTITION_SHIFT);
 	if (g == NULL) {
@@ -540,7 +537,7 @@
 				"cannot allocate disk structure for disk %d\n",
 				dev_no);
 		blk_cleanup_queue(q);
-		return;
+		return 0;
 	}
 
 	d->disk = g;
@@ -563,8 +560,17 @@
 	g->private_data = d;
 	set_capacity(g, d->size >> 9);
 
+	printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
+			"CHS=%d/%d/%d sector size %d%s\n",
+			dev_no, (unsigned long)(d->size >> 9),
+			(unsigned long)(d->size >> 20),
+			(int)d->cylinders, (int)d->tracks,
+			(int)d->sectors, (int)d->bytes_per_sector,
+			d->read_only ? " (RO)" : "");
+
 	/* register us in the global list */
 	add_disk(g);
+	return 1;
 }
 
 /* returns the total number of scatterlist elements converted */
@@ -725,6 +731,29 @@
 }
 
 /*
+ * Get the driver to reprobe for more disks.
+ */
+static ssize_t probe_disks(struct device_driver *drv, char *buf)
+{
+	ssize_t count = 0;
+	struct viodasd_device *d;
+
+	for (d = viodasd_devices; d < &viodasd_devices[MAX_DISKNO]; d++) {
+		if ((d->disk == NULL) && probe_disk(d)) {
+			count += scnprintf(&buf[count], PAGE_SIZE - count,
+					"%s\n", d->disk->disk_name);
+		}
+	}
+	return count;
+}
+
+static DRIVER_ATTR(probe, S_IRUSR, probe_disks, NULL)
+
+static struct vio_driver viodasd_driver = {
+	.name = "viodasd"
+};
+
+/*
  * Initialize the whole device driver.  Handle module and non-module
  * versions
  */
@@ -767,6 +796,9 @@
 	for (i = 0; i < MAX_DISKNO; i++)
 		probe_disk(&viodasd_devices[i]);
 
+	vio_register_driver(&viodasd_driver);	/* FIX ME - error checking */
+	driver_create_file(&viodasd_driver.driver, &driver_attr_probe);
+
 	return 0;
 }
 module_init(viodasd_init);
@@ -776,6 +808,9 @@
 	int i;
 	struct viodasd_device *d;
 
+	vio_unregister_driver(&viodasd_driver);
+	driver_remove_file(&viodasd_driver.driver, &driver_attr_probe);
+
         for (i = 0; i < MAX_DISKNO; i++) {
 		d = &viodasd_devices[i];
 		if (d->disk) {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

Hi Andrew,

On Sun, 23 May 2004 23:29:20 -0700 Andrew Morton <akpm@osdl.org> wrote:
>
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > This patch allows us to dynamically add virtual disks to an iSeries
> >  partition. It works like this: after you have created the virtual disk
> >  file on OS/400 and attached it to the Linux partition, you need to read
> >  /sys/bus/vio/drivers/viodasd/probe.  This will do the probe and list any
> >  new disks discovered.
> > 
> >  This was the nicest way I could think of doing this as the interface to
> >  the hypervisor is polled ...
> 
> Is it possible to present all the virtual disks as partitions of a single
> disk, use the "partition table" to query what is present?

The virtual disks are just that: disks.  They present as /dev/iseries/vda
etc and have their own partitions. I can't change that, I will get skinned
by current users.  It was bad enough when I removed the ide emulation hack
... :-)

(Just in case of confusion: the "Linux partition" I referred to above is a
logical partition fo the whole machine.)

> Or to generate a hotplug event when a disk is added?  Even if there's no
> notification to the kernel, it should be possible to generate the hotplug
> events in response to a /proc-based trigger.

I guess that would be possible.  In this case I am trying to do the
minimum change.

> It's a shame you didn't cc linux-kernel on this - the blockdev police would
> have better ideas than I.

I have now sent the patch to LKML and cc'd this reply there as well.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Dave Hansen]

On Mon, 2004-05-24 at 00:05, Stephen Rothwell wrote:
> On Sun, 23 May 2004 23:29:20 -0700 Andrew Morton <akpm@osdl.org> wrote:
> > Or to generate a hotplug event when a disk is added?  Even if there's no
> > notification to the kernel, it should be possible to generate the hotplug
> > events in response to a /proc-based trigger.
> 
> I guess that would be possible.  In this case I am trying to do the
> minimum change.

I think this would be a worthy change.  It's the same kind of thing that
we're planning for memory hotplug on ppc64: initiate a probe in /sys
somewhere, and get a few hotplug events in short order.  The only
difference is that we'll probably require a write for the probe to
trigger.  You don't want a 'grep -r foo /sys' to cause probes, do you?

> +       printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
> +                       "CHS=%d/%d/%d sector size %d%s\n",
> +                       dev_no, (unsigned long)(d->size >> 9),
> +                       (unsigned long)(d->size >> 20),
> +                       (int)d->cylinders, (int)d->tracks,
> +                       (int)d->sectors, (int)d->bytes_per_sector,
> +                       d->read_only ? " (RO)" : "");
> +

Isn't it a little naughty to be spitting out so many values in a /sys
file?  

-- Dave

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On Mon, 24 May 2004 00:23:28 -0700 Dave Hansen <haveblue@us.ibm.com> wrote:
>
> On Mon, 2004-05-24 at 00:05, Stephen Rothwell wrote:
> > On Sun, 23 May 2004 23:29:20 -0700 Andrew Morton <akpm@osdl.org> wrote:
> > > Or to generate a hotplug event when a disk is added?  Even if there's no
> > > notification to the kernel, it should be possible to generate the hotplug
> > > events in response to a /proc-based trigger.
> > 
> > I guess that would be possible.  In this case I am trying to do the
> > minimum change.
> 
> I think this would be a worthy change.  It's the same kind of thing that
> we're planning for memory hotplug on ppc64: initiate a probe in /sys
> somewhere, and get a few hotplug events in short order.  The only
> difference is that we'll probably require a write for the probe to
> trigger.  You don't want a 'grep -r foo /sys' to cause probes, do you?

OK, I am convinced.  :-)  Now to learn about hotplug. :-(

> > +       printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
> > +                       "CHS=%d/%d/%d sector size %d%s\n",
> > +                       dev_no, (unsigned long)(d->size >> 9),
> > +                       (unsigned long)(d->size >> 20),
> > +                       (int)d->cylinders, (int)d->tracks,
> > +                       (int)d->sectors, (int)d->bytes_per_sector,
> > +                       d->read_only ? " (RO)" : "");
> > +
> 
> Isn't it a little naughty to be spitting out so many values in a /sys
> file?  

Thats not the sys file ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On Sun, 23 May 2004 23:29:20 -0700 Andrew Morton <akpm@osdl.org> wrote:
>
> Or to generate a hotplug event when a disk is added?  Even if there's no
> notification to the kernel, it should be possible to generate the hotplug
> events in response to a /proc-based trigger.

Of course, it occurs to me that hotplug events must be happening (I guess
add_disk does it) as udev was quite happily creating hte device nodes for
me ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Andrew Morton]

Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Sun, 23 May 2004 23:29:20 -0700 Andrew Morton <akpm@osdl.org> wrote:
> >
> > Or to generate a hotplug event when a disk is added?  Even if there's no
> > notification to the kernel, it should be possible to generate the hotplug
> > events in response to a /proc-based trigger.
> 
> Of course, it occurs to me that hotplug events must be happening (I guess
> add_disk does it) as udev was quite happily creating hte device nodes for
> me ...

Handy.  So the patch stands as-is?

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

On Mon, 24 May 2004 01:49:01 -0700 Andrew Morton <akpm@osdl.org> wrote:
>
> Handy.  So the patch stands as-is?

I would like that, but the recursive grep through /sys example
bascially convinced me that having a writable file is better.

New patch soon.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] dynamic addition of virtual disks on PPC64 iSeries

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3262 bytes --]

Hi Andrew, Linus,

Second attempt.

This patch allows us to dynamically add virtual disks to an iSeries
partition. It works like this: after you have created the virtual disk
file on OS/400 and attached it to the Linux partition, you need to write
to /sys/bus/vio/drivers/viodasd/probe (it doesn't matter what you write). 
This will do the probe. It calls add_disk() for each new disk, so we get
hotplug events as a side effect.

This was the nicest way I could think of doing this as the interface to
the hypervisor is polled ...

Please apply.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff -ruN 2.6.7-rc1/drivers/block/viodasd.c 2.6.7-rc1.dyndasd.1/drivers/block/viodasd.c
--- 2.6.7-rc1/drivers/block/viodasd.c	2004-05-24 16:29:46.000000000 +1000
+++ 2.6.7-rc1.dyndasd.1/drivers/block/viodasd.c	2004-05-24 21:17:24.000000000 +1000
@@ -40,8 +40,11 @@
 #include <linux/string.h>
 #include <linux/dma-mapping.h>
 #include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
 
 #include <asm/uaccess.h>
+#include <asm/vio.h>
 #include <asm/iSeries/HvTypes.h>
 #include <asm/iSeries/HvLpEvent.h>
 #include <asm/iSeries/HvLpConfig.h>
@@ -519,13 +522,6 @@
 		       "bad rc sending event to OS/400 %d\n", (int)hvrc);
 		return;
 	}
-	printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
-			"CHS=%d/%d/%d sector size %d%s\n",
-			dev_no, (unsigned long)(d->size >> 9),
-			(unsigned long)(d->size >> 20),
-			(int)d->cylinders, (int)d->tracks,
-			(int)d->sectors, (int)d->bytes_per_sector,
-			d->read_only ? " (RO)" : "");
 	/* create the request queue for the disk */
 	spin_lock_init(&d->q_lock);
 	q = blk_init_queue(do_viodasd_request, &d->q_lock);
@@ -563,6 +559,14 @@
 	g->private_data = d;
 	set_capacity(g, d->size >> 9);
 
+	printk(VIOD_KERN_INFO "disk %d: %lu sectors (%lu MB) "
+			"CHS=%d/%d/%d sector size %d%s\n",
+			dev_no, (unsigned long)(d->size >> 9),
+			(unsigned long)(d->size >> 20),
+			(int)d->cylinders, (int)d->tracks,
+			(int)d->sectors, (int)d->bytes_per_sector,
+			d->read_only ? " (RO)" : "");
+
 	/* register us in the global list */
 	add_disk(g);
 }
@@ -725,6 +729,26 @@
 }
 
 /*
+ * Get the driver to reprobe for more disks.
+ */
+static ssize_t probe_disks(struct device_driver *drv, const char *buf,
+		size_t count)
+{
+	struct viodasd_device *d;
+
+	for (d = viodasd_devices; d < &viodasd_devices[MAX_DISKNO]; d++) {
+		if (d->disk == NULL)
+			probe_disk(d);
+	}
+	return count;
+}
+static DRIVER_ATTR(probe, S_IWUSR, NULL, probe_disks)
+
+static struct vio_driver viodasd_driver = {
+	.name = "viodasd"
+};
+
+/*
  * Initialize the whole device driver.  Handle module and non-module
  * versions
  */
@@ -767,6 +791,9 @@
 	for (i = 0; i < MAX_DISKNO; i++)
 		probe_disk(&viodasd_devices[i]);
 
+	vio_register_driver(&viodasd_driver);	/* FIX ME - error checking */
+	driver_create_file(&viodasd_driver.driver, &driver_attr_probe);
+
 	return 0;
 }
 module_init(viodasd_init);
@@ -776,6 +803,9 @@
 	int i;
 	struct viodasd_device *d;
 
+	driver_remove_file(&viodasd_driver.driver, &driver_attr_probe);
+	vio_unregister_driver(&viodasd_driver);
+
         for (i = 0; i < MAX_DISKNO; i++) {
 		d = &viodasd_devices[i];
 		if (d->disk) {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]