Re: [Qemu-devel] [PATCH 0/4] Add virtio disk identification support

[john cooper <john.cooper@redhat.com>]

john cooper wrote:
> I'm all for putting this issue to rest, but if we're
> going to live with an ioctl interface retrieving the
> id string, let's make it a little more friendly from
> the user's perspective.

The qemu side of the patch is ok as-is.  The guest-user
interface issue is contained in the driver.  While I
see the example ioctl patch has been incorporated into
the virtio_blk driver, there can be no data retrieved
through this interface as virtblk_get_id() will fail
without the qemu counterpart.  So we can clean up the
details without concern of existing usage.

The only difference (as above) is allowing the caller
to pass a buffer size to the driver and the driver
informing the caller of the total number of bytes
available:

#include <stdio.h>
#include <strings.h>
#include <fcntl.h>

#define IOCTL_CMD       'VBID'
#define BUFSZ           10

main()
{
        int fd, rv;
        char buf[255];

        bzero(buf, sizeof (buf));
        buf[0] = BUFSZ;
        if ((fd = open("/dev/vda", O_RDONLY)) < 0)
                perror("open");
        else if ((rv = ioctl(fd, IOCTL_CMD, buf)) < 0)
                perror("ioctl");
        else
                printf("[%s] %d of %d bytes\n", buf,
                       BUFSZ < rv ? BUFSZ : rv, rv);
}


Signed-off-by: john cooper <john.cooper@redhat.com>
---

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 83fa09a..6237732 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -225,15 +225,29 @@ static int virtblk_ioctl(struct block_device *bdev, fmode_t mode,
 	struct gendisk *disk = bdev->bd_disk;
 	struct virtio_blk *vblk = disk->private_data;
 
+	/* user passes the address of a char[] for return of the id string
+	 * and has set char[0] to the array size.  copy id string to this
+	 * char[] and return the number of non-nul characters in the internal
+	 * id string.  The caller can then determine if all were received.
+	 */
 	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;
+		unsigned char idlen;
+		int rv;
+
+		if (copy_from_user(&idlen, usr_data, sizeof (idlen)))
+			return -EFAULT;
+		if (VIRTIO_BLK_ID_BYTES < idlen)
+			idlen = VIRTIO_BLK_ID_BYTES;
+		if ((rv = virtblk_get_id(disk, id_str)))
+			return rv;
+		if (copy_to_user(usr_data, id_str, idlen))
+			return -EFAULT;
+		for (rv = 0; rv < VIRTIO_BLK_ID_BYTES; ++rv)
+			if (!id_str[rv])
+				break;
+		return rv;
 	}
 	/*
 	 * Only allow the generic SCSI ioctls if the host can support it.

-- 
john.cooper@redhat.com