| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-08 | |||
| 12:43:47 | sean-k-mooney | so if we have to enable an offload in a ovs specific way then that should not be in the generic profile | |
| 12:44:12 | jangutter | sean-k-mooney: yep.... you do realise it's making more of Jay's case for him :-p | |
| 12:44:33 | sean-k-mooney | if we have some traits like thing we can use as an indirection and have the driver interperate that then thats fine | |
| 12:45:12 | jangutter | sean-k-mooney: but I understand what you mean, the distinction between offload metadata and plugging modes should be separate. | |
| 12:45:19 | sean-k-mooney | jangutter: not really i just said storing backend sepcific metadata in the generic represtor profile would not be ok | |
| 12:45:27 | sean-k-mooney | that include via compostion | |
| 12:45:39 | sean-k-mooney | jangutter: ya | |
| 12:46:30 | jangutter | sean-k-mooney: cool, so you had in mind "add one more class" not "convert one more class to a random bag of dicts"... | |
| 12:46:48 | sean-k-mooney | yes | |
| 12:47:21 | sean-k-mooney | random bag of dicts that will some day be sent over api is less then ideal | |
| 12:47:37 | jangutter | sean-k-mooney: verily. | |
| 12:48:35 | jangutter | sean-k-mooney: thanks very much, will archive this review as a warning to future developers. | |
| 12:48:56 | sean-k-mooney | im fine with VIFPortProfileOVSRepresentor having no addtional field and inheriting from VIFPortProfileRepresentor by the way | |
| 12:49:28 | jangutter | sean-k-mooney: that won't work, unfortunately, will have to be multiple inheritance. | |
| 12:49:37 | sean-k-mooney | the thing is right now we dont have any offload metadata that we are sending so the pluggins modes info is all that class contains | |
| 12:49:44 | fried_rice | mdbooth: I set up gertty during the PTG and it took like three days before it finished loading everything down. | |
| 12:49:54 | sean-k-mooney | jangutter: oh why? | |
| 12:50:16 | mdbooth | efried: Ah, ok. That's when I started, and it took so long I assumed I'd done it wrong and deleted it. | |
| 12:50:31 | efried | mdbooth: That had been my experience the first time I tried it. | |
| 12:50:35 | jangutter | sean-k-mooney: with VIFHostDevice or VIFVhostUser we need the bridge name and other thingies too. | |
| 12:50:54 | efried | mdbooth: Perhaps broadband you could get it to load up faster, though actually I suspect not. I wasn't getting close to maxing out my bw when I was doing it. | |
| 12:51:01 | jangutter | sean-k-mooney: so OVSRepresentor would have to have multiple inheritance (OVS and Representor). | |
| 12:51:09 | sean-k-mooney | efried: ya there are some knonw issue. if its worth anything the pip version works better the package manager | |
| 12:51:22 | mdbooth | Well I'm currently on 4G mobile data, so I guess I'd better kill it | |
| 12:51:25 | efried | I don't remember how I installed it. | |
| 12:51:27 | sean-k-mooney | jangutter: no it wouldn't | |
| 12:51:33 | jangutter | efried: yeah, apparently ONLY USE THE PIP VERSION. | |
| 12:51:35 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Add native implementation OVSDB API https://review.openstack.org/482226 | |
| 12:51:41 | sean-k-mooney | VIFPortProfileOVSRepresentor is a port profile not a vif | |
| 12:52:08 | mdbooth | No, seriously | |
| 12:52:55 | jangutter | sean-k-mooney: ah you're right: interface_id is what's currently used. | |
| 12:52:59 | sean-k-mooney | mdbooth: it takes a while to sync because its cloneing all the git repos you subsribe to | |
| 12:53:23 | mdbooth | My gate is finally open. | |
| 12:53:28 | sean-k-mooney | jangutter: interface_id is used for what exactly | |
| 12:54:17 | jangutter | sean-k-mooney: when the port gets plugged into OVS, there's an interface_id UUID added to the OVSDB config. | |
| 12:54:45 | sean-k-mooney | yes that is pulled form the ID field in the base os-vif VIF object | |
| 12:54:47 | jangutter | sean-k-mooney: last time I checked, Neutron listens to OVSDB for that interface_id UUID to pop up in order to confirm the plugging. | |
| 12:55:22 | sean-k-mooney | jangutter: for the ml2 agent yes it check the interface_id in the external_ids colume of the port table | |
| 12:55:56 | jangutter | sean-k-mooney: yep, but is that guaranteed to be the same as VIFBase.id ? | |
| 12:56:12 | sean-k-mooney | jangutter: yes that is where we read if from | |
| 12:56:23 | jangutter | sean-k-mooney: why is interface_id then a field in the OVS port profile? | |
| 12:57:09 | sean-k-mooney | legacy reasons | |
| 12:57:15 | sean-k-mooney | basically it was used by libvirt | |
| 12:57:38 | sean-k-mooney | it got copied when we did the import from nova but its the same id | |
| 12:57:51 | jangutter | sean-k-mooney: o.m.w. here I was thinking that it's the whole reason behind port profiles! | |
| 12:58:02 | sean-k-mooney | jangutter: nope | |
| 12:58:46 | jangutter | sean-k-mooney: so, one VIF can only ever ever have one interface ID, and that's the same as its UUID? | |
| 12:58:59 | sean-k-mooney | looks like we are using it https://github.com/openstack/os-vif/blob/master/vif_plug_ovs/ovs.py#L124 | |
| 12:59:15 | sean-k-mooney | jangutter: yep it should be | |
| 12:59:46 | sean-k-mooney | let me check nova to confirm but they should never be different as far as i know | |
| 12:59:53 | jangutter | sean-k-mooney: yep, that was the line of code that made me thought it could differ from vifbase.id. | |
| 13:00:27 | sean-k-mooney | i dont think it can but im checking | |
| 13:00:47 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Remove IPTools deprecated implementation https://review.openstack.org/605422 | |
| 13:02:04 | sean-k-mooney | jangutter: so ya the neutron port uuid is stored in the id field https://github.com/openstack/nova/blob/master/nova/network/os_vif_util.py#L248-L264 | |
| 13:03:08 | jangutter | sean-k-mooney: interface_id=vif.get('ovs_interfaceid') or vif['id'] | |
| 13:03:14 | sean-k-mooney | jangutter: and here we even default it to the vif[id] if ovs_interfaceid is not set https://github.com/openstack/nova/blob/master/nova/network/os_vif_util.py#L288 | |
| 13:03:23 | sean-k-mooney | yes | |
| 13:03:47 | jangutter | sean-k-mooney: yep.... Guess what, I've also wrote code like that. | |
| 13:04:18 | jangutter | sean-k-mooney: but my interpretation was that "ovs_interfaceid" is the new hotness and vif['id'] is old-and-busted. | |
| 13:05:26 | sean-k-mooney | jangutter: nope othere way around ovs_interfaceid i think was a nova networks thing | |
| 13:05:57 | sean-k-mooney | using neutron i dont think they can ever be different at least not currently | |
| 13:06:09 | jangutter | https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_vif.py#L84 | |
| 13:06:46 | jangutter | sean-k-mooney: yeah, in the tests, there's a definite difference between uuids.ovs and uuids.vif | |
| 13:07:05 | sean-k-mooney | jangutter: that does not mean the tests are correct :) | |
| 13:07:30 | jangutter | sean-k-mooney: yep! thanks for showing me the error of my ways again! Important safety tip. | |
| 13:07:54 | sean-k-mooney | jangutter: https://github.com/openstack/nova/blob/a330c9a143dea8095a3d1c3eabd56193ad6f38b1/nova/network/neutronv2/api.py#L2651 so this is where its set :) | |
| 13:08:01 | jangutter | sean-k-mooney: makes me think of basic particle physics where time symmetry is a thing. | |
| 13:09:51 | sean-k-mooney | jangutter: by the way want to open a bug for the incorrect tests? | |
| 13:10:02 | jangutter | sean-k-mooney: this is pretty hilarious now that I think of it, I was almost actively undoing the direction that Nova, Neutron and OS-VIF is going. | |
| 13:10:41 | jangutter | sean-k-mooney: I'll try to get on it today: that looks like something nice to fix for future Jan's not to step into. | |
| 13:11:10 | sean-k-mooney | jangutter: ya i was thinking it would make a nice low hanging fruit style bug. | |
| 13:15:35 | sean-k-mooney | jangutter: this change kind of hurts me to read... https://github.com/openstack/nova/commit/1c07735f8e3b28f64fcd1252372aa9e6e917d960 | |
| 13:18:22 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Handle missing marker during online data migration https://review.openstack.org/608572 | |
| 13:19:28 | jangutter | sean-k-mooney: I would literally interpret that as: "the ovs interface-id should be different from the vif id" | |
| 13:20:42 | sean-k-mooney | jangutter:this code is from 6 years ago when quantum was not the default network backend in nova and nova-networks was still alive and well | |
| 13:21:36 | sean-k-mooney | it looks like there used to be a mapping table and nova used to generate uuids for the libvirt xml | |
| 13:23:03 | jangutter | sean-k-mooney: yep, with neutron and nova sharing the same UUID for the VIF, that's not needed now. | |
| 13:24:05 | sean-k-mooney | jangutter: i think origianly nova nad nutron used to use the name of the port as the common thing and the uuid only came a little later as more backends stared to appear | |
| 13:34:01 | mriedem | gibi: probably need some help from you on how the refactored functional assertFlavor.... checks should be done in https://review.openstack.org/#/c/606106/ | |
| 13:34:14 | mriedem | i've found i still just need a simple assertFlavorMatchesAllocation method | |
| 13:34:56 | mrch_ | how to get rid of the "Instance not resizing, skipping migration." WARNINGS Spam, non of the req-IDs stand in the list: MariaDB [nova]> select * from instance_actions where action = "live-migration" and deleted = "0" | |
| 13:35:34 | gibi | mriedem: looking.. | |
| 13:35:49 | mriedem | mrch_: i tried removing that here https://review.openstack.org/#/c/560467/ | |
| 13:35:54 | mriedem | but that change needs to be rebased | |
| 13:40:05 | gibi | mriedem: I think the solution for that is here https://github.com/openstack/nova/blob/5c0235a579ccb52f7bce5de9bcb3c927c94b23b7/nova/tests/functional/test_servers.py#L4871 | |
| 13:42:47 | mrch_ | mriedem: what does "rebased" mean? | |
| 13:51:45 | mriedem | mrch_: i need to rebase it on the current master branch and resolve merge conflicts | |
| 13:51:51 | mriedem | i.e. it's an old patch | |
| 13:52:24 | mriedem | gibi: hmm, ok, so maybe i should move that into the base provider usage test class? | |
| 13:53:21 | mriedem | mrch_: i'll rebase it quick | |
| 13:53:26 | jaypipes | alex_xu: answered your question on https://review.openstack.org/#/c/555081/. Hopefully that explains things a bit better. let me know if you have further questions. | |
| 13:53:55 | openstackgerrit | Jay Pipes proposed openstack/nova-specs master: Standardize CPU resource tracking https://review.openstack.org/555081 | |
| 13:54:00 | gibi | mriedem: sure, you can move _check_allocation_during_evacuate I think it does not depend on anything in the current class | |
| 13:54:10 | mrch_ | mriedem: thx, but when the old one is queens im ok with it | |
| 13:54:48 | mriedem | mrch_: if my change is accepted on master then we (or you) would have to backport it to stable/rocky and then stable/queens | |
| 13:54:57 | mriedem | unless you're just going to run with that downstream | |
| 13:56:29 | openstackgerrit | Merged openstack/nova master: api-ref: Move the evacuate action to admin action https://review.openstack.org/607896 | |
| 13:56:36 | openstackgerrit | Merged openstack/nova master: Update doc https://review.openstack.org/605640 | |
| 13:56:48 | openstackgerrit | Merged openstack/nova master: libvirt: remove unused attribute driver for LibvirtConfigNodeDevice https://review.openstack.org/583246 | |
| 13:56:58 | openstackgerrit | Merged openstack/nova master: Set defult value of num_nvme_discover_tries=5 https://review.openstack.org/602351 | |