[lvm-devel] [PATCH 4/7] Introduce vg_add_lc and vg_remove_lv functions.

[Milan Broz]

Petr Rockai wrote:
> Btw., would vg_link_lv and vg_unlink_lv describe better what these two
> functions do? It seems a little easy to confuse _add_lv and vg_add_lv, while
> the two do quite a different thing.
yes, renamed

>> @@ -331,7 +330,7 @@ int import_lv(struct dm_pool *mem, struct logical_volume *lv, struct lv_disk *lv
>>  		lv->alloc = ALLOC_NORMAL;
>>  
>>  	if (!lvd->lv_read_ahead)
>> -		lv->read_ahead = lv->vg->cmd->default_settings.read_ahead;
>> +		lv->read_ahead = cmd->default_settings.read_ahead;
>>  	else
>>  		lv->read_ahead = lvd->lv_read_ahead;
>>  
> I'm not sure why is this change needed? (Adding cmd_context parameter to
> import_lv.)

just code shuffle, because lv->vg is initialized later in vg_add_lv and we
need cmd for default settings.
(probably explains some code shuffle later too)

> Does not change what the code does, just shuffles the lines somewhat. It does
> change semantics of import_lv, but that function is never used outside _add_lv
> anyway (would it make sense to make it static _import_lv, in a separate
> patch?).
ok, I'll move it to separate patch

>> +		//FIXME: cow still in VG, fix count, remove this later
>> +		if (lv->status & SNAPSHOT)
>> +			lv->vg->lv_count++;
> ^^ This is indeed ugly (see also my previous mail: it might be worth getting
> rid of lv_count altogether) ... although this patch is a definite improvement,
> it is still suboptimal in this respect.

later patch removes that, I keep it here just that every separate patch can pass
full testing.

Milan