-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: Package directory volumes when using OCI #676
base: staging
Are you sure you want to change the base?
feat: Package directory volumes when using OCI #676
Conversation
Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Signed-off-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Output string `local:"true" long:"output" short:"o" usage:"Save the package at the following output"` | ||
Platform string `local:"true" long:"plat" short:"p" usage:"Filter the creation of the package by platform of known targets"` | ||
Target string `local:"true" long:"target" short:"t" usage:"Package a particular known target"` | ||
Volume []string `local:"true" long:"volume" short:"v" usage:"Path to volume to bundle within the package (passing a path will automatically generate a CPIO image)"` |
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.
Based on the implementation, there is no CPIO generation so the usage should be updated.
AnnotationKernelVolumePath = "org.unikraft.kernel.volume-%d.path" | ||
AnnotationKernelVolumeMount = "org.unikraft.kernel.volume-%d.mount" |
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.
To be consistent with Unikraft's core:
- org.unikraft.kernel.volume-%d.path"
+ org.unikraft.volume.fs%d.path"
- org.unikraft.kernel.volume-%d.mount"
+ org.unikraft.volume.fs%d.mount"
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.
Let's also add the type here:
org.unikraft.volume.fs%d.type
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 am thinking about the attributes that are coming from the new addition occuring in the core to handle volumes: unikraft/unikraft#979
@@ -17,6 +17,8 @@ const ( | |||
AnnotationKernelKConfig = "org.unikraft.kernel.kconfig." | |||
AnnotationKernelArch = "org.unikraft.kernel.arch" | |||
AnnotationKernelPlat = "org.unikraft.kernel.plat" | |||
AnnotationKernelVolumePath = "org.unikraft.kernel.volume-%d.path" | |||
AnnotationKernelVolumeMount = "org.unikraft.kernel.volume-%d.mount" | |||
AnnotationFilesystemPath = "org.unikraft.filesystem" | |||
AnnotationDiskIndexPathPattern = "org.unikraft.disk-%d" |
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 this is what filesystem
and disk-
was meant to be. So these can be removed.
@nderjung is this still needed? I feel like there was no real push to have volumes in packages. Should I close this? |
Prerequisite checklist
make fmt
on your commit series before opening this PR;Description of changes