Ryan Harper wrote: > * john cooper <john.cooper@xxxxxxxxxx> [2010-06-21 01:11]: >> Rusty Russell wrote: >>> On Sat, 19 Jun 2010 04:08:02 am Ryan Harper wrote: >>>> 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 NULL-terminated. The buffer is also zero-padded >>>> meaning that if the serial is 19 chars or less that we get a NULL terminated >>>> string. When copying this value into a string buffer, we must be careful to >>>> copy up to the NULL (if it present) and only 20 if it is longer and not to >>>> attempt to NULL terminate; this isn't needed. >>>> >>>> Signed-off-by: Ryan Harper <ryanh@xxxxxxxxxx> >>>> Signed-off-by: john cooper <john.cooper@xxxxxxxxxx> >>>> --- >>>> drivers/block/virtio_blk.c | 32 ++++++++++++++++++++++++++++++++ >>>> 1 files changed, 32 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c >>>> index 258bc2a..f1ef26f 100644 >>>> --- a/drivers/block/virtio_blk.c >>>> +++ b/drivers/block/virtio_blk.c >>>> @@ -281,6 +281,31 @@ static int index_to_minor(int index) >>>> return index << PART_BITS; >>>> } >>>> >>>> +/* Copy serial number from *s to *d. Copy operation terminates on either >>>> + * encountering a nul in *s or after n bytes have been copied, whichever >>>> + * occurs first. *d is not forcibly nul terminated. Return # of bytes copied. >>>> + */ >>>> +static inline int serial_sysfs(char *d, char *s, int n) >>>> +{ >>>> + char *di = d; >>>> + >>>> + while (*s && n--) >>>> + *d++ = *s++; >>>> + return d - di; >>>> +} >>>> + >>>> +static ssize_t virtblk_serial_show(struct device *dev, >>>> + struct device_attribute *attr, char *buf) >>>> +{ >>>> + struct gendisk *disk = dev_to_disk(dev); >>>> + char id_str[VIRTIO_BLK_ID_BYTES]; >>>> + >>>> + if (IS_ERR(virtblk_get_id(disk, id_str))) >>>> + return 0; >>> 0? Really? That doesn't seem very informative. >> Propagating a prospective error from virtblk_get_id() should >> be possible. Unsure if doing so is more useful from the >> user's perspective compared to just a nul id string. > > I'm not sure we can do any thing else here; maybe printk a warning? > > Documentation/filesystems/sysfs.txt says that showing attributes should > always return the number of chars put into the buffer; so when there is > an error; zero is the right value to return since we're not filling the > buffer. So we return a nul string in the case the qemu user didn't specify an id string and also in the case a legacy qemu doesn't support retrieval of an id string. Not too much difference and if needed going forward the error return can be elaborated. >>> /* id_str is not necessarily nul-terminated! */ >>> buf[VIRTIO_BLK_ID_BYTES] = '\0'; >>> return virtblk_get_id(disk, buf); >> The /sys file is rendered according to the length >> returned from this function and the trailing nul >> is not interpreted in this context. In fact if a >> nul is added and included in the byte count of the >> string it will appear in the /sys file. > > Yeah; I like the simplicity; but we do need to know how long the string > is so we can return that value. Which we're getting from serial_sysfs() without having to accommodate an unused nul. I'd hazard the primary reason the sysfs calling code keys off a return of byte count vs. traversing the string itself is due to the called function almost always having the byte count available. -john -- john.cooper@xxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html