From 10bc310c27af1ed358e62351e7ac1d0110c3da27 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 15 Jun 2010 14:43:48 +0200 Subject: [PATCH 1/3] virtio_blk: support barriers without FLUSH feature If we want to support barriers with the cache=writethrough mode in qemu we need to tell the block layer that we only need queue drains to implement a barrier. Follow the model set by SCSI and IDE and assume that there is no volatile write cache if the host doesn't advertize it. While this might imply working barriers on old qemu versions or other hypervisors that actually have a volatile write cache this is only a cosmetic issue - these hypervisors don't guarantee any data integrity with or without this patch, but with the patch we at least provide data ordering. Signed-off-by: Christoph Hellwig Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 258bc2ae2885..2d6191aa5948 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -366,12 +366,32 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) vblk->disk->driverfs_dev = &vdev->dev; index++; - /* If barriers are supported, tell block layer that queue is ordered */ - if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH)) + if (virtio_has_feature(vdev, VIRTIO_BLK_F_FLUSH)) { + /* + * If the FLUSH feature is supported we do have support for + * flushing a volatile write cache on the host. Use that + * to implement write barrier support. + */ blk_queue_ordered(q, QUEUE_ORDERED_DRAIN_FLUSH, virtblk_prepare_flush); - else if (virtio_has_feature(vdev, VIRTIO_BLK_F_BARRIER)) + } else if (virtio_has_feature(vdev, VIRTIO_BLK_F_BARRIER)) { + /* + * If the BARRIER feature is supported the host expects us + * to order request by tags. This implies there is not + * volatile write cache on the host, and that the host + * never re-orders outstanding I/O. This feature is not + * useful for real life scenarious and deprecated. + */ blk_queue_ordered(q, QUEUE_ORDERED_TAG, NULL); + } else { + /* + * If the FLUSH feature is not supported we must assume that + * the host does not perform any kind of volatile write + * caching. We still need to drain the queue to provider + * proper barrier semantics. + */ + blk_queue_ordered(q, QUEUE_ORDERED_DRAIN, NULL); + } /* If disk is read-only in the host, the guest should obey */ if (virtio_has_feature(vdev, VIRTIO_BLK_F_RO)) From a5eb9e4ff18a33e43557d44b205f953b0c1efade Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 23 Jun 2010 22:19:57 -0500 Subject: [PATCH 2/3] virtio_blk: Add 'serial' attribute to virtio-blk devices (v2) Create a new attribute for virtio-blk devices that will fetch the serial number of the block device. This attribute can be used by udev to create disk/by-id symlinks for devices that don't have a UUID (filesystem) associated with them. ATA_IDENTIFY strings are special in that they can be up to 20 chars long and aren't required to be nul-terminated. The buffer is also zero-padded meaning that if the serial is 19 chars or less that we get a nul-terminated string. When copying this value into a string buffer, we must be careful to copy up to the nul (if it present) and only 20 if it is longer and not to attempt to nul terminate; this isn't needed. Changes since v1: - Added BUILD_BUG_ON() for PAGE_SIZE check - Removed min() since BUILD_BUG_ON() handles the check - Replaced serial_sysfs() by copying id directly to buffer Signed-off-by: Ryan Harper Signed-off-by: john cooper Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 2d6191aa5948..7a93b3f68849 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -281,6 +281,27 @@ static int index_to_minor(int index) return index << PART_BITS; } +static ssize_t virtblk_serial_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct gendisk *disk = dev_to_disk(dev); + int err; + + /* sysfs gives us a PAGE_SIZE buffer */ + BUILD_BUG_ON(PAGE_SIZE < VIRTIO_BLK_ID_BYTES); + + buf[VIRTIO_BLK_ID_BYTES] = '\0'; + err = virtblk_get_id(disk, buf); + if (!err) + return strlen(buf); + + if (err == -EIO) /* Unsupported? Make it empty. */ + return 0; + + return err; +} +DEVICE_ATTR(serial, S_IRUGO, virtblk_serial_show, NULL); + static int __devinit virtblk_probe(struct virtio_device *vdev) { struct virtio_blk *vblk; @@ -465,8 +486,15 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) add_disk(vblk->disk); + err = device_create_file(disk_to_dev(vblk->disk), &dev_attr_serial); + if (err) + goto out_del_disk; + return 0; +out_del_disk: + del_gendisk(vblk->disk); + blk_cleanup_queue(vblk->disk->queue); out_put_disk: put_disk(vblk->disk); out_mempool: From 6c99a8528f9a81dda6bbbf83854b8475ce79c745 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 23 Jun 2010 22:19:58 -0500 Subject: [PATCH 3/3] virtio_blk: Remove VBID ioctl With the availablility of a sysfs device attribute for examining disk serial numbers the ioctl is no longer needed. The user-space changes for this aren't upstream yet so we don't have any users to worry about. Signed-off-by: Ryan Harper Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 7a93b3f68849..23b7c48df843 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -225,16 +225,6 @@ static int virtblk_ioctl(struct block_device *bdev, fmode_t mode, struct gendisk *disk = bdev->bd_disk; struct virtio_blk *vblk = disk->private_data; - if (cmd == 0x56424944) { /* 'VBID' */ - void __user *usr_data = (void __user *)data; - char id_str[VIRTIO_BLK_ID_BYTES]; - int err; - - err = virtblk_get_id(disk, id_str); - if (!err && copy_to_user(usr_data, id_str, VIRTIO_BLK_ID_BYTES)) - err = -EFAULT; - return err; - } /* * Only allow the generic SCSI ioctls if the host can support it. */