16:02:12 <joanna> #startmeeting ironic_bfv
16:02:12 <openstack> Meeting started Thu Apr 20 16:02:12 2017 UTC and is due to finish in 60 minutes.  The chair is joanna. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:02:13 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:02:16 <openstack> The meeting name has been set to 'ironic_bfv'
16:02:41 <joanna> morning! I am sorry for the late start :)
16:03:07 <mjturek> o/
16:03:17 <hshiina> o/
16:03:22 <joanna> #topic Announcements/Reminders
16:04:12 <joanna> Does anyone have any announcements?
16:04:50 <mjturek> I updated the cinder driver patch yesterday, but would like to discuss the comments later on
16:05:01 <mjturek> There are a couple I didn't address
16:05:28 <joanna> mjturek: great! I see that in the agenda - will it be fine to discuss in Discussion part?
16:05:36 <mjturek> joanna: totally
16:05:41 <joanna> great :)
16:05:43 <joanna> moving on
16:05:45 <joanna> #topic Current Status
16:05:58 <joanna> follow up patch for common code got merged :)
16:06:20 <hshiina> great!
16:06:35 <mjturek> \o/
16:08:38 <joanna> From what I see in Etherpad, that was the only merged patch since last meeting, other patches are WIP and have new review comments
16:09:08 <joanna> mjturek: thank you for taking care of the driver patch!
16:09:18 <mjturek> joanna: np it's been really interesting
16:09:30 <joanna> mjturek: awesome! :)
16:09:41 <joanna> shall we move to Planning?
16:09:48 <mjturek> +1
16:10:02 <joanna> #topic Planning/Priorities
16:10:03 <hshiina> shall we update other patches to solve conflicts?
16:10:34 <joanna> I think that driver patch should stay a priority since it's a direct dependency of the merged patches
16:11:03 <mjturek> agreed
16:11:05 <joanna> We can think about additional patch to put to weekly pirority list on Monday to accelerate
16:12:22 <joanna> how about https://review.openstack.org/#/c/406290 ?
16:12:58 <mjturek> joanna: I think there's going to need to be a db patch for getting volume target from volume id (I'd like to handle this). It'll probably be needed to get the driver patch merged
16:13:17 <mjturek> but I'm fine with 406290
16:14:02 <hshiina> i agree, let's rebase and add it to list
16:14:07 <joanna> mjturek: If you plan to update the wiring patch, you can rebase it on top of your DB reading change, so it won't get lost
16:14:32 <mjturek> surely
16:15:05 <joanna> mjturek: when the read volume target is ready it can be added to priority list instead. The point is to get reviews as fast as possible :)
16:15:28 <joanna> mjturek, hshiina: great!
16:15:30 <mjturek> ahhh understood joanna
16:15:53 <joanna> #info Add https://review.openstack.org/#/c/406290 to weekly priority list
16:16:43 <joanna> should we move to Discussion?
16:17:14 <mjturek> sure!
16:17:16 <hshiina> yes
16:17:18 <joanna> #topic Discussion
16:17:36 <mjturek> joanna: hshiina: will be referring to comments here https://review.openstack.org/#/c/366197/39/ironic/drivers/modules/storage/cinder.py
16:17:52 <joanna> mjturek: do you want to do that one by one?
16:17:58 <mjturek> yeah please
16:18:04 <mjturek> so the first one on line 217
16:18:29 <mjturek> I beleive vdrok is right. The function seems to achieve the same thing as detach_volumes
16:19:07 <mjturek> is anyone opposed to removing the function>
16:19:09 <mjturek> ?*
16:19:21 <joanna> mjturek: it does it in two ways
16:19:30 <joanna> first it tries to run with no errors allowed
16:20:04 <joanna> then, if it fails, it logs that aborting volume attached failed on detach (that's something specfifc)
16:20:14 <joanna> and then tries once again woth allowing errors
16:20:30 <mjturek> is the error reporting that important though?
16:20:45 <mjturek> that we need a new function for it?
16:21:00 <joanna> I think the idea of retrying with allowed errors was to detect additional issues with the system
16:21:21 <mjturek> hmmm, alright. I'll bring that up with vdrok and see what he thinks
16:21:28 <mjturek> I'll ping you and hshiina as well to weigh in
16:21:37 <joanna> I can imagine that if there's something wrong with cinder, so all the ops should fail, as an admin, I'd like to know that
16:22:13 <joanna> and I think unsuccessful attach may happen even when there are no issues, but if also detach fails that's something worth looking at
16:22:13 <mjturek> yeah I might see the value there
16:23:00 <mjturek> right
16:23:04 <joanna> but vdrok is right saying that it uses the functionality of detach, it's just a failure handler to add context to logs
16:23:24 <mjturek> got it
16:23:42 <joanna> we can be less informative, or use these contents inline
16:24:06 <joanna> However it would be also worth for other possible usages of this function
16:24:27 <mjturek> right right
16:24:39 <joanna> does it help at all :)?
16:24:57 <mjturek> yeah definitely, but I think I'll need to bring it up with vdrok
16:25:06 <mjturek> before making a decision
16:25:06 <joanna> sure :)
16:25:10 <mjturek> :)
16:25:21 <mjturek> so moving on to generate_connector comment on 376
16:25:36 <joanna> ln 376?
16:25:43 <mjturek> yep!
16:26:04 <mjturek> mariojv was asking if Cinder does this. I haven't found anything but haven't looked too much
16:26:33 <mjturek> and I genuinely don't know the answer to 2 or 3 :)
16:26:39 <mjturek> I would assume no to 2
16:26:47 <joanna> hmm
16:26:52 <mjturek> but was wondering if either of you had insight
16:26:56 <mjturek> if not I'll bug Julia :)
16:27:31 <joanna> target_iqn should be in volume_target
16:27:59 <hshiina> nova passes ip and iqn to cinder
16:28:51 <mjturek> so she's saying that we need to translate from the connector_id to the IQN
16:29:01 <mjturek> the connector_Id is the IP
16:30:05 <mjturek> but by the time we hit nova we should have the IQN?
16:31:18 <mjturek> and looking at the data structure that this returns, it looks like we'll need IQN
16:31:31 <mjturek> I'm probably going to see if Julia has any insight
16:32:09 <joanna> mjturek: that is a good idea
16:32:17 <mjturek> cool cool
16:32:22 <joanna> mjturek: from how I see it we have to pass all the connection info to nova
16:32:30 <mjturek> ahhh
16:32:40 <mjturek> okay so then it is probably a TODO that must be handled
16:32:46 <joanna> however, it should already be in volume_connector
16:33:01 <mjturek> oh
16:33:14 <mjturek> okay cool
16:33:19 <joanna> and the IQN can be generated from the other data contained in volume_target, so even if for any reason it's not available, it can be generated
16:33:40 <mjturek> makes sense
16:33:41 <joanna> (although it might be a good idea to inform that the volume_connector info is incomplete)
16:33:54 <joanna> s/volume_target/volume_connector/, sorry :)
16:34:07 <mjturek> right :)
16:34:20 <mjturek> okay, makes perfect sense then
16:34:46 <mjturek> so final comment is 409
16:35:29 <mjturek> I think what vdrok means here is that we may have more than one volume connector but one might be broken
16:36:27 <mjturek> would checking len(data) be helpful here?
16:36:32 <joanna> I see, because  vailid variable is reused in the switch case
16:37:15 <mjturek> ahhhh right
16:37:23 <joanna> yes I think it's a good idea
16:37:23 <joanna> :)
16:37:34 <mjturek> cool, will do then :)
16:37:56 <mjturek> that's all I had
16:38:03 <joanna> great!
16:38:14 <hshiina> regarding last issue, i don't think multi volume connectors mean multipath
16:38:57 <joanna> hshiina: this is about that there might be both ip and iqn for the same volume?
16:39:59 <joanna> hshiina: please take a look at comment in revision 34
16:40:09 <hshiina> joanna, yes
16:40:17 <joanna> ln 409
16:42:48 <joanna> hshiina: does it answer the quesion?
16:45:14 <hshiina> joanna, i haven't fully understood multipath. but, multipath seems more complicated
16:46:05 <mjturek> so it sounds like it simply means that multiple paths are available to the volume. The fact that an IP and IQN are available means multiple paths are available, even if it's just for one driver
16:46:11 <mjturek> one volume*
16:46:19 <joanna> from what I understand what Julia is saying, there is no harm in setting multipath to true, as driver should handle that
16:48:12 <joanna> mjturek: I think that since it's confusing, maybe it's a goos idea to add a comment there explaining why multipath is handled this way?
16:48:43 <joanna> also, can you confirm with Julia that it's a proper way to handle multipath?
16:49:26 <mjturek> joanna: agreed
16:49:39 <joanna> hshiina: are you ok with that? :)
16:50:29 <hshiina> multipath is listed in feature capabilities in the approved spce: https://specs.openstack.org/openstack/ironic-specs/specs/not-implemented/boot-from-volume-reference-drivers.html
16:50:58 <hshiina> joanna, it's ok to confirm with Julia
16:52:05 <joanna> hshiina: it's listed here as a potential capability - so maybe it's a good idea to have a happy scenario now, and when it's ready add better support for multipath if what we have is insufficient?
16:53:28 <hshiina> joanna, yes
16:53:58 <joanna> mjturek: if that will be fine with Julia, this comment may be a TODO comment for the future, then :)
16:54:14 <joanna> cool! that was the only item, should we move to open discussion?
16:54:24 <mjturek> sure, I'll reach out to her later and ping hshiina as well
16:54:32 <joanna> mjturek: that's awesome!
16:54:32 <mjturek> +1
16:54:41 <hshiina> mjturek, thanks
16:54:47 <joanna> #info mjturek to follow up on multipath with hshiina and TheJulia
16:54:51 <joanna> #topic Open Discussion
16:55:44 <joanna> so this might be the last BFV meeting I attend. I will miss working on it! Please do not hesitate to contact me with any questions or anything :)
16:56:22 <mjturek> joanna: :( will do
16:56:37 <mjturek> joanna: do we need someone to facilitate the meeting>
16:56:47 <mjturek> I'd be happy to help unless hshiina would rather do it
16:57:15 <joanna> TheJulia should be back next week
16:57:22 <joanna> so I think there's no such need :)
16:57:26 <mjturek> joanna: okay great!
16:57:31 <joanna> awesome!
16:57:34 <joanna> so are we done?
16:57:46 <hshiina> yes.
16:57:51 <joanna> great!
16:57:54 <mjturek> ttyl all
16:57:57 <joanna> good luck & have fun! :)
16:58:04 <hshiina> joanna, thank you for your work!
16:58:04 <mjturek> you too joanna :)
16:58:04 <joanna> thank you :)
16:58:05 <joanna> #endmeeting