vmware / govmomi

Go library for the VMware vSphere API
Apache License 2.0
2.32k stars 913 forks source link

How to get paired name/size values for each snapshot associated with a VM? #2243

Closed atc0005 closed 3 years ago

atc0005 commented 3 years ago

Here is a PowerCLI snippet (either or):

Get-VM testVM | Get-Snapshot | Measure-Object -Sum SizeMB
Get-Snapshot -VM testVM | Measure-Object -Sum SizeMB

which helped me generate a list of snapshot details like this from a Nagios plugin that I wrote about a year ago:

OK: 4 snapshots found for non-excluded VMs, but configured thresholds not exceeded [WARNING: 30 GB, CRITICAL: 50 GB]

* 'testVM' [Age: 291, SnapSize: 13.861 GB, Combined SnapSize: 28.551 GB, SnapName: '2020-03-29']
* 'testVM' [Age: 406, SnapSize: 6.251 GB, Combined SnapSize: 28.551 GB, SnapName: '2019-12-05']
* 'testVM' [Age: 457, SnapSize: 8.438 GB, Combined SnapSize: 28.551 GB, SnapName: '2019-10-15']
* 'testVM' [Age: 1045, SnapSize: 0.000 GB, Combined SnapSize: 28.551 GB, SnapName: 'Fresh install, activation and patches']

I'm looking to do the same thing with govmomi, but after digging the closest I've found is #1197. I expect I'm overlooking something obvious.

Is the approach here to loop over vm.LayoutEx.File (where vm is a mo.VirtualMachine), filter type to snapshotData and build a mapping of sizes and keys, then loop over vm.Snapshot.RootSnapshotList and pair together the id values here with key values from the other structure?

Thanks in advance for any pointers.

atc0005 commented 3 years ago

I think I found the correlation. I'll work on this over the next few days and will link to the results in case it is helpful to someone else.

atc0005 commented 3 years ago

Scratch notes, since this seem as good a place as any to place them.


Hierarchy of types

Levels above and some below skipped for simplicity and due to my ignorance.

Fields expanded

atc0005 commented 3 years ago

Unfortunately this didn't work. I ended up with the size of the vmsn files associated with the snapshots instead of the snapshots themselves.

atc0005 commented 3 years ago

Downloaded, compiled and ran the VMSnapshot C# project from the vSphere Management SDK and it did not report the size of snapshots, so no go on using it as a guide for an equivalent govmomi approach.

I pulled this from page 155 of vSphere Web Services SDK Programming Guide - VMware vSphere 7.0 (U1):

File Extension Usage File Description
.vmsd vmname.vmsd Virtual machine snapshot file.
.vmsn vmname.vmsn Virtual machine snapshot data file.
**.delta.vmdk Snapshot difference file. A number preceding the extension increases with more snapshots.
**.vmdk Metadata about a snapshot.
-Snapshot#.vmsn Snapshot of virtual machine memory. Snapshot size is equal to the size of your virtual machine's maximum memory.

This is starting to look like I'll need to match all of those file types in order to calculate snapshot size. The challenge will be tying the specific files back to a specific snapshot ID value. Digging further.

I still can't escape the feeling that I'm overlooking something obvious.

dougm commented 3 years ago

Looks like you're right. I tracked down the PowerCLI source and afaict it does what you describe above. You'll need to use DatastoreBrowser to get those file sizes. I think the only example we have of that is the govc datastore.ls command.

atc0005 commented 3 years ago

@dougm: Looks like you're right. I tracked down the PowerCLI source and afaict it does what you describe above.

Thanks for checking and confirming that.

You'll need to use DatastoreBrowser to get those file sizes. I think the only example we have of that is the govc datastore.ls command.

I'll look there, thank you for the tip.

atc0005 commented 3 years ago

I thought that maybe I could use the same approach I stubbed out earlier, but instead of limiting to snapshotData file type, I could use the types mentioned in the vSphere Web Services SDK Programming Guide:

image

Unfortunately it doesn't appear that the **.delta.vmdk extension is present for vSphere 6.7.

I noticed that the snapshot VMDKs had a six digit suffix dot vmdk suffix (e.g., 000001.vmdk), so I thought that maybe that could be a useful pattern to match against. I created a VM with a six digit suffix to see how well that would work.

image

As the first file in the listing shows, knowing that pattern doesn't help that much.

I'll go review the datastore.ls implementation as originally suggested. ;)

atc0005 commented 3 years ago

I'll go review the datastore.ls implementation as originally suggested. ;)

I took a look, but I don't understand how what I'm looking at maps to a file listing from a datastore. Once I get that, I'm not sure how I can reliably filter the list to only relevant files.

Using the fields I listed previously, I believe I understand how vm.LayoutEx.Snapshot, vm.LayoutEx.Disk and vm.LayoutEx.File can be used to gather the desired details, but I don't understand (yet?) how to diff between snapshot states to determine the file size changes. As in, if I'm evaluating the first snapshot in the list, I'm not sure I can reliably determine which are the original disks and which are the snapshot disks (files alone don't seem to be enough).

Field Maps to Description Example
vm.LayoutEx.Disk[].Chain[].FileKey diskDescriptor, diskExtent pairs [3 4], [11 12], [24 25]
vm.LayoutEx.Disk[].Key vm.LayoutEx.Snapshot[].Disk[].Key 2000
vm.LayoutEx.Snapshot[].Disk[].Key vm.LayoutEx.Disk[].Key 2000
vm.LayoutEx.Snapshot[].Disk[].Chain[].FileKey diskDescriptor, diskExtent pairs [3 4], [11 12], [24 25]
vm.LayoutEx.Snapshot[].Key.Value Managed Object Reference snapshot-163887
vm.LayoutEx.Snapshot[].DataKey vm.LayoutEx.File[].Key individual file keys (logs, vmdks, etc) 13, 14, 15, 16, 26, 29
vm.LayoutEx.File[].Key vm.LayoutEx.Snapshot[].DataKey individual file keys (logs, vmdks, etc) 13, 14, 15, 16, 26, 29
vm.LayoutEx.File[].Size file size in bytes (int64) 8.4 GB

Any chance the source for the PowerCLI implementation can be shared? ;) I assume the answer is no, but could a pseudocode version be shared?

dougm commented 3 years ago

Yeah, I can't share it, but looking at it again I realize the code using DatastoreBrowser is for older API versions.. there is a separate method for the modern versions of vSphere. Apologies for the confusion. It does still involve iterating over the LayoutEx structure as you've outlined above. I'll try to parse through it more and put something useful together when I get a chance.

atc0005 commented 3 years ago

Thanks!

dougm commented 3 years ago

Below is the converted C# to Go, hopefully this will be useful :) Looks like it's meant to get the size of a single snapshot. You could refactor to get the size of all the VM's snapshots or walk the snapshot tree and add them up. For an example see the govc snapshot.tree source and https://github.com/vmware/govmomi/blob/add8be5a3c4efaa28d6791ae142b165b7b08c8ec/object/virtual_machine.go#L647-L648

Feel free to take the code and run with it:

func extractDiskLayoutFiles(diskLayoutList []types.VirtualMachineFileLayoutExDiskLayout) []int {
    var result []int

    for _, diskLayout := range diskLayoutList {
        for _, diskUnit := range diskLayout.Chain {
            for i := range diskUnit.FileKey {
                result = append(result, int(diskUnit.FileKey[i]))
            }
        }
    }

    return result
}

func removeKey(l *[]int, key int) {
    for i, k := range *l {
        if k == key {
            *l = append((*l)[:i], (*l)[i+1:]...)
            break
        }
    }
}

func SnapshotSize(info *types.VirtualMachineSnapshotInfo, parent *types.ManagedObjectReference, vmlayout *types.VirtualMachineFileLayoutEx, isCurrent bool) int {
    var fileKeyList, parentFiles, allSnapshotFiles []int

    diskFiles := extractDiskLayoutFiles(vmlayout.Disk)

    for _, layout := range vmlayout.Snapshot {
        diskLayout := extractDiskLayoutFiles(layout.Disk)

        allSnapshotFiles = append(allSnapshotFiles, diskLayout...)

        if layout.Key.Value == info.CurrentSnapshot.Value {
            // The .vmsn file
            fileKeyList = append(fileKeyList, int(layout.DataKey))
            // The .vmdk files
            fileKeyList = append(fileKeyList, diskLayout...)
        } else if parent != nil && layout.Key.Value == parent.Value {
            parentFiles = append(parentFiles, diskLayout...)
        }
    }

    for _, parentFile := range parentFiles {
        removeKey(&fileKeyList, parentFile)
    }

    for _, file := range allSnapshotFiles {
        removeKey(&diskFiles, file)
    }

    fileKeyMap := make(map[int]types.VirtualMachineFileLayoutExFileInfo)

    for _, file := range vmlayout.File {
        fileKeyMap[int(file.Key)] = file
    }

    size := 0

    for _, fileKey := range fileKeyList {
        file := fileKeyMap[fileKey]
        if parent != nil ||
            (file.Type != string(types.VirtualMachineFileLayoutExFileTypeDiskDescriptor) &&
                file.Type != string(types.VirtualMachineFileLayoutExFileTypeDiskExtent)) {
            size += int(file.Size)
        }
    }

    if isCurrent {
        for _, diskFile := range diskFiles {
            file := fileKeyMap[diskFile]
            size += int(file.Size)
        }
    }

    return size
}

I tested with this change:

modified   govc/vm/snapshot/tree.go
@@ -25,6 +25,7 @@ import (

    "github.com/vmware/govmomi/govc/cli"
    "github.com/vmware/govmomi/govc/flags"
+   "github.com/vmware/govmomi/units"
    "github.com/vmware/govmomi/vim25/mo"
    "github.com/vmware/govmomi/vim25/types"
 )
@@ -147,7 +148,7 @@ func (cmd *tree) Run(ctx context.Context, f *flag.FlagSet) error {

    var o mo.VirtualMachine

-   err = vm.Properties(ctx, vm.Reference(), []string{"snapshot"}, &o)
+   err = vm.Properties(ctx, vm.Reference(), []string{"snapshot", "layoutEx"}, &o)
    if err != nil {
        return err
    }
@@ -161,6 +162,8 @@ func (cmd *tree) Run(ctx context.Context, f *flag.FlagSet) error {
    }

    cmd.info = o.Snapshot
+   size := SnapshotSize(o.Snapshot, nil, o.LayoutEx, true)
+   fmt.Printf("size=%s\n", units.ByteSize(size))

    cmd.write(0, "", o.Snapshot.RootSnapshotList)
atc0005 commented 3 years ago

@dougm Thank you! I appreciate the time/effort you spent to put this together. I'm going over the code now and comparing against my earlier efforts.

dougm commented 3 years ago

No problem @atc0005 , it was straightforward to translate the code and follow the notes you'd outlined. Definitely not obvious how to calculate the snapshot size from the API doc, so it'll be great to have this as a reusable method within govmomi.

atc0005 commented 3 years ago

@dougm

Thanks again for your help, and please let me know if my questions/commentary become tiresome. I'll switch over to other items and give you a break. :)

func SnapshotSize(
    info *types.VirtualMachineSnapshotInfo, 
    parent *types.ManagedObjectReference, 
    vmlayout *types.VirtualMachineFileLayoutEx, 
    isCurrent bool,
) int {

I see that info (vm.Snapshot) is passed in, which contains the current snapshot MOID (assuming I'm using the right terminology) and the full snapshot tree. But inside this function, the full snapshot tree isn't examined (afaict), info is used only used to check if a snapshot layout is for the active snapshot (using the MOID). If the snapshot is active, the vmsn and vmdk file keys are saved in a list. I see where the parent snapshot file keys are explicitly removed from the file keys list.

Later towards the end of the function the isCurrent boolean value is checked so that those disk files are included in the snapshot size calculation. I know you said that the code is meant to get the size of a single snapshot, but is this limited to the active snapshot only?

Here is a screenshot of the snapshots for a test VM in a vSphere 6.7 cluster:

image

I've reverted to a snapshot named 2019-10-15 of 8.44 GB which is second in the tree. The "You are here" entry in the vSphere web client just below it shows a disk usage of 1 MB. Running a version of govc with the provided changes, the current snapshot for a test VM is listed as 1 MB in size.

This probably makes sense in a literal way, but I would have expected the snapshot named 2019-10-15 of 8.44 GB to be listed as the current snapshot along with its snapshot size.

Based on what I'm seeing here, if I want to start with a crawl of vm.Snapshot.RootSnapshotList and calculate the size of each snapshot, I will need to keep track of which snapshots have parents in order to include or exclude files from the calculation. That include/exclude behavior from the provided code should help with that.

I'm still digging into this and hoping for the "ah ha!" moment. Once I have it, I suspect the direction will be obvious.


EDIT: In case it is helpful in any way, I've taken the provided code and applied it against a branch in my fork (atc0005/govmomi@83bd29f6fa0044853ddedb9b67fc953949006353). I've heavily annotated it as I attempt to figure out what each part of the code is doing.

dougm commented 3 years ago

Hey @atc0005 , I don't mind helping, solving these types of mysteries can be fun. I hadn't looked at the sizes displayed by the UI. The PowerCLI total matches the Go SnapshotSize(), but that total is less than what the UI displays?

In any case, the info param should just be a moref:

func SnapshotSize(snapshot types.ManagedObjectReference, parent *types.ManagedObjectReference, vmlayout *types.VirtualMachineFileLayoutEx, isCurrent bool) int {

And the diff below is how you would call it when walking the tree.

Does PowerCLI have an option to display the size of each snapshot or just the total?

modified   govc/vm/snapshot/tree.go
@@ -25,6 +25,7 @@ import (

    "github.com/vmware/govmomi/govc/cli"
    "github.com/vmware/govmomi/govc/flags"
+   "github.com/vmware/govmomi/units"
    "github.com/vmware/govmomi/vim25/mo"
    "github.com/vmware/govmomi/vim25/types"
 )
@@ -39,7 +40,8 @@ type tree struct {
    fullPath    bool
    id          bool

-   info *types.VirtualMachineSnapshotInfo
+   info   *types.VirtualMachineSnapshotInfo
+   layout *types.VirtualMachineFileLayoutEx
 }

 func init() {
@@ -78,7 +80,7 @@ func (cmd *tree) Process(ctx context.Context) error {
    return nil
 }

-func (cmd *tree) write(level int, parent string, st []types.VirtualMachineSnapshotTree) {
+func (cmd *tree) write(level int, parent string, pref *types.ManagedObjectReference, st []types.VirtualMachineSnapshotTree) {
    for _, s := range st {
        sname := s.Name

@@ -92,7 +94,10 @@ func (cmd *tree) write(level int, parent string, st []types.VirtualMachineSnapsh
            names = append(names, sname)
        }

+       isCurrent := false
+
        if s.Snapshot == *cmd.info.CurrentSnapshot {
+           isCurrent = true
            if cmd.current {
                names = append(names, ".")
            } else if cmd.currentName {
@@ -105,6 +110,12 @@ func (cmd *tree) write(level int, parent string, st []types.VirtualMachineSnapsh
            var attr []string
            var meta string

+           if true {
+               size := SnapshotSize(s.Snapshot, pref, cmd.layout, isCurrent)
+
+               attr = append(attr, units.ByteSize(size).String())
+           }
+
            if cmd.id {
                attr = append(attr, s.Snapshot.Value)
            }
@@ -127,7 +138,7 @@ func (cmd *tree) write(level int, parent string, st []types.VirtualMachineSnapsh
            }
        }

-       cmd.write(level+2, sname, s.ChildSnapshotList)
+       cmd.write(level+2, sname, &s.Snapshot, s.ChildSnapshotList)
    }
 }

@@ -147,7 +158,7 @@ func (cmd *tree) Run(ctx context.Context, f *flag.FlagSet) error {

    var o mo.VirtualMachine

-   err = vm.Properties(ctx, vm.Reference(), []string{"snapshot"}, &o)
+   err = vm.Properties(ctx, vm.Reference(), []string{"snapshot", "layoutEx"}, &o)
    if err != nil {
        return err
    }
@@ -161,8 +172,9 @@ func (cmd *tree) Run(ctx context.Context, f *flag.FlagSet) error {
    }

    cmd.info = o.Snapshot
+   cmd.layout = o.LayoutEx

-   cmd.write(0, "", o.Snapshot.RootSnapshotList)
+   cmd.write(0, "", nil, o.Snapshot.RootSnapshotList)

    return nil
 }
atc0005 commented 3 years ago

@dougm: Hey @atc0005 , I don't mind helping, solving these types of mysteries can be fun.

I'm glad to hear that, I was concerned that I was encroaching too much on your time!

I hadn't looked at the sizes displayed by the UI. The PowerCLI total matches the Go SnapshotSize(), but that total is less than what the UI displays?

This is what I see in the UI for the top-most snapshot:

image

Notice how it shows the disk usage as 20 GB?

With the base disk size of 20 GB, this (incorrectly) led me to assume (the first time I saw it) that the base disk + snapshot = 40 GB. As I understand it, the potential is 40 GB if the snapshot completely fills out, but that's not what the UI suggests (to me anyway).

PowerCLI seems to show the snapshot as size 0.02670192718505859375 MB.

PowerCLI output snippet:

Name             : Fresh install, activation and patches
Id               : VirtualMachineSnapshot-snapshot-18946
ParentSnapshotId :
SizeMB           : 0.02670192718505859375
SizeGB           : 0.0000260761007666587829589844
Full PowerCLI output ```console Get-VM RHEL7-TEST-RBD | Get-Snapshot | Select-Object -Property Name, Id, ParentSnapshotID, SizeMB, SizeGB | Format-List Name : Fresh install, activation and patches Id : VirtualMachineSnapshot-snapshot-18946 ParentSnapshotId : SizeMB : 0.02670192718505859375 SizeGB : 0.0000260761007666587829589844 Name : 2019-10-15 Id : VirtualMachineSnapshot-snapshot-126800 ParentSnapshotId : VirtualMachineSnapshot-snapshot-18946 SizeMB : 8642.019138336181640625 SizeGB : 8.439471814781427383422851562 Name : 2019-12-05 Id : VirtualMachineSnapshot-snapshot-138143 ParentSnapshotId : VirtualMachineSnapshot-snapshot-126800 SizeMB : 6401.019138336181640625 SizeGB : 6.2509952522814273834228515625 Name : 2020-03-29 Id : VirtualMachineSnapshot-snapshot-163887 ParentSnapshotId : VirtualMachineSnapshot-snapshot-138143 SizeMB : 7953.019138336181640625 SizeGB : 7.7666202522814273834228515625 Name : Test Snapshot Id : VirtualMachineSnapshot-snapshot-229096 ParentSnapshotId : VirtualMachineSnapshot-snapshot-18946 SizeMB : 1.018657684326171875 SizeGB : 0.0009947828948497772216796875 Name : Test Child snapshot Id : VirtualMachineSnapshot-snapshot-229097 ParentSnapshotId : VirtualMachineSnapshot-snapshot-229096 SizeMB : 1.018657684326171875 SizeGB : 0.0009947828948497772216796875 ``` ```console Get-VM RHEL7-TEST-RBD | Get-Snapshot | Select-Object -Property Name, Id, ParentSnapshotID, SizeMB, SizeGB | Format-Table Name Id ParentSnapshotId SizeMB SizeGB ---- -- ---------------- ------ ------ Fresh install, activation and patches VirtualMachineSnapshot-snapshot-18946 0.02670192718505859375 0.0000260761007666587829589844 2019-10-15 VirtualMachineSnapshot-snapshot-126800 VirtualMachineSnapshot-snapshot-18946 8642.019138336181640625 8.439471814781427383422851562 2019-12-05 VirtualMachineSnapshot-snapshot-138143 VirtualMachineSnapshot-snapshot-126800 6401.019138336181640625 6.2509952522814273834228515625 2020-03-29 VirtualMachineSnapshot-snapshot-163887 VirtualMachineSnapshot-snapshot-138143 7953.019138336181640625 7.7666202522814273834228515625 Test Snapshot VirtualMachineSnapshot-snapshot-229096 VirtualMachineSnapshot-snapshot-18946 1.018657684326171875 0.0009947828948497772216796875 Test Child snapshot VirtualMachineSnapshot-snapshot-229097 VirtualMachineSnapshot-snapshot-229096 1.018657684326171875 0.0009947828948497772216796875 ```

In my use case I looped over all snapshots returned by Get-Snapshot and added up the size values to get the total. With the PowerCLI plugin I wrote I ended up including the aggregate total with each snapshot PowerShell object I created/collected. This made both values easily accessible when printing the snapshots list.

To the question: the web UI suggests 20 GB as the value, PowerCLI shows 0.02670192718505859375 MB as the value. I'm not sure what files are being added up to equal that specific size.

I think it's based off of the snapshot data file size, which when I end up evaluating it with Go I am given 27999, or 27.3KB (after units.ByteSize() wrapped).

Yesterday evening just before I pulled my hair out I finally got the stars to align and successfully (I think) calculated the size of multiple snapshot trees. I spent some more time this morning getting it together and I think I'll be ready to share the results in a day or so. Even so, it will still be in need of heavy refactoring. I think the net result is the same, though the process is handled a little differently. It's probably just a lack of understanding on my part.

I'll apply the latest changes you provided tomorrow AM and study them. I appreciate your feedback and help with this.

Quick question though:

+if true {

Did you mean to write if isCurrent { here?

dougm commented 3 years ago

Glad to hear you got the stars to align, look forward to your findings.

+if true { Did you mean to write if isCurrent { here?

No, just meant that as a flag like the other attributes. So it should be something like if cmd.size { , then govc tree -s ... would include size in the output (similar to -D, etc).

atc0005 commented 3 years ago

Glad to hear you got the stars to align, look forward to your findings.

Evidently I spoke too soon; looks like I have more testing to do. I had something working for the specific example that I've listed here, but my implementation fails to handle a single snapshot branch multiple levels deep where the active snapshot is the last in the chain.

atc0005 commented 3 years ago

Evidently I spoke too soon; looks like I have more testing to do. I had something working for the specific example that I've listed here, but my implementation fails to handle a single snapshot branch multiple levels deep where the active snapshot is the last in the chain.

The more I troubleshoot this, the more my implementation is morphing to look like the provided logic. It seems that I didn't quite have a moment of insight like I thought I did. That said, it does seem like the current implementation matches the results as shown by the web UI, minus the first snapshot which is reported as the full disk size in the web UI, and just the snapshot data file per Go.

Separate from govmomi, I am seeing odd results just between the size values for web UI and PowerCLI for the leaf snapshot (active). This is a different VM than previously shown in this issue. For this VM, the web UI shows 404.03 MB when selecting the name and 12.11 GB when selecting the "You are here" marker. The VM is powered on and is assigned 5 GB of RAM.

PowerCLI shows:

Name             : post-obsolete-package-removal-upgrade-complete
SizeMB           : 7688.02824497222900390625
Id               : VirtualMachineSnapshot-snapshot-230725
ParentSnapshotId : VirtualMachineSnapshot-snapshot-230724
IsCurrent        : True
PowerState       : PoweredOff

It seems that PowerCLI is handling the calculations differently than the web UI's logic. I initially thought that the problem might be my PowerCLI version, so I upgraded it to the latest version.

Get-Module | Where-Object {$_.Name -like 'VMware*'} | Select-Object -Property Name, Version

Name                          Version
----                          -------
VMware.Vim                    7.0.1.16997275
VMware.VimAutomation.Cis.Core 12.1.0.16997582
VMware.VimAutomation.Common   12.1.0.16997174
VMware.VimAutomation.Core     12.1.0.16997984
VMware.VimAutomation.Sdk      12.1.0.16997004

That's after a fresh removal, closure of open sessions, fresh install and another try. The results remained consistent between PowerCLI 11.x (previous) and 12.1.x (now).

This almost seems like a case where the file keys associated with the VM's active state (growth since the last snapshot) are being ignored by the web UI, but recognized/reported by PowerCLI. Is this intentional? Is the differential disk growth between the last snapshot and the running state tracked in a manner visible to PowerCLI, but not the web UI / govmomi?

When I run the calculations using Go/govmomi, I get the 404.03 MB value. I've not yet tested the latest provided changes with govc to see what it reports (that will be next).

atc0005 commented 3 years ago

I dug further, and I found where I had a mistake in my logic. While I was getting the same results as shown by the web UI display, I wasn't getting the same results as the PowerCLI implementation. In particular, the PowerCLI implementation appears to combine the last (fixed) snapshot state with the current (growing) running state as one value. The web UI on the other hand displays a fixed value for the point-in-time that the snapshot was taken, then when you click into the You are here entry, you presumably get the dynamic portion for the current running state. In my case that value is still not equal to what PowerCLI shows, but I assume that other content such as swap files are being included.

Once I stepped through the initial code sample from https://github.com/vmware/govmomi/issues/2243#issuecomment-763336117 again, I believe I now understand what is taking place. I probably should have just admitted that I wasn't following the logic in the provided code, but I felt like asking for doc comments to explain the logic would have been imposing on top of the time/effort that had already been expended to assist with this GH issue.

Either way, I'm now getting consistent results across two different VMs with varying snapshot depths, so I'll work on cleaning the implementation up and then link to it. I still consider myself fairly new-ish to Go, so it's likely that I've overcomplicated things. It's also specific to my use case and not govc, but maybe it can still prove useful.

atc0005 commented 3 years ago

@dougm

I was going to take the code you provided, apply it to the current master branch and then submit as a PR (assigning credit of course).

Regarding code style, do you prefer to omit comments unless necessary? For example, convert code like this:

    // flat list of diskDescriptor, diskExtent values
    var fileKeyList []int

    // files associated with parent snapshot only
    var parentFiles []int

    // all snapshot files, regardless of whether snapshot has a parent
    var allSnapshotFiles []int

to this?

    var fileKeyList []int
    var parentFiles []int
    var allSnapshotFiles []int