-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for Block CIMs #2261
base: main
Are you sure you want to change the base?
Support for Block CIMs #2261
Conversation
Currently we have a map which maintains a mapping of CIM & containerd ID to the volume at which a CIM is mounted for the given container. This was required before the layer refactoring work when we needed to get the volume path from the layer cim path. However, this isn't needed anymore. As of now, this map doesn't provide much value and makes the code a bit complicated. Moreover, we will need to rewrite some of this code anyway when we do the work required for handling `shim delete` cleanups properly (containerd/containerd#9727). Signed-off-by: Amit Barve <ambarve@microsoft.com>
ad3aed4
to
2afb976
Compare
pkg/cimfs/cimfs.go
Outdated
|
||
// IsBlockedCimSupported returns true if block formatted CIMs (i.e block device CIM & | ||
// single file CIM) are supported on the current OS build. | ||
func IsBlockedCimSupported() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we be consistent about if cim
is written as Cim
or CIM
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we will have to modify some existing code for this too. I would prefer to do this in a separate PR to avoid making these commits larger. Is that okay?
//sys CimMountImage(imagePath string, fsName string, flags uint32, volumeID *g) (hr error) = cimfs.CimMountImage? | ||
//sys CimDismountImage(volumeID *g) (hr error) = cimfs.CimDismountImage? | ||
|
||
//sys CimCreateImage(imagePath string, oldFSName *uint16, newFSName *uint16, cimFSHandle *FsHandle) (hr error) = cimfs.CimCreateImage? | ||
//sys CimCloseImage(cimFSHandle FsHandle) = cimfs.CimCloseImage? | ||
//sys CimCreateImage2(imagePath string, flags uint32, oldFSName *uint16, newFSName *uint16, cimFSHandle *FsHandle) (hr error) = cimfs.CimCreateImage2? | ||
//sys CimCloseImage(cimFSHandle FsHandle) = cimfs.CimCloseImage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was removing the ?
intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The CimCloseImage
API doesn't return anything. However, if you keep the ?
for an API that returns void, the mkwinsyscall
still generates the function definition with a return value. Ideally, we should fix this bug in mkwinsyscall
but I unblocked the CimFS work by removing that ?
.
return winapi.CimDeletePath(c.handle, path) | ||
} | ||
|
||
// Adds a tombstone at given path. This ensures that when the the CIMs are merged, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a description of what merging CIMs means somewhere. I may have missed it if it's already present though :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a package documentation here , let me know if you want me to add any more details in there.
CimFS now supports a new format for storing CIMs, named BlockCIM. A block CIM format can store the entire CIM on a block device (like a VHD) or a file formatted like a block device. This commit adds Go wrappers for the new CimFS APIs that allow creation, merging and mounting of such Block CIMs. Some new flags required when creating and mounting these CIMs are added and some deprecated flags have been removed. New type has been introduced to represent a block CIM. Unit tests have been added to test the newly added CimFS functionality. Lastly, CimFS flags aren't a part of the hcs schema (only the CimMount request is), those flags are moved from the hcs/schema2 package to the cimfs package. Signed-off-by: Amit Barve <ambarve@microsoft.com>
This commit adds a layer writer that can be used for extracting an image layer tar into a Block CIM format. Existing forked CIM layer writer was renamed to a common base type `cimLayerWriter`. Forked CIM layer writer & Block CIM layer writer both now extend this common base type to write layers in that specific format. Signed-off-by: Amit Barve <ambarve@microsoft.com>
This commit adds the ability to parse block CIM layer mounts and to mount the merged block CIMs to be used as a rootfs for a container. Signed-off-by: Amit Barve <ambarve@microsoft.com>
2afb976
to
cda4bee
Compare
This PR adds support for creating, mounting and using block CIMs for running windows process isolated containers.
Individual commits provide additional details on what changes they bring in.