qemu: use x-ignore-shared to implement vm template #1799
Conversation
|
@bergwolf govmm pr was merged |
|
@devimc Thanks. It still needs to wait for the qemu patch which is being reviewed on the qemu mailing list though. |
|
@bergwolf ping, any updates on the qemu patch? |
|
@raravena80 I asked Dave and he said the patch will be included in the next qemu migration pull request. Once that is done, I'll move forward the kata side changes. |
Then we can use x-ignore-shared to do migration and drop the extra patch once we move to qemu 4.1.0 or later. Fixes: kata-containers#640 Depends-on: github.com/kata-containers/runtime#1799 Signed-off-by: Peng Tao <[email protected]>
|
Dropping |
|
/test |
Codecov Report
@@ Coverage Diff @@
## master #1799 +/- ##
=========================================
Coverage ? 52.41%
=========================================
Files ? 108
Lines ? 13976
Branches ? 0
=========================================
Hits ? 7325
Misses ? 5781
Partials ? 870 |
Codecov Report
@@ Coverage Diff @@
## master #1799 +/- ##
=========================================
Coverage ? 52.56%
=========================================
Files ? 108
Lines ? 13982
Branches ? 0
=========================================
Hits ? 7349
Misses ? 5762
Partials ? 871 |
There was a problem hiding this comment.
Thanks @bergwolf. I appreciate this isn't strictly required since you're refactoring code that was already there, but could you consider adding a few unit tests?
virtcontainers/qemu.go
Outdated
| @@ -585,6 +584,85 @@ func (q *qemu) vhostFSSocketPath(id string) (string, error) { | |||
| return utils.BuildSocketPath(store.RunVMStoragePath, id, vhostFSSocket) | |||
| } | |||
|
|
|||
| func (q *qemu) setupVirtiofsd(timeout int) (remain int, err error) { | |||
There was a problem hiding this comment.
timeout is internal and never changed before reaching here. I'm not against validating it but what is the place to do the validation along the call stack? That said, we do make sure it is non-negative when changing it below.
| // connection with QEMU closes. Therefore we do not keep track | ||
| // of this child process after returning from this function. | ||
| sourcePath := filepath.Join(kataHostSharedDir, q.id) | ||
| args := []string{ |
There was a problem hiding this comment.
This function does three things:
- constructs arguments.
- goroutine and channel logic.
- adjusts the timeout.
If you broke it into sub-functions for (atleast) (1) and (3), you could add some simple unit tests for those.
There was a problem hiding this comment.
Sounds good. I'll add some UTs for it.
There was a problem hiding this comment.
I've split the function and added UTs. PTAL.
qemu upstream has x-ignore-shared that works similar to our private bypass-shared-memory. We can use it to implement the vm template feature. Fixes: kata-containers#1798 Depends-on: github.com/kata-containers/packaging#641 Signed-off-by: Peng Tao <[email protected]>
|
/test |
|
Found this error on the nemu job: restarted job |
As we want to move to upstream qemu, we should drop as many private qemu patches as possible. One big change we carry with qemu-lite is the migration bypass-shared-memory patch. Qemu 4.0 already has a x-ignore-shared migration capability. With a small change to it, we can make it work similar to bypass-shared-memory and thus get rid of the private qemu patch. Then we can use x-ignore-shared instead of bypass-shared-memory to implement the VM template feature.