| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-12 | |||
| 18:45:32 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/770533/12/nova/virt/libvirt/host.py#1236 | |
| 18:45:41 | stephenfin | e.g. 1 PF, 4 VFS, and 1 vDPA dev per VF | |
| 18:45:59 | sean-k-mooney | is this generating the nodedev xmls | |
| 18:46:04 | stephenfin | yes | |
| 18:46:22 | stephenfin | and then we let the libvirt driver generate the PciDevice objects | |
| 18:46:32 | sean-k-mooney | ok so not the data rutrined by _get_pcidev_info or the PciDevice objects | |
| 18:46:36 | stephenfin | no | |
| 18:47:05 | stephenfin | so I'd expect to see see five devices in the database | |
| 18:47:14 | stephenfin | one with type-PF, because I'm whitelisting that | |
| 18:47:18 | stephenfin | and four with 'vdpa' | |
| 18:47:21 | sean-k-mooney | yes | |
| 18:47:38 | stephenfin | we can expand the test to verify that if you like. That would be a good addition | |
| 18:47:55 | sean-k-mooney | no i just want to fiture out how to reuse | |
| 18:47:57 | sean-k-mooney | this | |
| 18:48:05 | sean-k-mooney | i want to create 1 PF and 1 vdpa device | |
| 18:48:29 | sean-k-mooney | can you tell me how to do that | |
| 18:48:33 | stephenfin | you'll want to copy lines 722 - 751 | |
| 18:48:43 | stephenfin | drop the for loop | |
| 18:48:47 | stephenfin | and replace idx with '1' | |
| 18:48:53 | stephenfin | sorry, 0 | |
| 18:49:28 | stephenfin | so what that'll do is generate 1 PCI device with PF caps, 1 PCI device with VF caps, and 1 vDPA device | |
| 18:49:28 | sean-k-mooney | what lines? | |
| 18:49:38 | stephenfin | https://review.opendev.org/c/openstack/nova/+/780112/4/nova/tests/functional/libvirt/test_pci_sriov_servers.py#722 | |
| 18:49:47 | stephenfin | from 'pci_info.add_device(' | |
| 18:50:00 | sean-k-mooney | oh your looking at a differnt version then i was | |
| 18:50:24 | stephenfin | ah yeah, I fixed the broken tests and gibi's comments | |
| 18:50:32 | stephenfin | so that is complete now | |
| 18:50:35 | stephenfin | I think | |
| 18:50:51 | sean-k-mooney | so dev_type PF is not a thing right it should be type-PF that is the value of the constant | |
| 18:50:58 | sean-k-mooney | PF is not a valid PF | |
| 18:51:06 | sean-k-mooney | *dev_type value | |
| 18:51:51 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/fields.py#L753-L761 | |
| 18:52:27 | sean-k-mooney | which is what the pcidevice object uses https://github.com/openstack/nova/blob/63bba50f4336f4b8bf0609b0cbe4717e5d0591d7/nova/objects/pci_device.py#L112 | |
| 18:52:53 | stephenfin | we're not generating PCI devices here | |
| 18:53:07 | stephenfin | https://review.opendev.org/c/openstack/nova/+/780112/4/nova/tests/unit/virt/libvirt/fakelibvirt.py#340 | |
| 18:53:07 | stephenfin | that's just a marker we use to tell the fakelibvirt fixture what type of fake XML to generate | |
| 18:53:29 | stephenfin | we could also pass 'MDEV_TYPES', in which case it would generate XML for a PCI device with mdev capabilities | |
| 18:53:33 | sean-k-mooney | ugh ya | |
| 18:53:46 | sean-k-mooney | we really should not use those contansts though | |
| 18:53:54 | sean-k-mooney | can we make them the same as the real ones | |
| 18:54:19 | sean-k-mooney | i guess we have been using these like this for a while | |
| 18:54:32 | stephenfin | Not really, because we'd need a way to indicate mdevs | |
| 18:54:36 | stephenfin | for XML generation purposes | |
| 18:54:45 | sean-k-mooney | i ocationllay modify this but its really annoying that we have PF and type- | |
| 18:54:50 | sean-k-mooney | type-PF | |
| 18:55:14 | sean-k-mooney | ya im going to sotp using the pci_info as the name | |
| 18:55:31 | stephenfin | I'd rather we moved away from passing the type and instead had e.g. 'add_pci_device', 'add_pci_device_with_mdev', 'add_pci_device_with_vf' etc. helpers | |
| 18:55:38 | stephenfin | that would be clearer IMO | |
| 18:55:52 | sean-k-mooney | ya | |
| 18:56:17 | stephenfin | That would be a good follow-up, but I think my changes there are already too invasive without tacking on even more | |
| 18:56:41 | sean-k-mooney | althoguh i still want fakelibvirt.HostPCIDevicesInfo( num_pci=0, num_pfs=1, num_vfs=0, num_vdpa=2) | |
| 18:57:00 | stephenfin | by all means, go for it. It should be relatively easy to add | |
| 18:57:17 | stephenfin | I only had one caller so I stuck with explicit | |
| 18:57:24 | stephenfin | if we've got more, we can make it more generic | |
| 18:58:28 | sean-k-mooney | im finding it quite hard to follow what that test is doing if im honest | |
| 18:59:05 | sean-k-mooney | espcailly the self.stub_out( | |
| 18:59:07 | sean-k-mooney | 'nova.virt.libvirt.guest.Guest.create', | |
| 18:59:39 | stephenfin | that's a pattern I copied from the SR-IOV test. I want to inspect what the XML being used to create the guest is | |
| 18:59:41 | sean-k-mooney | is vdpa_info.add_device(f'vdpa_vdpa{idx}', idx, vf) | |
| 18:59:42 | stephenfin | so I'm spying on it | |
| 18:59:46 | sean-k-mooney | what makes it a vdpa device | |
| 19:00:11 | sean-k-mooney | right but i dont think we should be doing that in this test | |
| 19:00:22 | sean-k-mooney | or at least i dont think my test should do that | |
| 19:00:35 | stephenfin | no, yours shouldn't. We only need it once | |
| 19:00:44 | stephenfin | it's just to make sure everything is wired up correctly | |
| 19:00:47 | sean-k-mooney | right i would not have put that in the basic create | |
| 19:00:58 | sean-k-mooney | i would have put that explicy in its own test | |
| 19:01:24 | stephenfin | I didn't want two tests that did the exact same thing and only changed what they were looking at | |
| 19:01:59 | stephenfin | If I had another test for e.g. resizing, I wouldn't bother with this | |
| 19:02:18 | stephenfin | I just want to make sure what we're handing off to neutron and libvirt is correct | |
| 19:02:51 | stephenfin | i.e. black box testing, looking at only at the inputs and outputs (from the nova service to other services) | |
| 19:03:07 | sean-k-mooney | yep i know anyway i think i have figured out what is needed an not thanks | |
| 19:03:31 | sean-k-mooney | stephenfin: it just a case of you are asserting two thing in one test and i was trying to fiture out what was the minium i needed | |
| 19:03:43 | stephenfin | gotcha | |
| 19:03:53 | stephenfin | yeah, to recap you don't need the libvirt spy | |
| 19:04:02 | stephenfin | nor do you really need to look at the neutron requests | |
| 19:04:11 | stephenfin | or PCI device counts in the DB | |
| 19:04:37 | stephenfin | just create a port, create a server, and then try your various and see that they're correctly rejects, I guess? | |
| 19:04:44 | sean-k-mooney | and i should not need to pass the libvirt and qemu version explictly either | |
| 19:05:01 | sean-k-mooney | since you are passing the default | |
| 19:05:20 | stephenfin | you will | |
| 19:05:32 | stephenfin | the FakeLibvirtFixture defaults to the minimums | |
| 19:05:34 | stephenfin | which are too low | |
| 19:05:40 | sean-k-mooney | oh ok | |
| 19:05:53 | stephenfin | sean-k-mooney: btw, the 'test_vtpm' module has examples you can use | |
| 19:05:54 | sean-k-mooney | i though it was max | |
| 19:06:04 | stephenfin | e.g. test_live_migrate_server | |
| 19:06:08 | stephenfin | test_shelve_server | |
| 19:06:11 | sean-k-mooney | i have some working | |
| 19:06:13 | sean-k-mooney | ya | |
| 19:06:20 | sean-k-mooney | ill take a look | |
| 19:06:20 | stephenfin | great :) | |
| 19:06:30 | sean-k-mooney | im trying to use/extend the integrated helpers | |
| 19:06:55 | sean-k-mooney | so added attach/detach interface with calls to the fakenotifier to wait | |
| 19:07:10 | sean-k-mooney | that kind of thing | |
| 19:07:21 | sean-k-mooney | for shelve i think we already have that there | |
| 19:14:55 | openstackgerrit | Merged openstack/nova stable/ussuri: Prevent archiving of pci_devices records because of 'instance_uuid' https://review.opendev.org/c/openstack/nova/+/760977 | |
| 19:16:28 | gibi | stephenfin: thanks for the update of the vdpa func test, it looks good now | |
| 19:22:46 | sean-k-mooney | stephenfin: what did you fix between v3 and v4 | |
| 19:23:01 | sean-k-mooney | i might need to rebase my patch to pick up the v4 changs | |