Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-12
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
20:03:43 mriedem something like strutils.bool_from_string
20:04:21 mriedem ok in my case i want to add a new option but have it's default be backward compatible to whatever is set for another option
20:04:27 mriedem unless you explicitly override it
20:06:37 bnemec I guess I should note that the more "correct" way to do this is to use get_location: https://docs.openstack.org/oslo.config/latest/reference/api/oslo_config.html#oslo_config.cfg.ConfigOpts.get_location
20:06:49 bnemec That will tell you if a user set the option or not in an unambiguous way.
20:08:16 bnemec Or maybe even better, use set_default and set the new opt's default to the value of the old opt: https://docs.openstack.org/oslo.config/latest/reference/api/oslo_config.html#oslo_config.cfg.ConfigOpts.set_default
20:10:49 mriedem the get_location stuff is relatively new right?
20:10:55 mriedem like, newer than ocata?
20:13:00 mriedem yeah rocky https://review.opendev.org/#/c/537397/
20:13:47 mriedem i don't plan on backporting this nova workaround too far, but it is super latent (much older than ocata anyway)
20:21:39 bnemec Yeah, although set_default has been around forever and is probably the right thing to do here conceptually.
20:22:12 bnemec You're setting a default for an opt based on a thing calculated in code (in this case from the value of another opt) but want the user to be able to override it if they set the opt explicitly.
20:23:11 mriedem where would i call set_default? where i'm using the new option?
20:23:50 bnemec Sometime between when you register the old opt and when you use the new one.
20:23:53 mriedem iow it's like dict.set_default
20:23:56 mriedem *setdefault
20:25:24 bnemec Something like that, but you just call it once and the config object remembers that you overrode the default for that opt.
20:26:22 bnemec I guess dict inserts the key with the default so that is actually the same.
20:26:37 mriedem this is what i've got right now http://paste.openstack.org/show/756357/
20:28:22 mriedem http://paste.openstack.org/show/756358/
20:28:22 mriedem this is probably better to avoid calling set_default when i don't need to
20:28:34 mriedem otherwise i have to do this somewhere in wsgi init after parsing options and starting the service
20:29:03 mriedem which takes it quite far away from where it's used, which is ok but kind of hard to remember all of this
20:29:40 bnemec Heh, if you're still checking for None then I'm not sure you gain much from using set_default.
20:29:46 bnemec But either should work.
20:33:10 mriedem yeah true :/
20:35:34 bnemec It's probably not worth satisfying my pet peeve about the BoolOpt default by moving the setdefault call to some weird location though.
20:39:56 sean-k-mooney dict not only set the defult i return the value in the dict. so if it was already set it returns the value that was found in the dict and if not it returns you default after setting it in the dict
20:40:22 sean-k-mooney you can discard the return if you dont care but its a nice behavior to have
20:40:38 sean-k-mooney so if we had a CONF.set_default it would be nice to keep it the same
20:41:48 sean-k-mooney that would change your example to http://paste.openstack.org/show/756359/
20:43:01 sean-k-mooney its a minor change but its basicly free to return the value and it makes the useage terser
21:31:42 openstackgerrit Matt Riedemann proposed openstack/nova master: Allow disabling cold migration to the same host https://review.opendev.org/676022
22:02:14 openstackgerrit Matt Riedemann proposed openstack/nova master: Add nova-lxc job to the experimental queue https://review.opendev.org/676024
22:02:16 mriedem donnyd: sean-k-mooney: ^ and now i'm out o/
22:05:57 donnyd Yesssss
22:16:25 openstackgerrit Merged openstack/nova stable/queens: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/672161
22:16:54 openstackgerrit Merged openstack/nova master: Avoid timeout from service update notification tests https://review.opendev.org/675935
22:22:58 openstackgerrit Eric Fried proposed openstack/nova master: Use SDK for node.list https://review.opendev.org/656027

Earlier   Later