Re: [Nbd] [PATCH] nbd: nbd sysfs framework

[Paul Clements <paul.clements@...856...>]

On Wed, Aug 3, 2011 at 7:34 PM, Greg KH <greg@...926...> wrote:
> On Wed, Aug 03, 2011 at 07:15:51PM -0400, Paul Clements wrote:
>> Description: The patch creates a new sysfs entry framework for nbd.
>
> Why?  What does this buy us except an increase in code size for no added
> benifit?  You seem to be stripping out the driver layer here and using
> "raw" kobjects, why?

Not sure what I was thinking...just in a hurry, I think.

Is the following a little better?

Thanks for your feedback.

--
Paul

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index e6fc716..a1e7615 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -383,39 +383,17 @@ harderror:
 	return NULL;
 }
 
-static ssize_t pid_show(struct device *dev,
-			struct device_attribute *attr, char *buf)
-{
-	struct gendisk *disk = dev_to_disk(dev);
-
-	return sprintf(buf, "%ld\n",
-		(long) ((struct nbd_device *)disk->private_data)->pid);
-}
-
-static struct device_attribute pid_attr = {
-	.attr = { .name = "pid", .mode = S_IRUGO},
-	.show = pid_show,
-};
-
 static int nbd_do_it(struct nbd_device *lo)
 {
 	struct request *req;
-	int ret;
 
 	BUG_ON(lo->magic != LO_MAGIC);
 
 	lo->pid = current->pid;
-	ret = sysfs_create_file(&disk_to_dev(lo->disk)->kobj, &pid_attr.attr);
-	if (ret) {
-		printk(KERN_ERR "nbd: sysfs_create_file failed!");
-		lo->pid = 0;
-		return ret;
-	}
 
 	while ((req = nbd_read_stat(lo)) != NULL)
 		nbd_end_request(req);
 
-	sysfs_remove_file(&disk_to_dev(lo->disk)->kobj, &pid_attr.attr);
 	lo->pid = 0;
 	return 0;
 }
@@ -735,6 +713,75 @@ static const struct block_device_operations nbd_fops =
  *  (Just smiley confuses emacs :-)
  */
 
+struct nbd_sysfs_entry {
+	struct device_attribute dev_attr;
+	ssize_t (*show)(struct nbd_device *, char *);
+	ssize_t (*store)(struct nbd_device *, const char *, size_t);
+};
+
+static ssize_t pid_show(struct nbd_device *lo, char *page)
+{
+	return sprintf(page, "%ld\n", (long)lo->pid);
+}
+
+#define NBD_ATTR(_var, _name, _perm, _show, _store) \
+	static struct nbd_sysfs_entry _var = { \
+	.dev_attr = __ATTR(_name, _perm, nbd_attr_show, nbd_attr_store), \
+	.show = _show, \
+	.store = _store \
+}
+
+// static struct nbd_sysfs_entry nbd_pid =
+// __ATTR(pid, S_IRUGO, pid_show, NULL);
+
+static ssize_t
+nbd_attr_show(struct device *dev, struct device_attribute *attr, char *page)
+{
+	struct nbd_sysfs_entry *entry = container_of(attr,
+			struct nbd_sysfs_entry, dev_attr);
+	struct nbd_device *lo =
+			(struct nbd_device *)dev_to_disk(dev)->private_data;
+	ssize_t rv;
+
+	if (!entry->show || !lo)
+		return -EIO;
+	rv = entry->show(lo, page);
+	return rv;
+}
+
+static ssize_t
+nbd_attr_store(struct device *dev, struct device_attribute *attr,
+	      const char *page, size_t length)
+{
+	struct nbd_sysfs_entry *entry = container_of(attr,
+			struct nbd_sysfs_entry, dev_attr);
+	struct nbd_device *lo =
+			(struct nbd_device *)dev_to_disk(dev)->private_data;
+	ssize_t rv;
+
+	if (!entry->store)
+		return -EIO;
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+	mutex_lock(&lo->tx_lock);
+	rv = entry->store(lo, page, length);
+	mutex_unlock(&lo->tx_lock);
+
+	return rv;
+}
+
+NBD_ATTR(nbd_pid, pid, S_IRUGO, pid_show, NULL);
+
+static struct attribute *nbd_attrs[] = {
+	&nbd_pid.dev_attr.attr,
+	NULL,
+};
+
+static struct attribute_group nbd_attr_group = {
+	.name = "nbd",
+	.attrs = nbd_attrs,
+};
+
 static int __init nbd_init(void)
 {
 	int err = -ENOMEM;
@@ -786,6 +833,7 @@ static int __init nbd_init(void)
 	dprintk(DBG_INIT, "nbd: debugflags=0x%x\n", debugflags);
 
 	for (i = 0; i < nbds_max; i++) {
+		int error;
 		struct gendisk *disk = nbd_dev[i].disk;
 		nbd_dev[i].file = NULL;
 		nbd_dev[i].magic = LO_MAGIC;
@@ -805,6 +853,8 @@ static int __init nbd_init(void)
 		sprintf(disk->disk_name, "nbd%d", i);
 		set_capacity(disk, 0);
 		add_disk(disk);
+		error = sysfs_create_group(&disk_to_dev(disk)->kobj,
+				&nbd_attr_group);
 	}
 
 	return 0;
@@ -824,6 +874,8 @@ static void __exit nbd_cleanup(void)
 		struct gendisk *disk = nbd_dev[i].disk;
 		nbd_dev[i].magic = 0;
 		if (disk) {
+			sysfs_remove_group(&disk_to_dev(disk)->kobj,
+					&nbd_attr_group);
 			del_gendisk(disk);
 			blk_cleanup_queue(disk->queue);
 			put_disk(disk);