Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-07-14
18:31:17 opendevreview Merged openstack/nova-specs master: Configurable instance domains https://review.opendev.org/c/openstack/nova-specs/+/849765
20:31:17 opendevreview Rico Lin proposed openstack/nova master: libvirt: Ignore LibvirtConfigObject kwargs https://review.opendev.org/c/openstack/nova/+/830644
20:31:17 opendevreview Rico Lin proposed openstack/nova master: libvirt: Remove unnecessary TODO https://review.opendev.org/c/openstack/nova/+/830645
20:31:18 opendevreview Rico Lin proposed openstack/nova master: add locked_memory extra spec and image property https://review.opendev.org/c/openstack/nova/+/778347
20:31:18 opendevreview Rico Lin proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646
20:31:19 opendevreview Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507
#openstack-nova - 2022-07-15
05:25:47 auniyal Hello
05:25:56 auniyal please review these 3
05:26:02 auniyal https://review.opendev.org/c/openstack/nova/+/848886
05:26:16 auniyal https://review.opendev.org/c/openstack/nova/+/849104
05:26:27 auniyal https://review.opendev.org/c/openstack/nova/+/849532
10:03:45 sean-k-mooney stephenfin: if you get time can you fix the requirmetns file in https://review.opendev.org/c/openstack/nova/+/849867 then it should be good to merge
10:45:09 opendevreview Balazs Gibizer proposed openstack/nova master: Reproduce bug 1981813 in func env https://review.opendev.org/c/openstack/nova/+/849985
10:45:33 gibi sean-k-mooney: ^^ this is the reproduction for vnic_type change bug
11:25:42 opendevreview Merged openstack/nova master: libvirt: remove default cputune shares value https://review.opendev.org/c/openstack/nova/+/824048
11:34:03 artom gibi, I left a comment, I'm... not sure?
11:34:09 artom Smells like more of a unit test
11:35:15 gibi artom: you probably found a bug in that test
11:36:03 gibi it should only pass if _heal_instance_info_cache updates the vnic_type in our cache and then restart
11:36:30 artom My point is that the vnic change and heal cache isn't needed
11:36:38 gibi probably made a two broad mock
11:36:52 artom If you just start the server with a macvtap and mock the PCI code to return NotFound, you'll still get the exact same service startup error
11:36:58 gibi yeah
11:37:30 artom And like, I know in real life the NotFound comes from changing the vnic_type
11:37:52 artom OK, I think I got it.
11:38:33 artom No, wait.
11:41:06 artom The thing that Nova is doing wrong is attempting to find the wrong PCI device? Or because the device is already used by the instance, we can't find it?
11:42:27 gibi it is used by the instance so we cannot find it afaik
11:42:58 gibi or more precisely
11:43:02 gibi /sys/bus/pci/devices/0000:19:0a.7/net is not exists
11:43:22 gibi as the pci device is attached to the instance there is no netdev on the host
11:45:42 gibi after the vnic_type changed to macvtap the vif plug code wants to call set_vf_interface_vlan on the pci device
11:45:55 artom So wait, shouldn't that also happen if you just restart nova-compute with any macvtap device?
11:46:23 gibi hm
11:46:30 gibi that is a good question
11:46:33 gibi wait
11:46:38 gibi if it is consumed as a macvtap
11:46:44 gibi then the VF is not consumed
11:46:49 gibi and probably the netdev exists
11:47:02 artom Ah, right.
11:47:06 artom Sorry, creating confusion.
11:47:07 gibi the problem is that we consumed the VF and then looking up the VF's netdev
11:47:27 artom Bingo.
11:47:35 artom So the functional test should probably be asserting *that*
11:49:42 gibi so probably mocking get_ifname_by_pci_address is too much and I should inject the fault at set_vf_interface_vlan as that is the last specific call
11:49:55 gibi for the macvtap plug
11:49:56 artom Also, this is in the plug logic, right?
11:50:03 artom So should also happen during instance hard reboot?
11:50:18 artom We don't do any PCI accounting update during reboot...
11:50:22 gibi yes
11:50:30 gibi probably the plug fails at hard reboot too
11:50:45 gibi I can try as I have real sriov hw right now
11:52:25 gibi still the test will not be perfect as with mocking I can only simulate that the VF is consumed
11:52:40 gibi so if you move the port set before boot but keep the mock as is then it will still fail
11:52:57 gibi as it will still simulate that the VF is cosumed
11:53:04 gibi even if in reality it would not be
11:53:48 artom Right, there's no way around the mocking
11:54:00 artom But... I think I'd an asserting what what device we tried to look for
11:54:04 artom *assertion
11:54:14 artom "<gibi> the problem is that we consumed the VF and then looking up the VF's netdev"
11:54:15 artom :)
11:55:13 gibi even always try to look for the VF of the macvtap in both cases (a) when booting with a proper macvtap, b) when change the port to macvtap and then rebooting)
11:55:36 gibi s/even//
11:55:58 artom So the first case is legit, obviously
11:56:02 gibi yes
11:56:06 gibi second shoudl fail
11:56:17 gibi but I cannot distinguish on the mock level
11:56:25 artom The breakage in the second case from the fact that we didn't free up the device when the port type changed
11:56:58 gibi in the func env we never consume VF the pci stuff is mocked out globally
11:57:11 artom No? We can't do something like assert_called_with(<netdev_path>)?
11:57:40 gibi we can do both in both case the path would be the same
11:57:52 gibi *but
11:58:16 gibi the difference is that the boot consumed someting in b) but not in a)
11:58:22 gibi but we does not track consumption
11:58:27 gibi as that is on the host OS level
11:58:58 artom Like, I'm not saying "stop asserting the service start failure"
11:59:12 artom Oh, I think I get it
11:59:46 artom There's no assertion we can make that would be different between "start with legit macvtap device" and "start with vnic_type changed macvtap"
11:59:52 gibi yes
11:59:58 gibi as the path and the logic is the same
12:00:03 gibi the diff happens during boot
12:00:30 gibi this is like extrnal state that we don't modell in test
12:00:50 gibi the boot changes the external state (the host OS) and the reboot will depend on that state
12:00:57 gibi but we don't carry that state in the test env
12:01:16 gibi we could, but we don't today
12:01:35 gibi we could create a proper stub for the pci module and track pci devices
12:03:11 artom That smells like a lot of work... :)
12:03:38 artom But... you understand why I find the test weird, right? Like, we go through all these change vnic_type steps, but they're all moot because of the mock
12:04:00 gibi yes, that would be a piece of work :)
12:04:15 gibi and yes, I got you, I try to figure out something better...
12:05:00 artom Sorry for not bringing any better solution :P
12:05:11 gibi basically the mock and the vnic_type change need to be coupled somehow...
12:05:43 gibi artom: no worries, you have a valid point, and it was a good excersise to talk it through
12:20:20 gibi artom: interestingly the hard reboot did not fail
12:20:57 gibi it created a proper macvtap interface and passed to the instance
12:21:28 gibi it is probably works because hard reboot unplugs the vif -> the VF will be freed the plug can lookup the netdev
12:22:10 gibi and the accounting in nova side is actually correct as the macvtap dev needs to keep the parent VF allocated to the instance (and that does not change during the reboot)
12:22:21 gibi so probably a direct -> direct-phyical change would not work
12:29:31 artom Ah, right, because it's the host OS accounting
12:29:36 artom So as long as we unplug first, we're fine

Earlier   Later