Skip to contentSkip to conversation

Add serial number support for virtio_blk, V4

PATCH Applied john cooper 4 participants 5 messages May 29, 2009 – Jun 3, 2009

Subsystems (4)
KVM Block layer virtio core virtio block and SCSI drivers
lore
Share & tools
lore

john cooper

author
#0 lore raw

PATCH 2/2 Add serial number support for virtio_blk, V4

virtio_blk-serial-4.patch

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

=================================================================

drivers/block/virtio_blk.c +38 -3
@@ -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 = {

=================================================================

include/linux/virtio_blk.h +7 -0
@@ -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. */
Signature · 1 line
[email protected]
#1 lore raw
Quoted 3 lines · john cooper

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)

#2 lore raw
In reply to john cooper
Quoted 1 line · john cooper

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.

Quoted 15 lines
drivers/block/virtio_blk.c +15 -0
+/* 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);
#3 lore raw
Quoted 5 lines · Rusty Russell
Quoted 1 line · john cooper

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.

Quoted 1 line

Also, why u16?

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

Quoted 20 lines

Thanks,
Rusty.

Quoted 15 lines
drivers/block/virtio_blk.c +15 -0
+/* 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);
Signature · 1 line
Jens Axboe
author
#4 lore raw
Quoted 6 lines · Jens Axboe
Quoted 4 lines · Rusty Russell
Quoted 1 line · john cooper

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...

Quoted 6 lines

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

Quoted 1 line

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

Quoted 16 lines
Quoted 15 lines
Quoted 15 lines
drivers/block/virtio_blk.c +15 -0
+/* 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);
Signature · 1 line
[email protected]