[PATCH, resend] blkfront: ioctls/geometry, 2.6

[PATCH, resend] blkfront: ioctls/geometry, 2.6

[Jan Beulich]

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

Add backing support for HDIO_GETGEO ioctl to blkfront.

Inspired by an earlier patch from Charles Coffing.

Signed-Off-By: Jan Beulich <jbeulich@novell.com>


[-- Attachment #2: xenlinux-blkfront-getgeo.patch --]
[-- Type: text/plain, Size: 2509 bytes --]

Index: head-2006-05-11/drivers/xen/blkfront/blkfront.c
===================================================================
--- head-2006-05-11.orig/drivers/xen/blkfront/blkfront.c	2006-05-12 09:39:21.000000000 +0200
+++ head-2006-05-11/drivers/xen/blkfront/blkfront.c	2006-05-15 09:59:52.000000000 +0200
@@ -452,10 +452,6 @@ int blkif_ioctl(struct inode *inode, str
 		      command, (long)argument, inode->i_rdev);
 
 	switch (command) {
-	case HDIO_GETGEO:
-		/* return ENOSYS to use defaults */
-		return -ENOSYS;
-
 	case CDROMMULTISESSION:
 		DPRINTK("FIXME: support multisession CDs later\n");
 		for (i = 0; i < sizeof(struct cdrom_multisession); i++)
@@ -473,6 +469,23 @@ int blkif_ioctl(struct inode *inode, str
 }
 
 
+int blkif_getgeo(struct block_device *bd, struct hd_geometry *hg)
+{
+	/* We don't have real geometry info, but let's at least return
+	   values consistent with the size of the device */
+	sector_t nsect = get_capacity(bd->bd_disk);
+	sector_t cylinders = nsect;
+
+	hg->heads = 0xff;
+	hg->sectors = 0x3f;
+	sector_div(cylinders, hg->heads * hg->sectors);
+	hg->cylinders = cylinders;
+	if ((sector_t)(hg->cylinders + 1) * hg->heads * hg->sectors < nsect)
+		hg->cylinders = 0xffff;
+	return 0;
+}
+
+
 /*
  * blkif_queue_request
  *
Index: head-2006-05-11/drivers/xen/blkfront/block.h
===================================================================
--- head-2006-05-11.orig/drivers/xen/blkfront/block.h	2006-05-12 09:39:21.000000000 +0200
+++ head-2006-05-11/drivers/xen/blkfront/block.h	2006-05-10 11:06:51.000000000 +0200
@@ -140,6 +140,7 @@ extern int blkif_open(struct inode *inod
 extern int blkif_release(struct inode *inode, struct file *filep);
 extern int blkif_ioctl(struct inode *inode, struct file *filep,
                        unsigned command, unsigned long argument);
+extern int blkif_getgeo(struct block_device *, struct hd_geometry *);
 extern int blkif_check(dev_t dev);
 extern int blkif_revalidate(dev_t dev);
 extern void do_blkif_request (request_queue_t *rq);
Index: head-2006-05-11/drivers/xen/blkfront/vbd.c
===================================================================
--- head-2006-05-11.orig/drivers/xen/blkfront/vbd.c	2006-05-12 09:39:21.000000000 +0200
+++ head-2006-05-11/drivers/xen/blkfront/vbd.c	2006-05-10 11:07:16.000000000 +0200
@@ -91,6 +91,7 @@ static struct block_device_operations xl
 	.open = blkif_open,
 	.release = blkif_release,
 	.ioctl  = blkif_ioctl,
+	.getgeo = blkif_getgeo
 };
 
 DEFINE_SPINLOCK(blkif_io_lock);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH, resend] blkfront: ioctls/geometry, 2.6

[Keir Fraser]

On 2 Jun 2006, at 11:05, Jan Beulich wrote:

> Add backing support for HDIO_GETGEO ioctl to blkfront.
>
> Inspired by an earlier patch from Charles Coffing.

Does this have any effect? It looks to me as though HDIO_GETGEO is 
handled by block/ioctl.c:blkdev_ioctl(). I also note that *no* other 
blkdev drivers define their own HDIO_GETGEO handler. Shouldn't we just 
remove that case from our ioctl switch statement and define a 'getgeo' 
function hook?

Given that this patch can't have any effect, what drove you guys to 
implement it? :-)

  -- Keir

Re: [PATCH, resend] blkfront: ioctls/geometry, 2.6

[Jan Beulich]

>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 02.06.06 12:19 >>>
>
>On 2 Jun 2006, at 11:05, Jan Beulich wrote:
>
>> Add backing support for HDIO_GETGEO ioctl to blkfront.
>>
>> Inspired by an earlier patch from Charles Coffing.
>
>Does this have any effect? It looks to me as though HDIO_GETGEO is 
>handled by block/ioctl.c:blkdev_ioctl(). I also note that *no* other 
>blkdev drivers define their own HDIO_GETGEO handler. Shouldn't we just 
>remove that case from our ioctl switch statement and define a 'getgeo' 
>function hook?

But that is exactly what the patch does. It instead adds a getgeo function to vbd, which is what several other drivers
also have, and which is what backs blkdev_ioctl()'s handling of HDIO_GETGEO.

>Given that this patch can't have any effect, what drove you guys to 
>implement it? :-)

We saw GrUB failing when used inside a domU.

Jan

Re: [PATCH, resend] blkfront: ioctls/geometry, 2.6

[Keir Fraser]

On 2 Jun 2006, at 11:53, Jan Beulich wrote:

>
> But that is exactly what the patch does. It instead adds a getgeo 
> function to vbd, which is what several other drivers
> also have, and which is what backs blkdev_ioctl()'s handling of 
> HDIO_GETGEO.

Huh, so it does. It looks okay then. :-) I'll apply to -unstable and 
-testing.

  -- Keir