Skip to content

Conversation

@gabriel-samfira
Copy link
Contributor

@gabriel-samfira gabriel-samfira commented Feb 28, 2023

It seems we missed a few metadata files last time. This adds the hive deltas that get created in WcSandboxState.

Hopefully I got them all this time.

This adds the hive deltas that get created in WcSandboxState

Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Copy link
Member

@kzys kzys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

What would be the authoritative source? MSDN?

@kzys
Copy link
Member

kzys commented Mar 6, 2023

Ping @jterry75 @kevpar

@gabriel-samfira
Copy link
Contributor Author

What would be the authoritative source? MSDN?

Empirical observation, sadly. These are overlay registry hives that get created once a layer is committed with the changes that happened in that particular layer. But there is no publicly available documentation for this, or at least none that I could find.

@TBBle
Copy link
Contributor

TBBle commented Mar 6, 2023

Perhaps if we ignored the entire WcSandboxState undocumented/OS-use folder recursively somehow (is that bool used for anything right now?) then we'd be safe from any internal changes made in the future. System Volume Information would probably also make sense to ignore recursively. These are akin to lost+found in ext3, in that they're OS-managed and may be modified by apparently-unrelated user action, and so probably never make sense to compare.

The registry hive list in Windows/system32/config is pretty fixed, as they need to exist in a base layer as part of its creation. For those, we have the code in hcshim that can create non-bootable empty-but-valid instances. That's also empirical observation, but in an MS-owned project at least, and with unit tests. And I suspect changing the number or names of registry hives in Windows at this point would take decades to unwind, given that they've been unchanged AFAIR since the 1990's.

@gabriel-samfira
Copy link
Contributor Author

is that bool used for anything right now?

It's used here: https://github.com/containerd/continuity/blob/main/fs/fstest/continuity_util.go#L49-L53

If missing or explicitly set to false, HasDiff() will return true.

Perhaps if we ignored the entire WcSandboxState undocumented/OS-use folder recursively somehow

Being able to recursively ignore a folder would be great, but I think that should be part of a different PR. It may be useful on both Windows and Linux, in the future.

@estesp
Copy link
Member

estesp commented Mar 7, 2023

Going to go ahead and merge this for now; if there are future improvements to managing the metadata list we can handle with a follow-up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants