| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-21 | |||
| 16:18:51 | jroll | bauzas: right, which I guess is why I need reviews on this, too :) https://etherpad.openstack.org/p/nova-ironic-flavor-transition-plan | |
| 16:19:04 | bauzas | oh, never heard of that yet ^ | |
| 16:19:13 | jroll | bauzas: read more scheduler meeting :P | |
| 16:19:29 | jroll | bauzas: but yeah, I guess my spec kind of depends on this plan being solid | |
| 16:19:49 | bauzas | jroll: heh, not my fault, but my kid's one | |
| 16:19:58 | jroll | bauzas: we log for a reason! | |
| 16:20:01 | jroll | I'm mostly kidding | |
| 16:20:24 | jamespage | mriedem: correct | |
| 16:20:27 | bauzas | argh, no good fake reason to provide, I'm doomed | |
| 16:21:00 | mriedem | jamespage: ok, i'll tinker with seeing how easy a fix that is | |
| 16:21:15 | jamespage | mriedem: thankyou much appreciated | |
| 16:22:51 | jroll | bauzas: line 39-42 there is me walking through rebuild in my head, I think it's okay | |
| 16:27:39 | sgordon | mriedem, jamespage mmm yes | |
| 16:27:56 | sgordon | mriedem, jamespage revert of what specifically? | |
| 16:28:10 | sgordon | mriedem, jamespage libvirt has actually been fixed to handle the old xml more gracefully | |
| 16:28:22 | mriedem | sgordon: https://review.openstack.org/#/c/447169/ | |
| 16:28:46 | jamespage | sgordon: yeah we're all good on forward version compat I think | |
| 16:28:46 | openstack | Launchpad bug 1665698 in libvirt (Ubuntu Yakkety) "/etc/qemu-ifup not allowed by apparmor" [Medium,Triaged] | |
| 16:28:46 | mriedem | sgordon: yes but according to https://bugs.launchpad.net/nova/+bug/1665698/comments/50 that's in libvirt 3.1 | |
| 16:28:54 | mriedem | which is lightyears from where we'll be testing against in the gate | |
| 16:29:09 | jamespage | its backwards compat with 1.3.1 in Ubuntu Xenial | |
| 16:29:10 | sgordon | mriedem, yes so the distributor should backport it | |
| 16:29:29 | jamespage | backport what exactly? | |
| 16:29:35 | mriedem | the '' or None fix i guess | |
| 16:29:41 | mriedem | but what about someone not using a distro package? | |
| 16:29:48 | sgordon | jamespage, the libvirt fix | |
| 16:29:48 | jamespage | nova has established min version compats | |
| 16:29:49 | mriedem | or using not red hat or ubuntu distro package? | |
| 16:29:56 | mriedem | like suse | |
| 16:30:04 | mriedem | gentoo, etc | |
| 16:30:20 | sgordon | jamespage, yes but you obvs bumped the distro libvirt version at some point or you would not be hitting the issue | |
| 16:30:30 | mriedem | anyway, let me see how hard the conditional version check is in the code before we talk about making packagers handle this :) | |
| 16:30:34 | sgordon | jamespage, which is also why it only appeared on RHEL/CentOS 7.3 | |
| 16:30:59 | jamespage | sgordon: the libvirt version in xenial has been 1.3.1 since 16.04 release last year | |
| 16:31:03 | mriedem | ubuntu has libvirt 1.3.1 in stock 16.04 | |
| 16:31:07 | mriedem | that's what we test with in the gate | |
| 16:31:17 | mriedem | we just don't test with the same vif types that this hits | |
| 16:31:45 | sgordon | jamespage, the problem was only introduced in libvirt 1.3.2 | |
| 16:31:54 | sgordon | jamespage, so at some point it's been brought in via a backport | |
| 16:33:13 | mriedem | sgordon: why does the release note, that i think you wrote, say 1.3.3? https://review.openstack.org/#/c/411936/6/releasenotes/notes/libvirt-script-with-empty-path-2b49caa68b05278d.yaml | |
| 16:33:34 | jamespage | behaviour changed in 1.3.3 | |
| 16:34:49 | johnthetubaguy | dane-fichter: would be good to catch up on those | |
| 16:34:59 | cfriesen | mriedem: thanks for the updates and approval on the flavor API spec. you got to them before I even checked. :) | |
| 16:35:27 | mriedem | is there a way to see which tags are applied to a libvirt commit? i.e. https://libvirt.org/git/?p=libvirt.git;a=commit;h=9c17d665fdc5f0ab74500a14c30627014c11b2c0 | |
| 16:36:25 | mriedem | nvm it's 1.3.3 | |
| 16:36:31 | mriedem | because 1.3.2 was https://libvirt.org/git/?p=libvirt.git;a=commit;h=dca504a1b977060b844393daedd0da4bdb21ee76 | |
| 16:37:59 | cfriesen | I don't see any tags applied to that commit. were you wondering what branch it was in? | |
| 16:38:12 | mriedem | no i sorted it out | |
| 16:38:23 | cfriesen | I usually use gitk to figure out branches/versions/etc. | |
| 16:38:45 | cfriesen | it shows "follows" and "preceeds" versions. | |
| 16:39:40 | mriedem | now to see if os-vif is doing any of this | |
| 16:40:33 | mriedem | doesn't look like it, whew | |
| 16:41:13 | mriedem | so this affects ivs_ethernet, iovisor, midonet, tap and vrouter | |
| 16:41:23 | mriedem | sean-k-mooney: ^ we don't have os-vif support for those vif types yet right? | |
| 16:42:04 | mriedem | doesn't look like it | |
| 16:53:51 | sgordon | mriedem, digging back through the commentary i think the right approach is the branch you suggest | |
| 16:54:17 | mriedem | should have a patch up shortly | |
| 16:54:18 | sgordon | mriedem, a straight revert is just going to switch back to the other group of upset operators yelling about it because the are on libvirt >= 1.3.3 | |
| 16:54:22 | mriedem | this is going to be a bit of a mess in newton i suspect | |
| 16:54:30 | sgordon | mriedem, yeapppp | |
| 16:54:39 | mriedem | in ocata we have access to the libvirt version in the vif code | |
| 16:54:45 | mriedem | but i don't think i have that access in newton | |
| 16:54:50 | mriedem | so will have to poke around | |
| 16:54:54 | sgordon | mriedem, the revert approach also removed the rel note but given the combinations involved here i still think we will want to add one | |
| 16:55:07 | sgordon | mriedem, "if you see this behavior then do we have a deal for you..." | |
| 16:55:24 | mriedem | buy one get one half off bug sale?! | |
| 17:08:26 | johnthetubaguy | #success First few patches adding real docs for policy have now merged in Nova. A much improved sample file: https://docs.openstack.org/developer/nova/sample_policy.html | |
| 17:08:28 | openstackstatus | johnthetubaguy: Added success to Success page | |
| 17:20:29 | openstackgerrit | John Garbutt proposed openstack/nova master: Remove unused policy rule in admin_actions.py https://review.openstack.org/447611 | |
| 17:26:29 | mriedem | sgordon: jamespage: actually the same fix i have should work on newton, we have the stuff i needed for the version check | |
| 17:27:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: conditinally set script path for ethernet vif types https://review.openstack.org/448203 | |
| 17:27:20 | mriedem | sgordon: jamespage: review/test of ^ would be appreciated | |
| 17:28:28 | sgordon | mriedem, thanks i'll add sfinucan as my proxy with a brain here ;) | |
| 17:29:50 | dane-fichter | johnthetubaguy: hey I'm back, had to step out for a meeting | |
| 17:30:09 | sfinucan | I'm starting to get why sysadmins don't like upgrading package versions | |
| 17:30:46 | johnthetubaguy | dane-fichter: no worries | |
| 17:31:41 | dane-fichter | johnthetubaguy: did my comment answer questions you have? | |
| 17:32:56 | dane-fichter | johnthetubaguy: it seems like the drivers have to support trusted flavors based on how the download() method gets called | |
| 17:33:35 | johnthetubaguy | dane-fichter: well I think you could hide that using driver capabilities | |
| 17:34:07 | dane-fichter | johnthetubaguy: Can you explain a little more? where in the code does that happen? | |
| 17:34:28 | johnthetubaguy | dane-fichter: yeah, I will try find an example, one sec | |
| 17:35:17 | johnthetubaguy | dane-fichter: here is one example: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1730 | |
| 17:35:49 | johnthetubaguy | dane-fichter: its not perfect, but its a pattern I think you could re-use, that makes things consistent | |
| 17:37:04 | dane-fichter | johnthetubaguy: so that seems useful to just prevent the wrong drivers from trying to boot a trusted flavor instance | |
| 17:37:35 | dane-fichter | johnthetubaguy: but there's no real reason other than the semantics of download() that all drivers can't support this | |
| 17:38:04 | dane-fichter | johnthetubguy: seems like we should just modify download() to take the instance or flavor object as well | |
| 17:38:09 | johnthetubaguy | dane-fichter: oh, yes, agreed with you, well not all drivers use that download() method, the ones that do can support it | |
| 17:38:35 | dane-fichter | johnthetubaguy: ohh was not aware of that | |
| 17:39:01 | sfinucan | sgordon: Done | |
| 17:39:30 | dane-fichter | johnthetubaguy: so we could probably do both things here; use driver capabilities to prevent unsupported drivers from booting trusted flavors | |
| 17:39:37 | johnthetubaguy | dane-fichter: xenapi has its own in hypervisor route, to avoid a double hop, because nova-compute doesn't run somewhere with access to the hypervisor disk | |
| 17:40:15 | johnthetubaguy | dane-fichter: yeah, sounds like we are talking slightly cross purposes in there, agreed with with doing both things | |
| 17:40:58 | johnthetubaguy | dane-fichter: honestly, lots of that detail we can leave till code review, its just a saw detail in the spec that didn't look quite right to me, if that makes sense | |
| 17:41:37 | dane-fichter | johnthetubaguy: yeah. so maybe just tone down the detail in that implementation steps section? | |
| 17:42:08 | dane-fichter | johnthetubaguy: i'll definitely specify the driver capabilities wrinkle | |
| 17:42:41 | johnthetubaguy | dane-fichter: yeah, I think we need to add how to deal with "strange" virt drivers, but yeah the method calls and where the code exactly lives probable best left till later | |
| 17:43:49 | johnthetubaguy | s/probable/probably/ | |
| 17:44:44 | dane-fichter | johnthetubaguy: yes, agreed. thanks john | |
| 17:45:15 | johnthetubaguy | dane-fichter: no worries | |
| 18:11:26 | mriedem | sfinucan: i've got to update that bug fix patch, missed a couple tests | |