| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-22 | |||
| 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 | |
| 19:47:28 | stvnoyes | kk | |
| 20:00:09 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to Instance object https://review.openstack.org/489408 | |
| 20:00:59 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certificates to REST API https://review.openstack.org/486204 | |
| 20:09:52 | openstackgerrit | Merged openstack/nova master: Add tests to validate instance_list handles faults correctly https://review.openstack.org/505392 | |
| 20:30:55 | mriedem | sdague: one sort of weird thing in the live snapshot patch https://review.openstack.org/#/c/454323/ | |
| 20:31:05 | mriedem | the config option help talks about libvirt 1.2.2, | |
| 20:31:13 | mriedem | but we've required at least 1.2.9 since pike | |
| 20:31:22 | sdague | mriedem: right, which is under our minimum | |
| 20:31:35 | sdague | yeh, honestly, I don't know if other libvirt versions will experience the same issue | |
| 20:31:50 | mriedem | right we test against 2.5.0 right now | |
| 20:31:56 | sdague | yeh | |
| 20:32:25 | sdague | so we could also deprecate the option, I would honestly do that as a follow on from the default change under the assumption that we don't see the issue show up again | |
| 20:32:39 | sdague | it would be nice to have a month of burn in data on that | |
| 20:32:58 | mriedem | we'll be using 3.6.0 if/when we switch to pike uca | |
| 20:33:00 | cburgess | Or a release. | |
| 20:33:05 | mriedem | ok so that was my question, if we also deprecate | |
| 20:33:08 | mriedem | i'm ok with doing it in a separate change | |
| 20:39:22 | openstackgerrit | Matt Riedemann proposed openstack/nova master: fix nova accepting invalid availability zone name with ':' https://review.openstack.org/490722 | |
| 20:42:37 | sdague | yeh, that would make me more comfortable to know that all this was going to work for reals for a while | |
| 20:42:49 | sdague | it took us a while to see the fail pattern before | |
| 20:42:59 | sdague | and while the 10 rechecks seem fine, stuff emerges | |
| 20:48:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional migrate force_complete test https://review.openstack.org/496202 | |
| 20:52:51 | mriedem | anyone noticed an annoying quirk with new gerrit where commenting lower in a file jumps you back up to a previous comment or something higher up? | |
| 20:53:08 | mriedem | i swear i had to fix this somehow 2 years ago with the last major upgrade | |
| 20:56:49 | openstackgerrit | Dan Smith proposed openstack/nova master: Add get_instance_objects_sorted() https://review.openstack.org/505417 | |
| 20:56:50 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix a pagination logic bug in test_bug_1689692 https://review.openstack.org/505661 | |
| 20:56:50 | openstackgerrit | Dan Smith proposed openstack/nova master: Copy some tests to a cellsv1 mixin https://review.openstack.org/505442 | |
| 20:56:51 | openstackgerrit | Dan Smith proposed openstack/nova master: Remove legacy fault-loading routines https://review.openstack.org/505456 | |
| 20:56:51 | openstackgerrit | Dan Smith proposed openstack/nova master: Use improved instance_list module in compute API https://review.openstack.org/505418 | |
| 20:56:52 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix minor input items from previous patches https://review.openstack.org/506416 | |
| 20:56:52 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix CellDatabases fixture swallowing exceptions https://review.openstack.org/506312 | |
| 20:56:53 | openstackgerrit | Dan Smith proposed openstack/nova master: Make 'fault' a valid joined query field for Instance https://review.openstack.org/506774 | |
| 21:00:31 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add alternate hosts https://review.openstack.org/486215 | |
| 21:00:32 | openstackgerrit | Ed Leafe proposed openstack/nova master: Add Selection objects https://review.openstack.org/499239 | |
| 21:21:34 | mriedem | huh, placement randomly crashed in my devstack at some point in the last week | |
| 21:52:10 | mriedem | ooo yeah, bursting 20 instances at once with no quota and the fake virt driver | |
| 21:52:22 | mriedem | now i understand why this nfv thing is so fun! | |
| 21:54:21 | cburgess | lol | |