Re: [PATCH] rbd: replace the rbd sysfs interface

[Greg KH <greg@kroah.com>]

On Fri, Nov 19, 2010 at 12:42:51PM -0800, Yehuda Sadeh Weinraub wrote:
> On Thu, Nov 18, 2010 at 6:08 PM, Greg KH <greg@kroah.com> wrote:
> > On Thu, Nov 18, 2010 at 02:53:35PM -0800, Yehuda Sadeh Weinraub wrote:
> >> Yes, pretty much. One problem that I do see is that if we define the
> >> snaps/ as a device (and not just as a kobj) as you suggested before,
> >> it'll automatically create a 'uevent' entry under it which can be a
> >> real issue in the case we have a snapshot named like that. Shouldn't
> >> we just create it as a kobj in that case?
> >
> > No.  Just use the subdirectory option of an attribute group to handle
> > that and you will not need to create any device or kobject with that
> > name, the driver core will handle it all automatically for you.
> >
> 
> One issue with using the groups name, is that it's not nested (unless
> I'm missing something), so we can't have it done for the entire
> planned hierarchy without holding a kobject on the way. Just a
> reminder, the device-specific hierarchy would look like this:
> 
> 1. /sys/bus/rbd/devices/<id>/
> 2. /sys/bus/rbd/devices/<id>/<device_attrs>
> 3. /sys/bus/rbd/devices/<id>/snaps/
> 4. /sys/bus/rbd/devices/<id>/snaps/<snap_name>/
> 5. /sys/bus/rbd/devices/<id>/snaps/<snap_name>/<snap_attrs>
> 
> One solution would be to create kobjects for (3) and for (4), without
> using a group name.

Ick, no.

> Another way, we can create groups for (2), and (3)
> under (1), but that's about it,

attribute group for 2 is fine.

> you can't create the snap specific directory this way without
> resorting to some internal sysfs directory creation, which will be
> horribly wrong. At that point we don't have anything for 'snaps', and
> we don't really need to do any operations under that directory, we
> just need it to exist so that it contains the snapshot-specific
> directories.

But you need to do something with those snapshots, right?  So, why even
have "snaps" be a subdir?  Why not just make <snap_name> a struct device
with <id> being the parent, and it living on the same bus_type, but
being a different device_type (like partitions and block devices are), 

> Another way would be to create a group for (2) under (1) and create a
> kobject for (3), for which you can create group per snapshot.
> 
> Am I missing something? We already have the first solution (kobjects
> only) implemented, is there some real benefit for using the third
> method? We'll have to manually add remove groups anyway, as snapshots
> can be removed and new snapshots can be added.

Never add kobjects to a struct device, that is showing you that
something is wrong, and that userspace really will want to get that
create/destroy event of the sub child.

sorry for the delay, was gone last weekend.

greg k-h