Re: [PATCH] add virtio disk geometry feature

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Ryan Harper <ryanh@...>
Cc: <virtualization@...>, <linux-kernel@...>, <kvm-devel@...>, Anthony Liguori <aliguori@...>
Date: Wednesday, April 16, 2008 - 5:15 pm

On Thursday 17 April 2008 04:56:37 Ryan Harper wrote:

Hi Ryan,

   Looks good! Some brief review below.  Mainly just "how I would have done
things" stuff.  BTW, does this help in real life?  I assume something in
userspace wants it?


Unnecessary err initialization.  Sometimes gcc catches bugs when you defer 
initializations of err to as late as possible (ie. paths where err isn't
set properly), so I tend to do it.


Kind of ugly; we can represent this in the data structure explicitly tho...


... using a struct-within-a-struct.

Here's the result:
Subject: add virtio disk geometry feature
Date: Wed, 16 Apr 2008 13:56:37 -0500
From: Ryan Harper <ryanh@us.ibm.com>

Rather than faking up some geometry, allow the backend to push the disk
geometry via virtio pci config option.  Keep the old geo code around for
compatibility.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (modified to single struct)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -157,10 +157,25 @@ static int virtblk_ioctl(struct inode *i
 /* We provide getgeo only to please some old bootloader/partitioning tools */
 static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo)
 {
-	/* some standard values, similar to sd */
-	geo->heads = 1 << 6;
-	geo->sectors = 1 << 5;
-	geo->cylinders = get_capacity(bd->bd_disk) >> 11;
+	struct virtio_blk *vblk = bd->bd_disk->private_data;
+	struct virtio_blk_geometry vgeo;
+	int err;
+
+	/* see if the host passed in geometry config */
+	err = virtio_config_val(vblk->vdev, VIRTIO_BLK_F_GEOMETRY,
+				offsetof(struct virtio_blk_config, geometry),
+				&vgeo);
+
+	if (!err) {
+		geo->heads = vgeo.heads;
+		geo->sectors = vgeo.sectors;
+		geo->cylinders = vgeo.cylinders;
+	} else {
+		/* some standard values, similar to sd */
+		geo->heads = 1 << 6;
+		geo->sectors = 1 << 5;
+		geo->cylinders = get_capacity(bd->bd_disk) >> 11;
+	}
 	return 0;
 }
 
diff --git a/include/linux/virtio_blk.h b/include/linux/virtio_blk.h
--- a/include/linux/virtio_blk.h
+++ b/include/linux/virtio_blk.h
@@ -9,6 +9,7 @@
 #define VIRTIO_BLK_F_BARRIER	0	/* Does host support barriers? */
 #define VIRTIO_BLK_F_SIZE_MAX	1	/* Indicates maximum segment size */
 #define VIRTIO_BLK_F_SEG_MAX	2	/* Indicates maximum # of segments */
+#define VIRTIO_BLK_F_GEOMETRY	4	/* Legacy geometry available  */
 
 struct virtio_blk_config
 {
@@ -18,6 +19,12 @@ struct virtio_blk_config
 	__le32 size_max;
 	/* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
 	__le32 seg_max;
+	/* geometry the device (if VIRTIO_BLK_F_GEOMETRY) */
+	struct virtio_blk_geometry {
+		__le16 cylinders;
+		__u8 heads;
+		__u8 sectors;
+	} geometry;
 } __attribute__((packed));
 
 /* These two define direction. */
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH] add virtio disk geometry feature, Ryan Harper, (Wed Apr 16, 2:56 pm)
Re: [PATCH] add virtio disk geometry feature, Rusty Russell, (Wed Apr 16, 5:15 pm)
Re: [PATCH] add virtio disk geometry feature, Anthony Liguori, (Wed Apr 16, 5:32 pm)
Re: [PATCH] add virtio disk geometry feature, Ryan Harper, (Thu Apr 17, 10:37 am)
Re: [PATCH] add virtio disk geometry feature, Hollis Blanchard, (Wed Apr 16, 5:57 pm)