| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 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 | |
| 16:49:10 | mriedem | who wants to update him? | |
| 16:49:19 | mriedem | i can start | |
| 16:49:23 | mriedem | geguileo: so you're familiar with https://bugs.launchpad.net/nova/+bug/1754716 right? | |
| 16:49:25 | dansmith | geguileo: it's about this: https://bugs.launchpad.net/nova/+bug/1754716 | |
| 16:49:25 | 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:49:39 | dansmith | geguileo: which was a cinder backend fix for the dell driver, plus making nova not call initialize_connection() a second time | |
| 16:49:46 | mriedem | geguileo: i'm trying to understand if initialize_connection is supposed to be idempotent or not | |
| 16:49:48 | smcginnis | Swanson: You may be interested too. ^^ | |
| 16:49:55 | Swanson | smcginnis, already there | |
| 16:50:03 | dansmith | geguileo: you asserted in our downstream bug that calling initialize_connection really late during disconnect after a live migration was wrong and we should avoid doing it | |
| 16:50:04 | geguileo | mriedem: the problem is that Cinder never asked drivers to be idempotent | |
| 16:50:05 | Swanson | here | |
| 16:50:06 | jgriffith | mriedem: you're just begging for 3 different answers :) | |
| 16:50:11 | smcginnis | mriedem: It is, but that was a surprise to most around the Liberty timeframe. | |
| 16:50:37 | geguileo | dansmith: it's wrong because Cinder never specified that initialize_connection should be idempotent :-( | |
| 16:50:37 | smcginnis | Or rather, it wasn't, until we found out around liberty that nova was initializing repeatedly. | |
| 16:50:48 | jgriffith | ummm..... | |
| 16:50:55 | mriedem | smcginnis: yeah we do it all the time during migrations because we only have one copy of the connection_info at any time with the old flow | |
| 16:51:19 | mriedem | so the only copy of the connection_info is in the nova bdm table, and it's host-specific | |
| 16:51:25 | mriedem | on which was the last host to update it | |
| 16:51:34 | mriedem | with the new flow, that's all stored in cinder per-host attachment | |
| 16:51:37 | mriedem | so nova doesn't have to care | |
| 16:51:43 | jgriffith | initialize_connection has never been designed as an idempotent thing, in fact it was intentionally abused for that very reason | |