Patchwork [2/2] Remove virtio_blk VBID ioctl

login
register
mail settings
Submitter Ryan Harper
Date June 18, 2010, 6:38 p.m.
Message ID <1276886283-1571-2-git-send-email-ryanh@us.ibm.com>
Download mbox | patch
Permalink /patch/56212/
State New
Headers show

Comments

Ryan Harper - June 18, 2010, 6:38 p.m.
With the availablility of a sysfs device attribute for examining disk serial
numbers the ioctl is no longer needed.  The user-space changes for this aren't
upstream yet so we don't have any users to worry about.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 drivers/block/virtio_blk.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)
Rusty Russell - June 21, 2010, 1:30 a.m.
On Sat, 19 Jun 2010 04:08:03 am Ryan Harper wrote:
> With the availablility of a sysfs device attribute for examining disk serial
> numbers the ioctl is no longer needed.  The user-space changes for this aren't
> upstream yet so we don't have any users to worry about.

If John Cooper acks this, I'll push it to Linus immediately.

Unfortunately we offered this interface in 2.6.34, and we're now removing it.
That's unpleasant.

Thanks,
Rusty.
PS.  John should have been cc'd on these patches!
Ryan Harper - June 21, 2010, 2:30 a.m.
* Rusty Russell <rusty@rustcorp.com.au> [2010-06-20 20:31]:
> On Sat, 19 Jun 2010 04:08:03 am Ryan Harper wrote:
> > With the availablility of a sysfs device attribute for examining disk serial
> > numbers the ioctl is no longer needed.  The user-space changes for this aren't
> > upstream yet so we don't have any users to worry about.
> 
> If John Cooper acks this, I'll push it to Linus immediately.
> 
> Unfortunately we offered this interface in 2.6.34, and we're now removing it.
> That's unpleasant.

Yes; well.  There's a story as there always is.  John can tell it better
than I, but it goes something like:

John cooked up some patches, one of which was an example use of the
serial string including a VBID ioctl.  No one got around to doing a
sysfs interface and somehow the ioctl side in virtio-blk got picked up.

Working with what was available, I pushed some patches to linux-hotplug
to get this whole virtio-blk serial and disk/by-id symlinks working and
was met with:  why does a new kernel driver have an ioctl interface and
we don't want to collect additional single-use binaries in the udev
tree.

So now, the sysfs serial attribute patch and with it, no need and no
users of ioctl.

*whew*

> 
> Thanks,
> Rusty.
> PS.  John should have been cc'd on these patches!

He's cc'ed on the others, just not on this removal one.

I need to learn git-send-email better, I explicitly added him as --cc on
when sending; next time I'll look closer at the headers during
--dry-run.
john cooper - June 21, 2010, 5:07 a.m.
Rusty Russell wrote:
> On Sat, 19 Jun 2010 04:08:03 am Ryan Harper wrote:
>> With the availablility of a sysfs device attribute for examining disk serial
>> numbers the ioctl is no longer needed.  The user-space changes for this aren't
>> upstream yet so we don't have any users to worry about.
> 
> If John Cooper acks this, I'll push it to Linus immediately.

Actually I'm the one who suggested removing it.
The code in question was only intended as example
usage of accessing the s/n data in the driver, for
the /sys interface under discussion back then.
That effort subsequently stalled and Ryan had
recently picked it up.  As such I believe this
overshadows the general need for an ioctl.  Even if
for some reason an ioctl would be justified going
forward, a more usage friendly form would be better.
So let's just drop it for now as the corresponding
qemu-side code hasn't been merged.
 
> Unfortunately we offered this interface in 2.6.34, and we're now removing it.
> That's unpleasant.

Indeed.  This entire effort, aside from being an exercise
in protracted agony, probably violates a Rube Goldberg
patent.

Thanks,

-john

Patch

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index f1ef26f..9e382dd 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -225,16 +225,6 @@  static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
 	struct gendisk *disk = bdev->bd_disk;
 	struct virtio_blk *vblk = disk->private_data;
 
-	if (cmd == 0x56424944) { /* 'VBID' */
-		void __user *usr_data = (void __user *)data;
-		char id_str[VIRTIO_BLK_ID_BYTES];
-		int err;
-
-		err = virtblk_get_id(disk, id_str);
-		if (!err && copy_to_user(usr_data, id_str, VIRTIO_BLK_ID_BYTES))
-			err = -EFAULT;
-		return err;
-	}
 	/*
 	 * Only allow the generic SCSI ioctls if the host can support it.
 	 */