| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-22 | |||
| 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 | superdan | I would have thought | |
| 19:04:10 | mriedem | superdan: fault testing is weird since the instance has to be deleted or in error state | |
| 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 | |
| 19:34:18 | superdan | melwitt: okay that might not have found this because it was a show not a list | |
| 19:34:50 | stvnoyes | mriedem: btw, I don't see an init connection in xen driver's post_live_migrate. Is that a problem? (not for this rv) | |
| 19:35:15 | mriedem | stvnoyes: not sure, i'm not even sure how this is not failing.. | |
| 19:35:34 | jgriffith | mriedem you just made my week!!! "mriedem:that was one of the main things jgriffith was shooting for anyway" | |
| 19:36:04 | mriedem | <3 | |
| 19:40:45 | mriedem | stvnoyes: re the xen driver, it doesn't do the volume connect/disconnect like the libvirt driver, so i can't really say | |
| 19:40:58 | mriedem | we would need the xen driver team to take a look at it again and test things out | |
| 19:41:03 | stvnoyes | ok | |
| 19:41:38 | mriedem | that would be jianghua wang | |
| 19:42:12 | mriedem | heh, actually, they said in july: | |
| 19:42:13 | mriedem | "The connection_info from bdm works well for XenAPI both in v2 and v3.27. It'd be good if someone can help us to understand why libvirt needs initialize_connection." | |
| 19:42:20 | mriedem | :) | |
| 19:42:51 | mriedem | ah that's why this works | |
| 19:42:53 | mriedem | "Per the commit message from the following commit, the connection_info from DBM is for destination. So if the connection info for source side is different from destination, it will have problem. That's why libvirt changed to use initialize_connection. https://github.com/openstack/nova/commit/8b649aa86fb26e998d66e75e5cebfd19c396942d" | |
| 19:43:12 | mriedem | so apparently the bdm.connection_info is already getting updated with the dest host connection_info | |
| 19:44:04 | mriedem | nvm, that doesn't explain why your change isn't failing when we disconnect on the source host | |
| 19:44:50 | mriedem | let's just be safe and pull that thing out of cinder if we're new flow, we know it's there, and we have the attachment_id to get it via the migrate_data object | |