Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-05
15:34:18 gibi mriedem: is it safe to access the instance.info_cache from the API?
15:34:22 mriedem gibi: yes
15:34:36 mriedem gibi: the addresses portion of the server response is built from the cache
15:34:51 gibi ohh, sean-k-mooney has a good point about shelve offloaded instances
15:35:15 mriedem yes he does
15:35:16 mriedem grumble
15:35:18 sean-k-mooney gibi: in that case it would not be in the cache and we can fallback to hitting the api
15:35:33 sean-k-mooney so check the cache if its there use it if not hit api
15:35:37 mriedem sean-k-mooney: but then you're going to always hit the api because not all instances have these types of ports
15:35:44 mriedem so...
15:35:58 mriedem you'd have to condition based on the instance.vm_state, but anyway, let's not worry about this right now
15:36:34 openstackgerrit Andrey Volkov proposed openstack/nova stable/queens: Test for plug_contrail_vif with utf8 VM name https://review.openstack.org/641048
15:36:49 sean-k-mooney ok there is a comment in the code to optimise this anyway so ya we can look at that in train
15:37:19 gibi if in train we can implement the move operations then we can drop the whole check from the aPI
15:37:34 gibi bah, not ture
15:37:35 gibi true
15:37:49 mriedem not true b/c older microversions need to fail right?
15:37:55 gibi yeah
15:38:03 gibi we need to support old microversions
15:38:19 gibi but we have the microversion check in place
15:38:31 gibi so on old microversions we can simply check the microversion not the port allocations
15:38:50 gibi we only need the current check as the current 2.72 is not complete
15:39:11 gibi (or we can go back to the discussion how many microversin we need for this feature)
15:39:47 sean-k-mooney gibi well for old microverion you still need the check as we are only failing move operation if you have a port resouce request
15:39:55 mriedem correct ^
15:40:15 mriedem just because 2.80 supports moving those types of servers doesn't mean that all of a sudden 2.1 does also
15:41:21 gibi assume we merge the create support in 2.72. and the in Train we fix the move with a bugfix. then if the microversion < 2.72 then we have to check if the instance has ports with allocation
15:41:33 gibi so yeah we cannot drop that check
15:41:38 gibi that looks into the ports
15:41:56 gibi but we can make it smarter by checking the vm state and the info_cache
15:42:13 sean-k-mooney ya but we shoudl definetly be about to optimise it when we have more time
15:42:27 sean-k-mooney *able to
15:42:44 mriedem did we agree to support moves without a new microversion?
15:43:01 gibi mriedem: no we haven't yet.
15:43:05 mriedem ok
15:43:09 gibi mriedem: we postponed that decision to Train
15:43:19 mriedem we would at least need a minimum compute service version check
15:43:30 sean-k-mooney i think that is dansmith's perference e.g. non new micro
15:43:56 gibi mriedem: is the compute version check due to the virt driver impact?
15:44:36 mriedem gibi: the compute service is what unbinds and binds the ports and stuff right during a move
15:45:02 gibi mriedem: ohh so we need a new compute on both end of the migration for bind/unbind
15:45:04 mriedem so anything you'd need to add to compute in train to support migrations would require an ABI version bump
15:45:08 mriedem correct
15:45:12 gibi understood
15:46:33 sean-k-mooney that would be a check in the conductor liek we do for the multiple port bindigns or like we will be doing for numa and sriov migration
15:47:32 sean-k-mooney effectivly if both computes are not new enough reject the migration
15:48:01 gibi sean-k-mooney: ack
15:50:00 mriedem need another novaclient core to hit this https://review.openstack.org/#/c/640657/
15:51:03 mriedem jaypipes: are you coming back on the libvirt reshaper change today? https://review.openstack.org/#/c/599208/
15:51:51 jaypipes mriedem: yes. it's on my screen right now.
15:57:04 cfriesen mriedem: should I include the blueprint for the PciInvalidAlias commit, or leave it standalone?
15:59:08 mriedem cfriesen: that's fixing a separate bug right?
15:59:21 gibi mriedem: +Ad https://review.openstack.org/#/c/640657/
15:59:39 mriedem iow, handling PciInvalidAlias separately from the blueprint is so it could be backported right?
15:59:48 cfriesen mriedem: yep
15:59:54 mriedem cfriesen: if not, and it doesn't result in a 500 response today, then we don't need to backport it
16:00:46 mriedem cfriesen: btw are you sure that's not handled today?
16:00:49 mriedem by this https://github.com/openstack/nova/blob/c43c1d3fb9da5dd0a13e1f15623a696212f095ff/nova/api/openstack/wsgi.py#L367
16:00:56 mriedem PciInvalidAlias extends Invalid
16:01:14 mriedem so that should return a 400
16:01:43 cfriesen yeah, was just trying to track that down
16:02:20 mriedem if it is handled then i don't think we need anything separate
16:02:48 mriedem https://review.openstack.org/#/c/620706/27/nova/api/openstack/compute/servers.py@a857
16:02:59 cfriesen so Fault will end up being HTTPBadRequest?
16:03:38 cfriesen or I guess Fault(exception.ConvertedException(
16:04:15 mriedem without any testing for it it's hard to say
16:05:31 mriedem so we get that from get_pci_requests_from_flavor which is called today from _validate_and_build_base_options in the api on server create,
16:05:45 mriedem i don't see any existing tests that handle the PciInvalidAlias if it's raised from there
16:05:58 mriedem i suspect it is handled and converted to a 400 response
16:06:05 mriedem but we don't have any testing for that
16:06:09 cfriesen I'll try a test without specially adding it to the list
16:06:16 mriedem unlike https://review.openstack.org/#/c/640841/2/nova/tests/unit/api/openstack/compute/test_serversV21.py
16:10:45 cfriesen mriedem: looks like it fails: http://paste.openstack.org/show/747293/
16:11:41 cfriesen if I add PciInvalidAlias to the list in ServersController.create() , it passes
16:11:52 mriedem i assume those tests aren't using https://github.com/openstack/nova/blob/c43c1d3fb9da5dd0a13e1f15623a696212f095ff/nova/api/openstack/wsgi.py#L367
16:12:03 mriedem those are hitting the handler directly
16:12:10 cfriesen yeah, just realized that
16:12:10 mriedem whereas ^ wraps the handler
16:13:07 cfriesen okay, need to hack devstack then
16:13:09 mriedem anywho, i think PciInvalidAlias is probably handled as a 400 and we're OK, and for completeness you could add it to that big ass list of exceptions for the validation stuff
16:13:16 cfriesen works
16:13:42 mriedem you got a 400 for an invalid alias in devstack?
16:13:52 cfriesen haven't done it yet
16:14:01 mriedem oh
16:16:59 cfriesen I get: ERROR (ClientException): Unknown Error (HTTP 500)
16:17:35 mriedem is there a stacktrace in the api logs?
16:18:39 cfriesen whoops...typo. I had "faise" istead of "raise"
16:19:44 cfriesen okay, for real this time I get this: http://paste.openstack.org/show/747294/ checking logs
16:20:38 cfriesen here are the logs: http://paste.openstack.org/show/747295/
16:21:37 cfriesen adding the exception to the list makes it a 400 as expected
16:22:40 cfriesen report a bug or just put it in the git commit message?
16:27:01 gibi mriedem: regarding https://review.openstack.org/#/c/636360/21/releasenotes/notes/support-neutron-ports-with-resource-request-cb9ad5e9757792d0.yaml@21 I think unshelving is possible after a simple shelve without offload. And that operation is supported with bandwith as that does not need a new scheduling
16:29:37 mriedem efried: can we do an assertEqual(1, len(mdevs)) check in here instead? https://review.openstack.org/#/c/639854/4
16:29:42 jaypipes mriedem: off it goes.
16:29:58 efried ...
16:30:19 mriedem gibi: yeah true
16:32:35 jaypipes wouldn't it be great if Nvidia developers actually participated in the community of people trying to enable their hardware? :(
16:33:46 efried mriedem: responded
16:35:15 mriedem efried: fwiw i also commented in bauzas's change that we should just do that mdev naming conversion from dashes to underscores in _get_mediated_device_information
16:35:24 mriedem so callers, like the libvirt driver and your test, don't need to care about that conversion

Earlier   Later