On Wed, Sep 2, 2020 at 2:36 PM Germano Veit Michel <[email protected]> wrote:
> > > On Tue, Sep 1, 2020 at 5:39 PM Nir Soffer <[email protected]> wrote: > >> On Tue, Sep 1, 2020 at 2:27 AM Germano Veit Michel <[email protected]> >> wrote: >> > >> > >> > >> > On Mon, Aug 31, 2020 at 5:00 PM Nir Soffer <[email protected]> wrote: >> >> >> >> On Mon, Aug 31, 2020 at 4:48 AM Germano Veit Michel < >> [email protected]> wrote: >> >> > >> >> > >> >> > >> >> > On Sun, Aug 30, 2020 at 8:46 PM Nir Soffer <[email protected]> >> wrote: >> >> >> >> >> >> >> >> >> >> >> >> On Fri, Aug 28, 2020, 08:36 Germano Veit Michel <[email protected]> >> wrote: >> >> >>> >> >> >>> >> >> >>> >> >> >>> On Fri, Aug 28, 2020 at 9:29 AM Nir Soffer <[email protected]> >> wrote: >> >> >>>> >> >> >>>> >> >> >>>> >> >> >>>> On Thu, Aug 27, 2020, 16:38 Tal Nisan <[email protected]> wrote: >> >> >>>>> >> >> >>>>> >> >> >>>>> >> >> >>>>> On Fri, Aug 21, 2020 at 4:34 AM Germano Veit Michel < >> [email protected]> wrote: >> >> >>>>>> >> >> >>>>>> Hi, >> >> >>>>>> >> >> >>>>>> Is there a reliable way to figure out if a snapshot is in >> preview only using information obtained from the storage domain metadata? >> >> >>>>>> I'm trying to find a way to distinguish a problematic snapshot >> chain (double parent) from a snapshot in preview in order to improve >> dump-volume chains. >> >> >>>>>> >> >> >>>>>> Currently dump-volume-chains throws an error >> (DuplicateParentError) if a snapshot is in preview for the image, as there >> is a 'Y' shape split in the chain >> >> >>>>>> with 2 volumes (previous chain + preview) pointing to a common >> parent: >> >> >>>>>> >> >> >>>>>> image: dff0a0c0-b731-4e5b-9f32-d97310ca40de >> >> >>>>>> >> >> >>>>>> Error: more than one volume pointing to the same >> parent volume e.g: (_BLANK_UUID<-a), (a<-b), (a<-c) >> >> >>>>>> >> >> >>>>>> Unordered volumes and children: >> >> >>>>>> >> >> >>>>>> - e6c7bec0-53c6-4729-a4a0-a9b3ef2b8c38 <- >> 5eb2b29d-82d6-4337-8511-3c86705d566e >> >> >>>>>> status: OK, voltype: LEAF, format: COW, >> legality: LEGAL, type: SPARSE, capacity: 1073741824, truesize: 1073741824 >> >> >>>>>> >> >> >>>>>> - e0475853-4514-4464-99e7-b185cce9b67d <- >> deceff83-9d88-4f87-8304-d5bf74d119b1 >> >> >>>>>> status: OK, voltype: LEAF, format: COW, >> legality: LEGAL, type: SPARSE, capacity: 1073741824, truesize: 1073741824 >> >> >>>>>> >> >> >>>>>> - e6c7bec0-53c6-4729-a4a0-a9b3ef2b8c38 <- >> e0475853-4514-4464-99e7-b185cce9b67d >> >> >>>>>> status: OK, voltype: INTERNAL, format: COW, >> legality: LEGAL, type: SPARSE, capacity: 1073741824, truesize: 1073741824 >> >> >>>>>> >> >> >>>>>> - 00000000-0000-0000-0000-000000000000 <- >> e6c7bec0-53c6-4729-a4a0-a9b3ef2b8c38 >> >> >>>>>> status: OK, voltype: INTERNAL, format: RAW, >> legality: LEGAL, type: PREALLOCATED, capacity: 1073741824, truesize: >> 1073741824 >> >> >>>>>> >> >> >>>>>> From the engine side it's easy, but I'd need to solve this >> problem using only metadata from the storage. >> >> >>>>>> >> >> >>>>>> The only thing I could think of is that one of the volumes >> pointing to the common parent has voltype LEAF. Any better ideas? >> >> >>>>> >> >> >>>>> don't think that there is any, Engine is the orchestrator and >> due to that the info is only in the database >> >> >>>> >> >> >>>> >> >> >>>> There is no good way, but you can look at the length of the >> chain, and the "ctime" value. >> >> >>>> >> >> >>>> For example if this was the original chain: >> >> >>>> >> >> >>>> a <- b <- c >> >> >>>> >> >> >>>> if we preview a: >> >> >>>> >> >> >>>> a <- b <- c >> >> >>>> a <- d >> >> >>>> >> >> >>>> You know that d is a preview volume. >> >> >>>> >> >> >>>> If we preview b, we will have two chains of same length: >> >> >>>> >> >> >>>> a <- b <- c >> >> >>>> a <- b <- d >> >> >>>> >> >> >>>> But the ctime value of d will be larger, since preview is created >> after >> >> >>>> the leaf was created. >> >> >>>> >> >> >>>> ctime is using time.time() so it is not affected by time zone >> changes >> >> >>>> but it may be wrong due to host time changes, so it is not >> reliable. >> >> >>>> >> >> >>>> Can you open a bug for this? >> >> >>> >> >> >>> https://bugzilla.redhat.com/show_bug.cgi?id=1873382 >> >> >>> >> >> >>> I have a prototype working with some code I pasted in the >> bugzilla, but I don't think it's reliable and an overcomplication of what >> should be simple. >> >> >> >> >> >> >> >> >> I don't think the code in the bug is the way to handle this. >> >> >> >> >> >> It will be simpler and more useful to: >> >> >> 1. Find leaves >> >> >> 2. Follow the chain from each leaf, until the base (volume with no >> parent). >> >> >> 3. Display a tree instead of list, like lsblk. >> >> >> >> >> >> For example: >> >> >> >> >> >> dbf1e90c-41d5-4c2d-a8d2-15f2f04d3561 >> >> >> ├─ea6af566-922c-4ca2-af17-67f7cd08826c >> >> >> └─aa5643ef-8c74-4b28-91e0-8d45d6ee426b >> >> >> └─30c4f6d1-7f1d-470b-96ae-7594cf367dfa >> >> > >> >> > I like the idea of this visual representation, but it does not fix >> the problem. >> >> > >> >> > The problem is dump-volume-chains throwing incorrect errors in case >> there is a snapshot in preview. >> >> > >> >> > Error: more than one volume pointing to the same parent volume e.g: >> (_BLANK_UUID<-a), (a<-b), (a<-c) >> >> >> >> This error is wrong, you should remove it, and instead show the tree. >> >> >> >> > There is still a double parent on the representation above. So if >> the analysis is done (text output), there will >> >> > be an error detected no matter how we print it. If there is no way >> to distinguish a preview from a double parent >> >> > problem without leaving any doubt based only on storage metadata >> only then we can improve the >> >> > representation but ultimately the problem remains there. >> >> > >> >> > Ideally I'd like to keep DoubleParentError logic and detect Previews >> to eliminate the false errors. >> >> >> >> This is not possible now. >> >> >> >> > The analysis should be done in the image discrepancy tool on the >> engine, which has dump-volume-chains >> >> > output (json - no analysis) and the engine db. And we are already >> doing some basic checks there. Maybe >> >> > we should even move the entire analysis logic there and make >> dump-volume-chains just print and dump >> >> > data without doing analysis if the analysis cannot be done based on >> partial data. >> >> > >> >> > The main idea here was to simply stop false errors for those who >> look for them in dump-volume-chains >> >> > text output. >> >> > >> >> >> >> >> >> >> >> >> Users of the tool will have to check engine db to understand how to >> fix the disk. >> >> >> >> >> >> Even if it was easy to detect a volume in preview, how do you know >> which chain >> >> >> should be kept? Did it fail just after the user asked to commit the >> preview? >> >> > >> >> > >> >> > This tool is not used to diagnose and correct issues on its own. It >> is used for 2 things, but mainly the first: >> >> > a) Nice readable way to see volumes and their metadata, plus chain >> >> > b) Any obvious errors >> >> > >> >> > The duplicate parent is printing false problems during preview, >> breaking the tool for B. >> >> > >> >> > The main use is still A, use of dump-volume-chains is to stop >> collecting /dev/VG/metadata LV or *.meta files >> >> > and have this info for the volumes in the sosreport. >> >> > >> >> > I'm not aware of anyone using just the output of the tool to perform >> chain changes, every failure >> >> > also requires checking the DB too and most importantly the logs >> (unless rotated). >> >> > >> >> >> >> >> >> Storage format does not have a way to store info about the state of >> the disk, or make atomic >> >> >> changes like remove one chain when committing after a preview. This >> is also the reason we >> >> >> have trouble with removing snapshots. >> >> > >> >> > >> >> > Which means we cannot know for sure what is happening in the chain, >> right? >> >> > With this in mind, any suggestion to stop the false errors? >> >> >> >> Change the code to handle a tree instead of a list of volumes, error >> is gone. >> > >> > But then part of the validation is gone too. We open the possibility of >> validating trees, which are all invalid >> > except for the very specific case of a preview, which we have no data >> to determine for sure anyway. >> >> But the validation is incorrect. Trees are actually supported using >> preview, so >> failing and not showing the tree in dump-volume-chain is a bug. >> > Well, the tree is valid only if it is a snapshot preview and we have no > reliable way to determine if its a preview, > which means we cannot validate the tree reliably too. > > We could do the change, but then we swap one bug for another, from saying > all trees are errors to saying > non-preview trees are fine. False positive to false negative, not sure > which is preferable. > > If we can have a way to determine reliably if the chain is in preview, > then I totally agree it's worth > the effort of changing the logic. Otherwise I think it's not worth > changing right now. > > >> >> > There is not much that can be done as there is no reliable way to >> determine if the chain has a snapshot >> > in preview without several changes on engine and vdsm. And it's not >> worth implementing this, I'll close >> > the bug too. >> >> I think it is worth the time, so better leave this open. >> > Sure, no problem. I'll re-open it. > >> >> > Thanks for your help! >> > >> >> >> >> > >> >> > Since we cannot be sure of this based just on SD metadata, maybe the >> simplest is to remove the >> >> > duplicate parent error string and/or add some warning that it could >> be a snapshot in preview and just >> >> > print the unordered volumes. >> >> > >> >> > The improved visual representation could be handled separate from >> this. I've thought of something >> >> > similar in the past but found hard to print the volume metadata in a >> nice way (and we need to handle >> >> > big chains of several dozen snapshots). >> >> > >> >> > Thanks, >> >> > Germano >> >> > >> >> >> >> >> >> >> >> >> Nir >> >> >> >> >> >>> >> >> >>> >> >> >> >>
_______________________________________________ Devel mailing list -- [email protected] To unsubscribe send an email to [email protected] Privacy Statement: https://www.ovirt.org/privacy-policy.html oVirt Code of Conduct: https://www.ovirt.org/community/about/community-guidelines/ List Archives: https://lists.ovirt.org/archives/list/[email protected]/message/TP7NJK4I37SKBR5Z26GYRSHZOFGGU6VA/
