Skip to content

add logic to stack lcow layers on a single VPMEM device#930

Merged
anmaxvl merged 1 commit intomicrosoft:masterfrom
anmaxvl:user/maksiman/device-mapper
Jun 23, 2021
Merged

add logic to stack lcow layers on a single VPMEM device#930
anmaxvl merged 1 commit intomicrosoft:masterfrom
anmaxvl:user/maksiman/device-mapper

Conversation

@anmaxvl
Copy link
Contributor

@anmaxvl anmaxvl commented Jan 25, 2021

This PR adds logic to reuse an existing VPMEM device and stack multiple LCOW layers onto it.

The change depends on microsoft/opengcs#389

More background in #940

Signed-off-by: Maksim An maksiman@microsoft.com

@anmaxvl anmaxvl requested a review from a team January 25, 2021 23:55
@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch 4 times, most recently from f3ddb24 to 5459050 Compare January 26, 2021 07:22
@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch 5 times, most recently from 9c8e6fe to 3c00b47 Compare February 2, 2021 18:43
@anmaxvl anmaxvl marked this pull request as ready for review February 3, 2021 04:50
@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch 3 times, most recently from 710fdcb to bca9a89 Compare February 4, 2021 17:40
@anmaxvl anmaxvl requested a review from a team February 4, 2021 18:03
@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch from bca9a89 to a98df30 Compare February 4, 2021 23:31
@katiewasnothere
Copy link

It might be good if you could make an issue on hcsshim for this and describe the memory allocation method and any other additional big changes proposed here.

@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch 7 times, most recently from 5a41352 to 727b0ac Compare May 28, 2021 08:36
Comment on lines +180 to +188
target := 4 * GigaByte
for offset := range pi.free {
if offset < target {
target = offset
}
}
return mc, target, nil

Choose a reason for hiding this comment

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

does this mean we could potentially return that the next offset is at offset 4GBs? Is that valid?

Copy link
Contributor Author

@anmaxvl anmaxvl May 31, 2021

Choose a reason for hiding this comment

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

we should never have a pool that will have an offset of 4GB, since 4GB is the maximum size of memory block. I can add a check though? 4GB should probably be a const at this point.

@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch from 727b0ac to a5cb324 Compare May 31, 2021 21:05
Copy link

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

couple of non blocking nits, otherwise LGTM

@anmaxvl anmaxvl force-pushed the user/maksiman/device-mapper branch 2 times, most recently from d26f6ea to 026c3f9 Compare June 3, 2021 06:27
Copy link

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes, LGTM again :)

add VirtualPMemMapping schema and update gcs types

add memory allocator interface and implementation

VPMem multi-mapping support has been added in 19H1, which enables
packing multiple VHDs onto a single VPMem device.
This feature enables an optimization, where multiple LCOW container
layers can be packed onto a single VPMem device.

This change uses memory allocator introduced above to keep track
of the VPMem surface allocation.

Additionally, introduce new structs to keep track of the internal
state of each mapped LCOW layer VHD and update HCS/GCS calls
accordingly.

The optimization is enabled by default on supported systems and
fall-back to old behavior otherwise.

add CRI tests

Signed-off-by: Maksim An <maksiman@microsoft.com>
@dcantah
Copy link
Contributor

dcantah commented Jun 15, 2021

The force push was fixing some edgecases I think you said this morning?

@anmaxvl
Copy link
Contributor Author

anmaxvl commented Jun 15, 2021

The force push was fixing some edgecases I think you said this morning?

yeah, I added one more test case to cover rootfs.vhd vs initrd (it was defaulting to only do rootfs)

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.

4 participants