| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 16:25:44 | mdbooth | Hmm, perhaps I'm imagining the review comment. Or perhaps it was a different review. | |
| 16:25:52 | dansmith | yeah I don't see a comment | |
| 16:25:57 | mriedem | i don't see pre_live_migration() go through nova.virt.block_device to attach anything though | |
| 16:26:04 | dansmith | but BDMs get magically saved I think no? | |
| 16:26:08 | mriedem | the virt driver gets the bdms and connects them on the host directly | |
| 16:26:28 | mriedem | block_device_info = self._get_instance_block_device_info( | |
| 16:26:28 | mriedem | so, pre_live_migration on the dest host calls this | |
| 16:26:29 | mriedem | bdms=bdms) | |
| 16:26:29 | mriedem | context, instance, refresh_conn_info=True, | |
| 16:26:45 | mriedem | and that will initialize the connection on the dest host and update the connection_info and save it | |
| 16:26:47 | mriedem | so that's likely it | |
| 16:26:54 | mdbooth | It's in _get_instance_block_device_info() I think | |
| 16:27:01 | mriedem | yeah ^ | |
| 16:27:09 | mdbooth | Yeah, that's it | |
| 16:27:12 | mdbooth | refresh_conn_info | |
| 16:27:17 | mriedem | the thing dansmith just approved my multiattach patch | |
| 16:27:38 | mriedem | right so during pre_live_migration on the dest host the compute manager gets here https://github.com/openstack/nova/blob/2ec8c49f6cb4a0e7dba217e824c20d9c703d2105/nova/virt/block_device.py#L633 | |
| 16:27:45 | mriedem | uses the host connector from the dest host | |
| 16:27:49 | mriedem | gets a new connection_info and updates the bdm | |
| 16:27:54 | mriedem | because @update_db | |
| 16:27:54 | openstackgerrit | Saju M proposed openstack/python-novaclient master: pypy is not checked at gate https://review.openstack.org/554983 | |
| 16:28:05 | mriedem | and then uses that to connect the volumes in the virt driver | |
| 16:28:18 | mriedem | and that gets put into the LibvirtLiveMigrateBDMInfo objects | |
| 16:28:42 | mriedem | with the new flow, the connection_info for the source and dest attachments are stored in cinder with those attachments, so not a problem for the new flow | |
| 16:29:05 | mriedem | problem in nova is we have 1 bdm for all attachments so the connection_info gets overwritten | |
| 16:29:08 | dansmith | jump down, turn around, pick a bail of cotton | |
| 16:29:46 | dansmith | teag | |
| 16:29:48 | dansmith | er, yeah | |
| 16:29:52 | mriedem | now i'm thining about line dancing to brooks and dunn in 6th grade gym class, | |
| 16:29:53 | mriedem | thanks for that | |
| 16:30:00 | dansmith | mriedem: so did you see my comment about not changing the new path? | |
| 16:30:13 | dansmith | since I think it's immune | |
| 16:30:15 | mriedem | i'm just loading context from the first sentence in the bug report at this point :) | |
| 16:30:25 | mriedem | and also https://www.youtube.com/watch?v=d05tQrhNMkA | |
| 16:30:26 | dansmith | heh, okay | |
| 16:31:04 | dansmith | ain't nothin' wrong with that | |
| 16:31:14 | mriedem | there is plenty wrong with that | |
| 16:31:30 | mriedem | ok so back to this | |
| 16:32:36 | mriedem | ok so in _post_live_migration in the compute, we get the BDMs again but don't refresh their connection_info b/c that would screw up the bdms which are now on the dest host, | |
| 16:33:11 | mriedem | we call into the virt driver's post_live_migration method to disconnect the volumes and have to call initialize_connection from the source host to get the proper connection_info for the source host | |
| 16:33:15 | mriedem | for the old flow | |
| 16:34:14 | mriedem | and at this point, the cinder driver is returning different connection_info from what was used to originally connect the volume on the source host, and we blow up | |
| 16:35:00 | dansmith | mriedem: youtube tells me there's an upcoming Brooks & Dunn concert in MN in July.. better get on that | |
| 16:35:01 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Allow to specify granular CPU feature flags https://review.openstack.org/534384 | |
| 16:35:18 | mriedem | now, the goal here is to stash off the original source host connection_info so we can use that to disconnect later? | |
| 16:35:22 | mriedem | post-live migrate | |
| 16:35:30 | dansmith | yeah | |
| 16:35:44 | mriedem | what if we should be using the latest connection_info from cinder for the source host to disconnect? | |
| 16:35:53 | mriedem | like, what if the rbd driver changed the auth information for the ceph backend? | |
| 16:36:00 | mriedem | and the original stuff in the source host connection_info is stale | |
| 16:36:31 | dansmith | mriedem: did you check out the cinder bug and patch? | |
| 16:36:41 | dansmith | gorka says we shouldn't sending the latest | |
| 16:36:52 | mriedem | looking | |
| 16:37:04 | dansmith | I dunno if that means we shouldn't be sending the thing we grabbed before the migration if it changed right after, | |
| 16:37:10 | dansmith | but that'd be fairly obscure and hard to do I think | |
| 16:37:23 | dansmith | we'd have to catch it before it gets set and overwritten | |
| 16:38:12 | dansmith | well, I guess gorka's comments are in our downstream bug actually | |
| 16:38:27 | mriedem | yeah was going to say | |
| 16:38:38 | mriedem | i feel like i'll get different answers based on which cinder cores i ask | |
| 16:38:42 | mriedem | jgriffith: smcginnis | |
| 16:38:46 | mriedem | hemna | |
| 16:39:16 | mriedem | having said that, for the new flow, the original connection_info from the source host is stored in cinder with the attachment record for the source host, | |
| 16:39:27 | dansmith | mriedem: can you see this? https://bugzilla.redhat.com/show_bug.cgi?id=1552232 | |
| 16:39:28 | openstack | dansmith: Error: Error getting bugzilla.redhat.com bug #1552232: NotPermitted | |
| 16:39:29 | mriedem | so when we disconnect post live migration, we'll be using that original value, not something new | |
| 16:39:43 | mriedem | dansmith: nope | |
| 16:39:43 | dansmith | guess not | |
| 16:40:22 | dansmith | mriedem: yeah, using the original attachment id is definitely better than having to have these connectors line up and potentially be changed if you're saying that could happen mid-migration | |
| 16:41:54 | mriedem | i guess my point is just, i wouldn't be surprised if at some later date, someone reports a bug saying something in the storage backend or source host changed and they actually need to get the latest information to disconnect | |
| 16:42:21 | mriedem | sort of like the discussion in denver about always refreshing connection_info whenever we do stuff | |
| 16:42:29 | mriedem | to get things like latest ceph mon address and creds | |
| 16:42:32 | dansmith | mriedem: so actually, re-reading gorka's comments, | |
| 16:43:07 | dansmith | mriedem: what he's saying we shouldn't be doing is calling initialize_connection again to get the connector from the volume | |
| 16:43:18 | dansmith | which we're then using to detach | |
| 16:43:30 | dansmith | so old line 7724 here: https://review.openstack.org/#/c/551302/6/nova/virt/libvirt/driver.py | |
| 16:44:25 | mriedem | yeah i understand, but as noted, we have to do that because right now with the old flow, the bdm.connection_info is from the dest host, and the only way to get the connection_info with the old flow for the source host is to call initialize_connection with the source host connector | |
| 16:44:50 | dansmith | yeah I know | |
| 16:45:25 | mriedem | so i guess the point is calling initialize_connection when the volume is already connected on another host can cause problems if you call it again, for some cinder backends | |
| 16:45:38 | mriedem | but we've been doing this forever | |
| 16:45:43 | dansmith | mriedem: "So Nova should not be making a second initialize connection for a volume that is already attached to the node and use the information it already has in the DB to do the disconnect" | |
| 16:46:07 | mriedem | except we didn't have that information in the db | |
| 16:46:08 | dansmith | ^ gorka's comments from the downstream bug | |
| 16:46:11 | mriedem | hence the workaround | |
| 16:46:33 | dansmith | mriedem: well, it's just one backend remember, and that one is fixed now | |
| 16:46:35 | mriedem | if initialize_connection is supposed to be idempotent, which i always thought it was, i don't see the issue | |
| 16:46:43 | jgriffith | mriedem: you have a link to the bug? | |
| 16:46:51 | mriedem | jgriffith: nova bug https://bugs.launchpad.net/nova/+bug/1754716 | |
| 16:46:52 | openstack | Launchpad bug 1754716 in OpenStack Compute (nova) "Disconnect volume on live migration source fails if initialize_connection doesn't return identical output" [Undecided,In progress] - Assigned to Matthew Booth (mbooth-9) | |
| 16:46:56 | mriedem | jgriffith: cinder bug https://bugs.launchpad.net/cinder/+bug/1756914 | |
| 16:46:57 | openstack | Launchpad bug 1756914 in Cinder "Dell EMC SC: Initialize_connection returns all connections" [Undecided,Fix released] - Assigned to Tom Swanson (tom-swanson) | |
| 16:47:06 | mriedem | plus a red hat bz that we don't have access to | |
| 16:47:14 | dansmith | yeah, sorry :( | |
| 16:47:28 | dansmith | jgriffith: gorka has context on this but I don't see him around here | |
| 16:47:46 | mriedem | he's in -cinder | |
| 16:47:53 | jgriffith | Yeah, not in this channel, I'll ping him | |
| 16:47:58 | dansmith | I looked, maybe I don't know his nick? | |
| 16:48:05 | dansmith | I thought it was geguilar or something | |
| 16:48:08 | mriedem | he's been pung | |
| 16:48:27 | dansmith | huh, sorry, I looked and just missed it | |
| 16:48:36 | geguileo | mriedem: jgriffith hi | |