Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-08
21:30:58 mriedem cool
21:31:11 mriedem time for me to start my think friday
21:32:55 dansmith oh man, forgot about think friday
21:34:37 melwitt os-vif has been released a billion times so I don't need to do that
22:09:05 openstackgerrit Matt Riedemann proposed openstack/nova master: API: Remove evacuate/live-migrate 'force' parameter https://review.openstack.org/634600
22:17:27 dansmith mriedem: okay got N-3
22:18:09 dansmith the actual microversion patch is like half the size now, which is oh so much better
22:19:09 mriedem yeah
22:19:31 mriedem and most of it is samples
22:20:01 dansmith yeah
22:20:32 mriedem welp it's 4:20, too late to start think friday now
22:37:58 fried_rice o ovo wizards, remind me what the idiom is for getting extra specs out of a flavor obj
22:38:16 fried_rice if 'extra_specs' in flavor:
22:38:16 fried_rice extra_specs = flavor.extra_specs
22:38:16 fried_rice ?
22:38:36 mriedem anyone want to drop the nova-multiattach job from nova changes? https://review.openstack.org/#/c/606981/
22:38:50 mriedem fried_rice: yes, but i think it's always there
22:39:11 mriedem return cls._from_db_object(context, cls(context), db_flavor,
22:39:11 mriedem expected_attrs=['extra_specs'])
22:39:12 fried_rice apparently not, at least in test: http://logs.openstack.org/43/631243/8/check/openstack-tox-py27/e61ba3b/testr_results.html.gz
22:39:28 mriedem unit tests not withstanding
22:39:30 fried_rice maybe the test is just borked.
22:39:45 mriedem yeah in real runtime code the flavor would have the extra specs loaded on get from the db
22:40:02 fried_rice can it be None?
22:40:16 mriedem is the field marked as nullable=True?
22:40:19 fried_rice ...
22:40:27 mriedem 631243
22:40:28 mriedem oops
22:40:33 mriedem 'extra_specs': fields.DictOfStringsField(),
22:40:33 mriedem no
22:40:35 mriedem so it's not nullable
22:40:43 mriedem if it's set, it's not None
22:40:49 mriedem and cannot be set to None
22:41:02 fried_rice (Whether it is or not, I've learned not to trust my intuition when it comes to db objs and ovos)
22:41:18 mriedem you're usually pretty safe unless it's RequestSpec
22:41:23 mriedem then you're in for a world of pain
22:41:27 fried_rice okay, so it's always legal to say flavor.extra_specs.get('some_key') and not have to dork with checking whether extra_specs is a) present, b) a dict?
22:41:38 mriedem it's alwasy a dict
22:41:45 fried_rice ight.
22:41:47 mriedem in runtime code it should be present
22:43:00 mriedem yeah this is wonky https://review.openstack.org/#/c/631243/8/nova/objects/flavor.py@231
22:43:14 mriedem that could just be self.extra_specs.get('accel:device_profile')
22:43:16 fried_rice yeah, that's what I'm ripping on.
22:43:21 openstackgerrit Adam Spiers proposed openstack/nova master: Add detection of SEV support from QEMU/AMD-SP/libvirt on AMD hosts https://review.openstack.org/633855
22:44:35 aspiers fried_rice: thanks for the recheck - is the dsvm-nova gate broken, or is my code? ... or both? X-D
22:44:51 aspiers I can't get through to the NSX CI logs
22:44:58 fried_rice aspiers: hanging on by one rusty hinge
22:45:02 aspiers hehe
22:45:21 fried_rice it's not your code
22:45:46 fried_rice if your code were a problem, the whole CI would be red
22:45:53 openstackgerrit Matt Riedemann proposed openstack/nova master: neutron: make migrate_instance_finish call setup_instance_network_on_host https://review.openstack.org/635343
22:45:53 fried_rice (for that patch)
22:46:01 aspiers yeah I guess
22:46:04 openstackgerrit Matt Riedemann proposed openstack/nova master: neutron: make migrate_instance_finish call setup_instance_network_on_host https://review.openstack.org/635343
22:47:36 fried_rice mriedem: Um, you said something about RequestSpec being problematic?
22:48:08 fried_rice You mean if I'm trying to do this on RequestSpec fields themselves, or if I'm trying to pull RequestSpec.flavor.extra_specs.get('something') ?
22:48:11 openstackgerrit Matt Riedemann proposed openstack/nova master: Follow up (#2) for the bw resource provider series https://review.openstack.org/634767
22:48:46 mriedem fried_rice: i'm just be facetious
22:48:56 mriedem see my request spec rant this morning
22:49:03 fried_rice emphasis on feces.
22:49:09 mriedem HI-O!
22:49:34 fried_rice so this is just using fake_spec_obj, which exists previously https://review.openstack.org/#/c/631243/8/nova/tests/unit/scheduler/test_device_profile.py
22:50:12 fried_rice but apparently trying to pull extra specs out of that with spec_obj.flavor.get('extra_specs') is blowing up.
22:50:15 fried_rice (in test.)
22:51:16 mriedem req_obj.flavor = fake_flavor.fake_flavor_obj(ctxt)
22:52:09 mriedem my guess is there is a problem in how that sets the extra_specs on that fake Flavor object
22:52:47 mriedem yup
22:52:57 mriedem 'extra_specs' needs to be in expected_attrs
22:53:25 mriedem https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L258
22:53:40 mriedem extra_specs is a joined column so that fake_flavor_obj code is a bit misleading
22:54:09 fried_rice and that always happens in real life, so that test code ought to be updated accordingly?
22:54:15 mriedem yes
22:54:15 mriedem https://github.com/openstack/nova/blob/master/nova/tests/unit/fake_request_spec.py#L78
22:54:24 mriedem should pass expected_attrs=['extra_specs']
22:54:46 mriedem or fake_flavor_obj should just default to extra_specs if expected_attrs is empty
22:55:21 mriedem b/c that's what the get from db methods in the object do https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L378
22:55:36 fried_rice yeah, this fixes:
22:55:36 fried_rice --- a/nova/tests/unit/fake_flavor.py
22:55:36 fried_rice +++ b/nova/tests/unit/fake_flavor.py
22:55:36 fried_rice @@ -50,7 +50,7 @@ def fake_db_flavor(**updates):
22:55:36 fried_rice
22:55:37 fried_rice
22:55:37 fried_rice def fake_flavor_obj(context, **updates):
22:55:38 fried_rice - expected_attrs = updates.pop('expected_attrs', None)
22:55:38 fried_rice + expected_attrs = updates.pop('expected_attrs', ['extra_specs'])
22:55:39 fried_rice return objects.Flavor._from_db_object(context,
22:55:39 fried_rice objects.Flavor(), fake_db_flavor(**updates),
22:55:40 fried_rice expected_attrs=expected_attrs)
22:55:46 mriedem ++
23:06:48 openstackgerrit Matt Riedemann proposed openstack/nova master: Share snapshot image membership with instance owner https://review.openstack.org/630769
23:18:32 mriedem jroll: just a reminder that when https://review.openstack.org/#/c/635006/ is ready to go for wider review to throw it in the runways queue https://etherpad.openstack.org/p/nova-runways-stein
23:20:36 aspiers fried_rice, mriedem: was trying to understand the discussion around https://review.openstack.org/#/c/538498/ and it seems one point of debate was the corner case where a trait might need to be removed from a node, right?
23:20:51 aspiers fried_rice, mriedem: FWIW, that is not too unlikely with SEV
23:21:31 fried_rice aspiers: The basic idea is that there are some traits that are "owned" by compute, and any others are not.
23:21:36 aspiers It could be triggered by a downgrade of the kernel or QEMU or libvirt, or by disabling the sev option in the kvm_amd kernel module
23:21:50 aspiers fried_rice: yeah I got that, and I presume that the SEV trait would be owned by compute
23:22:01 fried_rice If a trait is owned by compute, you can't (un)set it as an operator or whatever. Next periodic will restore it to its "proper" (according to compute) value.
23:22:09 aspiers sure
23:22:24 fried_rice so if your trait is owned by compute, all you have to worry about is whether it needs to be set or not, and do that.
23:22:52 fried_rice If you need to be able to do that *without* compute service restarts, then I recommend doing it in update_provider_tree, not via a driver capability.

Earlier   Later