Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-21
15:25:01 sean-k-mooney ayoung: anyway the way that was all ment to work was ehn you boot the vm you provide a sperate kernel image with the kernel command line parmater set and then nova woudl use the root iamge and kernel image when booting and pass the kernel args form teh kernel image to qemu
15:25:10 ayoung And Nova/metadata server then would be responsible for multiplexing between the different instances
15:26:00 ayoung I see. I don;t think that the installer (terraform) is doing any of that. But I can reproduce this afternoon and determine what it IS doing
15:35:58 mriedem efried: heh, oops https://review.opendev.org/#/c/694897/
15:36:01 mriedem not sure i/we missed that
15:36:15 mriedem i mean, it's zvm so totally forgettable but otherwise
15:37:58 openstackgerrit Dan Smith proposed openstack/nova master: ZVM: Implement update_provider_tree https://review.opendev.org/694897
15:44:14 aarents Hi dansmith, can you confirm this last upaste is ok for you since your were holding -1 on that https://review.opendev.org/#/c/670000 ? thanks !
15:45:08 dansmith aarents: yeah, I was waiting for CI before, but I'll circle back
15:45:24 dansmith aarents: we probably want to have a few people look at that and sanity check my thinking there
15:45:52 dansmith maybe gibi since he was previously okay with the other fix, at least
15:46:09 dansmith obviously mriedem is always good at everything
15:46:20 aarents yes good idea
15:47:32 mriedem umm, rescue + disk bus = i defer to lyarwood
15:47:56 dansmith eh?
15:48:08 dansmith it's not really rescue related,
15:48:11 mriedem "There is a case during rescue where this value can be mistakenly updated
15:48:11 mriedem to reflect disk bus property of rescue image (hw_disk_bus)."
15:48:17 dansmith rescue is just one place where this side effect screws us
15:48:30 dansmith right, but.. the change is actually more generic
15:48:39 dansmith but yes, lyarwood would be a good person to look also
15:49:08 mriedem my gut feeling on something like this is it fixes one thing and breaks another
15:49:14 mriedem and i don't have the background on that
15:49:26 dansmith mriedem: check out my comments on the earlier set
15:49:43 dansmith mriedem: a patch from gary unceremoniously removed all instance.save()s he thought were unnecessary,
15:49:51 dansmith which removed the one right after this that was there originally,
15:49:57 dansmith so we just almost never save this change
15:49:59 dansmith and also,
15:50:10 dansmith making a change to instance within a "just generate me the xml" method is crazy wrong
15:50:50 dansmith but rescue just happens to tickle things right so we mangle the disk bus, but never save() it back to normal
15:51:06 sean-k-mooney we should not be saving the disk bus back but we shoudl be usign for the rescue xml
15:52:46 sean-k-mooney i dont think there has ever been an expection that the /dev/sd* names for the rescue boot would be the same as when the instnace is booted normally
15:53:06 dansmith sean-k-mooney: read the patches I linked in my analysis earlier
15:53:24 mriedem would have been useful to have that context in the commit message, if just summarized from the big comment in PS1
15:53:39 dansmith sean-k-mooney: the original change from like 2012 was trying to use get_xml as a hook to update the info late after libvirt had chosen defaults
15:54:26 mriedem tl;dr https://review.opendev.org/#/c/119622/ removed the code that would persist this change and yet we can still incorrectly persist it in edge cases incorrectly (like rescue) and we shouldn't be modifying the instance in a _get* method anyway.
15:54:55 sean-k-mooney ok ya we should not
15:55:07 dansmith mriedem: exactly
15:55:15 mriedem so do that, dan can +2 and ill +W
15:55:20 dansmith aarents: ^
15:59:41 aarents dansmith: So I rephrase commit message with more context ?
16:00:13 dansmith aarents: yeah just add all that context in there to make mriedem happy
16:00:23 aarents ok got it !
16:00:58 mgoddard hi mriedem, got a minute to discuss https://review.opendev.org/#/c/684849 ?
16:15:17 mriedem mgoddard: sure
16:15:32 mriedem preface: i have lost a lot of context on that bug and fix
16:16:15 mriedem so i'll likely just abandon my changes and you can move forward
16:16:30 mgoddard do you happen to remember how the RP association becomes stale after your patch
16:16:36 mgoddard I can't work it out from the code
16:17:21 mgoddard https://review.opendev.org/#/c/684849/2/nova/tests/functional/regressions/test_bug_1841481.py@129
16:21:35 mriedem i think the comment from L79
16:21:47 mriedem because host1 deletes the provider between
16:21:47 mriedem # _check_for_nodes_rebalance and _refresh_associations.
16:24:08 mriedem i think because we either don't add the provider uuid to _association_refresh_time or we pop it out on failure if the provider doesn't exist
16:25:06 mriedem it's all linked to when the ResourceTracker calls SchedulerReportClient.get_provider_tree_and_ensure_root
16:25:11 mriedem and then you go down the rabbit hole
16:29:34 mgoddard ok, I think I see. The RP wasn't in _association_refresh_time because it hasn't been in the local tree yet. If the RP exists in placement, that means we only update _association_refresh_time after _refresh_associations is done
16:30:23 mriedem i'm going to say yes
16:34:17 mgoddard that part makes sense now. I still don't see how the node not being removed from the RT compute_nodes prevents placement from getting healed though - each time through the loop we call _update, which calls _update_placement
16:36:00 mgoddard I probably need to stop thinking about this, going a little mad. I ran your functional test with my patch chain and it seemed to fix the issue.
16:37:42 efried mriedem: yeah, I was *sure* that one was already implemented. Oh well :(
16:43:22 mriedem mgoddard: i don't want to think about it anymore either which is why i abandoned my changes
16:54:07 mgoddard mriedem: hopefully you (or someone) will face thinking about it to review my patches at some point
16:57:17 mriedem can i get a stable core here? https://review.opendev.org/#/q/topic:bug/1849409+branch:stable/rocky
16:57:34 mriedem the changes to queens/pike/ocata are dependent on that since i have to redo them
16:59:38 mriedem melwitt: were these something you wanted to get downstream? https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:stable/stein+topic:heal_allocations_dry_run
16:59:47 mriedem i know eandersson was saying he used those in rocky
17:01:00 mriedem bauzas: could you take a look at these train backports? https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:stable/train+topic:bug/1852610
17:02:38 melwitt mriedem: I don't know that it's come up specifically (everything's on queens) but yeah definitely could use it I'm sure. really most likely is I'd want to get heal_allocations in queens in the first place (downstream) and then backport the --instance and --dry-run too
17:02:48 donnyd mriedem: :(
17:03:14 melwitt I dunno how doable that would be, I haven't tried it yet
17:03:27 mriedem donnyd: what? the lxc unicorn ci job that no one cares about?
17:03:35 donnyd I care
17:03:37 donnyd Lol
17:04:13 mriedem "care" and "care enough to work on" are different things, and i don't care enough to work on that anymore
17:04:54 donnyd Hence my :(
17:05:01 mriedem yeah i know
17:05:05 mriedem freedom ain't free and all that
17:06:04 donnyd Well I am very appreciative of the time that has been put in
17:07:12 melwitt https://www.youtube.com/watch?v=tzW2ybYFboQ
17:07:35 donnyd melwitt: zomg lol
17:07:59 melwitt :)
17:08:22 mriedem heh yo'uve never seen that?
17:08:34 mriedem it's the only reason i say it
17:09:21 donnyd Oh I have, and it makes me laugh every time
17:09:33 mriedem costs about a buck-o-five
17:10:05 mriedem ok on that note, my wife wants me out of the gd house for a few hours so i'm going to lunch, errands and then to be driven crazy working at a coffee shop so bbiab
17:17:14 openstackgerrit Lee Yarwood proposed openstack/nova master: block_device: Copy original volume_type when missing for snapshot based volumes https://review.opendev.org/694497
17:59:05 sean-k-mooney gibi: our downstream qa just found an issue with how we report the bandwidth provires
18:00:26 sean-k-mooney nova creates the compute node rp with the hypervior_hostname as the RP name
18:00:52 sean-k-mooney which means if you change the compute node host with the host config value in the nova and neutron config
18:01:19 sean-k-mooney we cannot find the root RP
18:04:23 sean-k-mooney so for it to work compute node host and hypervior_hostname meed to match
18:04:38 efried that sounds like a problem with how we report providers in general. Are you really supposed to be able to change that config for an existing service?
18:04:52 efried do we actually rename the provider correctly in that case?
18:05:12 sean-k-mooney this is a clean deployment
18:05:15 sean-k-mooney so no rename
18:05:25 sean-k-mooney but that would also be a problem
18:07:02 sean-k-mooney efried: http://paste.openstack.org/show/786502/
18:08:06 sean-k-mooney efried: nova is correctly using the host confiv vlaue "sriov01.localdomain" for the host as is neutron and that is also the vlaue set in the neutron port bindings

Earlier   Later