On Tue, 22 Jun 2010 02:13:21 am 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.
Ideally, the file shouldn't be set up if we don't have an ID. But we never
did add a feature bit for this :(
At a glance, we'll get -EIO if the host doesn't support it (or any other
transport error). -ENOMEM if we run out of memory.
printk is dumb, but it's nice to differentiate "host didn't supply one" vs
"something went wrong". How about return 0 on -EIO? Whatever is easiest
for udev is best here.
> > > /* 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.
So we're looking at something like:
/* id_str is not necessarily nul-terminated! */
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;
Then, please *test*!
Thanks,
Rusty.
--
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
