Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-08-05
11:46:38 sean-k-mooney if we need libvirt 6.0.0 we could not that as a min verion i guess.
11:46:42 sean-k-mooney but yes
11:46:53 sean-k-mooney both feel like we just did not find it in the xml and remove it
11:47:53 gibi regarding the leaking MAC addess after macvtap removal, who should do the removal of the MAC from the VF? is it libvirt?
11:49:12 sean-k-mooney gibi: libvirt should altrhough we also have code in nova to clear it for old libvirts
11:49:25 sean-k-mooney its also not reseting the programed vlan on the vf
11:49:33 sean-k-mooney but that is likely the same issue
11:50:14 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L774-L784
11:50:27 sean-k-mooney we seam not to be calling unplug
11:50:51 sean-k-mooney in the direct case it proably is not clearing the trused vf status
11:51:08 sean-k-mooney i didnt actuly test that
11:51:37 sean-k-mooney we should be calling https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L833 as part of detach
11:52:03 gibi sean-k-mooney: thanks I will trace this missing unplug in my env
11:52:06 sean-k-mooney we are apprently https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2201
11:53:14 gibi I even have a log in the dmesg about the MAC removal during macvtap
11:53:15 gibi [1911821.990047] ixgbe 0000:81:00.0: removing MAC on VF 2
11:53:30 gibi but right after it
11:53:31 gibi [1911822.241163] ixgbe 0000:81:00.0: Setting VLAN 100, QOS 0x0 on VF 2
11:53:42 gibi it seems the vlan is set back to it
11:53:57 sean-k-mooney oh god damit....
11:54:18 sean-k-mooney the neutron sriov nic agent is proably racing with unplug
11:54:51 sean-k-mooney although i guess it could be libvirt?
11:55:14 sean-k-mooney we might want to do unplug after detach?
11:55:28 gibi good points
11:55:45 gibi I will gather logs from libvirt and the neutron agent to see if this is a race
11:57:02 sean-k-mooney [centos@sriov-1 ~]$ libvirtd --version
11:57:04 sean-k-mooney libvirtd (libvirt) 6.0.0
11:57:09 sean-k-mooney so same libvirt version
11:57:46 sean-k-mooney im using differnet nics then you you have nicantic 10G nics and im using 1G e1000 nics but that should not matter
11:58:28 sean-k-mooney @chat:button1[centos@sriov-1 ~]$ /usr/libexec/qemu-kvm --version
11:58:31 sean-k-mooney QEMU emulator version 4.2.0 (qemu-kvm-4.2.0-19.el8)
11:58:41 sean-k-mooney i think that is the same qemu too?
11:58:49 sean-k-mooney yep
11:58:58 sean-k-mooney so ya likely not related to the versions
11:59:16 gibi cool, on set of possible differences is ruled out
11:59:19 gibi one
12:09:28 sean-k-mooney its proably libvirt
12:10:26 stephenfin gibi, lyarwood: Could you folks stick https://review.opendev.org/#/c/744021/ on your respective review queues, please? Feel free to chuck something my way too
12:12:07 lyarwood stephenfin: I was looking at that yesterday
12:12:30 lyarwood stephenfin: I *think* I get it, I just wanted to grep around a little more before voting
12:13:14 stephenfin nw. Lots of context needed for it, unfortunately
12:14:24 lyarwood yup indeed, func tests helped however so thanks for that at least
12:24:03 openstackgerrit Stephen Finucane proposed openstack/nova master: libvirt: Drop support for Xen https://review.opendev.org/743231
12:24:04 openstackgerrit Stephen Finucane proposed openstack/nova master: libvirt: Remove 'hypervisor_version' from 'libvirt_info' https://review.opendev.org/744199
12:35:41 gibi sean-k-mooney: yeah, it is a race between libvirt detaching the device and nova unpluging the vif (and reseting the MAC), If I move the unplug after the detach in the nova code then the VF MAC and VLAN is reset properly after the macvtap port is detached
12:36:05 sean-k-mooney i have been wondering if we should do unplug twice
12:36:30 sean-k-mooney we generally wantto disconnect the device form the network backend before removing it form the vm
12:36:40 sean-k-mooney but just doing it once might be fine too
12:36:43 lyarwood stephenfin: LGTM btw
12:36:51 sean-k-mooney at the end
12:37:25 sean-k-mooney gibi: did you confirm it was libvirt by stoping the sriov nic agent?
12:38:22 sean-k-mooney gibi: or did you manage to find a log message
12:42:47 gibi sean-k-mooney: stopping the neutron nic agent did not solved the race so I assumed it is libvirt
12:42:59 sean-k-mooney yep makes sense to me
12:43:34 sean-k-mooney i saw in the libvirtd log that it does set the mac a number of time sbut i dont have the devstack logs to corralte the timestamps
12:43:42 alex_xu efried: do you know what is the usecase for this https://review.opendev.org/#/c/693414/3/specs/ussuri/approved/provider-config-file.rst@247
12:45:02 gibi sean-k-mooney: I will add a separate patch into the series that moves the unplug
12:45:15 efried alex_xu: Yes.
12:45:15 efried The idea there was that you could define a "default" rule to apply to all your compute nodes, but then override it for specific ones.
12:47:00 efried Used for ironic, but also in cases where you want to have your rules centralized and ansibled out to the hosts.
12:47:13 alex_xu efried: I see, thanks
12:48:37 alex_xu efried: is there any reason we should ignore the addtional inventories and traits when conflict the virt driver managed ones, instead of error out the conflict?
12:49:47 efried We debated this at design time. I can tell you for sure the answer to your question is "yes". But I can't remember exactly why :P
12:50:41 alex_xu efried: ok, so the result is ignore, not the error out, right? I saw the code is error out, not ignore.
12:51:07 efried Oh, whatever the design says is what we decided on.
12:51:28 alex_xu efried: ok, thanks :) I'm not going dump our the history
12:51:51 efried For this issue, I'm reasonably sure whatever is in the design is going to be there because it's what we decided on, not because we accidentally missed it.
12:52:41 alex_xu efried: ok, cool
12:53:44 efried https://specs.openstack.org/openstack/nova-specs/specs/victoria/approved/provider-config-file.html#provider-config-consumption-from-nova says "ignore" under "Provider Tree Merging".
12:55:42 alex_xu efried: yes, that is what I read also
12:56:00 alex_xu I can't thinking of a reason the different between ignore and error out also
12:56:19 efried alex_xu: here's an explanation of that other thing https://review.opendev.org/#/c/693414/3/specs/ussuri/approved/provider-config-file.rst@137
12:58:07 alex_xu nice
13:00:18 efried alex_xu: https://review.opendev.org/#/c/612497/12/specs/train/approved/provider-config-file.rst@204
13:00:18 efried I remember now:
13:00:18 efried The conflicts in question should error on startup, but be ignored thereafter.
13:03:08 alex_xu efried: for the case, there are virt driver managed inventory or trait show up later?
13:26:27 alex_xu efried: virt driver's update_provider_tree always overwrited provider tree's inventory. so the conflict will be found at startup, and there won't be any conflict after startup. so the code feel like right
13:30:01 efried Yeah, I thought there was a theoretical edge case where we could come across an error at runtime. The chances were very small, but we wanted to make sure we didn't crater the driver if it did happen. I think that's what led to the design decision as it stands.
13:35:51 sean-k-mooney can there be a conflict if i make a call to placement directly and modify the inventory
13:36:34 sean-k-mooney i know you are not really ment to do that but users be users and they dont always do what we tell them
13:36:52 efried Heh. "You just voided your warranty. You're on your own."
13:37:57 sean-k-mooney have you started getting support request form customer yet
13:38:02 sean-k-mooney they do that alot...
13:38:12 sean-k-mooney then ask us to fix it anyway
13:39:26 efried of course.
13:40:13 efried Unless you really break things, any manual change to the placement inventory ought to be scrubbed back out on the next periodic.
13:40:36 sean-k-mooney we had one customer that for example when they wanted to spawn a vm on specifci core on a host, stoped nova-compute, updated the vcpu_pin_set to only have those cores, booted the vm with --avaiablity-zone <zone>:<host> and then complained that if someone did a concurent operation on another vm on the host it could cause issue with pinning ...
13:41:03 efried You could probably add things that neither the compute nor the config care about, and they would stick around. Probably. But that won't break the code, I don't think.
13:41:06 sean-k-mooney efried: ya it should heal on the next run
13:41:53 sean-k-mooney if you create your own RP it defnitly shoudl be ok
13:42:24 sean-k-mooney if you add inventories to one of nova's RPs via the api well thats not allowed so nova is free to delete it
13:43:16 sean-k-mooney im not sure if we actully will delete the inventory but we are allowed too
13:44:39 efried It's been a hot minute since I looked at update_from_provider_tree, but I think we would delete it, yes.
13:46:09 sean-k-mooney alex_xu: by the way i reworked https://review.opendev.org/#/c/739131/ after your comments in version 5, im hoping stephenfin will get back to it to be the second +2 later today but just an fyi incase you want to look at it before then.
13:46:49 sean-k-mooney efried: this is basicaly why we are providing the provider.yaml i.e. to enable a supported way to do this so ya.
13:47:19 sean-k-mooney efried: hows openshift land going?
13:49:08 efried correct
13:49:33 efried Things are going well. After a couple months of pretty serious culture shock and vertical learning curve, I'm getting my feet under me.
13:50:44 efried I've written an operator (in go) and feel pretty comfortable navigating openshift/kube APIs.

Earlier   Later