Re: [lvm-devel] [PATCH] add kobject to struct mapped_device

[Greg KH]

On Fri, Dec 20, 2002 at 09:44:47AM +0000, Joe Thornber wrote:
> On Fri, Dec 20, 2002 at 12:31:50AM -0800, Greg KH wrote:
> > Look reasonable?
> 
> Yes, it looks promising.  Some worries:
> 
> i) The 'status' and 'table' files do not contain a single value.
> Splitting it up into single values would be ungainly to say the least,
> eg.
> 
>   dm
>   |-- table
>       |-- target1
>       |   |-- sector_start
>       |   |-- sector_len
>       |   |-- target_type
>       |   `-- target args
>       |
>       |-- target2
>           |-- sector_start
> 
>   hmm ... maybe that's not too bad.

Yeah, it's not that bad, I'll look into it when I get to those files :)

> ii) If the table files are not split up then we have the problem that
>     they can be larger than a single page, which sysfs can't handle
>     (is this still true ?).

Yes, I think this is still true.  And it's also a good way to enforce
the "one value per file" rule :)

> iii) We need to be able to poll on the status file so that people can
>      block until there is a change of status.  eg, a snapshot uses up
>      another 5% of its COW storage, a mirror completes its initial
>      build, a path fails in the multipath target.

I saw that logic.  I don't think sysfs can handle poll right now, but we
could modify the last access/modify/change time when the status changes
so stat(2) can be used on the file.  This is how usbfs currently
notifies userspace of changes to their files.  Would this be acceptable?

> > Ok.  I can place all of the sysfs specific functions in dm.c, just like
> > drivers/block/genhd.c has, or if we place struct mapped_device into
> > dm.h, they can live in their own file.  Doesn't bother me either way.
> 
> Either put it in dm.c, or define some extra access functions (like
> dm_suspended() and dm_kdev()) to get the information you need.  I
> would prefer the latter, but we can always move things later.

Ok, will do.

thanks for your comments,

greg k-h