-
Notifications
You must be signed in to change notification settings - Fork 30
bake virtio 0.1.185 for windows server 2012 #1425
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
base: main
Are you sure you want to change the base?
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.
💡 To request another review, post a new comment with "/windsurf-review".
| finalBootIndex, err = virtv2v.GetBootableVolumeIndex(vminfo.VMDisks) | ||
| if err != nil { | ||
| return -1, errors.Wrap(err, "Failed to get bootable volume index") | ||
| return -1, "", errors.Wrap(err, "Failed to get bootable volume index") |
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.
The error return statement doesn't match the updated function signature. You need to return an empty string for the osRelease parameter when returning an error.
| return -1, "", errors.Wrap(err, "Failed to get bootable volume index") | |
| return -1, "", errors.Wrap(err, "Failed to get bootable volume index") |
| if useSingleDisk { | ||
| productName, err = RunCommandInGuest(diskPath, fmt.Sprintf("inspect-get-product-name %s", osPath), false) | ||
| } else { | ||
| productName, err = RunCommandInGuestAllVolumes(disks, "inspect-get-product-name", false, osPath) |
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.
There's an issue with how inspect-get-product-name is called. The osPath parameter should be part of the command string rather than passed as a separate argument.
| productName, err = RunCommandInGuestAllVolumes(disks, "inspect-get-product-name", false, osPath) | |
| productName, err = RunCommandInGuestAllVolumes(disks, fmt.Sprintf("inspect-get-product-name %s", osPath), false) |
spai-p9
left a comment
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.
lgtm
6f537b3 to
82466ac
Compare
a33af47 to
4d7f0f0
Compare
4d7f0f0 to
62b71b0
Compare
What this PR does / why we need it
This PR pre-bakes specific virtio version (0.1.189) to VJB VM. Now during migration v2v-helper determines the OS release and version and switches to this virtio-win version in case of windows server 2012
Which issue(s) this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged)fixes #1409
Special notes for your reviewer
Testing done
TBD