[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V
From: |
Christoph Hellwig |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3 |
Date: |
Wed, 27 May 2009 09:49:19 +0200 |
User-agent: |
Mutt/1.3.28i |
You should probably include rusty as he's collecting the patches
for the virtio guest drivers.
Also can you send the patch inline next time? That makes quoting it for
review a lot easier.
drivers/block/virtio_blk.c | 32 +++++++++++++++++++++++++++++---
include/linux/virtio_blk.h | 6 ++++++
2 files changed, 35 insertions(+), 3 deletions(-)
=================================================================
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -146,12 +146,37 @@ static void do_virtblk_request(struct re
vblk->vq->vq_ops->kick(vblk->vq);
}
+/* return serial# in IDE identify data (all other fields are currently zero)
+ */
+static int virtblk_identity(struct block_device *bdev, void *buf)
+{
+ struct virtio_blk *vblk = bdev->bd_disk->private_data;
+ u16 *id;
+ int rv;
+
+ if (!(id = kzalloc(ATA_ID_WORDS, GFP_KERNEL)))
+ rv = -ENOMEM;
+ else if ((rv = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_SN,
+ offsetof(struct virtio_blk_config, serial), &id[ATA_ID_SERNO],
+ ATA_ID_SERNO_LEN)))
+ ;
+ else if (copy_to_user(buf, id, ATA_ID_WORDS))
+ rv = -EFAULT;
+ else
+ rv = 0;
+ if (id)
+ kfree(id);
+ return (rv);
+}
+
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);
+ if (cmd == HDIO_GET_IDENTITY)
+ return (virtblk_identity(bdev, (void __user *)data));
+ else
+ return scsi_cmd_ioctl(bdev->bd_disk->queue, bdev->bd_disk,
+ mode, cmd, (void __user *)data);
}
This looks functionally correct, but pretty far from normal kernel coding
style. I'd envision something like this instead:
/*
* IDE-compatible identify ioctl.
*
* Currenlyt only returns the serial number and leaves all other fields
* zero.
*/
static int virtblk_identity(struct gendisk *disk, void *argp)
{
struct virtio_blk *vblk = disk->private_data;
u16 *id;
int error = -ENOMEM;
id = kzalloc(ATA_ID_WORDS, GFP_KERNEL)
if (!id)
goto out;
error = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_SN,
offsetof(struct virtio_blk_config, serial),
&id[ATA_ID_SERNO], ATA_ID_SERNO_LEN);
if (error)
goto out_kfree;
if (copy_to_user(argp, id, ATA_ID_WORDS))
error = -EFAULT;
out_kfree:
kfree(id);
out:
return error;
}
static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd, unsigned long arg)
{
struct gendisk *disk = bdev->bd_disk;
void __user *argp = (void __user *arg);
switch (cmd) {
case HDIO_GET_IDENTITY:
return virtblk_identity(disk, argp);
default:
return scsi_cmd_ioctl(disk->queue, disk, mode, cmd, argp);
}
}