diff mbox

[2/2] Add serial number support for virtio_blk, V4

Message ID 4A1F615F.1090701@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

john cooper May 29, 2009, 4:15 a.m. UTC 
virtio_blk-serial-4.patch



 drivers/block/virtio_blk.c |   41 ++++++++++++++++++++++++++++++++++++++---
 include/linux/virtio_blk.h |    7 +++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

Comments

Christoph Hellwig May 31, 2009, 11:07 a.m. UTC | #1 
On Fri, May 29, 2009 at 12:15:27AM -0400, john cooper wrote:
> virtio_blk-serial-4.patch
> 
> 

The patch looks good to me, but to get it applied to the kernel tree the
patch needs a good description and we also need your Signed-off-by line (See
Documentation/SubmittingPatches in the kernel source tree)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rusty Russell June 1, 2009, 7:10 a.m. UTC | #2 
On Fri, 29 May 2009 01:45:27 pm john cooper wrote:
> virtio_blk-serial-4.patch

Hate to ask dumb questions, but is there a scsi equivalent of this?  It'd be 
nice if we could avoid being ATA-specific in the long run...

Also, why u16?

Thanks,
Rusty.

> +/* return ATA identify data
> + */
> +static int virtblk_identify(struct gendisk *disk, void *argp)
> +{
> +	struct virtio_blk *vblk = disk->private_data;
> +	u16 *id;
> +	int err = -ENOMEM;
> +
> +	id = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
> +	if (!id)
> +		goto out;
> +
> +	err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY,
> +		offsetof(struct virtio_blk_config, identify), id,
> +		VIRTIO_BLK_ID_BYTES);

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jens Axboe June 3, 2009, 10:17 a.m. UTC | #3 
On Mon, Jun 01 2009, Rusty Russell wrote:
> On Fri, 29 May 2009 01:45:27 pm john cooper wrote:
> > virtio_blk-serial-4.patch
> 
> Hate to ask dumb questions, but is there a scsi equivalent of this?  It'd be 
> nice if we could avoid being ATA-specific in the long run...

SCSI has mode pages, where ATA pretty much stuffs everything into the
identify data.

> Also, why u16?

The identify page is word based, so u16 makes sense.

> 
> Thanks,
> Rusty.
> 
> > +/* return ATA identify data
> > + */
> > +static int virtblk_identify(struct gendisk *disk, void *argp)
> > +{
> > +	struct virtio_blk *vblk = disk->private_data;
> > +	u16 *id;
> > +	int err = -ENOMEM;
> > +
> > +	id = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
> > +	if (!id)
> > +		goto out;
> > +
> > +	err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY,
> > +		offsetof(struct virtio_blk_config, identify), id,
> > +		VIRTIO_BLK_ID_BYTES);
>
john cooper June 3, 2009, 3:08 p.m. UTC | #4 
Jens Axboe wrote:
> On Mon, Jun 01 2009, Rusty Russell wrote:
>> On Fri, 29 May 2009 01:45:27 pm john cooper wrote:
>>> virtio_blk-serial-4.patch
>> Hate to ask dumb questions, but is there a scsi equivalent of this?  It'd be 
>> nice if we could avoid being ATA-specific in the long run...
> 
> SCSI has mode pages, where ATA pretty much stuffs everything into the
> identify data.
> 
>> Also, why u16?
> 
> The identify page is word based, so u16 makes sense.

It is actually an artifact left over from the previous
version.  In that case the driver was only pulling
the serial number over, itself constructing a mocked-up
identify page, and needed to be aware of the internal
structure.

Currently qemu fabricates the identify page which the
driver treats as opaque data passed onto the user.  So
the u16 * structure has no meaning.

Sorry to be late in responding here.  A patch to address
this wrinkle and other feedback to follow shortly.

-john


>>> +/* return ATA identify data
>>> + */
>>> +static int virtblk_identify(struct gendisk *disk, void *argp)
>>> +{
>>> +	struct virtio_blk *vblk = disk->private_data;
>>> +	u16 *id;
>>> +	int err = -ENOMEM;
>>> +
>>> +	id = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
>>> +	if (!id)
>>> +		goto out;
>>> +
>>> +	err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY,
>>> +		offsetof(struct virtio_blk_config, identify), id,
>>> +		VIRTIO_BLK_ID_BYTES);
>
diff mbox

Patch

=================================================================
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -146,12 +146,46 @@  static void do_virtblk_request(struct re
 		vblk->vq->vq_ops->kick(vblk->vq);
 }
 
+/* return ATA identify data
+ */
+static int virtblk_identify(struct gendisk *disk, void *argp)
+{
+	struct virtio_blk *vblk = disk->private_data;
+	u16 *id;
+	int err = -ENOMEM;
+
+	id = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
+	if (!id)
+		goto out;
+
+	err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY,
+		offsetof(struct virtio_blk_config, identify), id,
+		VIRTIO_BLK_ID_BYTES);
+
+	if (err)
+		goto out_kfree;
+
+	if (copy_to_user(argp, id, VIRTIO_BLK_ID_BYTES))
+		err = -EFAULT;
+
+out_kfree:
+	kfree(id);
+out:
+	return err;
+}
+
 static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
 			 unsigned cmd, unsigned long data)
 {
-	return scsi_cmd_ioctl(bdev->bd_disk->queue,
-			      bdev->bd_disk, mode, cmd,
-			      (void __user *)data);
+	struct gendisk *disk = bdev->bd_disk;
+	void __user *argp = (void __user *)data;
+
+	switch (cmd) {
+	case HDIO_GET_IDENTITY:
+		return virtblk_identify(disk, argp);
+	default:
+		return scsi_cmd_ioctl(disk->queue, disk, mode, cmd, argp);
+	}
 }
 
 /* We provide getgeo only to please some old bootloader/partitioning tools */
@@ -356,6 +390,7 @@  static struct virtio_device_id id_table[
 static unsigned int features[] = {
 	VIRTIO_BLK_F_BARRIER, VIRTIO_BLK_F_SEG_MAX, VIRTIO_BLK_F_SIZE_MAX,
 	VIRTIO_BLK_F_GEOMETRY, VIRTIO_BLK_F_RO, VIRTIO_BLK_F_BLK_SIZE,
+	VIRTIO_BLK_F_IDENTIFY
 };
 
 static struct virtio_driver virtio_blk = {
=================================================================
--- a/include/linux/virtio_blk.h
+++ b/include/linux/virtio_blk.h
@@ -15,7 +15,13 @@ 
 #define VIRTIO_BLK_F_GEOMETRY	4	/* Legacy geometry available  */
 #define VIRTIO_BLK_F_RO		5	/* Disk is read-only */
 #define VIRTIO_BLK_F_BLK_SIZE	6	/* Block size of disk is available*/
+#define VIRTIO_BLK_F_IDENTIFY	8	/* ATA IDENTIFY supported */
 
+#define VIRTIO_BLK_ID_LEN 256
+#define VIRTIO_BLK_ID_BYTES (VIRTIO_BLK_ID_LEN * sizeof (u16))
+
+/* mapped into pci i/o region 0
+ */
 struct virtio_blk_config
 {
 	/* The capacity (in 512-byte sectors). */
@@ -32,6 +38,7 @@  struct virtio_blk_config
 	} geometry;
 	/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
 	__u32 blk_size;
+	__u16 identify[VIRTIO_BLK_ID_LEN];
 } __attribute__((packed));
 
 /* These two define direction. */