| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-21 | |||
| 20:17:50 | superdan | that's a double kick in the pands | |
| 20:18:03 | mriedem | meanwhile we've deleted your disks | |
| 20:18:19 | mriedem | this is really like, well, it's all busted, oh and btw we deleted your instance | |
| 20:18:20 | mriedem | you're welcome | |
| 20:18:28 | superdan | whatever, libvirt told us to :) | |
| 20:18:37 | mriedem | wow | |
| 20:18:49 | mriedem | remind me to never run live migration if i'm in ops | |
| 20:18:51 | superdan | if libvirt told us it was done, then there should be disks on the other side too | |
| 20:19:00 | mriedem | yeah hopefully | |
| 20:19:21 | superdan | it's scary how many people depend on live migration | |
| 20:24:45 | coffee_cat | lyarwood: are you around? | |
| 20:27:18 | coffee_cat | or anyone who could/would answer stupid BDM questions | |
| 20:27:31 | mriedem | i can answer stupid questions | |
| 20:27:44 | mriedem | coffee_cat: are you in budapest right now? | |
| 20:27:48 | mriedem | or the US? | |
| 20:27:52 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/destroy #4: full flavor https://review.openstack.org/391288 | |
| 20:28:05 | coffee_cat | mriedem: in the US until tomorrow evening | |
| 20:28:11 | mriedem | oh, lee is in the UK | |
| 20:28:17 | mriedem | so he's in a pub somewhere | |
| 20:28:18 | coffee_cat | mriedem: will fly back next Sunday though... | |
| 20:28:31 | coffee_cat | mriedem: ah, I forgot | |
| 20:29:13 | coffee_cat | mriedem: anyhow, I'm sure you can help me out here to understand what's going on :) | |
| 20:29:22 | coffee_cat | mriedem: so his comments are here: https://review.openstack.org/#/c/456971/2/nova/compute/manager.py | |
| 20:30:33 | coffee_cat | mriedem: and I started to wonder whether the BDM is host or instance specific? if this question even makes sense... | |
| 20:31:40 | mriedem | the bdm is instance + volume specific | |
| 20:31:50 | mriedem | so we get the bdm for the instance and existing (old) volume on L5011 | |
| 20:33:32 | mriedem | the connector is specific to the host | |
| 20:33:37 | mriedem | well, host + instance | |
| 20:35:23 | mriedem | so in the old way, after we swap the volume, we terminate the connection for the old volume, | |
| 20:36:02 | mriedem | and then update the bdm with the new connection_info for the new volume | |
| 20:41:23 | mriedem | coffee_cat: left comments inline | |
| 20:41:32 | mriedem | coffee_cat: this was also called out in john's spec i believe | |
| 20:41:52 | mriedem | https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/cinder-new-attach-apis.html#swap-volume | |
| 20:42:26 | coffee_cat | mriedem: ok, cool, makes sense, so basically need to create a new attachment for the new volume | |
| 20:42:46 | coffee_cat | and we might not be able to consolidate the workflow that much, but oh well | |
| 20:42:52 | mriedem | yes | |
| 20:43:07 | mriedem | where we used to call initialize_connection for the new volume, now we'll need to create a new attachment | |
| 20:43:08 | mriedem | for the new volume | |
| 20:43:28 | coffee_cat | ok, that sounds easily doable | |
| 20:43:32 | mriedem | if everything is ok, we delete the old attachment for the old volume and update the bdm with the new attachment_id | |
| 20:44:22 | mriedem | john's spec is talking about doing things in the api too, which i'm not really following | |
| 20:44:36 | mriedem | (api) create new attachment for the volume uuid-new, fail API call if we can’t create that attachment | |
| 20:44:52 | mriedem | ^ is what i was saying we could do on the compute where initialize_connection is called for the new volume | |
| 20:45:03 | mriedem | we could create the attachment for the new volume in the api, but we don't have a bdm for that | |
| 20:45:18 | mriedem | so not sure how we get that in the compute, unless we lookup the new volume details and get it's attachment list | |
| 20:45:21 | coffee_cat | yeah, BDM is problematic :/ | |
| 20:46:25 | mriedem | maybe he just meant create the new attachment in the api so we can fail fast | |
| 20:46:39 | mriedem | but, | |
| 20:46:42 | coffee_cat | probably as that was a topic many times | |
| 20:46:59 | mriedem | we'd only do that if we knew the old attached volume was created with the new flow i htink | |
| 20:47:01 | mriedem | *think | |
| 20:47:05 | mriedem | i'm not sure if that matters or not | |
| 20:47:08 | mriedem | but it probably does | |
| 20:47:33 | coffee_cat | so the volume is always created the same way | |
| 20:47:58 | mriedem | this is all new flow stuff | |
| 20:47:59 | mriedem | "In this section, we talk about what happens should the volume being swapped have the attachment_id present in the BDM, and as such we follow the new flow." | |
| 20:48:25 | mriedem | yeah, so if the bdm.attachment_id is set, we key off that for all of the swap volume goodies outlined in the spec | |
| 20:48:31 | mriedem | like creating a new attachment for the new volume, etc | |
| 20:48:36 | coffee_cat | and for the new volume we can create that with the new flow | |
| 20:49:01 | coffee_cat | if the API is available | |
| 20:49:11 | mriedem | well i think the point is, you can't swap from a volume attachd the old way, to a volume attached the new way | |
| 20:49:28 | mriedem | you have to swap from attached new way to attached new way | |
| 20:49:30 | mriedem | if that makes sense | |
| 20:49:42 | mriedem | i don't know if technically it makes a difference on the cinder side | |
| 20:49:46 | coffee_cat | I don't really see why not, but as we go by attachment_id in the BDM anyhow it pretty much doesn't matter | |
| 20:49:49 | sdague | mriedem: it would be less terrible if you used the fixture that actually captures the log stream | |
| 20:50:22 | mriedem | coffee_cat: the reason john and i said to do it this way in the spec is so nothing else in the code does anything using the new apis until we've plumbed in support for attaching volumes the new way | |
| 20:50:22 | smcginnis | mriedem: Should be OK on the Cinder side as long as each volume has its attachment info. | |
| 20:50:25 | mriedem | since everything keys off that | |
| 20:50:35 | coffee_cat | in the sense that we only detach --> attach with the new flow if the BDM.attachment_id is set | |
| 20:50:38 | smcginnis | mriedem: ie, we don't do some silly DB swap of some data to "make it easier" | |
| 20:51:04 | mriedem | coffee_cat: i think i'm agreeing with you | |
| 20:51:47 | mriedem | the thing i don't like about https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/cinder-new-attach-apis.html#swap-volume and creating the new attachment for the new volume in the API is, we'd have to do a service version check on that b/c we wouldn't know if the compute we're going to cast to is new enough to understand what we're doing | |
| 20:51:50 | coffee_cat | mriedem: ok, if we want to do this in the API, where we don't have the BDM that's another thing | |
| 20:52:04 | mriedem | i don't think we want to do it in the api | |
| 20:52:15 | mriedem | since that logic relies on if the compute is new enough i think | |
| 20:52:27 | mriedem | if we're casting to an ocata compute, it's not going to have any of this new bdm.attachment_id code | |
| 20:52:32 | coffee_cat | although there we need to check whether the new Cinder API is available, etc. | |
| 20:52:50 | mriedem | well, that's why we key off bdm.attachment_id, | |
| 20:52:53 | mriedem | if that is set, we're using new api | |
| 20:53:05 | mriedem | that's why new style attach comes at the very end | |
| 20:53:13 | mriedem | since everything else keys off that | |
| 20:53:13 | coffee_cat | mriedem: yeah, on the compute it's all fine and I think we're on the same page if we keep things there | |
| 20:53:26 | mriedem | coffee_cat: ok i can amend the spec for that | |
| 20:53:31 | mriedem | when i add in the stuff we talked about this morning | |
| 20:53:39 | mriedem | about begin_detaching | |
| 20:53:51 | coffee_cat | mriedem: sounds good, thank you | |
| 21:12:13 | mriedem | maybe john had us creating the new attachment in the api in place of calling reserve on the new volume | |
| 21:13:03 | mriedem | but i thought with the new flow, we still called reserve when doing an attach? | |
| 21:13:59 | mriedem | coffee_cat: do you remember? ^ | |
| 21:14:11 | mriedem | the spec doesn't talk about reserve | |
| 21:15:09 | mriedem | hmm, the cinder spec talks about it http://specs.openstack.org/openstack/cinder-specs/specs/ocata/add-new-attach-apis.html#proposed-change | |
| 21:15:33 | mriedem | "If the connector is not specified this call will only complete the volume reservation step and create an empty volume attachment. This can be used by Nova to call it from the API and start the volume attaching process by checking that the volume is available and creates the empty attachment." | |
| 21:16:19 | mriedem | and in the nova spec | |
| 21:16:20 | mriedem | https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/cinder-new-attach-apis.html#proposed-change | |
| 21:16:29 | mriedem | "In addition we should only use the new API when all of the nova-compute nodes have been upgraded." | |
| 21:17:05 | mriedem | so if we only create a new attachment when cinder 3.27 is available and all computes are upgraded, then during swap_volume we can create the new attachment for the new volume based on bdm.attachment_id being set for the old volume | |
| 21:17:31 | mriedem | we could pass the new attachment_id over rpc to swap_volume on the compute | |
| 21:18:32 | coffee_cat | mriedem: we don't call reserve with the new flow | |
| 21:18:53 | mriedem | coffee_cat: yeah ok so then we need to create the new attachment in the api | |