16:00:12 <ildikov> #startmeeting cinder-nova-api-changes
16:00:13 <openstack> Meeting started Thu Dec 14 16:00:12 2017 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:14 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:17 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
16:00:22 <ildikov> johnthetubaguy jaypipes e0ne jgriffith hemna mriedem patrickeast smcginnis diablo_rojo xyang1 raj_singh lyarwood jungleboyj stvnoyes
16:01:04 <jungleboyj> @!
16:01:05 <_pewp_> jungleboyj (*´･д･)ﾉ
16:01:08 <mriedem> o/
16:01:30 <ildikov> good morning gentlemen :)
16:01:45 <jungleboyj> Good morning.
16:02:06 <ildikov> we can quickly go through things I think
16:02:37 <ildikov> we have a bug fixing round that has bits and pieces in both Nova and Cinder for the new attach code
16:02:48 <ildikov> #link https://review.openstack.org/#/q/topic:bug/1737779+(status:open+OR+status:merged)
16:03:14 <mriedem> there is a cinder one now too
16:03:18 <ildikov> #link https://review.openstack.org/#/c/527852/
16:03:22 <mriedem> https://review.openstack.org/#/c/527852/
16:03:28 <mriedem> ahead of me
16:03:37 <ildikov> mriedem: yep, just didn't have the link handy :)
16:03:46 <mriedem> i'll update ^ once i'm out of meetings
16:03:48 <mriedem> and ML threads
16:03:51 <mriedem> and tc channels
16:03:53 <mriedem> oh my
16:03:57 <jungleboyj> mriedem:  Great and then I will review.
16:03:59 <ildikov> mriedem: thanks for stepping up on that one, I wasn't sure whether or not we would want to drop attachment_specs
16:04:53 <ildikov> mriedem: sorry about the meeting :( :)
16:05:23 <ildikov> mriedem: as for the Nova side, do we have anything else beyond testing on those patches?
16:05:42 <mriedem> just needs reviews
16:05:49 <mriedem> i already asked for that in the nova meeting earlier today
16:05:56 <mriedem> i'll bug melwitt later
16:06:03 <ildikov> ah ok, missed that part
16:06:05 <ildikov> thanks!
16:06:46 <ildikov> as for reviews, we have the Cinder side spec for multi-attach still open: https://review.openstack.org/#/c/523608/
16:07:09 <ildikov> with a recent comment to move it Rocky from tommylikehu
16:07:35 <ildikov> jungleboyj: what's your take on this?
16:08:59 <jungleboyj> ildikov:  In multiple meetings.
16:09:27 <ildikov> jungleboyj: ok, no worries, we can get back to it, when you have a minute
16:09:39 <ildikov> mriedem: I started to update the libvirt patch
16:09:40 <jungleboyj> ildikov:  Why was that put in there?
16:10:11 <ildikov> jungleboyj: I'm not sure I understand your question
16:10:17 <ildikov> jungleboyj: the spec is not approved yet
16:10:29 <mriedem> i haven't read the cinder spec yet
16:10:37 <mriedem> i wasn't sure why there was a cinder spec honestly
16:10:43 <smcginnis> It's up to jungleboyj, but I think we will still want that spec even though it's past spec freeze.
16:10:46 <ildikov> jungleboyj: and it seems we're over some deadline now that would suggest to move it Rocky now?
16:10:54 <mriedem> i thought nova just needed the shared_targets field on the volume to disconnect safely
16:10:54 <smcginnis> mriedem: Someone told jgriffith to write one.
16:10:57 * smcginnis looks at ildikov
16:11:15 <jgriffith> I was just following orders
16:11:15 <mriedem> is there anything in the cinder spec that nova actually needs to add multiattach support?
16:11:23 <ildikov> mriedem: we talked about that spec two weeks ago or three, johnthetubaguy had comments that got fixed
16:11:55 <ildikov> mriedem: it's mainly about the policies and how to specify 'multiattach' at volume creation and after
16:12:00 <jungleboyj> I don't think the spec was that far off and I think that we were close to having the pieces in place to integrate for Rocky.
16:12:02 <mriedem> ok
16:12:14 <mriedem> if it's mostly policy knobs, sure
16:12:16 <smcginnis> I belive the idea was to make sure to get written down some of our assumptions and decisions so we didn't have to rehash it 100 times.
16:12:18 <ildikov> mriedem: not a long read, if you have a minute to glance through if you see anything odd, I think it should be ok b now
16:12:19 <smcginnis> Just 50.
16:12:22 <jungleboyj> Long story short, I think we need to get the spec in for Queens.
16:12:35 <jgriffith> smcginnis: 75
16:12:35 <mriedem> you need the policy rules for sure
16:12:52 <mriedem> because some nova backends don't support multiattach so deployments will need to turn it off
16:12:57 <smcginnis> jgriffith: 60, that's my final offer.
16:13:05 <ildikov> smcginnis: jgriffith: my bad, I like having some notes on what we agreed to do :)
16:13:06 <jgriffith> smcginnis: sold
16:13:23 <jgriffith> ildikov: you weren't the only one that asked for it so don't worry about it
16:13:30 <smcginnis> ildikov: No, it does make sense. We just need someone to approve it.
16:13:34 <ildikov> 60 is a nice number :)
16:13:49 <jgriffith> and it is good to have it written down so somebody can work it out
16:14:09 <ildikov> jgriffith: yeah, I guess I wanted notes I can follow... :)
16:15:13 <ildikov> mriedem: the only argument we had was handling 'multiattach' as volume type at creation time and then blocking the ability to modify it once the volume is attached
16:15:27 <jungleboyj> I will go approve it if that means we can move on.
16:15:43 <jungleboyj> jgriffith: ildikov if I approve it do we think we can still get it in, in queens?
16:15:45 <mriedem> i think definitely on the latter
16:16:02 <mriedem> changing a multiattach volume that is in-use likely screws up stuff on the nova side
16:16:03 <ildikov> jungleboyj: that would be the idea
16:16:17 <mriedem> 409 IMO
16:16:23 <ildikov> jungleboyj: mriedem has been missing some sleep for a few days/weeks now to make it happen for Queens
16:16:25 <jungleboyj> ildikov:  Ok, I will review and we should target approving today.
16:16:39 <jungleboyj> mriedem is the best!
16:16:50 <ildikov> mriedem: yeah, we cannot play with the domain xml, or at least I don't want to
16:17:24 <ildikov> mriedem: so no modifying that value once the volume is attached, if it's back to available then I don't care who does what with it until the next attach
16:17:35 <ildikov> jungleboyj: cool, thanks
16:17:48 <mriedem> ildikov: same
16:17:54 <ildikov> and yes, mriedem is the best!!!
16:17:56 <mriedem> i'll have to read the spec about the volume type thing
16:18:09 <mriedem> i thought we said last time that the existing boolean flag on the volume was enough
16:18:16 <mriedem> even though the volume type might not support it
16:18:26 <mriedem> maybe fail the volume create in that case?
16:18:33 <mriedem> or fail the PUT /volumes change
16:18:40 <ildikov> mriedem: it is still a flag on the volume from the volume object perspective
16:18:40 <jungleboyj> Can we retrype a volume when it is attached?
16:18:46 <jungleboyj> jgriffith: ^^^
16:18:47 <mriedem> jungleboyj: yes
16:18:51 <mriedem> that's the swap volume API in nova
16:18:55 <ildikov> mriedem: jgriffith clarified it in the spec as I got confused as well
16:19:04 <jungleboyj> Holy shit.  Didn't know that.
16:19:12 <mriedem> retype in cinder posts to the swap volume api in nova
16:19:29 <mriedem> to attach the new block device in the guest and remove the old one
16:19:50 <jungleboyj> Ok, so we don't want to allow that on multi-attached volumes?
16:20:13 <jgriffith> jungleboyj: we don't want to allow that for multi-attach type
16:20:17 <jgriffith> jungleboyj: read the spec :)
16:20:29 <jungleboyj> jgriffith:  Going.  That is fine with me.
16:21:24 <smcginnis> mriedem: That's just retype with migration, right?
16:22:27 <jgriffith> smcginnis: That should be the case yes
16:22:36 <jgriffith> smcginnis: which you may recall :)
16:22:38 <jgriffith> just saying
16:22:43 * jungleboyj has the spec open and is going to approve.
16:22:59 <jungleboyj> after reading again.
16:23:06 <jungleboyj> mriedem:  Any concerns with that?
16:24:42 <mriedem> sorry, in tc
16:24:57 <mriedem> smcginnis: it's also migration w/o retype
16:25:16 <mriedem> swap volume is called from cinder if you (1) retype a volume or (2) don't retype but migrate to another backend
16:25:21 <mriedem> as far as i understand
16:25:23 <smcginnis> But just plain old retype is fine and would make it very simple to change the flag.
16:25:33 <mriedem> i think you' can't do 1 without 2
16:25:37 <mriedem> but you can do 2 without 1 yes?
16:25:43 <jgriffith> mriedem: migrate is a form of retype, but yes, we're saying the same thing
16:25:50 <smcginnis> There's 1) retype, 2) retype with --migration-policy on-demand, and there's 3) migration.
16:28:53 <ildikov> smcginnis: +1 to easily change the flag when the conditions are met
16:29:21 <mriedem> i'll try to read the spec this afternoon
16:29:22 <jungleboyj> Ok, the spec says flat out that you can't retype.  I think that is the safest thing.
16:29:25 <mriedem> if jungleboyj can hold off on the approval
16:29:30 <jgriffith> smcginnis: I left the migrate command out because it shouldn't include any changes to the multiattach type setting
16:29:31 <jungleboyj> mriedem:  Yep, will do.
16:29:33 <mriedem> i'm way overcommitted atm
16:29:40 <mriedem> and need to start actually doing some stuff
16:29:59 <jgriffith> smcginnis: there's no retype involved, so it should just keep the same setting etc, but maybe not
16:30:11 <ildikov> mriedem: fair enough
16:30:13 <smcginnis> jgriffith: Yeah, I think the same settings in that case.
16:30:34 <ildikov> mriedem: I think the libvirt patch is mainly ready except moving up the support-matrix.ini changes
16:30:55 <smcginnis> So #1 should be no big deal, #2 would have to raise SorryCantDoThat(), and migrate isn't involved.
16:31:12 <ildikov> mriedem: there was a concern on how to puch down the 'multiattach' info to the libvirt driver, which I sorted out by putting it into the connection_info dict
16:31:17 <jgriffith> smcginnis: yeah, that's what I came up with at least
16:31:34 <smcginnis> jgriffith: ++
16:31:35 <ildikov> mriedem: that might be the only questionable thing in it
16:32:07 <ildikov> smcginnis: jgriffith: +1
16:32:21 <mriedem> puch down?
16:32:24 <mriedem> is that hungarian?
16:32:41 <jungleboyj> smcginnis:  ++
16:32:42 <ildikov> doesn't seem to be :)
16:32:42 <mriedem> pass down?
16:32:56 <mriedem> ildikov: will have to look
16:33:06 <mriedem> ildikov: maybe leave a comment on the patch where you have a question?
16:33:08 <mriedem> to highlightit
16:33:50 <jungleboyj> I am good on the Spec unless we want to clarify the retype considerations but I think being conservative there is fine.  Don't need to respin.
16:34:05 <smcginnis> jungleboyj: We can always do an update later.
16:34:23 <jungleboyj> ++
16:34:27 <ildikov> mriedem: I will add a comment to be sure
16:35:00 <ildikov> mriedem: I just would like to be sure we don't have concerns with that part anymore
16:35:07 <jgriffith> jungleboyj: line #100 - 110
16:35:53 <jgriffith> actually just 107
16:36:16 <mriedem> tab opened to https://review.openstack.org/#/c/523608/ for later
16:36:31 <jungleboyj> jgriffith:  I saw that but I thought that was only a problem if it is retype with migration ?
16:36:47 <jungleboyj> jgriffith:  But I am fine with leaving it as is.
16:37:38 <ildikov> mriedem: thanks!
16:38:07 <jgriffith> jungleboyj: ahh, I see; yeah we can add some clarification in there; but in-use should cover that case IMO
16:38:43 <jungleboyj> jgriffith:  Ok.  I think staying on the safe side is best.
16:38:56 <jgriffith> retyping an in-use volume is the problem, not necessarily migrating, migrating an in-use volume however is no bueno, but that's a result of retype.  Geesh... so simple
16:40:02 <jungleboyj> Yep!
16:45:18 <ildikov> ok, it seems that we're on the same page with this
16:45:26 <ildikov> or are there any further concerns?
16:45:49 <jungleboyj> I don't think so.
16:46:00 <jungleboyj> Just need to get the spec through and keep working things.
16:46:03 <ildikov> ok, cool
16:46:32 <ildikov> jgriffith: I guess you're out of bandwidth regarding anything written in the Cinder spec
16:46:46 <ildikov> jgriffith: is that a valid assumption?
16:46:57 <jgriffith> ildikov: I'm just out of bandwidth to spend a year polishing the turd
16:47:37 <ildikov> jgriffith: fair enough I guess
16:47:38 <jgriffith> so yes, that's valid; I won't be picking up that work any time in the next few weeks
16:48:03 <jgriffith> I started it last week, but I won't get back to it any time soon I don't think
16:48:37 <ildikov> jgriffith: if there are any bits and pieces I can start from plz point me to it
16:48:45 <ildikov> jgriffith: otherwise I'll figure it out
16:49:04 <jgriffith> ildikov: yeah, I'll shoot you some info in Cinder channel
16:49:24 <ildikov> jgriffith: sounds good
16:49:28 <ildikov> jgriffith: thank you
16:49:31 <jungleboyj> ildikov:  Thank you.
16:50:08 <ildikov> ok, I don't have anything more for today
16:50:14 <ildikov> anything from anyone else?
16:50:52 <jungleboyj> ildikov:  I don't think so.  Thank you for all you do!
16:51:12 <jungleboyj> jgriffith:  Thank you for continuing to help.  I know this has been a big turd to polish.
16:51:19 <ildikov> jungleboyj: just trying to keep it together :)
16:51:31 <jungleboyj> :-)
16:51:39 <ildikov> ok, let's close this for today and do some work
16:51:45 <ildikov> thanks everyone!
16:51:46 <jungleboyj> ildikov:  ++
16:51:57 <ildikov> #endmeeting