Wednesday, 2022-07-06

vbelhi guys, I try to run tempest to test my cinder driver multiattach. All tests run but tempest.api.compute.admin.test_volume_swap.TestMultiAttachVolumeSwap.test_volume_swap_with_multiattach:  "/opt/stack/tempest/tempest/common/compute.py", line 217, in create_test_server07:33
vbelValueError: Multiple pingable or sshable servers not supported at this stage. I have enabled swap volume and run_validation and don't get how similar setups run, e.g. http://ec2-18-177-230-241.ap-northeast-1.compute.amazonaws.com/refs/changes/92/825492/18/iscsi/07:33
vbelany clue?07:34
opendevreviewGorka Eguileor proposed openstack/cinder stable/yoga: Log when waiting to acquire coordinator lock  https://review.opendev.org/c/openstack/cinder/+/84882109:14
*** dviroel|out is now known as dviroel11:25
opendevreviewBrian Rosmaita proposed openstack/cinder stable/victoria: Don't destroy existing backup by mistake on import  https://review.opendev.org/c/openstack/cinder/+/84872612:17
opendevreviewMerged openstack/cinderlib master: Update master for stable/yoga  https://review.opendev.org/c/openstack/cinderlib/+/84772912:21
raghavendratHi reviewers, requesting feedback on below patch. It is xena backport:12:33
raghavendrathttps://review.opendev.org/c/openstack/cinder/+/84479312:33
opendevreviewOleg proposed openstack/cinder master: Add NVMe/TCP support to Dell EMC PowerStore driver  https://review.opendev.org/c/openstack/cinder/+/81914912:55
opendevreviewBrian Rosmaita proposed openstack/cinderlib master: Open cinderlib for zed development  https://review.opendev.org/c/openstack/cinderlib/+/84884613:40
whoami-rajatCinder meeting in #openstack-meeting-alt at 1400 UTC13:58
whoami-rajatjungleboyj rosmaita smcginnis tosky whoami-rajat m5z e0ne geguileo eharney walshh_ jbernard sfernand enriquetaso hemna fabiooliveira yuval tobias-urdin13:58
enriquetasothanks!14:00
whoami-rajatnp!14:00
opendevreviewLuigi Toscano proposed openstack/cinder master: Remove importlib_metadata requirement, use im  https://review.opendev.org/c/openstack/cinder/+/84884814:03
toskyoh, what a nice review number14:04
opendevreviewEric Harney proposed openstack/cinder master: WIP: RBD: Flattening of child volumes during deletion  https://review.opendev.org/c/openstack/cinder/+/83538414:10
opendevreviewMerged openstack/cinder master: Tests: add microversion consistency unit tests  https://review.opendev.org/c/openstack/cinder/+/73761814:11
opendevreviewBrian Rosmaita proposed openstack/cinderlib master: Open cinderlib for zed development  https://review.opendev.org/c/openstack/cinderlib/+/84884614:15
opendevreviewEric Harney proposed openstack/cinder master: WIP: RBD: Flattening of child volumes during deletion  https://review.opendev.org/c/openstack/cinder/+/83538414:18
enriquetaso\o/14:19
opendevreviewLuigi Toscano proposed openstack/cinder master: Remove importlib_metadata requirement  https://review.opendev.org/c/openstack/cinder/+/84884814:24
opendevreviewLuigi Toscano proposed openstack/cinder master: Remove importlib_metadata requirement  https://review.opendev.org/c/openstack/cinder/+/84884814:37
opendevreviewLuigi Toscano proposed openstack/cinder master: Remove importlib_metadata requirement  https://review.opendev.org/c/openstack/cinder/+/84884814:40
*** dviroel is now known as dviroel|lunch14:59
enriquetaso#startmeeting cinder_bs15:00
opendevmeetMeeting started Wed Jul  6 15:00:42 2022 UTC and is due to finish in 60 minutes.  The chair is enriquetaso. Information about MeetBot at http://wiki.debian.org/MeetBot.15:00
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.15:00
opendevmeetThe meeting name has been set to 'cinder_bs'15:00
enriquetasoOnly one bug for today's meeting15:00
enriquetaso#link https://lists.openstack.org/pipermail/openstack-discuss/2022-July/029420.html15:01
enriquetaso#topic  creating a bootable volume fails async when vol_size < virtual_size of image 15:01
enriquetaso#link https://bugs.launchpad.net/cinder/+bug/198026815:01
enriquetasoSummary: When creating an instance from the bootable volume, if the virtual size of the image is greater than the volume size, the check is done later in the cinder-volume service and fails whereas nova waits for a reply and times out.15:01
enriquetasocinder logs15:02
enriquetaso"ERROR cinder.volume.manager ImageUnacceptable: Image <immage uuid> is unacceptable: Image virtual size is 10GB and doesn't fit in a volume of size 8GB.""15:02
enriquetasoFix proposed to master15:02
enriquetaso#link https://review.opendev.org/c/openstack/cinder/+/84733515:02
whoami-rajathi15:03
whoami-rajatyeah, I've added a check to the API layer for virtual_size15:04
geguileowhoami-rajat: but I think this is a Nova bug, right?15:04
whoami-rajatit is currently failing the grenade job which I've seen before but might be a different issue15:04
geguileobecause Cinder moves the status of the volume to "error", right?15:04
whoami-rajatgeguileo, it is kind of, can be handled at both ends15:04
geguileobut the right approach is fixing it in Nova15:04
geguileobecause they have to check the status of the volume for "error"15:05
geguileonot only "available"15:05
geguileoand if it goes into error they should not timeout, but instead fail15:05
whoami-rajatthat's true15:06
whoami-rajatbut it as it is helpful for us as well when creating a bootable volume since we will fail fast15:06
rosmaitai thought lyarwood fixed the nova side already15:06
geguileowhoami-rajat: yeah, but if we fix it too soon Nova may never fix their side15:06
rosmaitawe do need both, because there's no guarantee of virtual_size being populated15:07
whoami-rajatrosmaita, i discussed this with lyarwood and i don't think he did15:07
whoami-rajatit's discovered long ago but no changes on nova side were made to handle it15:07
rosmaitai thought i saw a patch, guess not15:08
whoami-rajatrosmaita, maybe you're right and i might have missed it15:08
geguileothis is a larger problem than this specific case15:08
rosmaitathat's a good point15:08
geguileobecause cinder can fail for literally 10^6 different reasons15:08
rosmaitaonly 10^6 ?15:08
geguileoand if they are just expecting the happy path, that's a real problem15:08
geguileorosmaita: yeah, feeling optimistic today  ;-)15:09
rosmaita:)15:09
geguileobtw, I'm not saying that I'm against the cinder fix, I actually think it's a great fix15:09
geguileoI'm in favor of fail-fast systems15:10
HappyStackerhave a question as I don't know if we can categorize it as bug or not. We at Dell have a storage system called PowerFlex which rounds volume size by 8. But in case an operator creates a volume then extend it to a size which is not a 8GB multiplier, cinder will report a size which doesn't match the backend because it doesn't expect a size different from what we have told him.15:10
whoami-rajatgeguileo, I understand your point but don't know what we can do to convince nova team to fix it or fix similar related errors ...15:10
geguileowhoami-rajat: I can write a bot that creates a 1000 replicas of the original bug report...15:11
geguileo};-)15:11
rosmaitadon't you mean 10^6 replicas?15:11
geguileowhoami-rajat: because there's a Nova bug, right?15:11
geguileorosmaita: lol15:12
whoami-rajatgeguileo, on launchpad? not one I'm aware of since i reported the cinder one, we can add nova in it15:12
geguileoHappyStacker: we can discuss that particular case at the end of the bug meeting15:12
geguileowhoami-rajat: well, if there is not a Nova bug then it would be hard for them to fix it15:13
HappyStackersure geguileo15:13
geguileoand I can't complain  XD15:13
geguileoanyway, the fix seems reasonable and appropriate15:13
geguileoI'll stop complaining15:13
enriquetasosorry, what should we do regarding nova bug?15:14
enriquetasoshould I link nova in the bug?15:14
geguileoenriquetaso: no, I think we need a different bug, because their bug is, if I understood it correctly, that they don't handle error status gracefully in that flow15:15
whoami-rajatgeguileo, thanks for your inputs, we should at least inform nova team to fix it and see if they get it fixed15:15
enriquetasowhoami-rajat, would you mind creating a bug for nova with the error?15:16
whoami-rajatenriquetaso, sure will do15:16
enriquetasothanks15:17
enriquetaso#topic  PowerFlex which rounds volume size by 815:17
enriquetasoHappyStacker, ^15:17
enriquetasowould you mind sharing the question again15:18
enriquetasoor do you have a launchpad bug for it?15:18
enriquetasohave a question as I don't know if we can categorize it as bug or not. We at Dell have a storage system called PowerFlex which rounds volume size by 8. But in case an operator creates a volume then extend it to a size which is not a 8GB multiplier, cinder will report a size which doesn't match the backend because it doesn't expect a size different from what we have told him.15:18
HappyStackerwe don't have any launchapd for that uet and it's on our to do lis15:18
HappyStackerthe question is do we need to raise a bug?15:19
HappyStackeris it a bug? as cinder reacts as expected15:19
geguileoHappyStacker: does this cause operational problems?15:19
HappyStackermonitoring storage capacity from within an operator standpoint15:19
HappyStackerthe backend is fine, but will not match what an operator see15:20
enriquetasoi think maybe override the extend() method on powerflex driver to do as you need?15:20
enriquetasooh15:20
enriquetasook15:20
geguileobut when you create a 1GB volume in that backend (results in an 8GB volume) and attach it to a VM, format it, and use it15:20
whoami-rajatenriquetaso, that method currently doesn't return a model update so cinder won't update the size even if they return new size15:20
enriquetasoso in the backend you have a 8GB multiplier but 'cinder list' shows something else? wow, that weird 15:21
geguileocan then it be migrated to another backend with a 1GB volume and still work?15:21
HappyStackeryes that's the case15:21
geguileoenriquetaso: not wow, it's the expected behavior15:21
HappyStackerthe real volume will be 8GB seen by the operating system15:21
enriquetasotrue 15:21
geguileocustomers should NEVER request a 1GB volume and suddenly see an 8GB volume15:21
geguileoregardless of how much is used in the backend15:22
enriquetasosame with encrypted volumes tho :P15:22
geguileoenriquetaso: that's related, yes15:22
geguileoHappyStacker: so would that operation work fine?15:22
whoami-rajatgeguileo, for the create case, the size is correct but for the extend case it's mismatched between cinder DB and actual backend vol15:22
geguileowhoami-rajat: how can it be correct for the create?15:22
geguileoooooooh, I see what you mean15:23
rosmaitaeharney always mentions this case, i think there is another driver that has this issue15:23
geguileothat they are returning the wrong value on the extend!15:23
whoami-rajatgeguileo, because we return size in the model update for create but not for extend15:23
HappyStackerAt the creation time, I think it work but it doesn't reflect the reality when we do an extend15:23
geguileoHappyStacker: reality is a subjective term  ;-)15:23
geguileoCinder must report the USER requeste size (1GB)15:23
HappyStackerWe wand to fix it, but we don't know where to start15:24
geguileonot actual allocated size15:24
geguileoHappyStacker: when you extend from 1GB to 2GB does the size end up saying 8GB?15:24
HappyStackerbut how an operator is supposed to monitor the size allocation?15:24
whoami-rajathere for create they return the updated size https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/dell_emc/powerflex/driver.py#L63515:24
HappyStackeryes15:24
geguileoHappyStacker: that's a different issue15:24
geguileowhoami-rajat: that's a bug right there15:25
whoami-rajatbut for extend, we don't return anything (but even if we return, cinder doesn't expect a return and won't do anything) https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/dell_emc/powerflex/driver.py#L80415:25
geguileoHappyStacker: one thing is a bug in a driver, another a new feature15:25
geguileowhoami-rajat: then extend is correct15:25
HappyStackeras whoami-rajat, there is no issue at the creation time15:25
geguileoand the bug they have is in create15:25
geguileoHappyStacker: on the contrary the creation is *WRONG*15:25
HappyStackeryes from a cinder standpoint15:26
whoami-rajatgeguileo, ok, so for creation, we shouldn't modify the size value?15:26
geguileoa driver should never, ever change the size15:26
geguileoif I'm a user and I request a 1GB volume and then check the volume and see an 8GB volume I'm going to call somebody to complain15:26
HappyStackerok so botomm line is at the creation the size should reflect the user request15:26
geguileoHappyStacker: at creation a driver should not return the size15:26
geguileothat's a cinder core thingy15:27
HappyStackerok15:27
geguileoso, there is a bug there15:27
geguileoon the other hand a new feature would be necessary for what you want15:27
geguileowhich is reporting the real/allocated size for a volume15:27
HappyStackerbut how can make our powerflex comply with cinder best practices?15:28
HappyStackeryes15:28
geguileonot returning the size15:28
geguileothat's the best practice15:28
HappyStackerso the bug is that cinder should ALWAYS return the size requested by the user and not returned by the backend15:29
geguileoa new DB field in volume and snapshots15:29
HappyStackerand the feature would be to have a new field calles allocation for instance15:29
geguileoHappyStacker: yes, though the way of fixing it is for the driver not to return the size15:29
geguileoyeah, or actual_size15:29
geguileoit would need to be in both volumes and snapshots DB tables15:30
HappyStackerI came with one problem, I'll leave with two, thanks guys! just kidding, gotcha15:30
geguileoHappyStacker: one is a problem which is fixed with a 1 line patch15:30
geguileoHappyStacker: the other is a feature, not a problem  ;-P15:30
geguileoHappyStacker: The feature would require a spec15:30
HappyStackeryes15:31
HappyStackerand the other needs to have a launchpad 15:31
HappyStackerwith a patchset15:31
geguileoyup15:31
HappyStackerincluding the one line fix15:31
HappyStackerok15:31
enriquetasogreat15:32
HappyStackerI'll bring it up to my team15:32
geguileoHappyStacker: feel free to ping us regarind the feature/spec15:32
HappyStackeryes, I'll certainly do as it won't be trivial for me15:33
vbelHappy powerflexing! ;) 15:34
HappyStackerROFL15:35
vbelthere is another old problem that is fixed - https://review.opendev.org/c/openstack/cinder/+/806605  . Unfortunately no +2s :(15:35
enriquetasoOK.. Moving on15:36
enriquetaso#topic open discussion 15:36
opendevreviewMerged openstack/cinder master: mypy: cinder/api/common.py  https://review.opendev.org/c/openstack/cinder/+/84112515:37
vbelIs anything else needed for getting new Tatlin Unified storage driver into Zed? https://review.opendev.org/c/openstack/cinder/+/82549215:38
enriquetasoCinder people please review ^15:40
vbelthanks!15:41
enriquetasolooks like eharney review it a time ago and the code was updated :)15:41
enriquetasoOK15:41
enriquetasorun out of time!!15:41
enriquetaso#endmeeting15:41
opendevmeetMeeting ended Wed Jul  6 15:41:58 2022 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)15:41
opendevmeetMinutes:        https://meetings.opendev.org/meetings/cinder_bs/2022/cinder_bs.2022-07-06-15.00.html15:41
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/cinder_bs/2022/cinder_bs.2022-07-06-15.00.txt15:41
opendevmeetLog:            https://meetings.opendev.org/meetings/cinder_bs/2022/cinder_bs.2022-07-06-15.00.log.html15:41
enriquetasothanks everyone!15:42
geguileosfernand: this is the link to the potential deadlock in a driver https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/dell_emc/powermax/masking.py#L644-L64715:44
opendevreviewVladislav Belogrudov proposed openstack/cinder master: Fix volume caching in PowerFlex driver  https://review.opendev.org/c/openstack/cinder/+/80660515:58
opendevreviewVladislav Belogrudov proposed openstack/cinder master: Fix volume caching in PowerFlex driver  https://review.opendev.org/c/openstack/cinder/+/80660516:03
*** dviroel|lunch is now known as dviroel16:08
geguileovbel: I added a new comment to the patch, sorry I missed it on the first review16:42
opendevreviewMerged openstack/cinder master: Correct VolumeMigrationStatusField  https://review.opendev.org/c/openstack/cinder/+/83982518:04
opendevreviewMerged openstack/cinder master: mypy: service.py  https://review.opendev.org/c/openstack/cinder/+/78836718:04
opendevreviewGorka Eguileor proposed openstack/cinder master: Deadlock prevention support in syncrhonize  https://review.opendev.org/c/openstack/cinder/+/84889919:31
opendevreviewGorka Eguileor proposed openstack/cinder master: PowerMax: Fix deadlock moving SGs  https://review.opendev.org/c/openstack/cinder/+/84890019:31
opendevreviewGorka Eguileor proposed openstack/cinder master: PowerMax: Fix deadlock moving SGs  https://review.opendev.org/c/openstack/cinder/+/84890019:34
*** dviroel is now known as dviroel|out20:50

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!