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. 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. 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/EUQIVNKK6B3IX3Z45XPPEMY5BNH55HQA/
