Rework domain manager to use virtblocks#2
Conversation
|
@andreabolognani FYI. Started the refactoring. |
|
@jnsnow FYI |
| @@ -0,0 +1,45 @@ | |||
| package virtwrap | |||
There was a problem hiding this comment.
I've been keeping up with the conversation, though I've just skimmed the code.
Monitoring is not something that we're likely to get around to during the experiment, so let's just have a dummy monitoring function that does nothing for now.
There was a problem hiding this comment.
Yep, I will do something simple, like just checking if the qemu pid is still there. If not send fail. Otherwise send running.
There was a problem hiding this comment.
Sounds simple enough to implement, thumbs up to the idea.
Signed-off-by: Roman Mohr <rmohr@redhat.com>
Instaed of listening to libvirt events, check "something" every second and report if the state changed. Signed-off-by: Roman Mohr <rmohr@redhat.com>
| } | ||
| }() | ||
|
|
||
| go func() { |
There was a problem hiding this comment.
Instead of listening to libvirt events, this right now simply polls every second "something" and checks if something interesting happened.
There was a problem hiding this comment.
Next I will update the tests. Then a full shell for virtblocks is ready which does not depend on anything from libvirt.
Signed-off-by: Roman Mohr <rmohr@redhat.com>
Signed-off-by: Roman Mohr <rmohr@redhat.com>
Signed-off-by: Roman Mohr <rmohr@redhat.com>
| } | ||
|
|
||
| type VirtBlockDomainImpl struct { | ||
| } |
There was a problem hiding this comment.
Nit: this (and VirtBlockDomain above) should be s/VirtBlock/VirtBlocks/g.
|
For the libvirt integration I've created a separate |
Signed-off-by: Roman Mohr <rmohr@redhat.com>
Signed-off-by: Roman Mohr <rmohr@redhat.com>
Starting a domain and connecting via serial console works
Signed-off-by: Roman Mohr <rmohr@redhat.com>
|
@andreabolognani I can do that. No problem. The PR now successfully uses virtblocks. It is possible to start VMIs, connect via serial console (kubectl.sh console myvm) and shut the VMI down. All managed as usual. Note that the current architecture here is still more complex than actually needed. In the current fashion it can co-exist with the libvirt implementation. |
| } else if err != nil { | ||
| return fmt.Errorf("could not determine if process is already running: %v", err) | ||
| } | ||
| cmdStr = append(cmdStr, "-uuid", "272b47a8-b72b-4cd7-9109-79005816cc31") |
There was a problem hiding this comment.
Can we just add UUID support to Virt Blocks instead of working around the missing feature in KubeVirt? Should be trivial to do so.
There was a problem hiding this comment.
Definitely. Just didn't get to it.
|
@rmohr FYI yesterday I updated https://github.com/virtblocks/libvirt/tree/virtblocks to take advantage of VM models; ideally you'd to the same here for KubeVirt, but if you don't have time right now that's okay too :) |
2 participants
What this PR does / why we need it:
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 #
Special notes for your reviewer:
Release note: