-
Notifications
You must be signed in to change notification settings - Fork 162
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
vtpm fix : use domain uuid instead of full domain name #4270
Conversation
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.
Lucky you, the tests are fine... I'm fixing kvm_tets for an hour now =D
Config Num comes from the cloud. They increase it with each config update. |
The virtualization test suite has failed three times. Strange. |
Yeah, it looks like something is broken...
|
It is the version number (and not the app number) which is changed, right? If so it makes sense to update the PR description. |
pkg/pillar/hypervisor/kvm.go
Outdated
if err != nil { | ||
return fmt.Errorf("failed to extract UUID from domain name: %v", err) | ||
} | ||
return requestVtpmLaunch(domainUUID.String(), wk, swtpmTimeout) |
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.
Nit, but if the argument type was uuid.UUID and not a string then the compiler would help check.
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.
Run Eden
@shjala, will you take a look at why the same Eden test fails all the time? |
All are green except virtualization, do you mean that or there is more? |
Yeah, I mean that one. I don't see the same test failing in other PRs all the time. Maybe it's just a coincidence, but it's better to check. |
Domain name contains UUID, version and app number, the version number might get changed, so use only the UUID part to presever the vtpm state. Signed-off-by: Shahriyar Jalayeri <[email protected]>
@OhmSpectator should be fixed now, rerun? |
All green! What exactly have you done? Just rebase? |
Eden rerun is appreciated. |
Domain name contains UUID, version and app number,the version number might get changed, so use only the
UUID part to preserve the vtpm state.
This PR is based on a observation in @OhmSpectator code comment :
I'll be happy if someone point me to part of the code that actually makes this change to
appversion number, I couldn't find it myself.