Add compile time /etc/eve-hv-type file #3231
Conversation
1) Set eve virttype in /run/eve-virt-type based on /run/eve-release 2) Add "eve virttype" option to return the virtualization type Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
pkg/dom0-ztools/rootfs/bin/eve
Outdated
| @@ -159,6 +160,10 @@ __EOT__ | |||
| uuid=$(cat /persist/status/uuid) | |||
| echo "$uuid" | |||
| ;; | |||
| virttype) | |||
There was a problem hiding this comment.
I the rest of EVE-OS this is called either "hv" or "eve-type". I'd suggest we use one of those.
| @@ -5,3 +5,16 @@ | |||
| ID=$(zboot curpart 2>/dev/null) | |||
| echo "${ID:-RAM}" > /run/eve.id | |||
| cp -p /etc/eve-release /run/eve-release | |||
| v=$(cat /run/eve-release) | |||
| echo $v | grep kubevirt | |||
There was a problem hiding this comment.
We need more specific patterns to avoid having e.g., a branch name like erik-fix-kubevirt match.
In grub rootfs.cfg we use patterns like
if regexp -- '-xen(-amd64$|-arm64$|-riscv64$|$)' $rootfs_title ; then
eve_flavor=xen
else
eve_flavor=kvm
to avoid such false matches.
Here we should be able to extract the part of the string before the ISA to make it more general.
There was a problem hiding this comment.
I am surprised to know branch name gets added to eve image name. I thought its only commit hash and HV type.
There was a problem hiding this comment.
Why do string extraction? Why don't we just have an explicit entry in eve-release that says, "hv="?
There was a problem hiding this comment.
eve-release already exists, I am just using it. Its just the image name which already has HV included in it.
There was a problem hiding this comment.
Yeah, I get that. Still, teasing it out from an image name is less than reliable. It isn't your change, it is the general approach that predates it. How hard would it be to just include it in eve-release and be done?
There was a problem hiding this comment.
Actually I am thinking simplest change is to ship /etc/eve-hv-type and copy it /run like we are doing with /etc/eve-release. Then we need not do anything and it's fixed at build time. I will update the commit if there is no issue with this approach.
There was a problem hiding this comment.
@zedi-pramodh are you then proposing to change all of the code (from grub and onwards) which today determines the eve-flavor/hv from the eve-release string so that there is no remaining read of that file (except for the check in
Line 465 in fa6d8bd
Or would you like to add a new file and logic without removing the old usage?
There was a problem hiding this comment.
@eriknordmark yes I am thinking of adding a new file which just contains HV type. say /etc/eve-virt-type or eve-hv-type. That file will be generated and added to image during the build process. During boot up we just copy that file to /run. In that way we can eliminate processing from eve-release.
There was a problem hiding this comment.
And the file content is generated from HV value used during make command.
There was a problem hiding this comment.
Updated PR, looks much cleaner now.
1) Ship /etc/eve-hv-type file with the image 2) At bootup copy /etc/eve-hv-type to /run/eve-hv-type 3) Add EVE_HV to rootfs yaml Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
zedi-pramodh
changed the title
There was a problem hiding this comment.
Looks reasonable but is the plan to also change pkg/grub/rootfs.cfg and storage-init to use this? If pkg/grub/rootfs.cfg no longer needs to read /etc/eve-release then @mikem-zed we might have less/no variation in PCR8 when the eve-release changes.
Don't know whether we care about the naming "hv-type" vs. "hv". I think what you have here is fine.
|
Looks like pkg/grub/rootfs.cfg reads /etc/eve-release just to get eve-flavor. Then we can replace that code to read value from /etc/eve-hv-type. Coming to storage-init.sh I do not see the need for that code anymore. That seems to be legacy code to debug zfs eve image. We can just delete it. |
It would be good to put those changes in this PR so we can test that it works as expected. Once we have that we can run it through the tests. |
1) Removed legacy code in storage-init.sh 2) set eve_flavor using /etc/eve-hv-type Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>
| eve_flavor=kvm | ||
| fi | ||
| fi | ||
| cat -s eve_flavor /etc/eve-hv-type |
There was a problem hiding this comment.
I don't know whether the build will produce a file with a CR at the end or not, but is this cat -s robust and can handle with and without?
This PR is a split from PR #3210
Testing done with kubevirt image
0e55274f-a647-4756-9e7f-bb6bbdcad366:~# cat /run/eve-virt-type
kubevirt
0e55274f-a647-4756-9e7f-bb6bbdcad366:
# eve# eve virttypeWelcome to EVE!
commands: enter [service] [command (assumed sh)]
enter-user-app
exec [service] command
list
status
start
pause
resume
destroy
persist list
persist attach
config mount
config unmount
firewall drop
verbose on|off
version
uuid
virttype
0e55274f-a647-4756-9e7f-bb6bbdcad366:
kubevirt