Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-11-27
16:33:38 efried so wait, which are we doing? just dan's thing, master & backports; or matt's thing for backports and dan's thing for master?
16:35:06 dansmith we're doing whichever of those mriedem wants
16:35:14 dansmith which I think is the latter
16:35:21 mriedem the latter
16:36:15 efried okay, so my job is to +2 mriedem's patch (done) and then wait for mdbooth to be satisfied with dansmith's patch before reviewing it.
16:37:18 dansmith efried: I think the -1 on mine isn't actually a thing
16:39:04 efried yeah, but I'm drowning, so looking for any excuse to defer
16:41:31 mdbooth dansmith: It's not a thing?
16:41:39 dansmith mdbooth: the security groups thing?
16:42:27 mdbooth dansmith: Yeah. IIRC I would have been happy with a command saying: # We broke this, but we don't care because it's not a thing any more.
16:42:38 mdbooth s/comment
16:43:10 dansmith I don't understand why it's a thing at all anyway.. if the instance is deleted, we're not going to do anything with the security groups we do fetch out of the db (if there are any anyway),
16:43:21 dansmith and, in any case where we did load the instance with a deleted=yes context,
16:43:24 dansmith we'd have gotten the same
16:44:55 dansmith so the only case where we'd do something different is where we had an instance, which wasn't deleted when we started, but was deleted underneath us and we go to lazy-load security groups.. if we were to go on and save that instance later, it'd fail and I can't think of anything else we'd do with it that would trigger some different behavior than we have now
16:45:01 dansmith which I think was also mriedem's reply
16:47:03 mriedem i was specifically worried about instance.services but mel said it was something else there
16:47:12 mriedem i.e. the bug she was working on
16:47:48 mdbooth dansmith: I think think so. I think the bug is that if you've got an instance which isn't deleted and you lazy load security groups, you get deleted security groups. I don't think you need to delete the instance in between or anything.
16:47:56 mdbooth s/think think/don't think/
16:48:47 mdbooth dansmith: But we only care about that if we care about nova security groups, which IIUC are nova-network only.
16:49:59 mriedem they are nova-net only
16:50:07 dansmith mdbooth: ah, right, I remember you clarifying this again post initial review
16:50:25 dansmith they're n-net only, and don't work with cellsv1, right? so we really shouldn't have any supported cases where they're there anymore
16:51:00 mriedem so,
16:51:21 mriedem SecurityGroup.get_by_instance calls db api security_group_get_by_instance which hard-codes read_deleted="no"
16:51:33 mriedem *SecurityGroupList.get_by_instance
16:51:44 mriedem so isn't the hard-coding in the DB API preventing that
16:52:47 dansmith oh does it?
16:53:08 mdbooth mriedem: Looks like I missed that. That's convenient :)
16:53:13 dansmith sure does
16:57:15 dansmith recorded for posterity
16:57:15 sean-k-mooney speaking of things in nova net the function that enables trusted vfs was added to the nova net module
16:57:32 sean-k-mooney it proably should be moved to an sriov plugin in os-vif
16:59:01 sean-k-mooney specifically https://github.com/openstack/nova/blob/c6218428e9b29a2c52808ec7d27b4b21aadc0299/nova/network/linux_net.py#L1773 should be moved
17:00:43 sean-k-mooney mriedem: by the way i havent forgot about looking into can we remove NeutronLinuxBridgeInterfaceDriver too but i havent got around to it yet either
17:01:17 mriedem ken'ichi says it's ok https://review.openstack.org/#/c/616995/
17:01:33 mdbooth mriedem: Going back a patch, I was wondering about adding an 'intent' to driver.cleanup and/or driver.destroy, i.e. driver.destroy(reason=MIGRATION|DELETE). We currently have destroy_disks, but the problem with this is that we second guess it in ComputeManager, which makes it basically useless.
17:02:22 mdbooth What we want the driver to do is, for delete: delete everything. For migrate/evacuate: delete everything local to this hypervisor.
17:03:03 mdbooth I think the definition of 'local to the hypervisor' is more cleanly defined internally by the hypervisor.
17:03:05 slaweq hi nova cores, can You take alook at https://review.openstack.org/#/c/619061/ - it should fix neutron-tempest-postgres-full periodic job in gate so it's quite important for us :)
17:03:08 slaweq thx in advance
17:03:27 sean-k-mooney mriedem: oh ok cool i think there is more in that file we are not using that we could remove too
17:03:56 mriedem mdbooth: i don't expect you'll get much in the way of replies on that in the ML
17:04:08 mriedem people, like me, will likely say they don't want to think about it
17:04:25 mdbooth mriedem: It's a source of a class of real bugs, thoough
17:04:26 mriedem mdbooth: so having said that, i assume you aren't proposing to change your current fix to incorporate that right?
17:04:32 mriedem mdbooth: oh i don't doubt it
17:04:34 mdbooth mriedem: Hell, no
17:04:55 mdbooth mriedem: I was thinking about lyarwood's original proposal to fix an extra thing in my patch
17:04:58 mriedem there is definitely a russian roulette of disk deletion in compute/virt
17:05:16 mdbooth I looked harder at that, and I don't think we can fix it like that
17:07:42 openstackgerrit Jack Ding proposed openstack/nova master: Improve libvirt image and snapshot handling https://review.openstack.org/616692
17:12:03 openstackgerrit Jack Ding proposed openstack/nova-specs master: [WIP] Flavor Extra Spec and Image Properties Validation https://review.openstack.org/618542
17:14:48 mdbooth mriedem: Practically speaking, what would be the most practical way to go about making that sort of change? Just post a patch? Problem is it wouldn't be a small amount of work, so I'd be cautious of that without some level of buy in beforehand. BP?
17:17:19 mriedem bp doesn't give you much in the way of details, spec allows you to really write the details down and get review on it without getting too far into implementation, but idk
17:17:47 mriedem gonna be a hard sell to get people to want to spend time/brain power on this
17:38:15 sean-k-mooney mdbooth: blueprints tend to be more for tracking then design if you want input you will want a spec or at least a etherpad + hangout if you have a quorum of people that are interested
17:44:07 openstackgerrit Matt Riedemann proposed openstack/nova master: Refactor TestEvacuateDeleteServerRestartOriginalCompute https://review.openstack.org/608705
17:44:07 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix InstanceNotFound during _destroy_evacuated_instances https://review.openstack.org/606122
17:44:08 openstackgerrit Matt Riedemann proposed openstack/nova master: Run evacuate tests with local/lvm and shared/rbd storage https://review.openstack.org/604400
17:44:08 openstackgerrit Matt Riedemann proposed openstack/nova master: Always read-deleted=yes on lazy-load https://review.openstack.org/575190
17:44:21 mriedem dansmith: rebasearoo
17:45:53 openstackgerrit Matt Riedemann proposed openstack/nova master: DNM: Run against mriedem's evacuate test https://review.openstack.org/604423
17:49:16 openstackgerrit Artom Lifshitz proposed openstack/nova-specs master: Re-propose numa-aware-live-migration spec https://review.openstack.org/599587
17:49:54 artom mriedem, ^^ I think I got everything, when you have a moment
17:51:16 mriedem ok
17:52:38 artom (Oh and the docstring patch as well)
17:52:47 artom But that's not as important
17:52:50 bauzas mriedem: artom: I'll try to look at this spec tomorrow
17:52:58 artom woot
17:53:03 artom Cheers bauzas!
17:55:03 bauzas artom: before cheering me, wait for my comments :)
17:55:26 artom I have 100% confidence they'll be constructive and useful.
17:55:47 artom Why no, my nose isn't brown, whydja ask? ;)
19:06:27 mriedem artom: dansmith: there you go https://review.openstack.org/#/c/599587/
19:40:16 artom mriedem, thank you!
19:41:37 mriedem belmoreira: hey, remember talking in berlin about a problem where if you have a lot of disabled computes for an upgrade, but with max_placement_results set low, you could get novalidhost because placement returns all disabled computes? you were going to open a bug for that but i never saw one.
19:42:09 mriedem i remember dansmith and i kicked around some ideas about that in berlin, but my memory is getting hazy
19:45:45 mriedem need another core on this concurrent i/o disk semaphore thing in runways https://review.openstack.org/#/c/609180/
19:45:48 mriedem it's pretty simple
20:21:02 cdent Are hooks still a thing that are being used in nova, or is it more that they haven't been cleared away yet?
20:22:38 mriedem don't build on them if you can help it
20:22:48 mriedem rely on notifications if possible
20:23:32 mriedem hooks are use at your own risk (we can break them and not care to fix them), and the last time someone complained about the interface breaking on one of the hooks, upstream got real serious about just yanking them
20:23:42 mriedem kind of one of those squeeky wheel kinds of things - best not to talk about it
20:23:54 cdent yeah, I was wondering if the yanking was still an option
20:24:23 mriedem if we had a good reason to
20:24:29 cdent because the race I'm watching just showed up with a trace from one of them, but it appears it is more likely a coincidence
20:24:34 mriedem and someone that cared enough
20:25:29 cdent my running theory at this point is a totally unsurprising "it's probably eventlet"
20:28:12 mriedem no one was ever fired for blaming eventlet
20:32:24 mriedem GET /jaypipes
20:32:25 mriedem https://review.openstack.org/#/c/603352/7
20:36:15 mriedem i imagine jay is fighting with code using hooks within oath
20:36:31 mriedem hooks that call off to some chef thing
20:49:42 openstackgerrit Eric Fried proposed openstack/nova master: Reduce calls to placement from _ensure https://review.openstack.org/615677
20:49:42 openstackgerrit Eric Fried proposed openstack/nova master: SIGHUP n-cpu to clear provider tree cache https://review.openstack.org/615646
20:49:43 openstackgerrit Eric Fried proposed openstack/nova master: Commonize _update code path https://review.openstack.org/615705
20:49:43 openstackgerrit Eric Fried proposed openstack/nova master: Consolidate inventory refresh https://review.openstack.org/615695

Earlier   Later