15:59:51 <ildikov> #startmeeting cinder-nova-api-changes
15:59:51 <openstack> Meeting started Thu Oct  5 15:59:51 2017 UTC and is due to finish in 60 minutes.  The chair is ildikov. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:59:52 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:59:54 <openstack> The meeting name has been set to 'cinder_nova_api_changes'
15:59:57 <mriedem> o/
16:00:00 <ildikov> johnthetubaguy jaypipes e0ne jgriffith hemna mriedem patrickeast smcginnis diablo_rojo xyang1 raj_singh lyarwood jungleboyj stvnoyes
16:00:36 * johnthetubaguy has a nasty clash this week, but tries to keep his eye on the channel
16:00:37 <smcginnis> Here but at a training event so not really here.
16:00:52 <ildikov> johnthetubaguy: I knew you're still hiding :)
16:01:19 <jungleboyj> @!
16:01:19 <_pewp_> jungleboyj |。･ω･|ﾉ
16:01:21 <jgriffith> o/
16:01:33 <ildikov> ok, let's start
16:01:41 <ildikov> I guess we can have a quick one today
16:01:59 <ildikov> so the gate is getting more friendly, so I have my hopes up again
16:02:20 <ildikov> mriedem: johnthetubaguy: shared_targets spec: https://review.openstack.org/#/c/507670/
16:02:45 <ildikov> mriedem: johnthetubaguy: and corresponding code changes: https://review.openstack.org/#/c/509005/
16:02:46 * jungleboyj crosses my fingers.
16:03:38 <ildikov> mriedem: johnthetubaguy: the spec has no multi-attach aspects; would we need one on the Cinder side that does?
16:04:04 <mriedem> i don't understand the question
16:04:24 <jgriffith> ildikov IMO we'll need multi-attach spec for multi-attach
16:04:29 <jgriffith> Not for shared_targets reporting
16:04:35 <johnthetubaguy> the policy stuff
16:04:46 <ildikov> mriedem: basically where to cover all the aspects of the shared_targets and if it's ok to just figure it out on the Nova side referencing the spec I linked in above?
16:05:17 <mriedem> it's something to mention in the nova multiattach spec
16:05:28 <ildikov> jgriffith: johnthetubaguy: policy on multi-attach on both sides?
16:05:42 <ildikov> as if my understanding is correct we said we will leave R/O for a later time
16:05:52 <jgriffith> I'm ignoring anything multi-attach right now so "carry on" :)
16:06:20 <jgriffith> Unless that's the specific topi right now, which is unrelated to the spec or shared_targets patch that's up IMO
16:06:21 <ildikov> jgriffith: we should do everything now that needs a version bump
16:06:26 <ildikov> jgriffith: ideally at least
16:06:34 <jgriffith> ildikov why?
16:06:49 <jgriffith> we haven't even merged the new attach API's?
16:06:56 <jgriffith> shouldn't we do that *first*?
16:06:59 <ildikov> jgriffith: that's what we agreed on and that's mainly what keeps the new attach patch from merging right now
16:07:04 <jgriffith> ok
16:07:27 <jungleboyj> ildikov: ++
16:07:38 <ildikov> jgriffith: we didn't want to do multiple version bumps and checks if that can be avoided
16:07:47 <mriedem> the policy stuff isn't a microversoin
16:07:54 <mriedem> we said you can add policy rules to cinder today
16:08:01 <ildikov> mriedem: that's not, but the shared_targets thing is
16:08:04 <mriedem> basic policy rule is, can you even create multi-attach volumes
16:08:19 <mriedem> the shared_targets cinder spec is approved and code is up though...
16:08:28 <mriedem> so what else do we need to hold up the nova side for
16:08:28 <mriedem> ?
16:08:30 * jgriffith really doesn't understand
16:08:55 <ildikov> ok, if everyone is good, I'm good :)
16:09:11 <mriedem> pretty sure this was all discussed in detail ad nauseum last week
16:09:18 <mriedem> i hope someone was taking notes :)
16:09:29 <jgriffith> mriedem :). as a matter o'fact
16:09:41 <ildikov> will update the spec on the Nova side with that bit and hopefully the code will get merged so I can update the new attach code too shortly
16:10:35 <ildikov> jungleboyj: in case we get the shared_targets patch merged along with the version bump in the client, is there anything that would keep us back from cutting yet another client?
16:11:17 <jungleboyj> ildikov:  Not that I am aware of.  Think I could make that happen.
16:11:21 <jgriffith> ildikov jungleboyj technically you don't need a new client anyway
16:11:26 <jgriffith> just sayin
16:11:33 <jungleboyj> jgriffith:  ?
16:11:50 <ildikov> jgriffith: the new attach patch will not pass on the gate without a new client
16:12:04 <ildikov> jgriffith: once I bump the version for the attach calls in it
16:12:57 <jgriffith> alrighty
16:13:10 <ildikov> :)
16:13:21 <jungleboyj> jgriffith:  I try not to argue with ildikov  :-)
16:13:52 <ildikov> jungleboyj: jgriffith: that's what happened before we cut the latest client the last time
16:14:48 <ildikov> jungleboyj: jgriffith: the client is not built from source on the gate, which is not necessarily a bad thing and there's a test in Nova that checks on the max version, which is going to fail till we don't have a new release out for a version bump
16:15:04 <ildikov> so one thing I didn't fully get after re-reading the logs from last week
16:15:24 <jungleboyj> ildikov:  Ok, that is good to know and actually makes more sense to me.
16:15:39 <ildikov> so I understand we don't want boot from a multiattach R/W volume
16:15:44 <ildikov> that's ok
16:16:00 <ildikov> what I didn't get is how and where we want to ensure that doesn't happen
16:16:38 <ildikov> is it on the Cinder side not to be able to even create a volume that's bootable with 'multiattach'=True at the same time?
16:16:53 <jgriffith> There are a couple of options, I'd probably enforce some things on the Cinder side using the bootable property of the volume
16:17:02 <ildikov> or will we block the volume getting used for boot for the second time on one on the sides?
16:17:49 <ildikov> jgriffith: when the volume is created or when it's used?
16:17:59 <jungleboyj> ildikov:  I think we want to go with that option as it is consistent with the default policy we were talking about.
16:18:07 <johnthetubaguy> I would go for when the volume is created
16:18:17 <jgriffith> ?
16:19:18 <ildikov> johnthetubaguy: that would be my thought as well if that's feasible
16:19:54 <johnthetubaguy> I think we confirmed when bootable = false Nova rejects you being able to boot from a volume, so that should cover it
16:20:22 <jungleboyj> So we would not allow multi-attach able volume to be made bootable?
16:20:31 <jgriffith> OMG
16:20:44 <jgriffith> look... a volume is marked bootable when it's created from image
16:20:57 <jungleboyj> Sorry ...
16:21:04 <jgriffith> the corner case of marking a volume as bootable after the fact is a corner case
16:21:21 <jgriffith> if you have an attachment finalized on a volume and it's marked as bootable
16:21:42 <jgriffith> Then if another request to connect is received we fail and respond with an appropriate error message
16:21:44 <ildikov> isn't it marked as bootable when it's created?
16:22:05 <smcginnis> Can be later.
16:22:31 <jungleboyj> jgriffith: Ok.  That makes sense to me.
16:22:33 <jgriffith> Are my messages not showing up in here?
16:22:49 <jungleboyj> I didn't think that was the same thing that johnthetubaguy  was saying.
16:23:37 <johnthetubaguy> I was thinking more about the Nova attach side of things
16:23:40 <ildikov> can we block having bootable=True and multiattach=True being set on a volume at the same time?
16:23:59 <jgriffith> I think perhaps we're each talking about different problems and trying to solve them all at the same time
16:24:34 <jgriffith> if bootable: multiattach=Prohibited
16:24:37 <jgriffith> or whatever you like
16:24:42 <jungleboyj> jgriffith:  I agree with you.
16:24:50 <ildikov> yep
16:24:58 <jungleboyj> jgriffith: Meaning it can be attached once and subsequent attempts fail.
16:25:00 <jgriffith> but it doesn't seem like it's difficult to solve that, at least for the primary cases
16:25:01 <ildikov> or well, simply false
16:25:31 <jgriffith> ildikov yes, or false depending on how the multi-attach stuff is implemented and what that looks like
16:25:41 <jgriffith> but I don't know becuase it doesn't exist yet :)
16:25:51 <jgriffith> jungleboyj yes, correct
16:25:53 <ildikov> and if the bootable flag can be set later, then don't allow to set it for a volume that created as multiattach
16:25:59 <jgriffith> the only draw back is possible race conditions
16:26:05 <jungleboyj> Excellent.  I am good with the plan.
16:26:47 <jgriffith> ohhhh.... donuts!
16:27:51 <ildikov> jgriffith: where is the race?
16:28:10 <jgriffith> down the hallway to get donuts
16:28:13 <jgriffith> :)
16:28:36 <ildikov> yeah, I certainly want donuts now, that part is ok :)
16:28:57 * jungleboyj is jealous
16:29:04 <ildikov> jgriffith: I thought to still elaborate on the draw back you raised :)
16:29:06 <jgriffith> the race is simultaneous bfv calls, but one will fail so it's ok
16:29:24 <jgriffith> it's just not completely deterministic but I really don't think it's a big deal
16:29:44 <smcginnis> Yeah, small and unlikely window.
16:29:55 <jgriffith> smcginnis and somewhat benign anyway
16:29:58 <ildikov> if multi-attach is false now for BFV then it shouldn't be a bigger problem than it is already
16:30:04 <ildikov> or am I missing something?
16:30:26 <jgriffith> sure
16:30:40 <ildikov> ok, cool
16:30:59 <smcginnis> The answer to "am I missing something" is almost always true. :)
16:31:26 <ildikov> jgriffith: would you mind summarizing once again the overall plan on this?
16:31:33 <ildikov> just to ensure I got it too :)
16:31:45 <jungleboyj> smcginnis:  The answer to 'does it matter' is not always yes.
16:31:53 <ildikov> smcginnis: :)
16:32:09 <jgriffith> merge new attach code
16:32:14 <smcginnis> jungleboyj: Also true.
16:32:15 <jgriffith> that's the plan
16:32:29 <ildikov> jgriffith: fair enough :)
16:32:39 <mriedem> so while you guys rehashed the multiattach policy again this week,
16:32:44 <mriedem> i reviewed jgriffith's cinder patch
16:32:56 <mriedem> please oh please dear sweet baby jesus,
16:33:00 <mriedem> don't do that data migration offline
16:33:12 <jgriffith> mriedem which one?
16:33:15 * jgriffith goes to gerrit
16:33:18 <mriedem> https://review.openstack.org/#/c/509005/
16:35:55 <jgriffith> mriedem if there's someone with the foo to do that without python I'd love to change it
16:36:18 <mriedem> jgriffith: i can find an example from nova
16:36:49 <jgriffith> mriedem let me know, when I looked that's how Nova was doing it too I think
16:37:04 <mriedem> https://github.com/openstack/nova/commit/3674a4268d177230375fa1b581dbdf6f62755cee
16:37:19 <mriedem> nova is not doing data migrations in the migration scripts
16:37:24 <mriedem> we do that stuff in the object
16:37:26 <mriedem> on read
16:37:44 <mriedem> and we have the online_data_migrations CLI for batching those as needed
16:39:54 <jgriffith> mriedem don't get offended or anything
16:40:16 <mriedem> i posted some links in the change
16:40:22 <jgriffith> thanks
16:40:24 <mriedem> np
16:40:45 <mriedem> i'm not offended,
16:40:46 <mriedem> i just,
16:40:56 <mriedem> my ass is still sore from years and years of operators ripping us on doing this
16:40:59 <mriedem> including our internal people
16:41:14 <jgriffith> mriedem cool, and I really appreciate you pointing it out!!
16:41:25 <mriedem> i only just put the donut away 2 months ago
16:41:30 <jgriffith> haha
16:41:48 <jgriffith> not a good kind of donut either
16:43:13 <ildikov> ok, it looks like we're on track with this too now
16:43:22 <mriedem> well, not good to eat
16:43:39 <jgriffith> good for sitting, not good for munching
16:43:51 <ildikov> mriedem: thanks for the pointers, etc.
16:43:57 <ildikov> :( :)
16:44:20 <mriedem> unless you've got a fetish, but we digress
16:44:22 <mriedem> end of meeting?!
16:44:29 <ildikov> I think so
16:44:51 <ildikov> unless someone objects
16:45:00 <jgriffith> ha
16:45:28 <ildikov> ok, so jgriffith will fix the shared_targets patch
16:45:43 <ildikov> johnthetubaguy will review the live_migrate patch
16:45:54 * ildikov will update the multi-attach spec
16:46:07 * ildikov will also look into start a Cinder side of that thing
16:46:29 <ildikov> and we will merge the new attach patch as soon as the new client with a new mv is out
16:46:46 <ildikov> thanks everyone!
16:47:11 <ildikov> have a good rest of your day!
16:47:18 <ildikov> #endmeeting