[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [lvm-devel] [PATCH 2/11] Add upward link from underlying LV to stacked seg

Alasdair G Kergon wrote:
> On Tue, Jan 15, 2008 at 11:39:13AM -0500, Jun'ichi Nomura wrote:
>>> +struct lv_segment *seg_using_lv(struct logical_volume *lv)
>> Returning NULL is valid if the list is empty.
> What I mean is: why would the list be empty if that function
> is being called?  If the list was known to be empty, you
> wouldn't be in a code path that called the function, surely?
> IOW it's a coding error if 'return NULL' is hit.

Ah, I understand now.
If it returns NULL, there should be internal error in LVM2 code.

> Or does the function have a secondary use to find out whether
> the LV is at the top of the stack?
> Is it better split into two?  First an is_ function to find out
> if the segment is the type expected with a single segment; then the function to
> give details of the one-and-only segment?

I don't think we need to split it.
If a code calls seg_using_lv(lv), it knows the lv should have
one-and-only user (upper-level) segment.
If the lv doesn't, it's a coding error.

Maybe it's handy to have a wrapper for !list_empty(&lv->segs_using_this_lv)
to check if the LV is top-level or not.
  int is_top_of_the_stack(lv) {
    return list_empty(lv->segs_using_this_lv);

>>> Where is this new list validated?
>> Some validation were included in the tests using find_mirror_seg().
>> I'll add generic ones.
> I mean the validation in the vg_write() path must have independent
> validity checks so we never write out the metadata if there's
> an internal inconsistency in it.  IOW it must be done somewhere under
> vg_validate().  IOW the pointers must be validated in both directions:
> all existing list elements are correct (and not duplicated), and no
> elements are missing from the list.

I'll add that type of validation check.

Duplication is currently allowed since 2 areas in a segment
can be on the same LV.

Jun'ichi Nomura, NEC Corporation of America

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]