21:00:06 <ildikov> #startmeeting cinder-nova-api-changes
21:00:07 <openstack> Meeting started Wed Apr 13 21:00:06 2016 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
21:00:08 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
21:00:11 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
21:00:17 <mriedem> o/
21:00:18 <ildikov> scottda ildikov DuncanT ameade cFouts johnthetubaguy jaypipes takashin alaski e0ne jgriffith tbarron andrearosa hemna erlon mriedem gouthamr ebalduf patrickeast smcginnis diablo_rojo
21:00:19 <smcginnis> o/
21:00:35 <scottda> hi
21:00:55 <andrearosa_web> hi
21:00:59 <ildikov> hi
21:01:34 <mriedem> #link etherpad https://etherpad.openstack.org/p/cinder-nova-api-changes
21:01:46 <hemna_> hey
21:02:12 <scottda> if jgriffith Does not show up we shall pile a bunch of work on his plate.
21:02:14 <ildikov> beyond the topics on the agenda I wonder whether we want to have meeting next week as well
21:02:19 <alaski> o/
21:02:41 <hemna_> fwiw, I finished the attach, detach and live migration sequence diagrams
21:02:49 <scottda> hemna_: Those are great.
21:02:53 <hemna_> I also exported them to draw.io editable 'xml' files
21:02:53 <ildikov> either case maybe we should spend a few seconds on whether we need to prepare with more things than what we already have
21:03:06 <ildikov> hemna_: diagrams are awesome!
21:03:09 <hemna_> so we can edit them as needed later.  my gliffy.com account expires in 6 days
21:03:20 <scottda> hemna_: Did you see my question in Cinder? I'm wondering where we need an update_connector API?
21:03:37 <hemna_> yah I saw that
21:03:39 <hemna_> I'm not sure honestly
21:03:43 <scottda> hemna_: It looks to me like we create a new connector from new_host, and delete old one from old_host.
21:03:49 <hemna_> the live migration process was always the need for it
21:04:12 <hemna_> as the attachment entry is being moved from host-1 to host-2
21:04:19 <hemna_> and hence the connector and hostname would change
21:04:35 <mriedem> i've got a high level question,
21:04:40 <scottda> So it is the attachment entry only that needs an update? That makes sense...
21:04:51 <mriedem> do the connector issues need to be ironed out for the live migration stuff as a prerequisite for multiattach?
21:05:01 <hemna_> scottda, yah I think so, and only for making sure the connector and host were updated for force detach.
21:05:15 <hemna_> mriedem, I think they are all connected
21:05:44 <hemna_> we need to make sure we support getting the correct attachment in initialize_connection for multi-attach
21:05:45 <ildikov> mriedem: the issue is when you get two attachments on the same host by migrating an instance
21:06:16 <ildikov> mriedem: in other cases this data is not critical
21:06:19 <hemna_> do we need a diagram for swap volume as well ? :(
21:06:32 <scottda> hemna_: Well, while you are at it....
21:06:38 <hemna_> *sigh*
21:06:38 <mriedem> so is volume-backed live migration broken today?
21:06:44 <mriedem> w/o multiattach
21:06:46 <ildikov> scottda: good answer :)
21:06:52 <hemna_> mriedem, for single attach, it's working afaik.
21:07:17 <mriedem> ok, it should be, we have a gate job for it
21:07:25 <mriedem> which fails anywhere from 25-75% of the time
21:07:26 <ildikov> not nice, but I think it's working
21:07:26 <scottda> mriedem: I couldn't find any bugs associated with live migration and single attach.
21:07:28 <mriedem> but that's besides the point
21:07:31 <hemna_> heh
21:07:42 <smcginnis> Yeah, other than maybe some specific backend errors, I haven't heard of live migration problems.
21:07:49 <smcginnis> IIRC, our test team here has done it.
21:07:59 <hemna_> I noticed a bunch of bugs related to encrypted volumes :(
21:08:00 <mriedem> ok, i was trying to see if there was some latent bug with volume-backed live migration which is why it kept coming up
21:08:09 <hemna_> but that's another topic....
21:08:31 <mriedem> so there are the 4 (really 3) alternatives in the etherpad,
21:08:42 <mriedem> and it sounds like that's really just 2 and 4
21:08:48 <mriedem> they are similar,
21:08:52 <hemna_> yah they are
21:09:00 <ildikov> and the current winner is #2
21:09:05 <mriedem> and john would prefer 2 becaues he doesn't want to change reserve_volume to create the attachment in the cinder db
21:09:06 <mriedem> is that right?
21:09:09 <ildikov> if no objections from this group either
21:09:19 <hemna_> the only thing I an unsure about is how initialize_connection finds the correct attachment
21:09:24 <hemna_> for the multi-attach case
21:09:25 <scottda> mriedem: Yes, that is why he likes #2 over #4
21:10:49 <ildikov> hemna_: in same host case or in every?
21:10:53 <mriedem> so for initialize_connection today we just have the volume id and connector
21:11:02 <hemna_> it looks like his changes in initialize_connection he always calls db.volume_attach
21:11:09 <hemna_> which creates a new volume_attachment DB table entry
21:11:15 <hemna_> :(
21:11:37 <hemna_> this is one of the reasons I liked os-reserve returning an attachment_id.
21:11:40 <mriedem> and with option 4, reserve volume creates the attachment, and nova passes that attachment id to initialize_connection, right?
21:11:49 <hemna_> then nova can call initialize_connection with the attachment_id
21:11:53 <hemna_> and it's explicit
21:11:55 <mriedem> and unreserve would delete the attachment?
21:12:00 <hemna_> mriedem, yes
21:12:02 <mriedem> in case attach fails
21:12:24 <mriedem> what does initialize_connection need to do with the attachment? store the connector info in the vol attachment table?
21:12:34 <hemna_> if initialize_connection took attachment_id, then it could also act as an 'update'
21:12:49 <hemna_> mriedem, yes
21:13:01 <hemna_> it should/needs to store the host and the connector
21:13:03 <jgriffith> mriedem: well... initialize connection does a lot
21:13:12 <scottda> hemna_: Yes, that's what i asked about the spec I posted for update_attachment..is it needed if we can use initialize_connection for that..
21:13:15 <mriedem> hemna_: the host info is in the connector dict isn't it?
21:13:24 <hemna_> mriedem, yah
21:13:26 <hemna_> it is
21:13:27 <hemna_> scottda, yes
21:14:07 <hemna_> sonus, I think w/o multi-attach in the picture, the WIP will have issues w/ live migration
21:14:19 <mriedem> so let's say we go with #2 and init_connection creates the attachment, what deletes it on failure?
21:14:23 <scottda> hemna_: yes as in "I hear what you are saying" or yes as in "new api is needed even if initialize_.... can update"
21:15:17 <hemna_> I hear what you are saying
21:15:37 <hemna_> I'm not sure we need the update if we can get update to work inside of initialize_connection
21:15:55 <scottda> hemna_: Cool. I like that.
21:16:10 <hemna_> crap
21:16:22 <hemna_> so...I left out one thing on the live migration diagram that I have to add in
21:16:24 <hemna_> my bad
21:16:54 <hemna_> post_live_migration calls initialize_connection, prior to calling terminate_connection.
21:17:15 <hemna_> to make sure it has the correct connection_info of the volume on the source n-cpu host.
21:17:31 <hemna_> because the bdm has the connection_info for the destination n-cpu host then.
21:17:46 <hemna_> I'll add that in to the diagram.  sorry about missing that.
21:18:20 <hemna_> it's the one time that nova calls initialize_connection with the intention of only fetching the up to date connection_info for an existing attach
21:18:33 <mriedem> so what rolls back the attachment that's created in the case that the attach fails?
21:18:44 <mriedem> for options 2 and 4?
21:18:45 <hemna_> https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6831
21:18:46 <hemna_> there
21:19:05 <scottda> mriedem: Today, there is no rollback
21:19:17 <hemna_> correct
21:19:19 <hemna_> there is no rollback
21:19:21 <mriedem> is this the force detach issue?
21:19:22 <ildikov> hemna_: which still looks weird, but at least we have a diagram to capture that now
21:19:25 <jgriffith> mriedem: can you describe a failure case?
21:19:29 <scottda> mriedem: Yes, this is
21:19:43 <jgriffith> mriedem: *where/when* is the failure you're considering?
21:19:55 <mriedem> jgriffith: os-attach fails (cinder is down), or nova times out waiting during boot from volume
21:20:09 <mriedem> let me find the compute manager code quick
21:20:28 <mriedem> https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4735
21:20:30 <hemna_> heh if cinder is down, not much we can do
21:20:41 <mriedem> yeah...
21:20:49 <mriedem> so ^ is just making the volume available again
21:20:51 <mriedem> i think
21:21:04 <hemna_> yah I think that resets the state
21:21:32 <jgriffith> mriedem: hemna_ yes it does
21:21:42 <jgriffith> hemna_: mriedem except :)
21:21:51 <jgriffith> your volume is in an error state
21:21:52 <hemna_> https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L581-L588
21:22:03 <scottda> and perhaps export to compute host still exists.
21:22:26 <mriedem> i guess we call terminate_connection before that https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L287
21:22:32 <jgriffith> I guess I'm unclear why any of the existing calls would go away?
21:22:41 <jgriffith> mriedem: exactly
21:22:50 <jgriffith> mriedem: add a call to terminate
21:22:53 <mriedem> so if initialize_connection creates the attachment, terminate_connection could delete it
21:23:00 <mriedem> well,
21:23:04 <mriedem> nvm that
21:23:25 <mriedem> you'd have to only delete the attachment on a failure...
21:23:29 <mriedem> not sure how to signal that
21:23:40 <hemna_> terminate_connection should update that attachment and set the volume state to either 'in-use' or 'available'
21:23:44 <hemna_> which I think it does
21:24:21 <scottda> yeah, terminate_conn calls unreserve()
21:24:35 <mriedem> ok, i guess i was thinking we (nova) could get confused if we needed to check the list of attachments on a volume and there are > 1 but one is actually not attached
21:25:07 <jgriffith> mriedem: what if you store the attachment ID though?
21:25:20 <jgriffith> mriedem: ie, dump it in the bdm table
21:25:25 <mriedem> that was another question, i guess that's a new proposal
21:25:42 <jgriffith> mriedem: it's actually the "nova" side of the wip I put up
21:25:48 <ildikov> I wish I would not need to touch bdm
21:25:49 <hemna_> jgriffith, +1
21:25:52 <mriedem> couldn't the connection_info that's returned from initialize_connection have the attachment id in it?
21:26:03 <jgriffith> mriedem: it does :)
21:26:03 <mriedem> the connection_info is already stored in the bdm
21:26:07 <jgriffith> mriedem: that's my proposal :)
21:27:48 <ildikov> mriedem: does Nova store connection_info as is?
21:27:58 <mriedem> and the bdm should be 1:1 with the volume id and instance uuid, which is the same for the attachment right?
21:28:04 <mriedem> ildikov: yes
21:28:22 <mriedem> ildikov: https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L289
21:28:49 <hemna_> mriedem, yes
21:28:59 <ildikov> mriedem: cool, tnx
21:29:09 <scottda> mriedem: How's that connection info stored? If we add attachment_id, does the DB table need to change?
21:29:13 <mriedem> ok, so what does nova do with the attachment id in the bdm.connection_info?
21:29:21 <mriedem> scottda: nope, just a json blob
21:29:28 <scottda> cool
21:29:38 <hemna> mriedem, pass it with terminate_connection
21:29:38 <mriedem> it's unversioned...which kind of sucks
21:29:56 <hemna> mriedem, and hopefully with a call to initialize_connection if it has it.
21:29:58 <jgriffith> mriedem: so that's the thing I would need to figure out next
21:30:02 <hemna> re: live-migration
21:30:06 <mriedem> ok, so nova will need to have conditional logic for handling this if it's talking to a new enough cinder to provide the attachment id
21:30:06 <jgriffith> mriedem: where/how that's stored and mapped to a volume
21:30:12 <mriedem> and if it's in there, nova can pass it back
21:30:42 <mriedem> https://github.com/openstack/nova/blob/master/nova/objects/block_device.py#L85
21:30:48 <scottda> mriedem: Yes. Some plumbing needed, probably involving the microversion the cinder api supports.
21:30:48 <mriedem> that's the object field
21:31:53 <mriedem> ok, so what does the client side check for nova look like wrt nova and new enough cinder? does nova fail if it's asked to attach a multiattach volume but cinder isn't new enough to provide the attachment id from initialize_connection?
21:32:11 <mriedem> fwiw, nova today can already get the attachment id given the volume id and instance uuid, i think that's what ildikov's code already does
21:32:12 <jgriffith> mriedem: Yes, IMO
21:32:44 <scottda> mriedem: Yes, I agree. WE can just say multi-attach will only work with newton+ Cinder and newton+ Nova
21:32:53 <mriedem> but w/o checking if cinder is new enough, i guess we don't know that the internal plumbing is new enough to handle the multi-detach on the same host?
21:32:57 <ildikov> mriedem: for detach Nova gets the attachment_id from the volume info from Cinder
21:33:28 <hemna> so the attachment_id has been around for a few releases now with Cinder
21:33:48 <mriedem> ildikov: yeah but it filters the attachment list from that volume based on the instance uuid
21:33:49 <mriedem> right?
21:33:50 <hemna> since I put the multi-attach code in cinder, which was...Kilo ?
21:34:00 <smcginnis> hemna: Yep
21:34:06 <ildikov> mriedem: yeap, that's correct
21:34:10 <scottda> But passing in both instance_uuid and host are not plumbed for Cinder, so cinder would need to be Newton+ for this to work anyway
21:34:24 <mriedem> scottda: passing in to which cinder API?
21:34:26 <mriedem> attach?
21:34:32 <smcginnis> scottda: Well, did we ever revert that change. I think it might still.
21:34:38 <scottda> initialize_connection
21:34:45 <mriedem> we pass the connector which has the host...
21:34:50 <ildikov> mriedem: yes, attach fails with having both in the request
21:34:51 <jgriffith> scottda: we don't need it though
21:35:03 <jgriffith> scottda: we already have it on initialize connection, that's been my point all along
21:35:06 <scottda> jgriffith: We don't need it with your Alt#2, that is correct.
21:35:11 <jgriffith> scottda: we have it, but we don't do it with anything
21:35:12 <mriedem> jgriffith: b/c of the connector dict has the host right?
21:35:19 <jgriffith> mriedem: yes, correct
21:35:22 <hemna> the connector has always had the host afaik
21:35:30 <scottda> I'm just saying that, no matter what, Cinder will have to be Newton+
21:35:38 <jgriffith> mriedem: the whole thing is kinda silly... we actually have all of the info we just weren't storing it
21:35:39 <ildikov> scottda: +1
21:35:52 <jgriffith> mriedem: then we got into passing it all again in another later call
21:35:52 <mriedem> jgriffith: and that's why we need to chekc that cinder is newton+ ?
21:35:56 <scottda> There is no controversy that Newton+ Nova with older Cinder won't have multi-attach. It simply will not work.
21:35:59 <hemna> I'm just not sure how we find the right attachment in initialize_connection
21:36:08 <jgriffith> mriedem: yes, that would be the requirement
21:36:17 <mriedem> jgriffith: ok, i'm finally starting to understand :)
21:36:25 <smcginnis> scottda: I think it's completely fair to require N+
21:36:45 <hemna> because we don't have the instance_uuid at initialize_connection time.
21:36:46 <ildikov> scottda: attach would work and detach too if it's not same host same target case, but we need to block the happy scenarios too
21:36:52 <jgriffith> hemna: well... we don't actually have multi-attach yet anyway, so I'd hack some code for that
21:36:56 <jgriffith> hemna: but, that being said
21:37:08 <jgriffith> hemna: if you already have a plan/solution I'm happy to turn it over to you
21:37:18 <hemna> jgriffith, but if we had the attachment_id being passed in to initialize_connection, we would always be able to find the right attachment
21:37:24 <jgriffith> hemna: ok
21:37:28 <jgriffith> hemna: go for it
21:37:43 <ildikov> are going for alternative #4 now?
21:37:49 <mriedem> hold up,
21:37:50 <jgriffith> ildikov: looks like it :)
21:37:53 <hemna> that would work for single attach and multi
21:37:56 <mriedem> so one issue on the nova side with #4 is,
21:38:04 <jgriffith> I'm just tired of beating my head against this wall for no reason :)
21:38:13 <mriedem> nova would have to store the attachment_id in the bdm before we have the connection_info,
21:38:17 <mriedem> that means a schema change to the nova db
21:38:18 <jgriffith> mriedem: yes
21:38:30 <jgriffith> mriedem: and it completely deviates from the purpose of the reserve call
21:38:36 <hemna> jgriffith, I don't think it's for no reason, we are trying to make sure any changes we make work for single attaches and multi-attach.  at least that's what I'm assuming.
21:38:56 <jgriffith> hemna: I mean no reason from my personal side and usage of time
21:39:14 <jgriffith> hemna: I believe I can address your concerns, but I don't care enough to continue arguing
21:39:24 <jgriffith> hemna: if you have code and a proposal that works I say go for it
21:39:25 <smcginnis> I think mriedem makes a good point.
21:39:34 <smcginnis> If we can avoid that, it would be good.
21:39:38 <mriedem> ok, so from a nova perspective, i'm not sure why we need the attachment id when we reserve the volume
21:39:47 <jgriffith> mriedem: and my point last week was that's *impossible* to do
21:39:47 <ildikov> smcginnis: I hear ya
21:39:50 <smcginnis> mriedem: You're saying #2 would be better from the Nova perspective?
21:39:57 <hemna> mriedem, could fake the connection_info at reserve time (/me shudders)
21:40:04 <mriedem> smcginnis: it seems cleaner, i'm just thinking it out
21:40:17 <smcginnis> Yeah, I think so too.
21:40:18 <mriedem> i'm hearing that nova only needs the attachment id at initialize_connection time to pass it in to that API
21:40:37 <mriedem> but we could avoid that with just initialize_connection creating the attachment and returning that id in connection_info
21:40:40 <mriedem> which is then stored in the nova bdm
21:40:47 <hemna> the attachment_id at initialize_connection time is so cinder can find the right attachment to operate on
21:40:50 <mriedem> and we can use to pass to cinder terminate_connection
21:41:11 <mriedem> hemna: but if initialize_connection creates the attachment at that time, then it knows
21:41:13 <jgriffith> mriedem: you are correct in your statements
21:41:14 <mriedem> because it just created it
21:41:24 <smcginnis> hemna: So the issue is that intialize_connection is called repeatedly?
21:41:39 <hemna> mriedem, but there are calls to initialize_connection that are for existing attachments (re: live-migration)
21:41:45 <hemna> smcginnis, yes
21:41:47 <smcginnis> So we only return it if we know we created a new one?
21:41:57 <hemna> live migration calls initialize_connection again right before it calls terminate_connection
21:42:06 <smcginnis> mriedem: Or can nova pass it in on subsequent calls?
21:42:13 <hemna> because the connection_info in the BDM is for the destination n-cpu host and is wrong.
21:42:29 <ildikov> hemna: can we pass attachment_id to Cinder only these cases?
21:42:37 <ildikov> hemna: I mean Nova
21:42:44 <mriedem> ok, so creating the attachment at init_connection doesn't work for when we need to 'update' the original attachment for live migration
21:42:50 <hemna> but in this case
21:42:53 <jgriffith> we could fix the live migration code to behave differently once it has attachment ID's
21:42:56 <hemna> we don't want cinder to update the attachment
21:43:10 <jgriffith> seems easier than building a house of cards and replumbing every API call
21:43:14 <hemna> because it will be updating it to the source n-cpu host, which will shortly be detached.
21:43:19 <mriedem> rather than update the old attachment from the source host, why not create a new attachment table entry for the new host, and then when we're done, delete the old attachment?
21:43:27 <mriedem> so we don't update, we just create a 2nd and delete the 1st?
21:43:30 <jgriffith> mriedem: +1
21:43:44 <hemna> mriedem, yah I think that's what I was going to suggest
21:43:51 <hemna> is that we have 2 attachments
21:43:53 <jgriffith> mriedem: which in fact is what we discussed at the mid-cycles
21:43:54 <hemna> for live migration
21:44:05 <smcginnis> mriedem: +1
21:44:13 <mriedem> ok, so that seems better from a nova pov
21:44:18 <mriedem> and is option #2 right?
21:44:21 <jgriffith> mriedem: hemna my proposal was and is that we treat live-migration as a special case of multi-attach
21:44:32 <hemna> jgriffith, +1
21:44:34 <smcginnis> Because it is multiattached briefly.
21:44:41 <hemna> yah we had chewed on that in the past
21:44:45 <hemna> smcginnis, yah it is
21:44:55 <ildikov> yeah, I remember some stories as well
21:45:02 <hemna> I'll get the live migration diagram up to date
21:45:04 <mriedem> i mean, we could do really hacky stuff and pass the instance uuid in the connector dict to initialize_connection if we needed too...
21:45:09 <mriedem> but i'd like to avoid that if possible
21:45:12 <hemna> it'll show 2 calls to initialize_connection
21:45:13 * jgriffith gathers everybody around the camp fire to tell a tale
21:45:28 <hemna> ew yah
21:45:28 <jgriffith> mriedem: please don't... we'll pay a price later
21:45:42 <mriedem> just saying :)
21:45:44 <smcginnis> jgriffith: Got smores?
21:45:52 <jgriffith> smcginnis: of courrse!
21:45:57 <mriedem> honestly that doesn't sound different from init_connection adding the attachment id to the connection_info that's returned
21:45:59 <mriedem> but ok
21:46:11 <jgriffith> mriedem: wait....
21:46:28 <jgriffith> mriedem: let me think about that for a second
21:46:51 <mriedem> alaski: you might enjoy this part...
21:47:08 <mriedem> alaski: this is why we have unversioned port details dicts with neutron probably, it opens us up to all sorts of fun hacks :)
21:47:09 <jgriffith> mriedem: so, it wouldn't be that terrible as a kwarg or something.  I'm still not completely sure it's necessary though
21:47:34 <mriedem> jgriffith: i think cinder would microversion the init_connection response to add the attachment id
21:47:47 <scottda> mriedem: Yes
21:47:48 <mriedem> something like that, but nova needs to check the microversion
21:47:49 <mriedem> ok
21:48:03 <mriedem> that makes it less of a hack because it signals that there is a new thing in the connection_info response
21:48:30 <hemna> yah that sounds good
21:49:04 <mriedem> note that nova can still find the attachment id to pass to terminate connection if it's needed w/o it being stored in the bdm.connection_info
21:49:06 <jgriffith> mriedem: so circling back to your other statement...
21:49:21 <jgriffith> mriedem: easy fix, add the instance-id to the connector that we already get
21:49:42 <jgriffith> mriedem: actually... I think that's already there in some cases
21:49:58 <mriedem> it might be for vmware i think
21:50:05 <hemna> ah
21:50:14 <mriedem> yup https://github.com/openstack/nova/blob/master/nova/virt/vmwareapi/volumeops.py#L302
21:50:21 <hemna> if we had the instance_uuid in the connector, we can get the correct attachment inside of initialize_connection.
21:50:23 <mriedem> connector is just another unversioned dict that we pass around
21:50:30 <jgriffith> mriedem: :)
21:50:32 <mriedem> hemna: yup
21:50:42 <hemna> problem solved.
21:51:13 <scottda> So what problems are left?
21:51:34 <mriedem> sdague: we're not gauranteed to be able to tell what microversion the volume endpoint is at right?
21:51:37 <mriedem> because some deployments disable that?
21:51:51 <mriedem> guaranteed even
21:52:21 <alaski> mriedem: oh boy. let's please not add unversioned things here
21:52:32 <scottda> mriedem: you can hit http://<vol_endpoint>:8776/ and get it if cinder is mitaka+
21:52:50 <scottda> That's a new Cinder API think in mitaka
21:52:51 <mriedem> scottda: i think version discovery can be disabled though
21:52:53 <mriedem> via policy
21:53:09 <mriedem> we found that out the hard way in adding microversion support to nova client
21:53:14 <mriedem> b/c rackspace and others had it disabled
21:53:15 <ildikov> alaski: not adding new things so far
21:53:19 <scottda> true, I think you are right.
21:53:49 <alaski> ildikov: okay. I'm trying to catch up here
21:54:13 <mriedem> so...
21:54:19 <scottda> So, if you cannot discover Cinder version, you cannot use any new stuffs. I don't think there's any way around that.
21:54:22 <ildikov> alaski: we are playing with already existing ones in theory atm
21:54:42 <mriedem> we can avoid unversioned things if cinder adds a microversion to the terminate_connection api such that it takes a new parameter
21:54:48 <hemna> scottda, but then you always assume the lowest API version no?
21:54:53 <scottda> Other than ugly try:
21:55:19 <hemna> mriedem, terminate_connection needs new things?   I believe it already supports attachment_id
21:55:20 <scottda> hemna: You just assume you are on /v2 (and /v3 at version 3.0 is the equivalent)
21:55:59 <jgriffith> hemna: nah... it gets a connector, but not an AID
21:56:26 <mriedem> hemna: it does? https://github.com/openstack/python-cinderclient/blob/master/cinderclient/v2/volumes.py#L73
21:56:32 <hemna> huh
21:56:34 <hemna> yah you are right
21:56:35 <hemna> nm
21:56:40 <hemna> ignore me.
21:57:21 <mriedem> so do we need initialize_connection to return the attachment id? i'm not sure that it does
21:57:36 <mriedem> nova can figure that out by filtering the volume attachments list by instance uuid,
21:57:49 <hemna> mriedem, I think jgriffith's proposal is that it will, inside the connection_info dict it's already returning.
21:57:51 <mriedem> or just pass the instance uuid to terminate_connection with a microversion that takes instance uuid as a 3rd parameter
21:58:02 <mriedem> hemna: but we don't need it, i don't think
21:58:19 <mriedem> unless it's purely about signaling that we're talking to a version of cinder that has that behavior
21:58:32 <jgriffith> mriedem: I think either is doable, I had planned to return it
21:58:34 <mriedem> in which case, that works for me
21:58:51 <mriedem> if it's not there and multiattach, nova fails
21:59:12 <mriedem> it would be nice if the nova api could detect this support earlier in the process though
21:59:21 <mriedem> which i guess is the version discovery api
21:59:24 <jgriffith> mriedem: the trick is as you mention the versioning to know if things are actually *in* the table or not.  If you solve that via micro-v then so beit
21:59:51 <jgriffith> mriedem: I'll write  an API method "discover-api-version" for you
21:59:56 <mriedem> #action mriedem to talk to sdague about version discovery
22:00:16 <mriedem> we've worked around some of this with neutron via their extension list
22:00:16 <scottda> mriedem: Please keep me in the loop on that conversation
22:00:29 <jgriffith> honestly it would probably be useful to have said call anyway
22:00:30 <mriedem> nova checks the neutron extension list to see if we can do a new thing
22:00:32 <scottda> Since My next cinderclient patch is automatic version discovery.
22:01:08 <scottda> Yeah, but isn't that because Neutron doesn't have microversions?
22:01:16 <smcginnis> Gotta run, thanks everyone.
22:01:16 <mriedem> i think so yeah
22:01:22 <scottda> I mean, this is the point of microversions.
22:01:24 <mriedem> i have to head out soon too,
22:01:32 <scottda> I need to head as well.
22:01:32 <mriedem> so are we in soft agreement on option 2
22:01:43 <mriedem> init connection returns the attachment id in the connection_info
22:01:58 <mriedem> nova passes the instance uuid or attachment id to terminate_connection
22:02:05 <scottda> ildikov: I think we might as well keep open a meeting for next week. Just for discussing details and other items.
22:02:08 <mriedem> and we have a todo to talk to sdague about version discovery
22:02:31 <mriedem> ^ sound good?
22:02:34 <ildikov> scottda: sure, I'll check back on Monday and send out a reminder mail as well if we feel the need
22:02:35 <smcginnis> +1
22:02:38 <scottda> mriedem: yes
22:02:43 <hemna> mriedem, nova pass the instance_uuid to initialize_connection as well ?
22:02:44 <ildikov> mriedem: +1
22:02:50 <hemna> so cinder can find the right attachment
22:02:54 <hemna> (for multi-attach)
22:02:55 <mriedem> hemna: not sure that is needed
22:02:59 <hemna> it is
22:03:03 <mriedem> then sure :)
22:03:06 <hemna> ok
22:03:08 <hemna> +A
22:03:23 <mriedem> i'll assume you guys are updating the etherpad
22:03:27 <mriedem> and gals
22:04:04 <scottda> #action scottda update etherpad with this meeting's conclusions
22:04:06 <mriedem> if we pass a new parameter to initialize_connection and it explodes, we'll know we're not talking to a new enough cinder
22:04:20 <mriedem> ^ in the case that we can't detect the version ahead of time
22:04:49 <hemna> #action hemna updates the live-migration sequence diagram to include the initialize_connection call in post_live_migration() time.
22:05:09 <scottda> mriedem: True enough. or if we don't detect the version, just don't try and assume old behaviour, which is no multi-attcach and don't pass it in.
22:05:25 <mriedem> yeah, fast fail in the api
22:05:49 <scottda> OK, I need to run as well. Thanks everyone
22:05:53 <mriedem> yup, thanks
22:05:58 <ildikov> thanks guys
22:06:38 <ildikov> #endmeeting