| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-22 | |||
| 18:55:43 | cburgess | sdague Yeah so to be clear.. we never deployed libvirt 1.2.2 because it was bad. We just skipped that version which is why we never hit that live snapshot bug. But yeah we have run with it since we merged the code upstream. | |
| 18:56:01 | sdague | cburgess: ok, cool | |
| 18:56:39 | cburgess | sdague rmk added that code... umm... 4 or 5 years ago and we have been running with it enabled ever since. | |
| 18:57:30 | sdague | cburgess: yeh, I knew you all had been using that for a long time, I just realized at PTG we hadn't come around and flipped that back since getting past 1.2.2 | |
| 18:58:13 | cburgess | sdague cool.. its +1 from me. | |
| 19:00:16 | mriedem | leakypipes: superdan: https://review.openstack.org/#/c/505417/6 | |
| 19:00:18 | sdague | for the land of fun... qemu-img --version | head -1 on centos7, ubuntu 16.04, ubuntu 17.04 | |
| 19:00:24 | sdague | qemu-img version 1.5.3, Copyright (c) 2004-2008 Fabrice Bellard | |
| 19:00:32 | sdague | qemu-img version 2.5.0 (Debian 1:2.5+dfsg-5ubuntu10.16), Copyright (c) 2004-2008 Fabrice Bellard | |
| 19:00:39 | sdague | qemu-img version 2.8.0(Debian 1:2.8+dfsg-3ubuntu2.4) | |
| 19:00:47 | cburgess | Yeah you will break RHEL with that change. | |
| 19:00:51 | cburgess | Forgot to mention that. | |
| 19:00:56 | sdague | cburgess: why? | |
| 19:01:12 | cburgess | sdague Opps yeah just realized I was wrong. We don't block on 2.10 now.. we just change the args. My bad. | |
| 19:01:20 | sdague | cburgess: right | |
| 19:01:26 | cburgess | sdague Ignore me. | |
| 19:01:28 | sdague | with 2.10 right now you just explode in a ball of fire | |
| 19:01:31 | sdague | which is what the gate did | |
| 19:01:34 | cburgess | sdague LOL | |
| 19:01:42 | sdague | because the ppa for pike has 2.10 | |
| 19:01:46 | cburgess | sdague qemu-img barfs back an error at you I assume? | |
| 19:01:55 | sdague | yeh | |
| 19:02:01 | sdague | which is a stack trace | |
| 19:02:07 | cburgess | Yeah... cute. | |
| 19:02:13 | mriedem | so uh, no group rates in sydney huh? | |
| 19:03:00 | sdague | cburgess: qemu actually just gives a clean exit, but it's definitely not expected by nova code, so nova code stack traces | |
| 19:03:02 | superdan | mriedem: ack on all thanks | |
| 19:03:09 | mriedem | \o/ | |
| 19:03:29 | cburgess | sdague Also depends on which version of RHEL 7 you are on. I believe later versions of RHEL, or the OSP repos include newer versions of QEMU. On 7.4 for instance we have 2.6 | |
| 19:03:46 | superdan | mriedem: I wonder if we don't have a thing that actually tests server['fault'] from the api? | |
| 19:03:58 | sdague | cburgess: ah, yeh with OSP it might be different. I just have a centos 7 vanilla install | |
| 19:04:01 | mriedem | superdan: i'd expect a functional test somewhere, | |
| 19:04:10 | mriedem | superdan: fault testing is weird since the instance has to be deleted or in error state | |
| 19:04:10 | superdan | I would have thought | |
| 19:04:15 | mriedem | always trips me up | |
| 19:04:17 | cburgess | sdague yeah there are various additional repos you get access to with OSP. | |
| 19:04:22 | superdan | mriedem: um, what? | |
| 19:04:38 | superdan | oh for the api to show it? | |
| 19:04:41 | superdan | I see | |
| 19:04:42 | mriedem | yeah, | |
| 19:04:47 | mriedem | like these failure recreate tests we do, | |
| 19:04:50 | superdan | like your cell0 test, you could just boot things what will never schedule | |
| 19:04:57 | mriedem | i'm always wanting to just poll until the fault shows up, but most of the time that won't work | |
| 19:05:04 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Fix 500 if list servers called with empty regex pattern https://review.openstack.org/506754 | |
| 19:05:08 | superdan | mriedem: gotcha | |
| 19:05:18 | mriedem | so i've become astute and the instance actions api | |
| 19:08:11 | superdan | mriedem: the args to that function aren't kwargs | |
| 19:08:23 | superdan | I can call them that way for reference, but it's wrong | |
| 19:08:36 | mriedem | i know they aren't kwargs | |
| 19:08:44 | superdan | can I comment? | |
| 19:08:45 | mriedem | but you could have some variables in the test instead or something, like you did elsewhere | |
| 19:09:02 | mriedem | i just hate having to remember what None, None, {}, None, [], [], None means | |
| 19:09:10 | superdan | yep, I can do that | |
| 19:10:01 | superdan | mriedem: I'm not seeing a functional test that actually checks fault | |
| 19:10:05 | superdan | you know we have one somewhere? | |
| 19:10:14 | mriedem | superdan: i don't | |
| 19:10:17 | mriedem | w/o searching | |
| 19:10:35 | superdan | okay | |
| 19:10:44 | mriedem | gd these sydney hotel rates | |
| 19:15:42 | stvnoyes | mriedem: going thru your review ccomments on live migrate/v3 - https://review.openstack.org/#/c/463987/18/nova/virt/libvirt/driver.py line 7077. | |
| 19:16:28 | stvnoyes | init connection should be done for the new flow so that is a problem that needs to be fixed | |
| 19:17:11 | stvnoyes | 2 ways to do it, leave libvirt driver as it was and it will get done for both new and old flows or | |
| 19:18:10 | stvnoyes | init_connection is already being done during pre_live_migrate but the updated bdm is not saved. If we make a change to save it during pre, it will be updated. | |
| 19:18:33 | stvnoyes | then it will be available on the source node | |
| 19:19:27 | stvnoyes | i like that better as then we can remove code from libvirt driver and I noticed that the xen driver doesn't do an init connection so this would fix that too. | |
| 19:22:39 | mriedem | so post_live_migration is on the source node, and init_connection is called to get the connection_info to disconnect from the source host | |
| 19:22:59 | stvnoyes | yes | |
| 19:23:16 | mriedem | pre_live_migration runs on the dest node, let me see what you're doing there | |
| 19:24:34 | stvnoyes | the bdm update is buried - _get_instance_block_device_info > driver_block_device.refresh_conn_infos > device.refresh_connection_info | |
| 19:24:38 | mriedem | hmm, the driver.pre_live_migration is what connects the volume on the dest host, but where do we call attachment_update? | |
| 19:24:56 | mriedem | oh right i commented on that too | |
| 19:25:17 | stvnoyes | very not crazy about all this stuff happening down under a get method | |
| 19:25:25 | mriedem | yeah | |
| 19:25:28 | mriedem | me neither | |
| 19:25:38 | mriedem | ok so https://review.openstack.org/#/c/463987/18/nova/compute/manager.py@5431 is what would eventually update the attachment with the host connector on the dest host | |
| 19:25:53 | mriedem | and then we call self.driver.pre_live_migration to connect the volume on the dest host | |
| 19:26:04 | mriedem | and then volume_api.attachment_complete to finalize on the dest | |
| 19:27:14 | mriedem | so this is where we have the updated bdm on the dest during pre_live_migration right? https://review.openstack.org/#/c/463987/18/nova/virt/libvirt/driver.py@6908 | |
| 19:27:21 | stvnoyes | I think the issue is that init_connection on the dest does not persist the new info to the db. so the source still has the stale info | |
| 19:27:31 | mriedem | stvnoyes: not in the nova db, but i think that's the idea, | |
| 19:27:49 | mriedem | attachment_update for the new attachment on the dest host passes the dest host connector, | |
| 19:27:56 | mriedem | and cinder stores the connection info in the cinder db | |
| 19:28:13 | stvnoyes | right. I was thinking that if we did a bdm.save() on the destination after getting the update, when the source later pulls the bdm from the db, the connector info would be up to date | |
| 19:28:18 | mriedem | that's why i said in https://review.openstack.org/#/c/463987/18/nova/virt/libvirt/driver.py@7077 can't we just get the attachment record from cinder? | |
| 19:28:59 | mriedem | if we update the bdm connection_info during pre_live_migration, it's pointing at the dest host, so anything that relies on that on the source host could get mixed up | |
| 19:29:17 | mriedem | although i'm not sure we use the bdm connection_info on the source host after pre_live_migration do we? | |
| 19:29:24 | mriedem | maybe on rollback | |
| 19:29:41 | openstackgerrit | Elod Illes proposed openstack/nova master: Add error notification for instance.interface_attach https://review.openstack.org/506643 | |
| 19:30:43 | mriedem | stvnoyes: i think the best thing for us to do is let cinder store the connection_info per attachment | |
| 19:30:52 | mriedem | and for the new flow, we just pull that from cinder when we need it | |
| 19:30:56 | mriedem | using the attachment_id | |
| 19:31:03 | mriedem | let's keep the nova bdm out of it | |
| 19:31:11 | stvnoyes | ok, I like that. | |
| 19:31:20 | mriedem | that was one of the main things jgriffith was shooting for anyway | |
| 19:31:25 | stvnoyes | thanks | |
| 19:31:28 | mriedem | when we started this like a year ago :) | |
| 19:31:53 | melwitt | superdan, mriedem: there's some instance fault testing stuff in nova/tests/functional/test_server_group.py | |
| 19:31:56 | mriedem | stvnoyes: so for your patch, i think that post_live_migration thing in the driver was my main hangup, otherwise there were just a couple of other small things | |
| 19:32:25 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Fix 500 if list servers called with empty regex pattern https://review.openstack.org/506760 | |
| 19:32:27 | stvnoyes | I've fixed most of them. After this part goes in, then just lots of testing before I update | |