Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-05
15:07:24 mdbooth mriedem: Didn't spot that was a backport.
15:07:35 sean-k-mooney efried: i assume you rebased the patch due to a merge confilt with cdents patch?
15:07:45 efried sean-k-mooney: yes
15:07:52 mdbooth mriedem: Lemme read the review comments on there, perhaps my concern was already addressed.
15:08:10 mriedem mdbooth: i doubt it, no one thinks of the image cache besides you :)
15:08:41 sean-k-mooney ok just +1'd it again i havent used it locally but it makes sense to me
15:09:14 mdbooth mriedem: Hmm. I'd consider reverting that, tbh.
15:10:05 mdbooth mriedem: The other solution proposed was much more targetted and presumably also works around the same issue.
15:11:06 mdbooth I'd rather fail than risk leaving an empty file in the image cache.
15:11:19 mdbooth The former you can just retry.
15:11:58 openstackgerrit Balazs Gibizer proposed openstack/nova master: Handle placement error during re-schedule https://review.openstack.org/639608
15:12:11 mriedem mdbooth: are you talking about this? https://review.openstack.org/#/c/633493/2
15:12:27 sean-k-mooney any objections to me repoposing the numa and sriov migration specs for train. i think we should be ablt to wrap those up by train m1 so im hoping we can just fast appove the stien specs again?
15:13:26 sean-k-mooney there is 3 days to ff so they are not going to get reivewed betwen now and then anyway and that will give us plenty fo time to work on func tests for both
15:16:38 mriedem sean-k-mooney: no one will object to reproposing specs to train for things that will not land in stein
15:16:51 mriedem and no one is going to review those train specs for awhile either
15:17:10 sean-k-mooney ya i just want to have them up before the ptg
15:17:54 sean-k-mooney i know peopel are going to be focusing on getting RC1 ready for the next few weeks and finalising stien
15:18:53 sean-k-mooney on that note im hoping we can land https://review.openstack.org/#/c/602432/ in stein/rc1 its not a change i think many would want to backport so i would like to close that bug in stien if we can
15:20:50 mdbooth mriedem: Yeah
15:21:23 mdbooth mriedem: Although obviously as you say you wouldn't fix that on a stable branch.
15:21:37 mriedem gibi: random thought on the instance_has_port_with_resource_request check in the API - could we replace the neutron GET /ports call in there now with checking the instance info cache for any VIFs whose profile has the 'allocation' key?
15:24:47 openstackgerrit Matthew Booth proposed openstack/nova master: Revert "Fixes race condition with privsep utime" https://review.openstack.org/641038
15:24:56 mdbooth mriedem: Proposed a revert ^^^
15:25:08 openstackgerrit Chris Dent proposed openstack/nova master: Correct lower-constraints.txt and the related tox job https://review.openstack.org/622972
15:25:22 mdbooth Eurgh.
15:26:59 sean-k-mooney mriedem: your suggesting modifying https://github.com/openstack/nova/blob/6516038dea5b80af2d83906a7e23f33e76061a42/nova/api/openstack/common.py#L571-L582 to check the instance info cache.
15:27:15 openstackgerrit Matthew Booth proposed openstack/nova master: Revert "Fixes race condition with privsep utime" https://review.openstack.org/641038
15:28:21 sean-k-mooney assuming its only ever called after the info cache is populated then maybe. if not we might be able to make it condtional
15:28:55 sean-k-mooney e.g. if the instance is not in the info cache when this is called hit neutron otherwise us cache versions
15:29:33 mriedem gibi: https://review.openstack.org/#/c/636360/
15:29:42 gibi mriedem: looking
15:30:14 sean-k-mooney mriedem: it looks like its only called on move operations
15:30:52 mriedem sean-k-mooney: correct
15:30:55 mriedem so the cache should be there
15:31:00 mriedem and ports would already be bound
15:31:32 sean-k-mooney yep so i guess it might be safe to always used the cache values.
15:33:08 sean-k-mooney we are just checkign if a port has allocation too not where they are from and since that should not change over the lifetime of a port it "should" not matter if the allcoation were stale for example because we justed moved.
15:33:18 sean-k-mooney im ignoring that move wont work in stien
15:33:32 mriedem bauzas: are you good with this now? https://review.openstack.org/#/c/509206/
15:33:51 mriedem you previously approved
15:33:56 mriedem and then we had to fix something for tempest
15:34:05 mriedem Sylvain Bauza Feb 18 5:34 AM Patch Set 10: Code-Review+2 Workflow+1
15:34:06 sean-k-mooney mriedem: could we have issue with unshelve?
15:34:17 mriedem sean-k-mooney: yes, gdi
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

Earlier   Later