| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-28 | |||
| 13:42:49 | mdbooth | And asserted that's what we ended up with | |
| 13:42:58 | mdbooth | Then I realised that they're the same thing | |
| 13:43:03 | mdbooth | So actually I hadn't changed anything | |
| 13:43:28 | stephenfin | mdbooth: But what about the other way round | |
| 13:43:42 | stephenfin | If you called 'self.guest.migrate' with a "u'test'" param value | |
| 13:43:58 | mdbooth | test_migrate_v3 already tests that | |
| 13:44:01 | stephenfin | I'd expect the call to 'self.domain.migrateToURI3' to be called with "b'test'" | |
| 13:44:05 | stephenfin | But only for Python 2 | |
| 13:44:18 | mdbooth | test_migrate_v3_unicode tests that | |
| 13:44:32 | mdbooth | Because it passes u'v1' and asserts 'v1' | |
| 13:44:36 | stephenfin | Oh FFS | |
| 13:44:46 | stephenfin | I read 'not six.PY2' as 'not six.PY3' | |
| 13:44:57 | stephenfin | and thought that test was only for Python 3. Sorry! | |
| 13:44:59 | mdbooth | HAHA, sorry! | |
| 13:45:14 | mdbooth | -ESTUPIDNEGATIONS | |
| 13:45:33 | stephenfin | :D | |
| 13:45:54 | stephenfin | mdbooth: OK, let me give that one more spin through so. Sorry for the confusion :) | |
| 13:46:02 | mdbooth | NP, thanks again | |
| 13:47:21 | mdbooth | FWIW I wrote it that way because I way being anal. | |
| 13:47:50 | mriedem | https://review.openstack.org/#/c/507938/ needs final +2, regression in pike | |
| 13:48:14 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif stable/newton: Updated from global requirements https://review.openstack.org/373293 | |
| 13:48:51 | mdbooth | mriedem: https://review.openstack.org/#/c/507202/ libvirt data corruptor since forever | |
| 13:52:48 | mriedem | mdbooth: ok i'm going to put a patch on top of that to test live block migration with something like this https://review.openstack.org/#/c/481290/ | |
| 13:53:25 | mdbooth | mriedem: kk | |
| 13:54:00 | kashyap | mriedem: Oh cool. Funnily enough, I was just checking an hour ago if we do (non-shared) live block migration in the Gate | |
| 13:54:17 | mdbooth | Although the bug itself is really confined to the calling conventions of migrateToURI3 () | |
| 13:54:52 | mdbooth | We're going to have to backport it a long way, btw. | |
| 13:56:51 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DNM: Run test_volume_backed_live_migration and iscsi test https://review.openstack.org/508163 | |
| 13:56:55 | mriedem | ^ | |
| 13:57:42 | kashyap | Hmm, as an aside, seems like Nova cgit is out of sync with GitHub: | |
| 13:57:45 | kashyap | https://github.com/openstack/nova/blob/master/nova/tests/live_migration/hooks/run_tests.sh | |
| 13:57:48 | kashyap | http://git.openstack.org/cgit/openstack/nova/tree/nova/tests/live_migration/hooks/run_tests.sh?id=068d851 | |
| 13:58:03 | kashyap | Maybe /me should check w/ -infra | |
| 13:58:15 | mriedem | mdbooth: if you plan on backporting this to essex, do we really need https://review.openstack.org/#/c/507488/ in the mix for the backports? | |
| 13:58:23 | mriedem | you can't isolate that for your fix and backport, and then do ^ on top? | |
| 13:58:54 | mdbooth | mriedem: I'll do that during the backport. IIRC it just poked a bug in a unit test. | |
| 13:58:55 | dansmith | bauzas: wanna re-+W this after fixes were applied? | |
| 13:58:56 | dansmith | https://review.openstack.org/#/c/498947/10 | |
| 13:59:07 | mriedem | mdbooth: why not do it now? | |
| 13:59:16 | mriedem | otherwise the stable reviewers have to figure out why there is a diff | |
| 13:59:33 | mdbooth | Because it makes it messier upstream, and it's like a 1 line diff. | |
| 13:59:38 | mdbooth | In a unit test. | |
| 13:59:54 | mriedem | mdbooth: so when you say backport, you only mean internal backports? | |
| 14:00:09 | mdbooth | I'll be doing both. | |
| 14:00:38 | mriedem | ok. so as a stable reviewer, why do i need to sort out this diff? just isolate the py3 thing into the fix patch for migrateToURI and do the more general thing on top | |
| 14:01:16 | mdbooth | You should read my backport commit messages :) They're immaculate. | |
| 14:02:07 | dansmith | -1 | |
| 14:03:37 | openstackgerrit | Chris Dent proposed openstack/nova-specs master: Add spec for symmetric GET and PUT of allocations https://review.openstack.org/508164 | |
| 14:04:20 | mdbooth | mriedem: Incidentally, ever seen this bash-hackery: https://github.com/mdbooth/openstack-dev-hacks/blob/master/openstack.bash#L1-L4 | |
| 14:05:00 | mriedem | bauzas: do you think you'll be able to get to https://review.openstack.org/#/c/507488/ today? if not, i can help clean it up | |
| 14:06:15 | bauzas | dans | |
| 14:06:19 | bauzas | dansmith: done | |
| 14:06:39 | bauzas | mriedem: not sure I understand you, you mean me reviewing https://review.openstack.org/#/c/507488/ ? | |
| 14:06:50 | dansmith | bauzas: thanks | |
| 14:06:51 | mriedem | bauzas: oops, wrong patch | |
| 14:07:02 | mriedem | bauzas: this one https://review.openstack.org/#/c/506093/ | |
| 14:07:12 | bauzas | mriedem: FWIW, I'll have to bail out in 20 mins because I have to attend a Lyon OpenStack meetup (and btw. I'll miss today's nova meeting) | |
| 14:07:23 | openstackgerrit | Alex Xu proposed openstack/nova-specs master: Request traits in Nova https://review.openstack.org/468797 | |
| 14:07:31 | bauzas | mriedem: yeah I can fix that | |
| 14:07:37 | bauzas | 20 mins is enough | |
| 14:10:57 | mriedem | dansmith: what would be the best way to go about renaming the 'recreate' parameter in the rebuild_instance method in the compute manager, given rpc | |
| 14:11:14 | mriedem | add an 'evacuate' kwarg? so we could eventually drop the recreate parameter in a major rpc versoin bump? | |
| 14:11:34 | dansmith | mriedem: check the min version and send it the right way depending on what we're pinned to, | |
| 14:11:47 | dansmith | but you have to do the "if recreate or evacuate" logic in the top of the manager function, | |
| 14:11:55 | dansmith | which will probably not improve confusion | |
| 14:12:00 | dansmith | or, reduce | |
| 14:12:07 | dansmith | you could just do this at the top of the manager: | |
| 14:12:10 | mriedem | ok, alternatively i was just going to do: | |
| 14:12:12 | mriedem | evacuate = recreate | |
| 14:12:14 | dansmith | evacuate = recreate | |
| 14:12:15 | dansmith | yeah | |
| 14:12:17 | mriedem | ok | |
| 14:12:17 | mriedem | and replace all usage of the variable | |
| 14:12:23 | openstackgerrit | Ed Leafe proposed openstack/nova-specs master: Return Alternate Hosts https://review.openstack.org/504275 | |
| 14:12:34 | edleafe | johnthetubaguy: ^^ hope this addresses your comments | |
| 14:15:00 | bauzas | mriedem: not sure I get your comment on https://review.openstack.org/#/c/506093/4/nova/tests/functional/regressions/test_bug_1718455.py@135 | |
| 14:15:21 | bauzas | mriedem: if the migration isn't done yet, we should fail the test right? | |
| 14:16:02 | mriedem | bauzas: that code is waiting for the migration status to be 'running' | |
| 14:16:05 | mriedem | which is not waiting for it to be done | |
| 14:16:19 | bauzas | mriedem: so, s/running/done ? | |
| 14:16:37 | mriedem | you'd have to see whatever status we set the migration to when it's done | |
| 14:17:20 | mriedem | completed | |
| 14:17:29 | mriedem | is what is in _post_live_migrate in the compute manager | |
| 14:17:39 | mriedem | *_post_live_migration | |
| 14:17:41 | bauzas | ah, I understand | |
| 14:17:57 | bauzas | so, when it's running, that means the migration is in progress | |
| 14:17:58 | bauzas | my bad | |
| 14:18:07 | mriedem | correcto | |
| 14:18:09 | mriedem | hence the race | |
| 14:18:47 | bauzas | okay, uploading | |
| 14:19:52 | alex_xu_ | mriedem: gmann, jaypipes , next week is holiday in china, I will begin the vacation from tomorrow, so I won't active next week, probably just update spec when online since those two traits spec are very close. | |
| 14:20:06 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Ensure instance can migrate when launched concurrently https://review.openstack.org/506093 | |
| 14:20:12 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Update the placement deployment instructions https://review.openstack.org/469048 | |
| 14:20:18 | mriedem | alex_xu_: ok | |
| 14:20:50 | bauzas | alex_xu_: I was planning a spec review week, I could help you by providing new updates if you agree | |
| 14:20:55 | cdent | stephenfin: good comments, but I decided to deny you the “that” because why not | |
| 14:21:16 | alex_xu_ | bauzas: yea, sure, please free to update, appreciate the help! | |
| 14:22:22 | bauzas | k | |
| 14:25:51 | jaypipes | alex_xu_: cool. you ok with me or bauzas updating? | |
| 14:26:04 | jaypipes | alex_xu_: enjoy your vacation! :) | |