| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 16:13:33 | mriedem | this is specific to network data though | |
| 16:13:59 | mriedem | idk what's the best solution here, i'm not an expert on the metadata service or how a guest would consume trusted SRIOV ports in the guest, | |
| 16:14:15 | mriedem | but it's obviously something we should think about how that gets modeled so the guest application can actually leverage the thing | |
| 16:14:44 | artom | mriedem, wait, we're talking about trusted VFs? | |
| 16:15:22 | artom | What's wrong with just having an interface: {mac: blah, trusted:true} blob in there. | |
| 16:15:22 | mriedem | yes | |
| 16:15:25 | artom | ? | |
| 16:15:35 | artom | The exact format is obviously to be worked out :) | |
| 16:15:36 | mriedem | hold dear caller | |
| 16:16:28 | mriedem | artom: https://review.openstack.org/#/c/485522/ | |
| 16:16:33 | mriedem | questions/comments are in there | |
| 16:16:48 | diga | jaypipes: Hi | |
| 16:17:10 | diga | jaypipes: assigned this bug - https://bugs.launchpad.net/nova/+bug/1751692 to me | |
| 16:17:11 | openstack | Launchpad bug 1751692 in OpenStack Compute (nova) "os_region_name an unnecessary required option for placement " [Low,Triaged] - Assigned to Digambar (digambarpatil15) | |
| 16:17:18 | artom | mriedem, aha, thanks - I keep being dragged off for downstream stuff, but I've already added myself in there, will try to take a look | |
| 16:23:19 | mriedem | dansmith: mdbooth: ok for this bug, first thing, the bug report says, "During live migration we update bdm.connection_info for attached volumes in pre_live_migration to reflect the new connection on the destination node." - in the libvirt driver pre_live_migration i see where we connect the volumes on the dest host: https://github.com/openstack/nova/blob/2ec8c49f6cb4a0e7dba217e824c20d9c703d2105/nova/virt/libvirt/driver.py#L | |
| 16:23:35 | mriedem | and we stash the bdm info for the migration data object https://github.com/openstack/nova/blob/2ec8c49f6cb4a0e7dba217e824c20d9c703d2105/nova/virt/libvirt/driver.py#L7621 | |
| 16:23:51 | mriedem | but don't see where the bdm.connection_info gets updated and saved off from the dest host into the db | |
| 16:24:07 | dansmith | hang on | |
| 16:24:14 | dansmith | oh, where it gets saved | |
| 16:24:18 | mdbooth | mriedem: I think I put that in a review comment somewhere because it's super obtuse | |
| 16:24:24 | dansmith | we look it up from the db again, but.. | |
| 16:24:32 | dansmith | is this the weird commit() decorator thing? | |
| 16:24:45 | mriedem | update_db()? | |
| 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 | |