On Mon, Sep 24, 2018 at 9:24 AM Germano Veit Michel <firstname.lastname@example.org> wrote:
> I'm working an a tool (vdsm-tool update-volume) to make modifying SD metadata easier and more importantly, safer. This is very useful to recover from failed LSMs or snapshot issues.
> The plan is to use the VDSM API (modified by some of these patches) and add a tool (vdsm-tool) that talks to the API and modifies the volumes metadata as required by the user. Currently this is done manually, i.e.: looking at MD_XXX tags, doing dd, sed and then dd back to the storage. Any wrong argument (like a skip in place of a seek) can ruin the entire metadata, so this tool can be quite handy.
> The code is not necessarily 100% finished yet, but I've been testing this for some time and it seems ok from a functional point of view. I'm just not sure everything I did (especially inside VDSM, example 94366) is correct. Your comments on what can/should be improved are very welcome at this point. Please see this series and help reviewing it.
I am not a maintainer of vdsm.storage, but I can say that I'm missing
a high-level description of the change that you are suggesting, and
its motivation. When do you see a need to manually change the metadata
of volumes? Shouldn't we fix the bug that causes this need?
Ohh, sorry. I thought it was obvious. This is for snapshot related issues. These bugs have been present for a while, they are fixed but new ones come up. In the end downstream support is constantly manually repairing chains. This is why this tool is needed. Both to make those changes safer and to save time.
I personally have a deep resentment to a "force" flags - not just
here, but everywhere. It is never clear what is being forced. Some
things cannot or should not be forced.
Nir already suggested to remove the force flag. I don't mind, the idea was just to keep the old behavior the same if the force flag is not used.
One last note: a CI+1 gives a positive psychological vibe to your reviewer.
Yes, I know. Most of them have CI+1. They keep randomly failing on FC28 (but EL7 succeeds) on every push.