[TEST PR - DNM] Nonetns #835
Conversation
caoruidong
force-pushed
the
nonetns
branch
3 times, most recently
from
c39451e to
224705c
Compare
|
/test |
Codecov Report
@@ Coverage Diff @@
## master #835 +/- ##
==========================================
+ Coverage 66.1% 66.23% +0.12%
==========================================
Files 87 87
Lines 10509 10539 +30
==========================================
+ Hits 6947 6980 +33
+ Misses 2863 2855 -8
- Partials 699 704 +5 |
|
/test |
| @@ -832,6 +838,12 @@ func doNetNS(netNSPath string, cb func(ns.NetNS) error) error { | |||
| } | |||
| defer currentNS.Close() | |||
|
|
|||
| // if netNSPath is empty, the callback function will be run in the current network namespace. | |||
| // So skip useless GetNS and Set, just call cb(). | |||
| if netNSPath == "" { | |||
There was a problem hiding this comment.
In such case, we don't need LockOSThread() nor GetCurrentNS(). Please skip the whole function and just call the callback function directly.
There was a problem hiding this comment.
OK, exactly. @bergwolf Thanks for your review. I'll address them in original PR. This pr is just a test and will be closed after I find why CI fails.
| NicName string | ||
| TAPIface NetworkInterface | ||
| VMFds []*os.File | ||
| VhostFds []*os.File |
There was a problem hiding this comment.
This is much alike NetworkInterfacePair struct. Please unify them with embedded types so that we have something like:
type VMInterface struct {
ID string
Name string
TAPIface NetworkInterface
NetInterworkingModel
VMFds []*os.File
VhostFds []*os.File
}
type NetworkInterfacePair struct {
VMInterface
NetInterworkingModel
VirtIface NetworkInterface
}
type TapEndpoint struct {
VMInterface
EndpointProperties NetworkInfo
EndpointType EndpointType
PCIAddr string
}
| case NetXConnectMacVtapModel: | ||
| if err := q.hotplugMacvtap(drive); err != nil { | ||
| case TapEndpointType: | ||
| drive := endpoint.(*TapEndpoint) |
There was a problem hiding this comment.
These two types are similar and can be handled similarly with a helper function.
|
Hi @caoruidong. I understand from https://github.com/kata-containers/runtime/pull/835/files#r226216859 that this is a test PR, based on #733. Raising test PRis is fine, but it would help users if you could explain this to avoid confusion :) I'll update the title of this PR to make it clearer... |
|
@jodh-intel Thank you. I will try to not open test pr anymore :) |
|
Hi @caoruidong - please do raise test PRs if you need to :) All I mean is that we have so many PRs "flying around", it helps a lot if you link back to the original PR and make it clear your PR is a test PR. |
Refactor these functions so differernt types of endpoints can use a unified function to hotplug nics. Fixes kata-containers#731 Signed-off-by: Ruidong Cao <[email protected]>
…point This model is for not creating a new net ns for VM and directly creating taps in the host net ns. Signed-off-by: Ruidong Cao <[email protected]>
|
original PR: #733 |
|
@jodh-intel Yeah. I mean I don't want to meet CI problem once more… |
To update device resource entries from host to guest, we search for the right entry by host major:minor numbers, then later update it. However block and character devices exist in separate major:minor namespaces so we could have one block and one character device with matching major:minor and thus incorrectly update both with the details for whichever device is processed second. Add a check on device type to prevent this. Fixes: kata-containers#835 Signed-off-by: David Gibson <[email protected]>
This is a test PR. Since last pr got some errors I can't understand. I test this to find out the reason.