openstackgerrit | Alexey Khodos proposed openstack/cinder master: Refactored NexentaStor5 driver https://review.openstack.org/586239 | 00:18 |
---|---|---|
*** tejdeep has joined #openstack-cinder | 00:41 | |
*** erlon has joined #openstack-cinder | 00:41 | |
*** tejdeep_ has joined #openstack-cinder | 00:43 | |
*** tejdeep has quit IRC | 00:43 | |
*** tejdeep has joined #openstack-cinder | 00:44 | |
*** dave-mccowan has joined #openstack-cinder | 00:54 | |
*** enriquetaso has quit IRC | 00:55 | |
*** erlon has quit IRC | 00:57 | |
*** tejdeep has quit IRC | 00:57 | |
*** whoami-rajat has joined #openstack-cinder | 01:12 | |
*** itlinux has joined #openstack-cinder | 01:25 | |
openstackgerrit | Ryan Liang proposed openstack/cinder stable/queens: VNX: update sg in cache https://review.openstack.org/642613 | 01:31 |
*** mvkr has quit IRC | 01:36 | |
*** mvkr has joined #openstack-cinder | 01:38 | |
*** gnufied has quit IRC | 01:41 | |
*** erlon has joined #openstack-cinder | 01:48 | |
*** mvkr has quit IRC | 01:49 | |
*** itlinux has quit IRC | 01:51 | |
*** mvkr has joined #openstack-cinder | 01:52 | |
openstackgerrit | Merged openstack/cinder master: Adds revert to snapshot feature to support matrix https://review.openstack.org/627971 | 02:20 |
*** openstackstatus has quit IRC | 02:22 | |
*** openstack has joined #openstack-cinder | 02:26 | |
*** ChanServ sets mode: +o openstack | 02:26 | |
*** Dinesh_Bhor has quit IRC | 02:49 | |
*** lixiaoy1 has joined #openstack-cinder | 02:58 | |
*** lixiaoy1 has quit IRC | 03:03 | |
*** erlon has quit IRC | 03:35 | |
*** dave-mccowan has quit IRC | 03:40 | |
*** udesale has joined #openstack-cinder | 03:49 | |
*** mchlumsky has quit IRC | 03:58 | |
*** rosmaita has left #openstack-cinder | 04:11 | |
*** udesale has quit IRC | 04:45 | |
*** udesale has joined #openstack-cinder | 04:46 | |
*** abhishekk has joined #openstack-cinder | 04:57 | |
*** abhishekk has quit IRC | 04:57 | |
*** markvoelker has joined #openstack-cinder | 05:06 | |
*** avishay has joined #openstack-cinder | 05:17 | |
avishay | Hi all, it looks like I have introduced some regression with the WWN verification patch in os-brick | 05:18 |
avishay | http://openstack-logs.purestorage.com/09/636709/6/check/PureISCSIDriver-tempest-dsvm-xenial-aio-multipath-chap/414e0f8/logs/screen-c-vol.txt.gz | 05:18 |
avishay | For some reason get_sysfs_wwn doesn't always return a WWN, and therefore the verification fails | 05:19 |
avishay | Possible solutions are: (1) skip the validation if no WWN is found, (2) use get_scsi_wwn() instead of get_sysfs_wwn(), (3) revert the WWN verification patch in os-brick until Train opens | 05:21 |
avishay | patrickeast, This only affects Pure Storage because it's the only driver that returns the expected WWN on initialize_connection. We can revert that patch if desired as an alternate short-term fix. | 05:22 |
*** sapd1 has joined #openstack-cinder | 05:24 | |
avishay | I would say that the safest is probably to revert the WWN verification in os-brick | 05:24 |
whoami-rajat | avishay: 3rd option seems best for now. | 05:24 |
avishay | whoami-rajat: Yes. What's the process for reverting? Open a bug and submit a revert patch? | 05:25 |
whoami-rajat | avishay: you can directly push the revert patch. but since os-brick is released[1], i'm not sure how we can add the revert to it. maybe need a new release. but atleast for now you can push the patch for it. | 05:30 |
whoami-rajat | [1] https://review.openstack.org/#/c/641863/ | 05:30 |
avishay | whoami-rajat: OK. I will submit the revert patch for now and I guess we'll have to wait until people wake up? | 05:36 |
openstackgerrit | Avishay Traeger proposed openstack/os-brick master: Revert "Verify WWN of connected iSCSI devices if passed" https://review.openstack.org/642648 | 05:38 |
avishay | whoami-rajat: ^^^ | 05:39 |
whoami-rajat | avishay: yeah, i think smcginnis would provide better inputs here. | 05:46 |
avishay | whoami-rajat: OK thanks a lot | 05:50 |
whoami-rajat | avishay: np! | 05:56 |
*** gary_perkins has quit IRC | 06:08 | |
*** irclogbot_0 has quit IRC | 06:09 | |
*** andreaf has quit IRC | 06:09 | |
*** gary_perkins has joined #openstack-cinder | 06:10 | |
*** irclogbot_0 has joined #openstack-cinder | 06:10 | |
*** andreaf has joined #openstack-cinder | 06:12 | |
*** e0ne has joined #openstack-cinder | 06:18 | |
openstackgerrit | Rajat Dhasmana proposed openstack/cinder master: Automate generation of snapshots api-ref samples https://review.openstack.org/642658 | 06:49 |
*** Luzi has joined #openstack-cinder | 06:53 | |
*** dpawlik has joined #openstack-cinder | 07:30 | |
*** pcaruana has joined #openstack-cinder | 07:39 | |
*** pcaruana has quit IRC | 07:43 | |
*** pcaruana has joined #openstack-cinder | 07:43 | |
*** tkajinam has quit IRC | 08:07 | |
*** e0ne has quit IRC | 08:17 | |
*** e0ne has joined #openstack-cinder | 08:23 | |
*** e0ne has quit IRC | 08:39 | |
*** sapd1 has quit IRC | 08:47 | |
*** sahid has joined #openstack-cinder | 08:48 | |
openstackgerrit | Yury Kulazhenkov proposed openstack/cinder master: Add support for VxFlex OS 3.0 to VxFlex OS driver https://review.openstack.org/639963 | 09:29 |
*** mpasserini has joined #openstack-cinder | 09:45 | |
mpasserini | Hi, I just posted a comment to this patch, I hope it's the right place: https://review.openstack.org/#/c/595372/2 | 09:46 |
mpasserini | This causes problems when deleting backups created before updating configuration, the error shown is like: "Delete backup aborted, the backup service currently configured [cinder.backup.drivers.swift] is not the backup service that was used to create this backup [cinder.backup.drivers.swift.SwiftBackupDriver]." It requires manual fixing in the DB like this: "update backups set service='cinder.backup.drivers.swift' where se | 09:46 |
mpasserini | ='cinder.backup.drivers.swift.SwiftBackupDriver';" | 09:46 |
mpasserini | Is there a better place where to track the issue? Bugzilla? | 09:46 |
whoami-rajat | mpasserini: Hi, thanks for the insights, you can report a bug on launchpad [1] (right side) | 10:02 |
whoami-rajat | [1] https://launchpad.net/cinder | 10:02 |
openstackgerrit | Avishay Traeger proposed openstack/os-brick master: Revert "Verify WWN of connected iSCSI devices if passed" https://review.openstack.org/642648 | 10:04 |
mpasserini | whoami-rajat: ok thanks! | 10:05 |
*** e0ne has joined #openstack-cinder | 10:16 | |
*** dviroel_ has joined #openstack-cinder | 10:32 | |
*** lpetrut has joined #openstack-cinder | 10:33 | |
*** luizbag has joined #openstack-cinder | 10:44 | |
*** bal has joined #openstack-cinder | 10:59 | |
*** carlos_silva has joined #openstack-cinder | 11:11 | |
*** sahid has quit IRC | 11:17 | |
*** sahid has joined #openstack-cinder | 11:20 | |
*** thgcorrea has joined #openstack-cinder | 11:20 | |
*** dave-mccowan has joined #openstack-cinder | 11:23 | |
*** abishop has joined #openstack-cinder | 11:53 | |
*** rosmaita has joined #openstack-cinder | 12:14 | |
*** markvoelker has quit IRC | 12:18 | |
*** e0ne has quit IRC | 12:36 | |
*** avishay has quit IRC | 12:37 | |
*** udesale has quit IRC | 12:50 | |
*** udesale has joined #openstack-cinder | 12:51 | |
*** mriedem has joined #openstack-cinder | 12:53 | |
*** enriquetaso has joined #openstack-cinder | 13:02 | |
*** FlorianFa has quit IRC | 13:03 | |
*** sapd1 has joined #openstack-cinder | 13:03 | |
smcginnis | mpasserini: What version are you running. That issue was fixed (or at least was supposed to have been). | 13:07 |
mpasserini | smcginnis I'm at queens | 13:09 |
mpasserini | openstack-cinder-12.0.4-3.el7ost.noarch | 13:09 |
smcginnis | Hmm, that should have it. | 13:10 |
smcginnis | https://github.com/openstack/cinder/commit/7bae5759c872d4536ee29887eb6567f668e75eec#diff-c821b31d1ba94112c25a0bd41d55d5fb | 13:10 |
smcginnis | Oh! | 13:11 |
smcginnis | mpasserini: I finally read it close enough. Your service is misconfigured. | 13:12 |
*** anks2k has joined #openstack-cinder | 13:16 | |
mpasserini | smcginnis: the system was working fine with the driver spelled as cinder.backup.drivers.swift.SwiftBackupDriver until we did the ugprade to Queens. With Queens the spelling was corrected in the code to cinder.backup.drivers.swift, so our backups created with the old method cannot be deleted anymore | 13:16 |
mpasserini | we had to change the 'service' column in the cinder backup DB for those old backups in order to make their deletion work | 13:17 |
*** lseki has joined #openstack-cinder | 13:21 | |
smcginnis | mpasserini: That's actually backwards. The format prior to Queens allowed using just the module name (cinder.backup.drivers.swift) but starting with Queens it got rid of that backwards compatibility so you actually have to provide the class name for the backup driver (cinder.backup.drivers.swift.SwiftBackupDriver). | 13:21 |
*** dave-mccowan has quit IRC | 13:22 | |
mpasserini | oh I see, so I should change this? [root@controller-3 ~]# grep drivers.swift /etc/cinder/cinder.conf | 13:26 |
mpasserini | backup_driver = cinder.backup.drivers.swift | 13:26 |
smcginnis | mpasserini: Correct | 13:26 |
smcginnis | You will also need to restart the cinder-backup service after editing /etc/cinder.conf | 13:27 |
smcginnis | And it sounds like another DB update to revert back the one you had done. | 13:27 |
*** e0ne has joined #openstack-cinder | 13:28 | |
mpasserini | I see ,then it's a problem with the puppet module version delivered with RHOSP13, which I'm using | 13:28 |
mpasserini | [root@controller-3 ~]# grep cinder.backup.drivers.swift /etc/puppet/modules/cinder/manifests/backup/swift.pp | 13:29 |
mpasserini | $backup_driver = 'cinder.backup.drivers.swift', | 13:29 |
smcginnis | mpasserini: Hmm, yeah. They should not be setting it to that. | 13:29 |
mpasserini | The last puppet module includes the right driver https://github.com/openstack/puppet-cinder/blob/4aaac7a5b42e4aa969ddef2205d9a4aa1d2db84c/manifests/backup/swift.pp#L78 | 13:32 |
mpasserini | thanks! | 13:32 |
smcginnis | mpasserini: No problem. Glad it got figured out! | 13:33 |
*** dave-mccowan has joined #openstack-cinder | 13:35 | |
*** avishay has joined #openstack-cinder | 13:39 | |
avishay | smcginnis: Hey | 13:40 |
*** enriquetaso has quit IRC | 13:48 | |
*** lennyb has quit IRC | 13:51 | |
*** openstack has joined #openstack-cinder | 15:42 | |
*** ChanServ sets mode: +o openstack | 15:42 | |
avishay | Thanks guys and sorry for that | 15:44 |
jungleboyj | avishay: It is ok. Better safe than sorry. | 15:44 |
avishay | Also better late than never :P | 15:45 |
*** e0ne has quit IRC | 15:45 | |
*** enriquetaso has quit IRC | 15:46 | |
smcginnis | jungleboyj: Might be good to get it going right away. | 15:47 |
*** enriquetaso has joined #openstack-cinder | 15:47 | |
jungleboyj | smcginnis: Ok. Cool. I thought that made the most sense but wasn't sure with the way you worded things. I will keep pushing this forward. | 15:47 |
jungleboyj | whoami-rajat: Has started the merge so I will request the release as soon as it merges. | 15:49 |
whoami-rajat | jungleboyj: after your +2, thought its better to merge it fast. :) | 15:50 |
jungleboyj | whoami-rajat: Yeah, makes sense. | 15:51 |
hemna_ | mep | 15:53 |
*** sapd1 has quit IRC | 15:53 | |
jungleboyj | I just hadn't gotten there yet in my processing of the situation. | 15:54 |
*** avishay has quit IRC | 15:54 | |
*** e0ne has joined #openstack-cinder | 15:58 | |
*** sahid has quit IRC | 16:00 | |
whoami-rajat | e0ne: since we merged the ScaleIO renaming change in os-brick[1] , and it's blocked in cinder[2], won't it cause any issue? | 16:01 |
whoami-rajat | [1] https://review.openstack.org/#/c/635530/ | 16:01 |
whoami-rajat | [2] https://review.openstack.org/#/c/634397/ | 16:01 |
*** sapd1 has joined #openstack-cinder | 16:01 | |
*** sahid has joined #openstack-cinder | 16:02 | |
e0ne | whoami-rajat: we reached requirements freeze for Stein, so it should be safe | 16:02 |
whoami-rajat | ok, i think the os-brick change is backward compatible so it seems safe. | 16:03 |
whoami-rajat | e0ne: ok thanks. | 16:03 |
*** enriquetaso has quit IRC | 16:05 | |
jungleboyj | e0ne: Though we are going to request a FFE for requirements for os-brick for this change: | 16:05 |
jungleboyj | https://review.openstack.org/#/c/642648/2 | 16:05 |
jungleboyj | Oh, but that merged before the previous os-brick release. So, if it is backward compatible then we should be ok. | 16:06 |
*** KeithMnemonic has quit IRC | 16:07 | |
*** anks2k has left #openstack-cinder | 16:07 | |
*** KeithMnemonic has joined #openstack-cinder | 16:08 | |
*** sapd1 has quit IRC | 16:08 | |
whoami-rajat | jungleboyj: some part of backward compatibility exists in cinder patch also[1] (as smcginnis suggested) | 16:11 |
whoami-rajat | [1] https://review.openstack.org/#/c/634397/6/cinder/volume/manager.py@185 | 16:11 |
jungleboyj | Right, but the old names will work in os-brick without the Server side merging. | 16:13 |
hemna_ | why do we have an os-brick specific patch for a cinder driver? | 16:14 |
hemna_ | review url ? | 16:15 |
hemna_ | https://review.openstack.org/#/c/640875/ | 16:17 |
hemna_ | that one? | 16:17 |
hemna_ | why is there branding specific names in the connection properties at all? | 16:18 |
whoami-rajat | hemna_: it's for renaming the connector https://review.openstack.org/#/c/635530/ | 16:18 |
hemna_ | no customer EVER sees that? | 16:18 |
hemna_ | why not make it just generic | 16:18 |
hemna_ | it's odd to me that there is branding as part of a connector | 16:19 |
jungleboyj | hemna_: Good point. | 16:19 |
hemna_ | smh | 16:19 |
hemna_ | now we have special cases in os-brick because of branding.... | 16:20 |
* hemna_ has a sad | 16:20 | |
eharney | why is the class named VxFlexOsConnector when all of the text calls it "VxFlexOS"? uhy | 16:20 |
* eharney just reviewed https://review.openstack.org/#/c/634397/ and kinda thinks we should just name it "EMC Thing #4" and give up on the marketing name changes | 16:21 | |
hemna_ | yah I agree | 16:21 |
hemna_ | they even changed the name of the protocol in the connection_properties that gets shipped between cinder and nova | 16:21 |
hemna_ | smh | 16:21 |
hemna_ | because....branding | 16:21 |
whoami-rajat | hemna_: i think due to custom driver protocols. | 16:22 |
jungleboyj | I should have pushed back against that. | 16:22 |
eharney | they could have just left the protocol as "scaleio". | 16:22 |
hemna_ | eharney: +1 | 16:22 |
hemna_ | well, I guess I should blame myself for not doing more reviews | 16:23 |
hemna_ | but I would have -1'd those for sure | 16:23 |
* jungleboyj was doing too many reviews to put enough thought into that. | 16:23 | |
hemna_ | I get renaming the driver in cinder | 16:24 |
hemna_ | we did that at HP -> HPE a few years back | 16:24 |
hemna_ | as it's in the public documentation eventually | 16:24 |
hemna_ | I kinda think we should revert that os-brick change for the protocol | 16:25 |
* hemna_ wonders if the connector mapping can ever get removed in the future | 16:26 | |
hemna_ | even if it is deprecated | 16:26 |
hemna_ | re: detaching a volume that's been attached for a long time | 16:27 |
hemna_ | if we remove that deprecated mapping, I think it will fail to detach | 16:27 |
hemna_ | as os-brick won't be able to find the right connector | 16:27 |
hemna_ | :( | 16:27 |
whoami-rajat | jungleboyj: wasn't the disco driver removed from cinder? | 16:27 |
hemna_ | whoami-rajat: can we just make the rebranded cinder driver use the same 'scaleio' protocol | 16:28 |
hemna_ | that will clean up os-brick a bit | 16:28 |
jungleboyj | whoami-rajat: Yes. It was. | 16:28 |
hemna_ | and just add a comment in the cinder driver that you have to keep the protocol name the same for backwards and forwards compatibility | 16:29 |
whoami-rajat | hemna_: yes, was looking for a case like that. they've also changed all the connection properties https://review.openstack.org/#/c/635530/6/os_brick/initiator/connectors/vxflexos.py | 16:29 |
hemna_ | ugh | 16:29 |
hemna_ | that's unfortunate | 16:30 |
hemna_ | :( | 16:30 |
whoami-rajat | jungleboyj: so i think this code isn't used anywhere ? https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/disco.py | 16:30 |
hemna_ | I think that once you have a protocol mapping in os-brick, it can't be removed | 16:30 |
jungleboyj | This sounds like a mess we should discuss in Denver. | 16:31 |
hemna_ | I think we should fix this prior to shipping the next os-brick (if we can) | 16:32 |
hemna_ | https://github.com/openstack/os-brick/blob/stable/rocky/os_brick/initiator/connector.py#L104 | 16:32 |
hemna_ | the only real change needed in the connector.py would be to change that line | 16:33 |
hemna_ | to the new connector name | 16:33 |
hemna_ | once the current state ships, we can't remove that mapping, as the new protocol name won't work | 16:34 |
jungleboyj | I am open to doing whatever is best for os-brick going forward. | 16:36 |
*** udesale has quit IRC | 16:37 | |
hemna_ | it's going to require making changes in the rebranded cinder driver as well | 16:37 |
hemna_ | https://review.openstack.org/#/c/640875/4/os_brick/initiator/connectors/vxflexos.py | 16:38 |
hemna_ | :( | 16:39 |
whoami-rajat | hemna_: the driver isn't renamed in cinder, the nova renaming required os-brick release (which is done) but still nova patch is on hold[1], if we just revert the os-brick change, what will be the issue? | 16:40 |
whoami-rajat | [1] https://review.openstack.org/#/c/634866/3 | 16:40 |
hemna_ | yah that should be -2'd | 16:41 |
hemna_ | what a mess | 16:41 |
jungleboyj | whoami-rajat: That would be nice. | 16:41 |
jungleboyj | Looks like Eric asked the right question. | 16:41 |
hemna_ | yah I think we should revert these changes | 16:44 |
hemna_ | do we have contact with Yury? | 16:45 |
whoami-rajat | hemna_: not sure, can't find him now, the revert can be proposed by anybody i guess? | 16:46 |
hemna_ | yah | 16:46 |
jungleboyj | We should. We can get hold of walshh also. | 16:46 |
hemna_ | either revert or new patches to fix the protocol | 16:46 |
jungleboyj | She, of course, is not on IRC. | 16:46 |
whoami-rajat | jungleboyj: i'm still waiting for walshh's answer to my previous query :) | 16:48 |
whoami-rajat | hemna_: i will try contacting yury tomorrow again. else we can continue with either of the approach. | 16:50 |
jungleboyj | I would vote for a revert, get this mess out of here since the merge has been blocked on the Nova side. | 16:50 |
whoami-rajat | jungleboyj: ++ | 16:50 |
jungleboyj | Also since we haven't merged the server side and then work things out properly in Train. | 16:51 |
hemna_ | ok that's probably the best at this point | 16:53 |
*** e0ne has quit IRC | 16:55 | |
jungleboyj | whoami-rajat: You are going to follow up with Yuri tomorrow and explain why we are doing this | 16:57 |
*** enriquetaso has joined #openstack-cinder | 16:58 | |
whoami-rajat | jungleboyj: sure... (if i find him in IST tz). | 16:59 |
jungleboyj | Ok. Who is going to propose the reversion patch? | 16:59 |
jungleboyj | hemna_: ? | 16:59 |
hemna_ | sure I can work on that | 17:00 |
jungleboyj | Cool. I will help to approve and then once everything has merged I will propose the new os-brick release. | 17:00 |
whoami-rajat | jungleboyj: also i found this [1] . RSD driver is on hold in cinder but respective os-brick changes are merged. (this probably looks more safe as more than the vxflex one) | 17:01 |
whoami-rajat | [1] https://review.openstack.org/#/c/620250/ | 17:01 |
whoami-rajat | jungleboyj: can i also add this as meeting agenda for tomorrow ? | 17:01 |
whoami-rajat | ^ the cinder - osbrick gaps part discussion | 17:02 |
jungleboyj | whoami-rajat: Yeah, that is a good topic. | 17:02 |
jungleboyj | I think the RSD connector is fine as it is an addition that was needed to support their driver. The code just isn't used now. | 17:03 |
jungleboyj | Unless hemna_ looks at that and disagrees. | 17:03 |
* jungleboyj is glad to have our os-brick expert back to some extent. | 17:03 | |
whoami-rajat | jungleboyj: thanks. added the topic. | 17:04 |
*** enriquetaso has quit IRC | 17:05 | |
*** enriquetaso has joined #openstack-cinder | 17:05 | |
*** eandersson_ has joined #openstack-cinder | 17:07 | |
jungleboyj | whoami-rajat: Thanks. | 17:17 |
jungleboyj | I have alerted prometheanfire that an FFE will be coming for requirements. | 17:18 |
thgcorrea | eharney, I changed the commit message to better match my fix on https://review.openstack.org/#/c/633596/ that you commented with erlon | 17:27 |
eharney | thgcorrea: thanks, will take a look | 17:28 |
*** mpasserini has left #openstack-cinder | 17:39 | |
openstackgerrit | Merged openstack/os-brick master: Revert "Verify WWN of connected iSCSI devices if passed" https://review.openstack.org/642648 | 17:39 |
hemna_ | back | 17:51 |
hemna_ | so that one I think is fine, as it's a brand new connector for nvme | 17:53 |
hemna_ | new connector, new protocol | 17:53 |
hemna_ | hrmm | 17:53 |
hemna_ | new protocol, but same connector..... | 17:54 |
hemna_ | "* Add a new protocol "NVMEOF" mapping to allow both Cinder and Nova to | 17:54 |
hemna_ | identify and use the nvme connector. | 17:54 |
hemna_ | " | 17:54 |
hemna_ | why | 17:54 |
hemna_ | there is already an nvme protocol | 17:55 |
*** zhubx has quit IRC | 17:55 | |
*** zhubx has joined #openstack-cinder | 17:55 | |
hemna_ | unless I'm missing something, I think that one was also unnecessary | 17:55 |
hemna_ | whoami-rajat: is there an associated nova patch to accomodate nvmeof protocol as well? | 17:56 |
*** henriqueof has joined #openstack-cinder | 17:57 | |
whoami-rajat | hemna_: don't think so | 17:58 |
hemna_ | smh | 17:58 |
hemna_ | wtf | 17:58 |
whoami-rajat | hemna_: where do we use the nvme connector ? Spdk driver ? | 17:59 |
hemna_ | I'm trying to find it | 17:59 |
jungleboyj | whoami-rajat: It will be in the RSD driver when they get that in place. | 17:59 |
jungleboyj | hemna_: So you are saying because the NVME-of driver just redirects to the NVME driver there was no need to make a new connector? | 18:00 |
hemna_ | spdk.py looks like | 18:00 |
hemna_ | no | 18:00 |
hemna_ | there is no need to create the new protocol | 18:00 |
hemna_ | https://review.openstack.org/#/c/620250/10/os_brick/initiator/__init__.py | 18:00 |
whoami-rajat | jungleboyj: they added nvme-of for rsd, nvme existed previously | 18:00 |
jungleboyj | Hmmm, didn't realize spdk was using it too. | 18:01 |
hemna_ | https://review.openstack.org/#/c/620250/10/os_brick/initiator/connector.py | 18:01 |
hemna_ | both NVME and NVMEOF point to the same connector | 18:01 |
jungleboyj | Ah, I see. Should have said protocol. | 18:01 |
jungleboyj | But I am following. | 18:01 |
*** e0ne has joined #openstack-cinder | 18:02 | |
whoami-rajat | I think they added the new connector due to the additional options used by rsd | 18:03 |
hemna_ | well that patch doesn't have a new connector | 18:03 |
hemna_ | they just updated the same connector | 18:03 |
hemna_ | is there a patch in cinder to use the NVMEOF protocol? | 18:04 |
hemna_ | because I don't see it in the current code for cinder | 18:04 |
hemna_ | making that os-brick patch useless | 18:04 |
eharney | hemna_: https://review.openstack.org/#/c/621465/ | 18:05 |
hemna_ | eharney: ah yah, just found that too | 18:05 |
whoami-rajat | hemna_: oh yes, maybe they wanted to create a new connector and inherit the nvme connector but the end patch merged seems wrong. | 18:05 |
hemna_ | yah this is borked | 18:06 |
hemna_ | as well | 18:06 |
hemna_ | :(!! | 18:06 |
*** e0ne has quit IRC | 18:06 | |
hemna_ | there is nothing special in the nvme.py connector that even looks at the nvmeof protocol | 18:07 |
hemna_ | so, yah, that shouldn't have happened either | 18:07 |
* jungleboyj is sad | 18:09 | |
hemna_ | what a mess | 18:10 |
whoami-rajat | hemna_: shouldn't this change in nvme connector have affected the spdk driver ? | 18:10 |
hemna_ | so | 18:11 |
hemna_ | the nvme.py connector changes adds a new 'system uuid' in the connector dict that gets sent back to cinder | 18:12 |
hemna_ | and the nvme.py connector also looks for host_nqn | 18:12 |
hemna_ | if it's there, it uses it | 18:12 |
hemna_ | so I'm guessing that the spdk driver in cinder should still work | 18:13 |
hemna_ | host_nqn is returned by the new rsd.py cinder driver, but not spdk.py driver | 18:15 |
*** erlon has quit IRC | 18:15 | |
*** sahid has quit IRC | 18:17 | |
*** e0ne has joined #openstack-cinder | 18:18 | |
whoami-rajat | hemna_: only if it's found in the connection_properties, this seems to be written to handle both drivers | 18:18 |
hemna_ | yah, there is no need for the nvmeof protocol | 18:18 |
* hemna_ wants to cry | 18:18 | |
hemna_ | this is such a mess now | 18:18 |
whoami-rajat | hemna_: :( | 18:19 |
jungleboyj | :-( Is this something we can fix before Stein goes out? | 18:19 |
hemna_ | what's the timing ? | 18:20 |
jungleboyj | Well, it depends how long they will take requirements FFEs. | 18:21 |
*** e0ne_ has joined #openstack-cinder | 18:21 | |
hemna_ | so I think I have several patches to revert just for the scaleio issue | 18:21 |
hemna_ | https://github.com/openstack/os-brick/commit/6b60614afaf88819ce92a2d0ed269035a3c8b261 | 18:21 |
hemna_ | https://github.com/openstack/os-brick/commit/04c6c0cc291bd8c472b2a24a333085739eb9f877 | 18:21 |
hemna_ | those 2 | 18:21 |
whoami-rajat | hemna_: how about removing the nvmeof from connector names and making changes in the rsd driver to use nvme? | 18:21 |
*** e0ne has quit IRC | 18:22 | |
hemna_ | and I think we can get away with simply removing the mapping in os-brick for nvmeof | 18:22 |
hemna_ | like whoami-rajat just mentioned, change the rsd driver to use nvme protocol | 18:22 |
hemna_ | I already -1'd that cinder rsd.py patch | 18:23 |
hemna_ | but that's a new driver, so it shouldn't land now anyway | 18:23 |
hemna_ | did the scaleio driver rebranding land in cinder? | 18:23 |
jungleboyj | hemna_: No. | 18:23 |
hemna_ | do you have that review url handy? | 18:24 |
jungleboyj | hemna_: In another meeting. Hold on. | 18:25 |
hemna_ | ok | 18:25 |
whoami-rajat | hemna_: although I prefer nvmeof name over nvme as it makes more sense here, but I guess that's a topic for other day! | 18:25 |
hemna_ | I just want to get my ducks in a row, before I go about reverting commits | 18:25 |
hemna_ | and get a plan in place | 18:25 |
hemna_ | whoami-rajat: yah | 18:25 |
*** gmann is now known as gmann_afk | 18:26 | |
whoami-rajat | https://review.openstack.org/#/q/owner:yury.kulazhenkov%2540dell.com+status:open | 18:27 |
whoami-rajat | hemna_: ^ | 18:27 |
hemna_ | wtf | 18:32 |
hemna_ | https://review.openstack.org/#/c/634397/6/cinder/volume/drivers/dell_emc/vxflexos/driver.py | 18:32 |
hemna_ | that patch still uses the scaleio connector protocol | 18:32 |
hemna_ | line 928 | 18:32 |
*** angela-s has joined #openstack-cinder | 18:32 | |
jungleboyj | whoami-rajat: Thanks for finding the link. | 18:33 |
angela-s | smcginnis Hi Sean, can you have a look at the Newton backport? Jay has already approved. Thank you! https://review.openstack.org/#/c/640624/ | 18:33 |
whoami-rajat | jungleboyj: np! | 18:34 |
hemna_ | ok, so the cinder driver still uses 'scaleio' as the protocol | 18:34 |
hemna_ | so we just need to backout the os-brick commits | 18:35 |
hemna_ | and then they can accomodate for a new patch to use the scaleio protocol | 18:35 |
hemna_ | and rid os-brick of the mapping | 18:35 |
hemna_ | yah the VXFLEXOS protocol isn't even used in his rebranding patches in cinder | 18:37 |
hemna_ | smh | 18:37 |
hemna_ | and none of the cinder and nova changes have landed | 18:37 |
hemna_ | so it's safe to revert IMHO | 18:37 |
openstackgerrit | Walter A. Boring IV (hemna) proposed openstack/os-brick master: Revert "Fix VxFlexOs KeyError after upgrade" https://review.openstack.org/642849 | 18:38 |
whoami-rajat | hemna_: is it worth considering to have names like emc connector for better generalized connectors? Also for the filename atleast | 18:39 |
*** pcaruana has quit IRC | 18:39 | |
hemna_ | maybe, but we have existing protocols that I don't want to have to create a mapping | 18:39 |
hemna_ | which is part of the problem here | 18:39 |
hemna_ | creates a mess of magic | 18:40 |
*** e0ne_ has quit IRC | 18:41 | |
openstackgerrit | Walter A. Boring IV (hemna) proposed openstack/os-brick master: Revert "rename ScaleIO connector to VxFlex OS" https://review.openstack.org/642852 | 18:41 |
hemna_ | ok both of the revert patches are up in os-brick | 18:42 |
whoami-rajat | hemna_: ok. Thanks for looking into all these. | 18:42 |
hemna_ | np | 18:43 |
hemna_ | glad we caught it before the release went out | 18:43 |
whoami-rajat | hemna_: yeah, maybe merging os brick changes before driver merge isn't a good idea. | 18:44 |
hemna_ | it's a little bit of a chicken and egg issue really | 18:45 |
jungleboyj | hemna_: Sorry for helping to make the mess. | 18:45 |
jungleboyj | hemna_: whoami-rajat We should use cross project dependencies to help control that. | 18:45 |
whoami-rajat | jungleboyj: ++ | 18:47 |
hemna_ | so https://review.openstack.org/#/c/620250/ | 18:48 |
hemna_ | I don't think we should revert the whole patch | 18:49 |
hemna_ | but add a new patch that just removes NVMEOF protocol | 18:49 |
hemna_ | https://review.openstack.org/#/c/620250/10/os_brick/initiator/connector.py | 18:49 |
hemna_ | basically undoing that | 18:49 |
hemna_ | and force the rsd driver to use nvme | 18:49 |
whoami-rajat | hemna_: agreed | 18:50 |
eharney | nvme isn't the same thing as nvmeof..? | 18:51 |
hemna_ | the technology? got me | 18:51 |
hemna_ | both of the connector protocols point to the same exact os-brick connector | 18:51 |
hemna_ | it might make sense if there was something specific about the nvmeof protocol that required an nvmeof connector child class | 18:52 |
hemna_ | but that's not even there | 18:52 |
openstackgerrit | Walter A. Boring IV (hemna) proposed openstack/os-brick master: Remove unnecessary NVMEOF connector protocol https://review.openstack.org/642860 | 18:54 |
hemna_ | ok | 18:54 |
hemna_ | I think that gets us back to sanity, unless I'm missing something else | 18:55 |
eharney | i didn't follow the "before the release went out" bit, wasn't this already all in os-brick 2.8.0? | 18:57 |
hemna_ | grr | 18:58 |
whoami-rajat | hemna_: why did they even have a protocol mapping when the scaleio protocol was being used. :/ | 18:58 |
whoami-rajat | I think it's better to ask the driver maintainers the reason for this unnecessary(or not) code added/changed | 18:58 |
hemna_ | so we did a release of os-brick with this stuff in it ? | 18:58 |
hemna_ | eharney:I was hoping that mess hadn't made it into a drop to pypi yet | 18:58 |
hemna_ | but nobody is using it either way at this point, so it's probably ok. | 18:59 |
* hemna_ ducks | 18:59 | |
hemna_ | too many fires....too little time | 18:59 |
whoami-rajat | hemna_: yes that's why we will be doing FFE | 19:00 |
eharney | hemna_: whoami-rajat: so the spdk driver uses storage_protocol "NVME-oF" | 19:05 |
eharney | was that supposed to be the user of nvmeof? | 19:06 |
jungleboyj | hemna_: If the connectors have been released but we don't have any drivers merged using them, then yes it should be safe to revert things. Right? | 19:18 |
hemna_ | jungleboyj:yah I believe so | 19:22 |
jungleboyj | Cool. | 19:22 |
eharney | so, i asked a question there about that ^ | 19:22 |
jungleboyj | eharney: About the spdk driver? | 19:23 |
hemna_ | oh the spdk.py uses a target driver | 19:23 |
hemna_ | looking | 19:23 |
hemna_ | hrmm | 19:24 |
hemna_ | so, who uses 'nvme' ? | 19:24 |
whoami-rajat | hemna_: the initiator name used in spdk is still nvme | 19:25 |
hemna_ | https://github.com/openstack/cinder/blob/master/cinder/volume/targets/nvmeof.py#L63 | 19:25 |
jungleboyj | whoami-rajat: That is what I am seeing in spdk | 19:25 |
hemna_ | whoami-rajat:url ? | 19:26 |
eharney | ??? | 19:26 |
hemna_ | I don't see 'nvme' for the connector protocol | 19:27 |
hemna_ | afaik spdk.py uses a target driver | 19:27 |
hemna_ | which results in 'nvmeof' | 19:27 |
hemna_ | quit confusing | 19:28 |
hemna_ | that target driver was added 7 months ago to use 'nvmeof' | 19:29 |
hemna_ | and yet 'nvmeof' was only added 4 months ago to os-brick | 19:29 |
* hemna_ scratches head..... | 19:29 | |
* jungleboyj is so confused at the moment. | 19:30 | |
hemna_ | sorry | 19:30 |
hemna_ | the nvvmeof.py target driver was added 2 years ago | 19:30 |
hemna_ | https://github.com/openstack/cinder/blame/master/cinder/volume/targets/nvmeof.py#L28 | 19:31 |
whoami-rajat | hemna_: oh its only for copy_image_to_vol and copy_vol_to_image https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/spdk.py#L337 | 19:31 |
hemna_ | isn't sure that there was an nvmeof supported connector in os-brick 2 years ago | 19:31 |
hemna_ | https://github.com/openstack/os-brick/blame/master/os_brick/initiator/__init__.py#L66 | 19:32 |
hemna_ | 4 months ago | 19:32 |
hemna_ | ! | 19:32 |
hemna_ | https://github.com/openstack/os-brick/commit/905e73129bea8a141485ef00259c5e40d609bafc | 19:33 |
hemna_ | that was added 2 years ago | 19:33 |
hemna_ | but the protocol was 'nvme' | 19:33 |
hemna_ | not 'nvmeof' | 19:33 |
jungleboyj | Oh, so you are saying that the spdk driver is using NVMEOF because both NVME and NVMEOF map to the same NVMeConnector ? | 19:33 |
hemna_ | so cinder was returning 'nvmeof' | 19:33 |
hemna_ | so this never workd | 19:33 |
hemna_ | worked | 19:33 |
hemna_ | jungleboyj: yup | 19:33 |
hemna_ | cinder has been returning 'nvmeof' (initialize_connection calls -> nova -> os-brick) | 19:34 |
hemna_ | but there hasn't been an 'nvmeof' supported protocol until 4 months ago | 19:34 |
hemna_ | so it was broken for a long time | 19:34 |
* jungleboyj sighs | 19:34 | |
hemna_ | https://review.openstack.org/#/c/642860/ | 19:35 |
hemna_ | so that revert should not merge IMHNO | 19:35 |
eharney | hemna_: https://review.openstack.org/#/c/482640/ seems to use nvmeof via brick | 19:35 |
eharney | maybe e0ne can fill in some info there | 19:36 |
hemna_ | hrmm yah it hard coded the use of NVME | 19:37 |
eharney | sorta | 19:37 |
eharney | i think nvmeof= in libvirt_volume_drivers means it could consume it as nvmeof... | 19:37 |
hemna_ | yah | 19:38 |
hemna_ | that's how nova finds the nova libvirt volume driver, which in turn uses os-brick | 19:38 |
hemna_ | and internally hard code maps to nvme protocol | 19:38 |
hemna_ | so it worked whenever that patch landed | 19:38 |
hemna_ | 10 months ago I suppose | 19:38 |
hemna_ | so I abandoned https://review.openstack.org/#/c/642860/ | 19:39 |
hemna_ | but we have 2 protocols pointing to the same connector | 19:39 |
hemna_ | smh | 19:39 |
hemna_ | nova should change it's libvirt volume driver to point to nvmeof | 19:40 |
hemna_ | very confusing | 19:40 |
hemna_ | once we do that, I don't see a need to have the nvme protocol support in os-brick | 19:40 |
hemna_ | unless someone can find an instance where 'nvme' is used | 19:40 |
whoami-rajat | hemna_ https://review.openstack.org/#/c/482640/31/nova/virt/libvirt/volume/nvme.py@35 this is creating a connector with NVME, why we are still keeping NVMEOF in connectors list, could you please explain? | 19:44 |
*** thgcorrea has quit IRC | 20:00 | |
*** e0ne has joined #openstack-cinder | 20:20 | |
*** dave-mccowan has quit IRC | 20:24 | |
*** senrique_ has joined #openstack-cinder | 20:32 | |
*** enriquetaso has quit IRC | 20:34 | |
*** luizbag has quit IRC | 20:46 | |
*** gmann_afk is now known as gmann | 20:54 | |
*** zhubx has quit IRC | 20:59 | |
*** lseki has quit IRC | 20:59 | |
*** zhubx has joined #openstack-cinder | 20:59 | |
*** lseki has joined #openstack-cinder | 21:00 | |
*** abishop has quit IRC | 21:01 | |
*** eharney has quit IRC | 21:03 | |
*** mriedem has quit IRC | 21:05 | |
*** mriedem has joined #openstack-cinder | 21:08 | |
*** e0ne has quit IRC | 21:26 | |
*** senrique__ has joined #openstack-cinder | 21:32 | |
*** senrique_ has quit IRC | 21:34 | |
*** senrique__ has quit IRC | 21:47 | |
*** rcernin has joined #openstack-cinder | 22:01 | |
*** bal has quit IRC | 22:05 | |
*** carlos_silva has quit IRC | 22:09 | |
*** hoonetorg has quit IRC | 22:09 | |
*** hoonetorg has joined #openstack-cinder | 22:10 | |
*** whoami-rajat has quit IRC | 22:12 | |
*** gnufied has quit IRC | 22:15 | |
openstackgerrit | Sam Morrison proposed openstack/cinder master: Include availability-zone of a pool in get-pools API request https://review.openstack.org/599866 | 22:23 |
openstackgerrit | Sam Morrison proposed openstack/cinder master: Include availability-zone of a pool in get-pools API request https://review.openstack.org/599866 | 22:26 |
*** enriquetaso has joined #openstack-cinder | 22:38 | |
*** angela-s has quit IRC | 22:39 | |
*** ianychoi_ is now known as ianychoi | 22:47 | |
*** mriedem has quit IRC | 22:51 | |
*** tkajinam has joined #openstack-cinder | 22:53 | |
*** alkhodos has joined #openstack-cinder | 23:02 | |
*** enriquetaso has quit IRC | 23:07 | |
*** itlinux has joined #openstack-cinder | 23:37 | |
*** lseki has quit IRC | 23:58 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!