Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-12
16:14:32 sean-k-mooney and ye just finished that conversation
16:14:48 sean-k-mooney but it is a minor pluse to the workaround still for me
16:15:04 sean-k-mooney *workaround conf
16:15:23 melwitt the reason I ask is because I wouldn't support a workaround like this for a super latent thing if it's a big complicated change. but if it's a tiny one liner and would solve a lot of pain for people, then it seems like a worthwhile thing to do
16:16:33 melwitt my concern would be more like, do we really expect to deprecate it from [workarounds] or not
16:16:42 dansmith I don't think it's a lot of people, and I just want to avoid the deprecation dance, effectively introducing something as deprecated
16:16:44 dansmith but yes, it's small and the deprecation is the major impact, so whatever
16:17:05 dansmith melwitt: I assume we'd land it, and deprecate it in the next patch, then just backport the first one
16:17:21 melwitt oh, right, I see
16:17:30 mriedem yes that would be what i'd do
16:18:04 sean-k-mooney dansmith: that depend on the long term fix no? e.g. if we make it work transparently in the fureure then the deprecation/removal should not break people
16:18:14 dansmith sean-k-mooney: we can't
16:18:28 dansmith sean-k-mooney: we're talking about moving what honors the thing from global/scheduler to local/compute
16:18:34 dansmith didn't we already discuss that part?
16:19:35 sean-k-mooney oh i missed that part of the discussion. ill go back to fixing my typos :)
16:20:07 sean-k-mooney i had assumed the workaround option would be set globally
16:21:25 melwitt if the trait thing is simple and we're sure we're going to land it this cycle, then deprecate immediately "works". immediate deprecation is definitely a weird thing to do having left it latent for so long, but if it's such a minor tiny change, I'm not opposed about it
16:22:36 melwitt *backport + immediate deprecation
16:22:49 sean-k-mooney it would not be the first workaound option that has been in that state
16:23:13 sean-k-mooney e.g the numa migration one was intended to be intoduced backported and deprecated all in one cycle
16:24:04 openstackgerrit Merged openstack/nova stable/queens: Disable limit if affinity(anti)/same(different)host is requested https://review.opendev.org/659247
16:24:08 sean-k-mooney we even change default behavior with that backport
16:29:01 openstackgerrit Matt Riedemann proposed openstack/nova master: rt: only map compute node if we created it https://review.opendev.org/675704
16:29:03 openstack bug 1839560 in OpenStack Compute (nova) "ironic: moving node to maintenance makes it unusable afterwards" [High,In progress] https://launchpad.net/bugs/1839560 - Assigned to Matt Riedemann (mriedem)
16:29:03 openstackgerrit Matt Riedemann proposed openstack/nova master: Add functional regression recreate test for bug 1839560 https://review.opendev.org/675705
16:30:35 sean-k-mooney if we treated the new can_cold_migrate_to_same_node workaround option the same way as enable_numa_live_migration is there a reason we cant just set this gloably the scheduler?
16:30:51 sean-k-mooney i know that would not be desirebal for mix hyperviors
16:31:03 sean-k-mooney but its simplifies thing greatly
16:31:09 dansmith sean-k-mooney: could you read the backscroll please?
16:31:15 dansmith we've already discussed that too
16:31:52 dansmith or even the comments on the patch
16:31:53 sean-k-mooney ok im just confused why we would treat this differently then enable_numa_live_migration which is effectivly the same
16:32:01 sean-k-mooney ill read both
16:32:34 dansmith it probably should be the same, I haven't really seen anything about that knob myself
16:35:43 sean-k-mooney enable_numa_live_migration is used in the api i think and reject any live migration if the instnace has a numa toplogy when set to false which we made the defaul.
16:36:40 sean-k-mooney the reasoning being while it was a behavior change teh people that understand how to deploy numa toployg are the people that modify thing in the config anyway and we whould not let most people break things by migaint with out understandint its broken with numa
16:37:25 sean-k-mooney we may have done the check late in teh scheduler in teh enable_numa_live_migration case but i cant see why we would not have done it in the api
16:37:30 openstackgerrit Stephen Finucane proposed openstack/nova master: Unplug VIFs as part of cleanup of networks https://review.opendev.org/663382
16:40:58 Sundar Hi dansmith: Thanks for your reviews. For https://review.opendev.org/#/c/631243/30/nova/objects/flavor.py@230, I am trying to understand the alternative you recommend. "Let the versioned fields be properties" --> extra_specs is just a DictOfStrings, so how do you propose to change that to include a devic eprofile name explicitly?
16:41:07 mriedem sean-k-mooney: it's not in the api, it's in conductor,
16:41:16 mriedem and it's after we've picked a host to check if it's qemu
16:41:38 mriedem the allow resize to same host option is read in the api before calling the scheduler
16:42:36 sean-k-mooney mriedem: oh yep your right
16:43:11 mriedem efried: https://review.opendev.org/#/c/675935/
16:43:21 mriedem easy test fix to make func job runs go faster
16:46:47 efried mriedem, gibi: Left a comment in there. If you disagree, I'll +W
16:48:14 dansmith Sundar: just access the extra specs whenever you're going to look at the device profile
16:48:33 dansmith Sundar: i.e. code should be: flavor.extra_specs['device_profile'] ... instead of flavor.device_profile
16:54:00 openstackgerrit Surya Seetharaman proposed openstack/nova master: API microversion 2.76: Add 'power-update' external event https://review.opendev.org/645611
16:58:55 dansmith tssurya: found something I think is a problem that wasn't obvious until your clean-up
16:59:01 dansmith tssurya: let me know if I'm missing something there
16:59:24 tssurya dansmith: checking
17:01:12 mriedem efried: i'm ok either way
17:01:36 efried mriedem, gibi: I just don't like lambda *a, **k: None if we don't need it.
17:01:51 mriedem efried: if you want to change i'll test it and approve
17:01:58 efried ...
17:03:07 efried mriedem: f it, would need to import (the real) fixtures. +W
17:03:15 mriedem heh
17:04:31 tssurya dansmith: replied, hope I am not messing something up
17:10:02 dansmith tssurya: ah, I was confused, yeah, but in your comment you mean avoid L4768 not 4770 right?
17:10:21 dansmith you're trying to avoid queuing this event because it's a no-op?
17:10:39 tssurya dansmith: right sorry I am trying to avoid L4768
17:11:01 tssurya dansmith: yes I don't want the event to go down to the manager
17:11:12 dansmith so I was going to say this earlier, but I'm not really sure why we need to do the (almost) silent drop of those in the api
17:11:21 tssurya I am being strict with the task/vm/power states of the vm
17:11:25 dansmith because it seems like the manager/driver would know better about whether or not that is still important
17:11:41 dansmith but, the thing I -1d for is wrong, so I'll drop that
17:12:16 tssurya I understand your point, but since I sort of copied the start/stop logic in the api,
17:12:22 tssurya it would blow up here: https://review.opendev.org/#/c/645611/11/nova/compute/api.py@4705
17:12:37 tssurya when I save the instance since the expected state is None and so on
17:13:05 tssurya its mostly because when I was testing if there is a power sync/receving of event almost close to each it could mess up things
17:13:20 tssurya mostly a fail safe that I did in the beginning itself
17:16:42 dansmith tssurya: ah, okay that's fair
17:20:41 dansmith tssurya: I'll let mriedem lead with a +2 on the api stuff and versioning bits but otherwise I think I'm good with it
17:21:07 tssurya dansmith: thank you :D ! yea let's wait for the CI to run
18:20:26 openstackgerrit sean mooney proposed openstack/nova master: Libvirt: add support for vPMU configuration. https://review.opendev.org/671338
18:20:57 sean-k-mooney gibi: stephenfin mriedem i think i have adress most of the comments
18:34:15 mriedem ok
18:41:33 openstackgerrit Matt Riedemann proposed openstack/nova master: Restore soft-deleted compute node with same uuid https://review.opendev.org/675496
18:43:00 mriedem mnaser: ^
18:46:02 mnaser mriedem: seems very sane
18:53:54 openstackgerrit sean mooney proposed openstack/nova master: WIP: Add a placement audit command https://review.opendev.org/670112
18:55:20 sean-k-mooney cdent i think i have adressed your comments on ^ but im not fully sure what sylvain was trying to do so ill take a look again tomorow after i read the bug
19:12:31 openstackgerrit Matt Riedemann proposed openstack/nova stable/stein: Add useful error log when _determine_version_cap raises DBNotAllowed https://review.opendev.org/675714
19:54:46 mriedem dansmith: melwitt: just had another thought on this allow cold-migrate to same host thing, which wouldn't involve a workaround and soon-to-be-deprecated config option,
19:55:08 mriedem if allow_resize_to_same_host=true, get the compute node for the instance and check it's hypervisor type, if it's anything besides vcenter we don't allow it
19:55:30 mriedem that would be a hack fix for any in-tree drivers and could be replaced with the more dynamic traits filtering later
19:56:09 mriedem don't allow cold migration to the same host that is
19:58:19 mriedem maybe that's *too* hacky
19:58:52 dansmith that check in the api seems like a high crime
20:00:25 mriedem impeachable?
20:00:36 dansmith automatic impeachment
20:00:46 dansmith and subsequent special counsel investigation
20:00:51 mriedem heh
20:01:03 dansmith all your dirty laundry will be aired
20:01:23 mriedem bnemec: for a BoolOpt, is it possible to leave it unset so you can set the default at runtime somehow, e.g. it's based on another options value?
20:02:08 bnemec mriedem: It is. I believe the default for BoolOpt is actually None, which implies that it was unset since that's not a valid boolean value.
20:02:58 openstack Launchpad bug 1764782 in oslo.config "BoolOpt defaults to None" [Medium,Triaged]
20:02:58 bnemec Which I actually think is a bug (https://bugs.launchpad.net/oslo.config/+bug/1764782) but as Doug points out there's no way to know if people have been relying on that behavior.
20:03:17 mriedem ah i see, looks like boolean type opts are really str opt under the covers and they get cast to booleans

Earlier   Later