Add support for HVSOCK on hyperv #18167
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
| ready := `[Unit] | ||
| After=remove-moby.service sshd.socket sshd.service | ||
| OnFailure=emergency.target | ||
| OnFailureJobMode=isolate |
There was a problem hiding this comment.
Can you elaborate on the isolate choice? That means all/most other units are stopped. Is the idea to nuke everything if this unit fails?
There was a problem hiding this comment.
this is existing code that works like a charm today ... i didn't want to change anything ... plus it came from @dustymabe and he speaks fluent systemd ... i dont
There was a problem hiding this comment.
maybe a copy and paste from an existing service? I don't remember giving specific instruction to use that, but maybe I did.
The question is, what do you want to happen if this unit fails?
There was a problem hiding this comment.
then this should work like you want it to. basically the machine won't be up in a half state. it shuts everything down and drops you to a console.
There was a problem hiding this comment.
Great! This is how I read the docs on isolate as well 👍 I haven't seen until reviewing the PR though.
| // ready is a unit file that sets up the virtual serial device | ||
| // where when the VM is done configuring, it will send an ack | ||
| // so a listening host knows it can being interacting with it | ||
| ready := `[Unit] |
There was a problem hiding this comment.
Consider moving the two units into a dedicated .go file as consts. Future changes/bug fixes may need to be applied to both units. If they're located in the same file, the risk of changing one but not the other is probably lower.
There was a problem hiding this comment.
they are not the same though. thoughts?
There was a problem hiding this comment.
I was more thinking about locality. Currently the units are hidden in the functions. Having them side-by-side may help future development/maintenance but that's more speculation.
baude
force-pushed
the
hypervvsock
branch
2 times, most recently
from
114a1a8 to
d643737
Compare
Windows HyperV uses HVSocks (Windows adaptation of vsock) for communicating between vms and the host. Podman machine in Qemu uses a virtual UDS to signal the host that the machine is booted. In HyperV, we can use a HVSOCK for the same purpose. One of the big aspects of using HVSOCK on Windows is that the HVSOCK must be entered into the Windows registry. So now part of init and rm of a podman machine, entries must be added and removed respectively. Also duplicates are a no-no. Signed-off-by: Brent Baude <[email protected]>
| return err | ||
| } | ||
| // Wait on notification from the guest | ||
| return m.ReadyHVSock.Listen() |
There was a problem hiding this comment.
Would there ever be an instance where Listen() hangs and doesn't return?
There was a problem hiding this comment.
yeah, but you can ctrl-c it. i believe qemu is doing this the same basic way.
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Windows HyperV uses HVSocks (Windows adaptation of vsock) for communicating between vms and the host. Podman machine in Qemu uses a virtual UDS to signal the host that the machine is booted. In HyperV, we can use a HVSOCK for the same purpose.
One of the big aspects of using HVSOCK on Windows is that the HVSOCK must be entered into the Windows registry. So now part of init and rm of a podman machine, entries must be added and removed respectively. Also duplicates are a no-no.
Does this PR introduce a user-facing change?
[NO NEW TESTS NEEDED]