[IMP] In some case, we might want to keep some lvm active

[cvinh]

No description provided.

[aesteve-rh]

Thanks! The patch looks generally fine (minus a few comments), but there are at least a couple missing points:

• Please add a some text/explanation in the body of the commit.
• Add a test (potentially at lmv_test.py) covering the new config option and new the deactivate unused LVs logic.

[aesteve-rh]

Wouldn't doing something like this achieve the same thing if we use lv names instead of full paths?

def deactivateUnusedLVs(vgname, skiplvs=KEEPACTIVE):

I mean, technically, skipping LVs is already part of the API.

[cvinh]

Humm, we have the same lv name in different vg, that's why we prefer using complete path
ie : /dev/vg1/lv1, /dev/vg2/lv1

[aesteve-rh]

Certainly skiplvs=KEEPACTIVE was not the best example here, as it mixes different vgs.

But my point was that there is already a logic for skiplvs in place, so I think it would be nice to consolidate the list before we get to the if lv.name in skiplvs: condition, filtering all lvs that pertain to vgname.

Something like (not tested):

skiplvs = skiplvs + tuple(lvm.split('/')[-1] for lvm in KEEPACTIVE if vgname == lvm.split('/')[-2])

In this solution maybe worth using different than the complete path to ease the parsing.

Or even convert the KEEPACTIVE list into a dictionary, where the VG is the key and the value is a list of lvs that we can directly append to skiplvs based on vgname.

[cvinh]

• Add a test (potentially at lmv_test.py) covering the new config option and new the deactivate unused LVs logic.

I'm sorry, I don't know how to add Tests, I would make some bad copy-paste ... 👎

[cvinh]

Can you please point me out where I could add the information in the documentation ?

[aesteve-rh]

I'm sorry, I don't know how to add Tests, I would make some bad copy-paste ... 👎

It's ok, start with something simple, and I can help you refine it. I think something similar to test_bootstrap would do. Just make sure you monkeypatch the KEEPACTIVE list.

You can see how to run the tests here:
https://github.com/oVirt/vdsm/blob/master/doc/development.md#running-the-tests

$ tox -e storage

Check pytest documentation if you are unfamiliar: https://docs.pytest.org/en/8.2.x/getting-started.html
You can also check for other tests within the project.