| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-22 | |||
| 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 | |
| 21:55:37 | mriedem | oh yeah, port quota | |
| 21:55:40 | mriedem | need to keep that in mind | |
| 21:57:22 | mriedem | superdan: i was going to create like 100 instances using the fake driver in devstack env and just compare listing instances before and after your changes, have you tried something like that yet? | |
| 21:59:04 | mriedem | i only have 1 cell though so it's not as fun | |
| 22:00:43 | superdan | mriedem: I've been scheming on some tests :) | |
| 22:00:53 | superdan | but I don't have any real numbers or anything no | |
| 22:27:00 | mikal | I'd appreciate a quick review of https://review.openstack.org/#/c/504429/ if anyone is bored, its a fix for a privsep snafu with ploop stuff | |