From patchwork Wed May 27 07:49:19 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 26392 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n4R7s0Lc011764 for ; Wed, 27 May 2009 07:54:00 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761559AbZE0Htd (ORCPT ); Wed, 27 May 2009 03:49:33 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1761178AbZE0Htc (ORCPT ); Wed, 27 May 2009 03:49:32 -0400 Received: from verein.lst.de ([213.95.11.210]:55697 "EHLO verein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760661AbZE0Hta (ORCPT ); Wed, 27 May 2009 03:49:30 -0400 Received: from verein.lst.de (localhost [127.0.0.1]) by verein.lst.de (8.12.3/8.12.3/Debian-7.1) with ESMTP id n4R7nJIF007754 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO); Wed, 27 May 2009 09:49:19 +0200 Received: (from hch@localhost) by verein.lst.de (8.12.3/8.12.3/Debian-6.6) id n4R7nJOw007752; Wed, 27 May 2009 09:49:19 +0200 Date: Wed, 27 May 2009 09:49:19 +0200 From: Christoph Hellwig To: john cooper Cc: KVM list , qemu-devel@nongnu.org, rusty@rustcorp.com.au Subject: Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3 Message-ID: <20090527074919.GB7356@lst.de> References: <4A1C88DE.6050608@redhat.com> Mime-Version: 1.0 Content-Disposition: inline In-Reply-To: <4A1C88DE.6050608@redhat.com> User-Agent: Mutt/1.3.28i X-Spam-Score: -0.001 () BAYES_44 X-Scanned-By: MIMEDefang 2.39 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org 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(-) 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); } } --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ================================================================= --- 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); }