| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-21 | |||
| 09:57:56 | sean-k-mooney | or well services they are not that small | |
| 09:59:20 | cdent | that's not really true | |
| 09:59:30 | sean-k-mooney | how so | |
| 09:59:43 | cdent | interactions via rpc are insufficiently hard boundaries | |
| 09:59:43 | sean-k-mooney | the api does not care what virt dirver you use | |
| 10:00:35 | sean-k-mooney | am im not sure i would agree | |
| 10:01:09 | stephenfin | gibi++ Thanks for hitting https://review.opendev.org/#/c/609460/ again. Much obliged :) | |
| 10:01:20 | sean-k-mooney | the could be more orbust in some ways but i would not consier it to less stict then something like grpc | |
| 10:02:28 | sean-k-mooney | its still versioned using ovo | |
| 10:02:29 | stephenfin | Do we have a bug to track those failing MYSQL migration unit tests? I'm used to them intermittently failing on py35 but now py27 is unhappy too http://logs.openstack.org/29/666629/1/check/openstack-tox-py27/93e40ec/testr_results.html.gz | |
| 10:03:05 | sean-k-mooney | stephenfin: if they are teh ones im think of yes | |
| 10:03:30 | sean-k-mooney | ya those fail on all paython version intermiently and there is a bug | |
| 10:03:42 | stephenfin | with nova or sqlalchemy? | |
| 10:03:49 | sean-k-mooney | nova | |
| 10:04:04 | sean-k-mooney | well with that error | |
| 10:04:10 | sean-k-mooney | so in the nova gate | |
| 10:04:21 | sean-k-mooney | and we have a bug in nova | |
| 10:04:30 | stephenfin | Damn | |
| 10:04:38 | stephenfin | I don't even know where to start with that | |
| 10:04:43 | stephenfin | It's rather irritating | |
| 10:04:58 | sean-k-mooney | stephenfin: gibi has a partial patch to simplfy the tests | |
| 10:05:09 | sean-k-mooney | to help with figuring it out | |
| 10:05:15 | sean-k-mooney | not sure if it has merged yet | |
| 10:05:43 | sean-k-mooney | stephenfin: https://review.opendev.org/#/c/662434/ | |
| 10:05:54 | sean-k-mooney | stephenfin: and this is the bug https://bugs.launchpad.net/nova/+bug/1823251 | |
| 10:05:55 | openstack | Launchpad bug 1823251 in OpenStack Compute (nova) "Spike in TestNovaMigrationsMySQL.test_walk_versions/test_innodb_tables failures since April 1 2019 on limestone-regionone" [High,Confirmed] | |
| 10:05:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Stop using getattr for VIF lookups https://review.opendev.org/666630 | |
| 10:05:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Resolve a TODO and update another https://review.opendev.org/666631 | |
| 10:06:01 | stephenfin | sean-k-mooney++ | |
| 10:07:43 | sean-k-mooney | stephenfin: instaed of doing an if else chain use a dict of vif_type -> functions | |
| 10:07:53 | stephenfin | I thought about that | |
| 10:08:09 | stephenfin | but it saved something like 4 lines | |
| 10:08:11 | sean-k-mooney | its faster and simpler to read | |
| 10:08:43 | sean-k-mooney | the dict hashse the string and finds the function O(1) | |
| 10:09:15 | sean-k-mooney | the if,elif,else tree you built has to do a tone of comparisons | |
| 10:09:46 | stephenfin | I wonder is it significant in the grand scheme of things though? | |
| 10:09:53 | stephenfin | The additional lookup time | |
| 10:10:12 | sean-k-mooney | we do this for every interface on every boot/reboot | |
| 10:10:24 | sean-k-mooney | but it mostly that the code is nicer to read | |
| 10:10:59 | stephenfin | Hmm, okay. If we're doing it elsewhere in nova I'm happy to switch | |
| 10:11:50 | stephenfin | Though I must say I do rather like us explicitly highlighting the VIF types we still use the legacy path for https://review.opendev.org/#/c/666630/3/nova/network/os_vif_util.py@517 | |
| 10:12:09 | stephenfin | Is DVS even a thing still? | |
| 10:12:54 | sean-k-mooney | you can still do that | |
| 10:13:13 | sean-k-mooney | and praobly but didnt we say we were kicking them out of tree | |
| 10:13:35 | sean-k-mooney | we proably should jsut port these all into os-vif to be honest | |
| 10:16:47 | jangutter | stephenfin: if only python had a "case" statement, right :-p | |
| 10:17:04 | sean-k-mooney | stephenfin: oh dvs is a vmware thing | |
| 10:17:25 | sean-k-mooney | jangutter: switch statement unless you are channeling sql | |
| 10:17:43 | jangutter | sean-k-mooney: eep, was channeling Pascal of all things. | |
| 10:18:08 | sean-k-mooney | ha fair enough | |
| 10:18:49 | sean-k-mooney | you know if python had static type swe could have overload a function on teh argument type too | |
| 10:19:16 | sean-k-mooney | so many ways to solve this issue in other languages | |
| 10:20:36 | jangutter | stephenfin, sean-k-mooney: I haven't cross-checked, but how many of the VIFs in use by Neutron Stadium things are still legacy? | |
| 10:20:59 | jangutter | stephenfin, sean-k-mooney: I guess I'll have to go through the plugins in https://governance.openstack.org/tc/reference/projects/neutron.html | |
| 10:24:28 | sean-k-mooney | i dont think that many alot use ovs underneat | |
| 10:25:00 | jangutter | sean-k-mooney: yah, and a _lot_ of these are 'noop' plugging. | |
| 10:25:01 | sean-k-mooney | at this point i would be happy to just add drivers to os vif for the legecay stuff to get it out of nova and then we can drop them form os-vif later | |
| 10:25:13 | sean-k-mooney | jangutter: we have a driver for that :) | |
| 10:25:25 | jangutter | sean-k-mooney: yep. | |
| 10:29:57 | stephenfin | sean-k-mooney: RE: https://review.opendev.org/#/c/666630/3/nova/network/os_vif_util.py@545 | |
| 10:30:13 | stephenfin | We're still going to have a lot of lookups in https://review.opendev.org/#/c/666630/3/nova/network/os_vif_util.py@513 | |
| 10:30:37 | stephenfin | and I really don't want to change that as noted above | |
| 10:30:56 | sean-k-mooney | you dont have too | |
| 10:31:27 | sean-k-mooney | that shoudl be relitivly quick but you should make it a set | |
| 10:31:31 | sean-k-mooney | not a tuple | |
| 10:32:00 | stephenfin | okay, let me try | |
| 10:32:08 | stephenfin | In the interim, want to make sure https://review.opendev.org/#/c/666631/ makes sense? | |
| 10:32:10 | stephenfin | :D | |
| 10:32:25 | sean-k-mooney | stephenfin: if you define the set at module sope as a constant taht woudl also be better | |
| 10:32:44 | stephenfin | Even though it's only used in one place? | |
| 10:32:47 | sean-k-mooney | e.g. if vif_type in LEGACY_VIFS | |
| 10:33:10 | sean-k-mooney | sure why not its cheap | |
| 10:33:33 | sean-k-mooney | litally put it like 10 lines up on line 495 | |
| 10:34:49 | sean-k-mooney | i dont think our min version is at 3.2.0 yet | |
| 10:34:52 | sean-k-mooney | for libvirt | |
| 10:34:58 | sean-k-mooney | i think its currently at 3.0.0 | |
| 10:35:10 | stephenfin | Yeah, it is. That's why I just updated the docstring for one | |
| 10:36:01 | sean-k-mooney | oh sorry you fix the todo below that | |
| 10:36:06 | stephenfin | yup | |
| 10:36:52 | sean-k-mooney | do we set the vlan in the libvirt xml | |
| 10:37:24 | sean-k-mooney | as libvirt will only do that if we do | |
| 10:38:58 | sean-k-mooney | ok i think we do | |
| 10:39:17 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L352-L354 | |
| 10:40:12 | sean-k-mooney | ya we should be setting it here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/designer.py#L116 | |
| 10:47:39 | stephenfin | okay, I really don't like using the dict lookup. I know it's slightly faster but it feel out of place in that file, given the amount of if-elses we have | |
| 10:48:00 | stephenfin | I have dragged the legacy VIFs out to their own variable though and used sets instead of tuples for 'in' lookups | |
| 10:48:15 | sean-k-mooney | ok | |
| 10:48:18 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Stop using getattr for VIF lookups https://review.opendev.org/666630 | |
| 10:48:19 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Resolve a TODO and update another https://review.opendev.org/666631 | |
| 10:49:20 | sean-k-mooney | when we were using getattr before it was a dict lookup which is why i wanted to keep it that way | |
| 10:50:19 | stephenfin | Understandable. I think the additional clarity is worth the small cost though | |
| 10:56:32 | sean-k-mooney | stephenfin: will you get around to rebaseing https://review.opendev.org/#/c/663382/ soon or will i do it? | |
| 10:56:53 | stephenfin | oh, forgot about that. I'll do that today, yeah | |
| 10:57:50 | sean-k-mooney | ok i was talking to a partner about it yesterday so the sooner it lands upstream the sooner we can backport and the sooner they will be happy | |
| 10:58:52 | sean-k-mooney | ok im actuly goint ot go lie down now for a bit ill be back in a few hours | |
| 11:01:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Remove dead minimum libvirt checks https://review.opendev.org/666814 | |
| 11:05:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Stop using getattr for VIF lookups https://review.opendev.org/666630 | |
| 11:05:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Resolve a TODO and update another https://review.opendev.org/666631 | |
| 11:05:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: vif: Remove dead minimum libvirt checks https://review.opendev.org/666814 | |
| 11:06:24 | openstackgerrit | Merged openstack/os-traits master: Sync Sphinx requirement https://review.opendev.org/666386 | |