interfaces: hotplug nested vm test, updated serial-port interface #6491
Conversation
…into hotplug-nested-vm
…into hotplug-nested-vm
…into hotplug-nested-vm
…into hotplug-nested-vm
…into hotplug-nested-vm
…into hotplug-nested-vm
…o hotplug-nested-vm
…t slot is handled by gadget.
| check_slot_not_present() { | ||
| SLOT_NAME="$1" | ||
| for _ in $(seq 10); do | ||
| if ! execute_remote "snap interfaces" | MATCH ":$SLOT_NAME"; then |
There was a problem hiding this comment.
can we use "connections" now instead?
There was a problem hiding this comment.
Yep! This uncovered a bug in integration between connections & hotplug - HotplugGone is not honored! Bugifx coming in separate PR.
There was a problem hiding this comment.
Changed to use snap connections, but it needs #6590 to land first.
There was a problem hiding this comment.
Thanks for converting serial port to be hot plug friendly!
I've added a couple of comments inline. Some are clear follow-up since they require additional debug infrastructure (those that avoid the jq-inspired editing and querying of state). Some are small changes to proposed code that can be done in this branch, if that is what you want.
One thing I'm worried about is the extensive use of "try a number of times, sleeping" pattern in tests. Could we move that out of individual query routines and into a "wait-for-settle" helper or something alike? I fear this may be masking real bugs.
| fi | ||
| } | ||
|
|
||
| DATA="$1" |
There was a problem hiding this comment.
Is data relevant to be an outside argument? I think this could work fine without that.
| . "$TESTSLIB/nested.sh" | ||
| create_nested_classic_vm | ||
|
|
||
| copy_remote "${GOHOME}"/snapd_*.deb |
| return nil, nil | ||
| } | ||
|
|
||
| slot := hotplug.ProposedSlot{ |
There was a problem hiding this comment.
For USB devices we could also synthesise a slot label. The value could come from the ID_MODEL_FROM_DATABASE and ID_VENDOR_FROM_DATABASE attributes. The label is not used for matching but is used for display in some cases (it is also underused because it was always empty).
For example, plugging the Arduino UNO with this change would show a nice human-readable description of the slot.
There was a problem hiding this comment.
Logic for that already exists and is outside of this PR as a general mechanism for hotplug - see suggestedSlotName function in hotplug.go - it currently looks at "NAME", "ID_MODEL_FROM_DATABASE", "ID_MODEL" attributes as input data for slot name. That logic doesn't fill in Label and doesn't look at ID_VENDOR_FROM_DATABASE yet, but we can experiment with enhancing it and all interfaces will automatically benefit from it. I think this will be a material for separate PR. The logic here in HotplugDeviceDetected shouldn't generally bother with name/label unless an interface wants something non-standard.
There was a problem hiding this comment.
+1, I would love to see the follow up that uses the label more prominently.
There was a problem hiding this comment.
Will do, but we will need to think what makes sense here, as we need to populate name and label (let's not discuss here).
There was a problem hiding this comment.
do I understand correctly that we remember slot names? so if we change naming strategy there will be no confusion/renaming of preexisting slots?
There was a problem hiding this comment.
@pedronis Yes we remember their names for as long as they have connections, and they wouldn't change if we change naming strategy. The names can only change if you unplug the device and it has no connections, in such case we will forget the slot and it can get a new name next time when device is plugged again.
Co-Authored-By: stolowski <stolowski@gmail.com>
Co-Authored-By: stolowski <stolowski@gmail.com>
Co-Authored-By: stolowski <stolowski@gmail.com>
Co-Authored-By: stolowski <stolowski@gmail.com>
… +e to remove "|| true" expressions, use "ID_MODEL=QEMU_USB_SERIAL" match expression for parsing udevadm output.
Update serial-port interface to support hotplug, and add a new spread test for nested vm to exercise hotplug subsystem using qemu's serial-port.
The test can be run manually like this:
for xenial:
spread -debug google-nested:ubuntu-16.04-64:tests/nested/classic/hotplug
for bionic:
SPREAD_NESTED_SYSTEM=bionic spread -debug google-nested:ubuntu-18.04-64:tests/nested/classic/hotplug
Thanks to Sergio for laying foundations for this test and figuring out qemu stuff!