[Qemu-devel] [PATCH 0/2] Add serial number support for virtio_blk, V4

[Qemu-devel] [PATCH 0/2] Add serial number support for virtio_blk, V4

[john cooper]

[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

-- 
john.cooper@redhat.com

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

[john cooper]

[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

-- 
john.cooper@redhat.com

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

[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 <john.cooper@redhat.com>
---

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 8dd3c7a..0b7ebe9 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -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);
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index dff3e0c..1be4342 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -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. */
diff --git a/sysemu.h b/sysemu.h
index 47d001e..d3df19f 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -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

-- 
john.cooper@redhat.com

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

[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 <john.cooper@redhat.com>
---

 drivers/block/virtio_blk.c |   41 ++++++++++++++++++++++++++++++++++++++---
 include/linux/virtio_blk.h |    6 ++++++
 2 files changed, 44 insertions(+), 3 deletions(-)
=================================================================
--- 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;
+	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 = {
=================================================================
--- a/include/linux/virtio_blk.h
+++ b/include/linux/virtio_blk.h
@@ -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. */

-- 
john.cooper@redhat.com

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

[Rusty Russell]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Thu, 4 Jun 2009 06:37:19 am john cooper wrote:
> 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:

> +/* 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.

[-- Attachment #2: Type: text/html, Size: 1757 bytes --]

[Qemu-devel] [PATCH] Add serial number support for virtio_blk, V4a

[Rusty Russell]

From: john cooper <john.cooper@redhat.com>

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 <john.cooper@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/block/virtio_blk.c |   37 ++++++++++++++++++++++++++++++++++---
 include/linux/virtio_blk.h |    4 ++++
 2 files changed, 38 insertions(+), 3 deletions(-)

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
@@ -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 = {
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
@@ -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. */

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

[Jens Axboe]

On Tue, Jun 09 2009, Rusty Russell wrote:
> From: john cooper <john.cooper@redhat.com>
> 
> 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!

-- 
Jens Axboe