Determine and pass PCI addresses for block devices #267
Conversation
Codecov Report
@@ Coverage Diff @@
## master #267 +/- ##
==========================================
- Coverage 65.64% 65.61% -0.04%
==========================================
Files 75 75
Lines 8008 8014 +6
==========================================
+ Hits 5257 5258 +1
- Misses 2181 2185 +4
- Partials 570 571 +1
Continue to review full report at Codecov.
|
virtcontainers/qemu.go
Outdated
| @@ -565,19 +565,19 @@ func (q *qemu) qmpSetup() (*govmmQemu.QMP, error) { | |||
| return qmp, nil | |||
| } | |||
|
|
|||
| func (q *qemu) addDeviceToBridge(ID string) (string, string, error) { | |||
| func (q *qemu) addDeviceToBridge(ID string) (string, *Bridge, error) { | |||
There was a problem hiding this comment.
you don't need to return a reference to the bridge, a copy should be enough
There was a problem hiding this comment.
Yes, copy should be enough, I tend to pass addresses to avoid extra copies. I believe any incorrect changes should be caught by our unit tests/CI.
I can change this though, would like more input on this.
There was a problem hiding this comment.
Like I mentioned copy should be enough. I tend to use addresses to avoid copies and create empty objects in case of error. I'll change this if this is a blocker.
|
@WeiZhang555 @bergwolf PTAL? |
|
@sboeuf Can you take a look at this as well? |
|
As we discussed offline, I think this change is fine in theory, but I realized that the CI is completely useless here. We don't have any Jenkins job actually testing
I'd prefer option 1 here, but the bottom line is that I would prefer that we take only changes that our CI is actually testing. |
|
@amshinde I have run the whole testswith kata-containers/agent#227 and qemu-lite 2.7. No regressions were found. |
|
@chavafg good news. Are you currently adding a new job for those tests ? I think you are, right ? |
|
@sboeuf Yes, I have opened kata-containers/tests#271 to address this. |
|
Great thank you @chavafg |
|
@sboeuf Can we merge this, now that we know that CI passes? |
Change the function to return the bridge itself that the device is attached to. This will allow bridge address to be used for determining the PCI slot of the device within the guest. Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Introduce a new field in Drive to store the PCI address if the drive is attached using virtio-blk. Assign PCI address in the format bridge-addr/device-addr. Since we need to assign the address while hotplugging, pass Drive by address. Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
We need to store the bridge address to state to use it for assigning addresses to devices attached to teh bridge. So we need to make sure that the bridge pointer is assigned the address. Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Store PCI address for a block device on hotplugging it via virtio-blk. This address will be passed by kata agent in the device "Id" field. The agent within the guest can then use this to identify the PCI slot in the guest and create the device node based on it. Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Store the PCI address of rootfs in case the rootfs is block based and passed using virtio-block. This helps up get rid of prdicting the device name inside the container for the block device. The agent will determine the device node name using the PCI address. Fixes kata-containers#266 Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
Don't create the mount destination in `generalMount()` as that is handled by `mountToRootfs()`. Fixes kata-containers#267. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
This PR introduces using PCI addresses of virtio-blk devices to determine the device node names within the guest.
A follow up PR will use the same to pass the PCI addresses for VFIO devices as well.
The agent will use this information to identify devices within the guest.