| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-26 | |||
| 11:50:09 | johnthetubaguy | Spazmotic: xenapi live-migrate... whats up there? | |
| 11:50:38 | Spazmotic | man john i've been crying your name for days :p | |
| 11:50:53 | johnthetubaguy | Spazmotic: your welcome | |
| 11:50:55 | Spazmotic | hehehehe | |
| 11:51:20 | Spazmotic | Looks like it's not working in XS7+ due to the SR not existing during the assert_can_live_migrate for src -> destand XS7 being more insistent that it does. | |
| 11:51:41 | johnthetubaguy | oh, they "fixed" that check | |
| 11:52:02 | Spazmotic | Yep, so it would seem. It actually insists now and throws MAP errors | |
| 11:52:34 | johnthetubaguy | yeah, we don't pass the SR map to the first check call, as it never used to do anything with it | |
| 11:52:37 | Spazmotic | johnthetubaguy: by the by I manually added you to a review with Xenapi the other week, I hope you don't mind. | |
| 11:53:10 | johnthetubaguy | that is no problem, although you might have to ping me to get to that, hoping to ramp my reviewing back up now things are more settled over here | |
| 11:53:15 | Spazmotic | Well in this case it's for our iSCSI volumes only causing an issue since they don't exist yet on the dest during its first pass call through that method | |
| 11:53:33 | Spazmotic | yeah I understand, I mentioned you a few times but figured yo uwere just busy | |
| 11:53:36 | johnthetubaguy | yeah, but I think you will need to send the map for it to find them | |
| 11:53:41 | Spazmotic | IT's pretty simple code but it directly involves things I know yo uworked on, so wanted your advice | |
| 11:54:09 | Spazmotic | I agree, my only real thought here is how to handle it. My best thought now is to generate the SR a little earlier in the code line | |
| 11:54:16 | johnthetubaguy | its a nasty one honestly, as the work isn't done until much later to create the volumes on the dest, as I am sure you saw | |
| 11:54:51 | johnthetubaguy | the problem is Cinder really, that extra connection has tracking info that doesn't get updated till after the check phase | |
| 11:55:29 | johnthetubaguy | the other (very unfortunate but quick) approach is to move the check later, before you call the live-migration | |
| 11:55:40 | Spazmotic | Without a major overhaul, the only thing I was able to think of that wasn't too gross was to use connect_to_volume provider at the destination check. IT would require that code pulling BDMS though.. which doesn't feel right. | |
| 11:55:58 | Spazmotic | at least that way the SR exists, but doesn't need to attach the volume yet..and it generates the map without error. | |
| 11:55:59 | johnthetubaguy | yeah, those BDMs are wrong at that point | |
| 11:56:26 | johnthetubaguy | you need the specific connection params for the destination hypervisor, which could be different credentails, in the general case | |
| 11:56:28 | Spazmotic | It would literally just be a throw away of the variable afterwards, which is a waste | |
| 11:57:16 | Spazmotic | That's true that data is stored in the SR isn't it. | |
| 11:58:05 | Spazmotic | I guess it's still workable since bdms pulls conn_data anyway | |
| 11:58:17 | Spazmotic | But still not right | |
| 11:58:22 | johnthetubaguy | hmm, not sure I understand what you mean, I thought each SR would have different passwords in conn_data, potentially, not sure we ever tested that mind | |
| 12:00:16 | johnthetubaguy | Spazmotic: basically, you need this code to run *before* you attach the SR from the bdm: https://github.com/openstack/nova/blob/d0d0262379a57230e546cddf233da8012d3fe5b1/nova/compute/manager.py#L5889 | |
| 12:01:06 | johnthetubaguy | That means due to the common code the earliest you can attach the SRs on the destination is this path: https://github.com/openstack/nova/blob/d0d0262379a57230e546cddf233da8012d3fe5b1/nova/compute/manager.py#L5944 | |
| 12:01:40 | Spazmotic | Yeah I saw that and agree as well unless I duplicated the bdms pull, which was why I say it's wrong | |
| 12:01:59 | Spazmotic | re-pulling bdms in the check call to be able to call the attachments.. which sucks | |
| 12:02:04 | johnthetubaguy | yeah, the bdms just don't have the info in them you think they do, if its done before that call | |
| 12:02:16 | Spazmotic | Ah no? are they modified before then? Didn't think that would change. | |
| 12:02:35 | Spazmotic | Ahhhh i see | |
| 12:02:43 | Spazmotic | I didn't read the loop close enough.. damn | |
| 12:03:01 | johnthetubaguy | they can be modified by this call: https://github.com/openstack/nova/blob/d0d0262379a57230e546cddf233da8012d3fe5b1/nova/compute/manager.py#L5905 | |
| 12:03:06 | johnthetubaguy | cool | |
| 12:04:02 | Spazmotic | The other option is to clean up sort out Huan Xie's code.. which just eats the exception and forms the map manually | |
| 12:04:10 | Spazmotic | So.. lots to mush face against on it | |
| 12:05:11 | Spazmotic | nothing really graceful showing up in my mind yet | |
| 12:05:52 | Spazmotic | Could push the bdm attachment setting into its own method to call twice.. but that's just adjusting the code to fit this specific situation. | |
| 12:06:47 | johnthetubaguy | its more we can't call that till the pre-live-migrate stage, else all the error handling is screwed up | |
| 12:07:41 | Spazmotic | mmhmm.. I can see that | |
| 12:07:47 | Spazmotic | Just needs more thought then I think | |
| 12:09:04 | openstackgerrit | Ameed Ashour proposed openstack/nova master: detach instance volumes when VM creation fails https://review.openstack.org/528385 | |
| 12:09:04 | openstackgerrit | Ameed Ashour proposed openstack/nova master: detaching volumes when deleted VM is in error state https://review.openstack.org/528069 | |
| 12:09:07 | openstackgerrit | Ameed Ashour proposed openstack/nova master: change instance_system_metadata column type https://review.openstack.org/526900 | |
| 12:09:40 | mdbooth | stephenfin: You can't spit-shine a turd ;) | |
| 12:09:55 | mdbooth | stephenfin: Thanks for trying, though. | |
| 12:09:58 | johnthetubaguy | Spazmotic: so the quickest fix is to skip the check_can_live-migrate for XS 7+ | |
| 12:10:17 | Spazmotic | I was wondering if we could do that but I could not think of a way | |
| 12:10:24 | johnthetubaguy | Spazmotic: that might not be as bad as it first sounds, given how the API has changed | |
| 12:10:28 | Spazmotic | We still do need the data from the source check, but if we could skip the assertation that would be ideal. | |
| 12:11:08 | Spazmotic | I guess I could use the XenAPI host calls to make a switch | |
| 12:11:15 | johnthetubaguy | Spazmotic: so you can call it right before where we call live_migrate, if the data is missing in migrate data? | |
| 12:11:49 | johnthetubaguy | Spazmotic: downside is failed live-migrations will take much longer to fail, and involve more work to rollback (disconnect the volumes) | |
| 12:12:09 | johnthetubaguy | but given you need to connect the volume first now anyway, its maybe not a big deal | |
| 12:12:37 | ameeda | gmann: gibi: is this ready for merge ? https://review.openstack.org/#/c/526900/ | |
| 12:12:53 | johnthetubaguy | Spazmotic: so I think I am on watch the baby duties in a few mins, over lunch, so maybe catch you later on | |
| 12:13:07 | Spazmotic | So we could potentially still generate the map and just have it just not run an assertation if the XS version is high.. but yea it's not a bad idea | |
| 12:13:17 | Spazmotic | worth looking into for usre.. I appreciate the brain dump man | |
| 12:13:23 | Spazmotic | and good luck with the baby :) | |
| 12:13:48 | johnthetubaguy | I may need it :) | |
| 12:14:08 | Spazmotic | Make your baby review my code.. will keep htem busy :p | |
| 12:14:45 | johnthetubaguy | Spazmotic: yeah, that works, generate the map, but skip the check check call based on version (there is some structures in place for version skips already) | |
| 12:15:08 | Spazmotic | Awesome.. maybe i'll look how those are done nad see if i can keep it standard | |
| 12:15:19 | Spazmotic | If you happen to know one feel free to let me know when you get a cahnce so I can take a look | |
| 12:22:48 | Spazmotic | Ah and johnthetubaguy since my irc nme is wierd.. the commit i added you to is https://review.openstack.org/#/c/533168/4.. Ignore zuul.. we fighting right now :P | |
| 12:27:51 | openstackgerrit | Radoslav Gerganov proposed openstack/nova master: WIP: VMware: implement update_provider_tree() https://review.openstack.org/536348 | |
| 12:35:59 | rgerganov | efried, is it safe to assume that when update_provider_tree is called, provider_tree.roots[0] points to the root RP of the compute node? | |
| 12:45:11 | Spazmotic | Nice.. host_rec['software_version']['product_version'].. super easy | |
| 12:47:55 | openstackgerrit | Merged openstack/nova master: Add the Nova libvirt StorPool attachment driver. https://review.openstack.org/140733 | |
| 12:48:01 | kholkina | Hi. I use this patch https://review.openstack.org/#/c/530224/ to update user-data. It works fine on 169.254.169.254/2009-04-04/user-data, but doesn't work when I use config drive. What should I do to update it on config-drive too? | |
| 12:48:11 | openstackgerrit | Merged openstack/nova master: Transform instance-evacuate notification https://review.openstack.org/482148 | |
| 12:52:16 | artom | kholkina, the config drive is essentially write-once, at instance creation. | |
| 12:52:40 | artom | kholkina, whatever metadata the instance was created with, that's what's on the config drive, and it can't be updated | |
| 13:01:58 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: DNM: test if tempest-full is triggered or not https://review.openstack.org/537936 | |
| 13:03:11 | kholkina | artom, ok, thanks | |
| 13:05:12 | bkero- | Hi there, does anybody know if it's possible to get the nova-cli to output in a different format? Sort of like OSC's -f and -c options | |
| 13:06:05 | artom | bkero-, if there is, nova --help should tell you :) | |
| 13:07:01 | bkero- | I did not see it in there, which is why I came here to ask | |
| 13:08:59 | openstackgerrit | Merged openstack/nova master: tests: Use correct response type in tests https://review.openstack.org/527182 | |
| 13:12:15 | artom | bkero-, then we don't do it :) Were you hoping for some secret options we only tell to the select few? ;) | |
| 13:14:43 | bkero- | pretty much | |
| 13:14:50 | bkero- | some environment variable or undocumented flag | |
| 13:15:39 | bkero- | Or "oh, we use this library for our cli binary, and you'd be setting this env var/flag according to it" | |
| 13:15:59 | artom | bkero-, hehe :) | |
| 13:16:12 | artom | bkero-, I'm not the ultimate expert on this, but I've never come across something like that | |
| 13:21:48 | Spazmotic | Heading out for the day. Will be on at home for a bit, hit me up on SpazZz if you need anything | |
| 13:51:38 | bkero- | Hm, is there any way to clear out 'nova migration-list'? | |
| 14:07:32 | gibi | mriedem: FYI I'm getting lost in zuul job definitions: http://lists.openstack.org/pipermail/openstack-dev/2018-January/126616.html | |
| 14:09:18 | mriedem | crack that nut | |
| 14:13:17 | mriedem | gibi: this is the last versioned notification transformation patch i'm tracking for queens https://review.openstack.org/#/c/465081/ so you could update https://review.openstack.org/#/c/518018/ based on that | |
| 14:14:31 | sean-k-mooney | finucannot: qq i dont have time to read mypy stuff right now (tyring to do the refactor before i the end of the day) but is it related in any way to https://www.python.org/dev/peps/pep-0484/ | |
| 14:14:44 | finucannot | sean-k-mooney: One and the same | |
| 14:15:29 | finucannot | sean-k-mooney: Only I'm using Python 2-compatible comments for now. It could be converted once Python 2 support is dropped, but that's a minimum of two years off | |
| 14:15:31 | sean-k-mooney | finucannot: from what i saw in your pathcs however it looked like mypy did it via comments not fuction changes | |
| 14:15:51 | sean-k-mooney | ah ok so mypy support both | |
| 14:16:06 | finucannot | Correct http://mypy.readthedocs.io/en/latest/python2.html | |
| 14:16:16 | sean-k-mooney | comments for backwards compatiblity and greeting(name: str) -> str: for native python 3 | |