| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-22 | |||
| 12:54:35 | gibi | it feels clearer | |
| 12:54:54 | gibi | as we define the rest of the infor in the profile at the same time like slot | |
| 12:55:14 | sean-k-mooney | ack | |
| 12:55:26 | sean-k-mooney | but in that case we need to permently keep the mac in the profile yes | |
| 12:55:47 | sean-k-mooney | so we shoudl be doing this for first spawn and sriov attach too | |
| 12:55:50 | sean-k-mooney | for consitency | |
| 12:55:58 | gibi | yes we do it | |
| 12:56:07 | gibi | any time we bind a SRIOV PF | |
| 12:56:28 | gibi | or create an inactive binding | |
| 12:56:45 | sean-k-mooney | ok and the key we use in teh profile for the PF mac is also different form teh key we use for the PF mac in the case of remote managed prots right | |
| 12:57:01 | sean-k-mooney | just making sure we dont have a collision there | |
| 12:57:17 | gibi | it is differen | |
| 12:57:18 | gibi | t | |
| 12:57:24 | gibi | the remote managed codepath stores pf_mac_address | |
| 12:57:29 | gibi | in the VF profiles | |
| 12:57:42 | gibi | my bugfix stores mac_address in the PF profile | |
| 12:57:57 | sean-k-mooney | ack | |
| 13:21:44 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: Fix migration with remote-managed ports & add FT https://review.opendev.org/c/openstack/nova/+/829974 | |
| 13:24:11 | dmitriis | sean-k-mooney, gibi: ^ found a missing piece related to migration when adding more func tests. Since binding:profile is updated differently during live migration there is a slight change needed to also update new fields. | |
| 13:25:08 | dmitriis | I added a test case for live migration, looking into adding resize/evacuate/cold_migrate now as well. | |
| 13:29:55 | sean-k-mooney | dmitriis: you will need to update the inactive port bindign with the dest host info | |
| 13:30:29 | sean-k-mooney | dmitriis: i assume you are goign to supprot live migration by hot unplugging and hot plugging the VF as we do for vnic_type=direct yes | |
| 13:30:56 | sean-k-mooney | in the future we might eb able to read if the vf supprot live migration from sysfs and skip that if it does | |
| 13:31:59 | dmitriis | sean-k-mooney: the fix is quite simple https://review.opendev.org/c/openstack/nova/+/829974/2/nova/compute/manager.py#10805 - as you say, it's updating the inactive port binding | |
| 13:32:41 | sean-k-mooney | ya although that is adding more coupling to sysfs | |
| 13:32:59 | sean-k-mooney | gibi how do you feel about ^ | |
| 13:33:17 | dmitriis | yes, I was thinking of storing more in extra_info but this kind of information is more volatile (pf mac, vf num) | |
| 13:33:38 | dmitriis | compared to the serial number that we currently store in extra_info | |
| 13:33:44 | sean-k-mooney | dmitriis: gibi was going to store it in extra info | |
| 13:34:00 | sean-k-mooney | well the pf mac is basically hardcoded in the flash | |
| 13:34:03 | gibi | I'm on the side of moving this info the PciDevice object populated by the virt driver | |
| 13:34:10 | sean-k-mooney | ya | |
| 13:34:24 | sean-k-mooney | we have that object here so we can read if form there | |
| 13:34:38 | chateaulav | to set image metadata during a ci tempest, i would most likely have to use a script as there are no variables, correct? | |
| 13:34:46 | sean-k-mooney | so really we shoudl put this info in the extra_info dict with properties to access it form teh object | |
| 13:35:20 | sean-k-mooney | chateaulav: am it depends if you are writing new test then you woudl do it in the test | |
| 13:35:24 | dmitriis | sean-k-mooney: well, the PF itself may be a vport of a NIC switch. I've seen HW with variable number of PFs configurable (per a given flash configuration though) | |
| 13:35:53 | sean-k-mooney | but if you are trying to reuse exsitng tempest then you woudl have a pre playbook that generates a local.sh that runs at the end of devstack before tempest is run to update the default image with the properties | |
| 13:36:26 | dmitriis | it's *probably* safe to assume that the PF mac won't change across reboots is what I am going for. But we may encounter odd cases too. | |
| 13:36:36 | sean-k-mooney | chateaulav: you can also config tempst to use a different image and have devstack download addtional images via the local.conf | |
| 13:36:40 | chateaulav | sean-k-mooney: k, thanks | |
| 13:37:32 | sean-k-mooney | dmitriis: changing the number of PF would change ther PCI adress which would not be supported if you had existing vms | |
| 13:37:55 | sean-k-mooney | we can also update the mac in the db on agent start | |
| 13:38:11 | sean-k-mooney | althoguh that wont update the pf mac in the neutron port so maybe we shoudl detect that and cause an error | |
| 13:38:22 | sean-k-mooney | or at least a warning | |
| 13:39:04 | dmitriis | VF numbers have an offset but they should remain stable unless the number of PFs changes | |
| 13:39:05 | sean-k-mooney | in the unlikely case a nic dies we likely want to be able to replace it with another nic | |
| 13:39:22 | sean-k-mooney | dmitriis: right which we dont want to supprot | |
| 13:39:52 | dmitriis | sean-k-mooney: ok, then I agree in principle that we can store vf_num and pf_mac in extra_info | |
| 13:40:01 | dmitriis | not hard to retrieve it from there either | |
| 13:40:09 | sean-k-mooney | we are already storing them in the neutron db in the port bidnign | |
| 13:40:16 | sean-k-mooney | so we already have the requirement that they be stable | |
| 13:40:23 | dmitriis | right | |
| 13:41:53 | sean-k-mooney | if we detach a change in the mac we can warn on that in agent start if we want to and tell the operator what the old mac was and the new one so that they know they have to manually update the ports. we could also automaticaly do that i guess but i woudl proably defer that to a followup patch | |
| 13:43:16 | dmitriis | ack | |
| 13:46:50 | dmitriis | So I think that's the change gibi is working on https://review.opendev.org/c/openstack/nova/+/829248 | |
| 13:48:39 | gibi | dmitriis: yes, but note the self -1 :) I realized that I need to change direction | |
| 13:48:46 | gibi | to use the PciDevice object | |
| 13:50:25 | dmitriis | gibi: ack. I'll focus on adding the remaining func test cases in the short term but I'll be around to help with moving things to PciDevice object | |
| 13:51:02 | gibi | dmitriis: thanks | |
| 13:57:22 | opendevreview | Felix Huettner proposed openstack/nova stable/queens: Gracefull recovery when attaching volume fails https://review.opendev.org/c/openstack/nova/+/829861 | |
| 14:06:42 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/830010 | |
| 14:33:44 | opendevreview | yuval proposed openstack/nova master: Lightbits LightOS driver https://review.opendev.org/c/openstack/nova/+/821606 | |
| 14:46:52 | opendevreview | Jonathan Race proposed openstack/nova master: zuul-job for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/828372 | |
| 14:46:52 | opendevreview | Jonathan Race proposed openstack/nova master: driver/secheduler/docs for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 14:53:34 | sean-k-mooney | gibi: can you respond ot https://review.opendev.org/c/openstack/placement/+/826492/8/placement/lib.py#219 i think that is the only thing im not sure about in your placement series | |
| 14:54:02 | sean-k-mooney | i have +w most of the patches before that so they should all be sitting in the ci queue and should merge up to that point later today | |
| 15:03:56 | gibi | sean-k-mooney: looking... | |
| 15:04:35 | sean-k-mooney | its pretty minor | |
| 15:04:46 | sean-k-mooney | just i dont think we are checkign for conflicts properly | |
| 15:05:14 | gibi | I have to load context... :) | |
| 15:05:25 | sean-k-mooney | like i think we bascially should be flattening the list of required traits and asserting that there is on intersection with forbidden | |
| 15:05:44 | sean-k-mooney | im not sure using all does that | |
| 15:11:08 | gibi | so the goal is to find a conflict in the request like required=T1,!T1 | |
| 15:11:18 | gibi | in the complex case | |
| 15:11:51 | gibi | required=in:T1,T2&required=!T2 | |
| 15:11:57 | opendevmeet | gibi: Error: "T2" is not a valid command. | |
| 15:11:57 | gibi | !T2 is not a conflict | |
| 15:12:37 | gibi | but | |
| 15:12:39 | sean-k-mooney | well it shoudl still be rejected | |
| 15:12:49 | gibi | it is satisfyable | |
| 15:13:00 | sean-k-mooney | only if we have t1 | |
| 15:13:02 | gibi | yes | |
| 15:13:11 | sean-k-mooney | right im not sure we shoudl allow that | |
| 15:13:15 | gibi | I think we cannot expect that the client will normalize the query | |
| 15:13:35 | sean-k-mooney | i dont think any frobiden traits should over lap with any required even if we in | |
| 15:13:45 | gibi | maybe these required triat pieces are coming form different direction in nova | |
| 15:13:47 | sean-k-mooney | since we cant make forbidne traits optional | |
| 15:14:26 | gibi | one prefilter says I need either T1 or T2 another prefilter says I don't like T2 | |
| 15:14:40 | gibi | I don't want nova to do the normalization in this case | |
| 15:14:58 | sean-k-mooney | i kind of feel like nova shoudl be preprocessing that rahter then having ot have placment fiture that out | |
| 15:15:08 | sean-k-mooney | but i see why you are trying to be more lax | |
| 15:15:32 | gibi | I think it is a lot easier to accept it than forcing the client to normalize | |
| 15:16:07 | sean-k-mooney | required=in:T1,T2&required=!T2 is logically jsut required=T1 | |
| 15:16:12 | gibi | yes | |
| 15:16:26 | sean-k-mooney | if we do accept that we shoudl really preprocess that before we generate the sql queries | |
| 15:16:34 | gibi | why? | |
| 15:16:44 | gibi | I think sql can handle this | |
| 15:17:07 | sean-k-mooney | i mean it could but i think that will not scale well | |
| 15:17:07 | gibi | I even think sql should have the query planner to optimize this out | |