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

Greg KH
2002-12-20T17:44:51Z

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