Skip to contentSkip to conversation

Add serial number support for virtio_blk, V4

PATCH Applied john cooper 3 participants 8 messages May 29, 2009 – Jun 9, 2009

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

john cooper

author
#0 lore raw

cover Add serial number support for virtio_blk, V4

[Rework of earlier patch to provide additional information in the response to an ATA identify request -- virtio_blk treats the data as opaque, content created by qemu's virtio-blk. Comments from Christoph also incorporated.]

This patch allows passing of a virtio_blk drive serial number from qemu into a guest's virtio_blk driver, and provides a means to access the serial number from a guest's userspace.

Equivalent functionality currently exists for IDE and SCSI, however it is not yet implemented for virtio. Scenarios exist where guest code relies on a unique drive serial number to correctly identify the machine environment in which it exists.

The following two patches implement the above:

qemu-vblk-serial-4.patch

which provides the qemu missing bits to interpret a '-drive .. serial=XYZ ..' flag, and:

virtio_blk-serial-4.patch

which extracts this information and makes it available to guest userspace via an HDIO_GET_IDENTITY ioctl, eg: 'hdparm -i /dev/vda'.

The above patches are relative to qemu-kvm.git and 2.6.29.3 respectively.

-john

Signature · 1 line
[email protected]
author
#1 lore raw

Re: [PATCH 0/2] Add serial number support for virtio_blk, V4a

[Cosmetic changes to prior version, documentation added to individual patches.]

This patch allows passing of a virtio_blk drive serial number from qemu into a guest's virtio_blk driver, and provides a means to access the serial number from a guest's userspace.

Equivalent functionality currently exists for IDE and SCSI, however it is not yet implemented for virtio. Scenarios exist where guest code relies on a unique drive serial number to correctly identify the machine environment in which it exists. The following patches, relative to qemu-kvm.git and 2.6.29.3 respectively, provide an equivalent mechanism for virtio_blk.

Usage:

With a patched qemu and guest virtio_blk driver, launch the guest with a virtio_blk drive specifying a serial number, eg:

    -drive ... ,if=virtio,serial=ThisSpace4Rent, ...

In the guest retrieve via HDIO_GET_IDENTITY ioctl to the virtio_blk driver, (eg):

    # hdparm -i /dev/vda

hdparm -i /dev/vda

/dev/vda:

Model=QEMU VIRT_BLK, FwRev=0.10.50, SerialNo=ThisSpace4Rent

-john

Signature · 1 line
[email protected]
author
#2 lore raw

Re: [PATCH 1/2] Add serial number support for virtio_blk, V4a

In reply to john cooper

This patch implements the missing qemu logic to interpret a '-drive .. serial=XYZ ..' flag for a virtio_blk device.

The serial number string is contained in a skeletal IDENTIFY DEVICE data structure and this structure is made available to the guest virtio_blk driver via pci i/o region 0.

Signed-off-by john cooper <[email protected]>

---

hw/virtio-blk.c +56 -0
index 8dd3c7a..0b7ebe9 100644
@@ -25,6 +25,7 @@ typedef struct VirtIOBlock
     BlockDriverState *bs;
     VirtQueue *vq;
     void *rq;
+    char serial_str[BLOCK_SERIAL_STRLEN + 1];
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)

@@ -285,6 +286,50 @@ static void virtio_blk_reset(VirtIODevice *vdev)
     qemu_aio_flush();
 }
 
+/* store identify data in little endian format
+ */
+static inline void put_le16(uint16_t *p, unsigned int v)
+{
+    *p = cpu_to_le16(v);
+}
+
+/* copy to *dst from *src, nul pad dst tail as needed to len bytes
+ */
+static inline void padstr(char *dst, const char *src, int len)
+{
+    while (len--)
+        *dst++ = *src ? *src++ : '\0';
+}
+
+/* setup simulated identify data as appropriate for virtio block device
+ *
+ * ref: AT Attachment 8 - ATA/ATAPI Command Set (ATA8-ACS)
+ */
+static inline void virtio_identify_template(struct virtio_blk_config *bc)
+{
+    uint16_t *p = &bc->identify[0];
+    uint64_t lba_sectors = bc->capacity;
+
+    memset(p, 0, sizeof(bc->identify));
+    put_le16(p + 0, 0x0);                            /* ATA device */
+    padstr((char *)(p + 23), QEMU_VERSION, 8);       /* firmware revision */
+    padstr((char *)(p + 27), "QEMU VIRT_BLK", 40);   /* model# */
+    put_le16(p + 47, 0x80ff);                        /* max xfer 255 sectors */
+    put_le16(p + 49, 0x0b00);                        /* support IORDY/LBA/DMA */
+    put_le16(p + 59, 0x1ff);                         /* cur xfer 255 sectors */
+    put_le16(p + 80, 0x1f0);                         /* support ATA8/7/6/5/4 */
+    put_le16(p + 81, 0x16);
+    put_le16(p + 82, 0x400);
+    put_le16(p + 83, 0x400);
+    put_le16(p + 100, lba_sectors);
+    put_le16(p + 101, lba_sectors >> 16);
+    put_le16(p + 102, lba_sectors >> 32);
+    put_le16(p + 103, lba_sectors >> 48);
+}
+
+/* coalesce internal state, copy to pci i/o region 0
+ */
+
 static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIOBlock *s = to_virtio_blk(vdev);

@@ -299,11 +344,15 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
     stw_raw(&blkcfg.cylinders, cylinders);
     blkcfg.heads = heads;
     blkcfg.sectors = secs;
+    virtio_identify_template(&blkcfg);
+    memcpy(&blkcfg.identify[VIRTIO_BLK_ID_SN], s->serial_str,
+        VIRTIO_BLK_ID_SN_BYTES);
     memcpy(config, &blkcfg, sizeof(blkcfg));
 }
 
 static uint32_t virtio_blk_get_features(VirtIODevice *vdev)
 {
+    VirtIOBlock *s = to_virtio_blk(vdev);
     uint32_t features = 0;
 
     features |= (1 << VIRTIO_BLK_F_SEG_MAX);

@@ -311,6 +360,8 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev)
 #ifdef __linux__
     features |= (1 << VIRTIO_BLK_F_SCSI);
 #endif
+    if (strcmp(s->serial_str, "0"))
+        features |= 1 << VIRTIO_BLK_F_IDENTIFY;
 
     return features;
 }

@@ -354,6 +405,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev)
     int cylinders, heads, secs;
     static int virtio_blk_id;
     BlockDriverState *bs;
+    char *ps;
 
     s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
                                           sizeof(struct virtio_blk_config),

@@ -365,6 +417,10 @@ VirtIODevice *virtio_blk_init(DeviceState *dev)
     s->vdev.reset = virtio_blk_reset;
     s->bs = bs;
     s->rq = NULL;
+    if (strlen(ps = (char *)drive_get_serial(bs)))
+        strncpy(s->serial_str, ps, sizeof(s->serial_str));
+    else
+        snprintf(s->serial_str, sizeof(s->serial_str), "0");
     bs->private = dev;
     bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
     bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
hw/virtio-blk.h +7 -0
index dff3e0c..1be4342 100644
@@ -30,6 +30,11 @@
 #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_SCSI       7       /* Supports scsi command passthru */
+#define VIRTIO_BLK_F_IDENTIFY   8       /* ATA IDENTIFY supported */
+
+#define VIRTIO_BLK_ID_LEN       256     /* length of identify u16 array */
+#define VIRTIO_BLK_ID_SN        10      /* start of char * serial# */
+#define VIRTIO_BLK_ID_SN_BYTES  20      /* length in bytes of serial# */
 
 struct virtio_blk_config
 {

@@ -39,6 +44,8 @@ struct virtio_blk_config
     uint16_t cylinders;
     uint8_t heads;
     uint8_t sectors;
+    uint32_t _blk_size;    /* structure pad, currently unused */
+    uint16_t identify[VIRTIO_BLK_ID_LEN];
 } __attribute__((packed));
 
 /* These two define direction. */
sysemu.h +3 -1
index 47d001e..d3df19f 100644
@@ -152,6 +152,8 @@ typedef enum {
     BLOCK_ERR_STOP_ANY
 } BlockInterfaceErrorAction;
 
+#define BLOCK_SERIAL_STRLEN 20
+
 typedef struct DriveInfo {
     BlockDriverState *bdrv;
     BlockInterfaceType type;

@@ -160,7 +162,7 @@ typedef struct DriveInfo {
     int used;
     int drive_opt_idx;
     BlockInterfaceErrorAction onerror;
-    char serial[21];
+    char serial[BLOCK_SERIAL_STRLEN + 1];
 } DriveInfo;
 
 #define MAX_IDE_DEVS	2
Signature · 1 line
[email protected]
author
#3 lore raw

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4a

In reply to john cooper

This patch extracts the opaque data from pci i/o region 0 via the added VIRTIO_BLK_F_IDENTIFY field. By convention this data takes the form of that returned by an ATA IDENTIFY DEVICE command, however the driver (except for structure size) makes no interpretation of the data. The structure data is copied wholesale to userspace via a HDIO_GET_IDENTITY ioctl command (eg: hdparm -i <dev>).

Signed-off-by john cooper <[email protected]>

---

 drivers/block/virtio_blk.c |   41 ++++++++++++++++++++++++++++++++++++++---
 include/linux/virtio_blk.h |    6 ++++++
 2 files changed, 44 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;
+	void *opaque;
+	int err = -ENOMEM;
+
+	opaque = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
+	if (!opaque)
+		goto out;
+
+	err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY,
+		offsetof(struct virtio_blk_config, identify), opaque,
+		VIRTIO_BLK_ID_BYTES);
+
+	if (err)
+		goto out_kfree;
+
+	if (copy_to_user(argp, opaque, VIRTIO_BLK_ID_BYTES))
+		err = -EFAULT;
+
+out_kfree:
+	kfree(opaque);
+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 +6 -0
@@ -15,7 +15,12 @@
 #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_BYTES	(sizeof (__u16[256]))	/* IDENTIFY DATA */
+
+/* mapped into pci i/o region 0
+ */
 struct virtio_blk_config
 {
 	/* The capacity (in 512-byte sectors). */

@@ -32,6 +37,7 @@ struct virtio_blk_config
 	} geometry;
 	/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
 	__u32 blk_size;
+	__u8 identify[VIRTIO_BLK_ID_BYTES];
 } __attribute__((packed));
 
 /* These two define direction. */
Signature · 1 line
[email protected]
Applied
#4 lore raw

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4a

Quoted 8 lines · john cooper

This patch extracts the opaque data from pci i/o region 0 via the added VIRTIO_BLK_F_IDENTIFY field. By convention this data takes the form of that returned by an ATA IDENTIFY DEVICE command, however the driver (except for structure size) makes no interpretation of the data. The structure data is copied wholesale to userspace via a HDIO_GET_IDENTITY ioctl command (eg: hdparm -i <dev>).

Thanks, applied with one subtraction:

Quoted 3 lines

+/* mapped into pci i/o region 0
+ */
struct virtio_blk_config

I removed this comment; it's true for virtio_pci, but not true in general.

Thanks!
Rusty.

#5 lore raw

PATCH 1/1 Add serial number support for virtio_blk, V4a

In reply to john cooper

From: john cooper <[email protected]>

This patch extracts the opaque data from pci i/o region 0 via the added VIRTIO_BLK_F_IDENTIFY field. By convention this data takes the form of that returned by an ATA IDENTIFY DEVICE command, however the driver (except for structure size) makes no interpretation of the data. The structure data is copied wholesale to userspace via a HDIO_GET_IDENTITY ioctl command (eg: hdparm -i <dev>).

Signed-off-by john cooper <[email protected]>
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/block/virtio_blk.c |   37 ++++++++++++++++++++++++++++++++++---
 include/linux/virtio_blk.h |    4 ++++
 2 files changed, 38 insertions(+), 3 deletions(-)
drivers/block/virtio_blk.c +34 -3
@@ -174,11 +174,43 @@ 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;
+	void *opaque;
+	int err = -ENOMEM;
+
+	opaque = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL);
+	if (!opaque)
+		goto out;
+
+	err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY,
+		offsetof(struct virtio_blk_config, identify), opaque,
+		VIRTIO_BLK_ID_BYTES);
+
+	if (err)
+		goto out_kfree;
+
+	if (copy_to_user(argp, opaque, VIRTIO_BLK_ID_BYTES))
+		err = -EFAULT;
+
+out_kfree:
+	kfree(opaque);
+out:
+	return err;
+}
+
 static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
 			 unsigned cmd, unsigned long data)
 {
 	struct gendisk *disk = bdev->bd_disk;
 	struct virtio_blk *vblk = disk->private_data;
+	void __user *argp = (void __user *)data;
+
+	if (cmd == HDIO_GET_IDENTITY)
+		return virtblk_identify(disk, argp);
 
 	/*
 	 * Only allow the generic SCSI ioctls if the host can support it.

@@ -186,8 +218,7 @@ static int virtblk_ioctl(struct block_de
 	if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_SCSI))
 		return -ENOIOCTLCMD;
 
-	return scsi_cmd_ioctl(disk->queue, disk, mode, cmd,
-			      (void __user *)data);
+	return scsi_cmd_ioctl(disk->queue, disk, mode, cmd, argp);
 }
 
 /* We provide getgeo only to please some old bootloader/partitioning tools */

@@ -393,7 +424,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_SCSI,
+	VIRTIO_BLK_F_SCSI, VIRTIO_BLK_F_IDENTIFY
 };
 
 static struct virtio_driver virtio_blk = {
include/linux/virtio_blk.h +4 -0
@@ -16,6 +16,9 @@
 #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_SCSI	7	/* Supports scsi command passthru */
+#define VIRTIO_BLK_F_IDENTIFY	8	/* ATA IDENTIFY supported */
+
+#define VIRTIO_BLK_ID_BYTES	(sizeof(__u16[256]))	/* IDENTIFY DATA */
 
 struct virtio_blk_config
 {

@@ -33,6 +36,7 @@ struct virtio_blk_config
 	} geometry;
 	/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
 	__u32 blk_size;
+	__u8 identify[VIRTIO_BLK_ID_BYTES];
 } __attribute__((packed));
 
 /* These two define direction. */
#6 lore raw

Re: [PATCH] Add serial number support for virtio_blk, V4a

Quoted 10 lines · Rusty Russell

From: john cooper <[email protected]>

This patch extracts the opaque data from pci i/o region 0 via the added VIRTIO_BLK_F_IDENTIFY field. By convention this data takes the form of that returned by an ATA IDENTIFY DEVICE command, however the driver (except for structure size) makes no interpretation of the data. The structure data is copied wholesale to userspace via a HDIO_GET_IDENTITY ioctl command (eg: hdparm -i <dev>).

Added!

Signature · 1 line
Jens Axboe
#7 lore raw

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4a

In reply to john cooper
Quoted 3 lines · you

This patch extracts the opaque data from pci i/o
region 0 via the added VIRTIO_BLK_F_IDENTIFY
field.

Thanks John, I fixed textual conflicts and forwarded it to Jens: he has the other pending virtio_blk patches as well.

I removed one comment:

Quoted 4 lines

+/* mapped into pci i/o region 0
+ */
struct virtio_blk_config
{

This is true for virtio_pci, but not for virtio in general.

Thanks,
Rusty.