15:04:01 <enriquetaso> #startmeeting cinder_bs
15:04:01 <opendevmeet> Meeting started Wed Aug 11 15:04:01 2021 UTC and is due to finish in 60 minutes.  The chair is enriquetaso. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:04:01 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:04:01 <opendevmeet> The meeting name has been set to 'cinder_bs'
15:04:11 <enriquetaso> Welcome to the cinder bug meeting.
15:04:17 <enriquetaso> We have 3 new bugs reported this last week. Lucky two of them are low-hanging-fruit and are related to the cinder-tempest-plugin README.
15:04:34 <enriquetaso> #topic search and tag the nas_secure bugs
15:04:40 <enriquetaso> But first, regarding the cinder meeting earlier today, I need to drop this action item here to don't forget about it:
15:04:46 <enriquetaso> #action(enriquetaso) search and tag the nas_secure bugs, prepare this for next week's cinder meeting.
15:04:54 <enriquetaso> Example:
15:04:54 <enriquetaso> #link https://bugs.launchpad.net/cinder/+bug/1938196
15:05:00 <enriquetaso> Full discussion:
15:05:00 <enriquetaso> #link https://meetings.opendev.org/meetings/cinder/2021/cinder.2021-08-11-14.01.log.html
15:06:45 <rosmaita> thanks, that will be really helpful in seeing how bad the situation is
15:06:58 <rosmaita> and hopefully find corner cases that should be addressed
15:07:12 <eharney> https://bugs.launchpad.net/cinder?field.searchtext=nas_secure   shows 5 to start
15:07:24 <enriquetaso> cool
15:07:43 <enriquetaso> ha
15:07:50 <enriquetaso> OK, moving on
15:07:55 <enriquetaso> #topic bug_1 "[victoria] max_over_subscription_ratio set auto mode rbd backend will have a negative value"
15:08:00 <enriquetaso> #link https://bugs.launchpad.net/cinder/+bug/1938869
15:08:05 <enriquetaso> Summary: If max_over_subscription_ratio is set to auto, c-sch doesn't allow the user to create volumes.
15:08:12 <enriquetaso> max_over_subscription_ratio = Representation of the over subscription ratio when thin provisioning is enabled. Default ratio is 20.0, meaning provisioned capacity can be 20 times of the total physical capacity. If the ratio is auto, Cinder will automatically calculate the ratio based on the provisioned capacity and the used space. If not set to auto, the ratio has to be a minimum of 1.0.
15:08:39 <enriquetaso> As the reporter is using the Victoria release I thought that  It would be nice to reproduce this in master.  I couldn't reproduce this.. so maybe this is a victoria only bug or I'm missing something. I left some comments in the bug report asking for more detailed steps information.
15:09:15 <eharney> we could put code in the manager/scheduler to detect when this situation happens (due to a driver bug or whatever) and at least push it back to a sensible value
15:09:32 <rosmaita> nice work, sofia, trying to reproduce
15:09:46 <rosmaita> more detailed info request sounds correct
15:10:23 <rosmaita> eharney: that strategy has a precedent in cinder, so probably not a bad idea
15:10:45 <enriquetaso> so, this is a possible scenario then
15:11:06 <enriquetaso> I'll add a note regarding this to the bug report
15:11:07 <eharney> well max_over_subscription_ratio being negative doesn't make sense
15:11:19 <rosmaita> well, always good to get more info
15:11:22 <eharney> but i'm a little unsure on the whole concept of why we have "auto" for it, too
15:11:37 <rosmaita> but eric is right, if we auto calculate, we should make sure we don't go negative
15:11:50 <rosmaita> i guess that would give you extra-thick volumes
15:12:07 <eharney> does "auto" give a behavior that someone actually wants?
15:12:47 <rosmaita> i dont' know ... sounds like cinder will compute whatever's necessary to jam the volume into the space that's there?
15:13:10 <eharney> i'm not sure about that
15:13:39 <rosmaita> yeah, me neither, but in general, it's probably not a good idea to let cinder figure this out for you
15:14:25 <rosmaita> i am afraid to look at what exactly is being computed
15:14:36 <Roamer`> hm, looking at the code (cinder/utils.py, calculate_max_over_subscription_ratio), it looks like there is a debug-level log message that would show how the negative value is derived from provisioned_capacity_gb, total_capacity_gb, and free_capacity_gb... maybe the reporter could retry this with verbose logging enabled and that would show them that Cinder cannot really do anything about it
15:14:52 <Roamer`> it seems, at least to me, that the calculation is, well, reasonable
15:16:01 <enriquetaso> #link https://opendev.org/openstack/cinder/src/branch/master/cinder/utils.py#L745
15:16:24 <Roamer`> yeah, sorry about that, I should have posted the link, looking at the files locally here :(
15:16:40 <Roamer`> and thanks
15:17:00 <enriquetaso> #action(enriquetaso) ask the reporter the reason of using 'auto'?? and ask for more verbose logs and steps
15:17:06 <eharney> you'd be surprised how much confusion there has been about the math/logic involved in these calculations in the past, i'm hesitant to say any of it is reasonable without close examination :)
15:17:10 <enriquetaso> Roamer`++
15:17:11 <Roamer`> ...it's more like https://opendev.org/openstack/cinder/src/branch/master/cinder/utils.py#L785
15:17:14 <eharney> at any rate, it's probably unreasonable to compute something < 1.0
15:18:44 <Roamer`> eharney, yeah, I get your point, ask me someday about figuring out which disks on which servers a new volume should be placed at :) or rather don't :) ...and yeah, maybe an explicit check for < 1 and returning a "nope, don't even think about going there" answer would be better
15:18:51 <Roamer`> although the end result is the same
15:20:40 <enriquetaso> OK.. moving on
15:20:49 <enriquetaso> #topic bug_2 "Add/remove things to cinder-tempest-plugin README"
15:20:57 <enriquetaso> #link https://bugs.launchpad.net/cinder-tempest-plugin/+bug/1939325
15:20:57 <enriquetaso> #link https://bugs.launchpad.net/cinder-tempest-plugin/+bug/1939322
15:21:03 <enriquetaso> The first bug report is to add placement service to README and the second: glance registry was deprecated in Queens and should be removed from README.
15:21:35 <eharney> why do we need to add placement-api to local conf for the tempest plugin?
15:21:39 <eharney> that doesn't sound right
15:22:32 <rosmaita> i think maybe nova won't work without it?
15:22:42 <eharney> then devstack should be installing it for nova anyway?
15:23:23 <eharney> i mean we don't have to write a config enabling the nova service, right?
15:23:24 <rosmaita> you'd think, unless it's only needed for some configurations, i guess
15:23:28 <Roamer`> devstack does include it in the default set of services (unless that is overridden)
15:23:43 <enriquetaso> good question, all our current jobs enable it https://zuul.opendev.org/t/openstack/build/ef71a0bc0d6046e88b1404f4050bf4ac/log/controller/logs/_.localrc_auto.txt#26
15:23:58 <eharney> this sounds like it isn't cinder-tempest-plugin's business
15:24:33 <enriquetaso> yes.. makes sense, the bug is invalid
15:24:38 <eharney> unless our tests actually require placement-api?
15:25:02 <eharney> i don't know
15:25:24 <rosmaita> i guess it depends on what exactly we are trying to say in the README
15:25:28 <enriquetaso> as far as I can see they don't but as I saw the placement-api in all the jobs I thought we need it
15:25:31 <rosmaita> https://opendev.org/openstack/cinder-tempest-plugin/src/branch/master/README.rst
15:25:44 <eharney> well then we need nova and glance and all kinds of other stuff too...
15:26:38 <eharney> oh, it's because we aren't just asking to enable some services, we are setting ENABLED_SERVICES to a specific list
15:27:53 <Roamer`> BTW it is actually also enabled in devstack's .zuul.yaml
15:27:55 <eharney> i would consider dropping that and just using enable_service for what we actually need
15:27:56 <rosmaita> hmm ... i dont' think there's a 'cinder' service
15:28:15 <Roamer`> at least the StorPool CI did not need to enable it explicitly, it was inherited from the "devstack" job
15:28:40 <Roamer`> #link https://opendev.org/openstack/devstack/src/branch/master/.zuul.yaml#L507
15:29:04 <eharney> what we have listed in the sample config overrides anything that would be inherited
15:29:10 <enriquetaso> drop it and enable the necessary makes more sense
15:30:40 <rosmaita> here's the default set: https://opendev.org/openstack/devstack/src/branch/master/stackrc#L56-L83
15:31:25 <Roamer`> hm... okay... so it's true that I've never actually tried overriding ENABLED_SERVICES explicitly, yeah, so eharney is right..... maybe what is in the Zuul config will also be ignored, not just what's in stackrc
15:32:26 <enriquetaso> #link https://opendev.org/openstack/devstack/src/branch/master/stackrc#L56-L83
15:32:57 <rosmaita> well, i think for the README you can just remove the ENABLED_SERVICES lines
15:33:07 <Roamer`> so yeah, I may have been spoiled by SoftwareFactory's nice configuration :)
15:33:26 <rosmaita> looks like the only thing you really don't need is horizon
15:34:27 <enriquetaso> #action(enriquetaso) the bugs are not necessary, remove the ENABLED_SERVICES lines and disable horizon
15:34:50 <eharney> i don't know why we are specifying VIRT_DRIVER, or CINDER_VOLUME_CLEAR either...
15:35:04 <eharney> or LIBVIRT_FIREWALL_DRIVER?  weird
15:35:26 <rosmaita> this was probably somebody's localrc that got copied in
15:35:52 <eharney> or SYSLOG, or... yeah
15:36:10 <rosmaita> we only say that it "should" work
15:36:35 <opendevreview> Luigi Toscano proposed openstack/cinder stable/wallaby: RBD: use correct stripe unit in clone operation  https://review.opendev.org/c/openstack/cinder/+/804265
15:38:24 <rosmaita> i guess maybe make this bug more general, something like "cleanup deployment suggestion in README"
15:38:54 <enriquetaso> that works for me... OK, we exceed the half an hour meeting
15:39:04 <enriquetaso> let me mention the last bug
15:39:08 <enriquetaso> #topic bug_3 "[IBM Storwize SVC]: use system_id in mkrelationship"
15:39:12 <enriquetaso> #link https://bugs.launchpad.net/cinder/+bug/1939145
15:39:30 <enriquetaso> Finally, as far as I can see there's a couple IBM Storwize bugs and patches related to. Nothing else to add in this case. I left some comments asking for more information because as i see it, the bug it's incomplete,
15:39:50 <enriquetaso> that's all I have for today's meeting
15:41:22 <Roamer`> BTW, yeah, sorry, I'm not quite sure what the etiquette is... I reported a couple of trivial bugs on Monday, but I suppose those will be discussed next week? if so, then sorry, and anyway, thanks for all your work on the bugs!
15:41:25 <rosmaita> the storwize team has a bad habit of filing bugs saying what change they want to make, instead of explaining what error is being produced
15:42:01 <enriquetaso> Roamer`, could you send the links here?
15:42:20 <enriquetaso> rosmaita++
15:42:35 <Roamer`> #link https://bugs.launchpad.net/cinder/+bug/1939242
15:42:47 <Roamer`> ^^ I'm not really sure how Glance with multistore enabled could ever actually work without this fix
15:43:07 <enriquetaso> oh sorry, I forgot about them :/
15:43:10 <Roamer`> #link https://bugs.launchpad.net/cinder/+bug/1939241
15:43:28 <rosmaita> i think there's already a bug for the glance multistore
15:43:30 <eharney> hmm, 1939242 sounds familiar, was someone else fixing this a while ago?
15:43:49 <rosmaita> i think me, but got stalled
15:43:52 <Roamer`> ^^ this one is totally on me, I should have reported it as a bug back in April when our CI system found it (the StorPool driver overrides _attach_volume() unnecessarily, os-brick knows how to do that, and the result is that StorPool cannot mount encrypted volumes)
15:43:56 <enriquetaso> i think rajat fix something related to that
15:44:01 <enriquetaso> but he's on PTO today
15:44:06 <Roamer`> ("this one" was about 1939241)
15:44:19 <enriquetaso> #topic Glance with multistore bugs
15:44:20 <rosmaita> https://review.opendev.org/c/openstack/cinder/+/755654
15:44:27 <enriquetaso> #link https://bugs.launchpad.net/cinder/+bug/1939242
15:44:33 <enriquetaso> #link https://bugs.launchpad.net/cinder/+bug/1939241
15:45:02 <enriquetaso> oh it was rosmaita sorry
15:45:03 <Roamer`> hmmmmm, yeah, 755654 is my fix done in the right way :)
15:45:14 <Roamer`> I mean, somebody else took the time to do it in the right way :)
15:45:32 <rosmaita> well, except i never finished, rajat wanted me to fix somethihng
15:45:38 <rosmaita> do you want to take it over?
15:45:44 <Roamer`> I can do that, yeah
15:45:49 <rosmaita> cool
15:46:29 <enriquetaso> Great
15:47:04 <enriquetaso> OK, i think that's all we have
15:47:07 <eharney> please close the new multistore bug as a dupe
15:47:08 <Roamer`> ahhh, right, I see rajat's comment, it makes sense... sure, I'll think about it
15:47:14 <Roamer`> and yes, I will close the new one
15:47:51 <enriquetaso> sure, need to search for the original bug report
15:48:01 <eharney> https://bugs.launchpad.net/cinder/+bug/1898075
15:48:05 <Roamer`> @link https://bugs.launchpad.net/cinder/+bug/1898075
15:48:06 <Roamer`> #link https://bugs.launchpad.net/cinder/+bug/1898075
15:48:17 <enriquetaso> #action(enriquetaso) mark 1939242 as duplicate of 1898075
15:48:20 <enriquetaso> thank you!!
15:48:53 <enriquetaso> excellent, any other topic for today?
15:49:13 <Roamer`> BTW just to make sure everything is clear: https://bugs.launchpad.net/cinder/+bug/1939241 is not about Glance multistore, it is a StorPool driver mess-up
15:50:29 <enriquetaso> thanks for point this bugs Roamer` not sure what happened there i forgot to tag them
15:50:51 <Roamer`> no worries, no worries at all, you're doing a lot of work!
15:51:03 <enriquetaso> #endmeeting