Skip to content

Don't create new scratch VHD per image for CimFS#9659

Merged
fuweid merged 1 commit intocontainerd:mainfrom
ambarve:cimfs_unionfs
Mar 7, 2024
Merged

Don't create new scratch VHD per image for CimFS#9659
fuweid merged 1 commit intocontainerd:mainfrom
ambarve:cimfs_unionfs

Conversation

@ambarve
Copy link
Copy Markdown
Contributor

@ambarve ambarve commented Jan 19, 2024

CimFS layers don't need to create a new scratch VHD per image. The scratch VHDs used with CimFS are empty so we can just create one VHD and copy it for every scratch snapshot. (Note that UVM VHDs are still unique per image because the VHD information is embedded in the UVM BCD during import)

@k8s-ci-robot
Copy link
Copy Markdown

Hi @ambarve. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ambarve ambarve force-pushed the cimfs_unionfs branch 2 times, most recently from d22732e to 38e490b Compare February 6, 2024 19:57
go.mod Outdated
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace github.com/Microsoft/hcsshim => github.com/ambarve/hcsshim v0.9.1-0.20240118174743-ed3de0e40cae
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a pending PR/change on hcsshim that this PR is dependent on?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah that PR is here: microsoft/hcsshim#2002

@kiashok
Copy link
Copy Markdown
Contributor

kiashok commented Feb 14, 2024

LGTM!
Needs to be updated with new hcsshim tag once microsoft/hcsshim#2002 is merged

@kiashok
Copy link
Copy Markdown
Contributor

kiashok commented Feb 14, 2024

@fuweid @thaJeztah could you please take a look at this PR when you have some time? Thanks!

Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

Just left one question and it looks good to me if there is rollback progress.

@ambarve ambarve force-pushed the cimfs_unionfs branch 2 times, most recently from 79a8857 to 0a74313 Compare March 5, 2024 16:44
CimFS layers don't need to create a new scratch VHD per image. The scratch VHDs used with CimFS are empty so
we can just create one base VHD and one differencing VHD and copy it for every scratch snapshot.
(Note that UVM VHDs are still unique per image because the VHD information is embedded in the UVM BCD during
import)

Signed-off-by: Amit Barve <ambarve@microsoft.com>
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid
Copy link
Copy Markdown
Member

fuweid commented Mar 7, 2024

ping @kiashok

@fuweid fuweid added this pull request to the merge queue Mar 7, 2024
@kiashok
Copy link
Copy Markdown
Contributor

kiashok commented Mar 7, 2024

ping @kiashok

signed off! Could you merge the PR? :)

Merged via the queue into containerd:main with commit 1afc6ca Mar 7, 2024
@fuweid
Copy link
Copy Markdown
Member

fuweid commented Mar 7, 2024

@kiashok yes! merged~

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants