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

2009-06-03 Thread Jens Axboe 
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);
 

-- 
Jens Axboe

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2009-06-03 Thread john cooper 
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);
 


-- 
john.coo...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2009-06-01 Thread Rusty Russell 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2009-05-31 Thread Christoph Hellwig 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2009-05-28 Thread john cooper 
virtio_blk-serial-4.patch



 drivers/block/virtio_blk.c |   41 ++---
 include/linux/virtio_blk.h |7 +++
 2 files changed, 45 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;
+   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_RO5   /* 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. */
-- 
john.coo...@redhat.com



--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html