Skip to content

Conversation

@ambarve
Copy link
Contributor

@ambarve ambarve commented May 27, 2021

This commit adds support in hcsshim to mount an extensible virtual disk data disk into a
container. In container config the host_path in the mount entry should
use the format evd://<evdType>/<evdPath> to specify an extensible virtual disk.

Signed-off-by: Amit Barve [email protected]

@ambarve ambarve requested a review from a team as a code owner May 27, 2021 21:55
@ambarve
Copy link
Contributor Author

ambarve commented May 27, 2021

This is basically the same as #998 but without the fixes in #1021.

@kevpar
Copy link
Member

kevpar commented Jun 4, 2021

Can this PR just be generic EVD support? That should give us support for a Spaces disk for free, right?

@ambarve
Copy link
Contributor Author

ambarve commented Jun 16, 2021

@kevpar I am not sure I understand what you mean. This PR adds the support for EVDs but also adds some verification in case that EVD is a storage space disk. So it is not limited to storage spaces.

@kevpar
Copy link
Member

kevpar commented Jun 16, 2021

@kevpar I am not sure I understand what you mean. This PR adds the support for EVDs but also adds some verification in case that EVD is a storage space disk. So it is not limited to storage spaces.

My thought is we can just add support for EVD and be done with it. There is no reason we need to support a special case for specific EVD providers like Storage Spaces. As long as we provide a generic EVD path, the user can use it and the lower levels of the stack should provide an error if they don't pass the right values.

@ambarve ambarve changed the title Support for storage spaces as data disks Support for extensible virtual disks as data disks Jun 16, 2021
@ambarve
Copy link
Contributor Author

ambarve commented Jun 16, 2021

@kevpar Thanks for the explanation. I have fixed it. PTAL.

@kevpar
Copy link
Member

kevpar commented Jun 25, 2021

Is there a corresponding cri fork change to create mounts with type extensible-virtual-disk on the container spec?

@ambarve
Copy link
Contributor Author

ambarve commented Jun 30, 2021

@kevpar yes there is another PR is cri here: jterry75/cri#97

Copy link
Contributor

@dcantah dcantah left a comment

Choose a reason for hiding this comment

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

lgtm. Minor comments

This commit adds support in hcsshim to mount an extensible virtual disk data disk into a
container. In container config the host_path in the mount entry should
use the format evd://<evdType>/<evdPath> to specify an extensible virtual disk.

Signed-off-by: Amit Barve <[email protected]>
Copy link
Member

@kevpar kevpar left a comment

Choose a reason for hiding this comment

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

LGTM

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