)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":20634,"name":"Chris MacNaughton","email":"chris.macnaughton@canonical.com","username":"Chris.MacNaughton"},"change_message_id":"370f68af7a074f287ca62266c85129c4de9dd15c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sahid Orentino Ferdjaoui \u003csahid.ferdjaoui@canonical.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-02-22 12:58:38 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fix wwn not found if mapth comes across too fast"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In a situation where the mpath is generate too fast, the iscsi devices"},{"line_number":10,"context_line":"are not listed anymore by udev in /dev/disk/by-id/* meaning that the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_eac5bd5d","line":7,"range":{"start_line":7,"start_character":21,"end_line":7,"end_character":26},"updated":"2019-03-04 07:59:58.000000000","message":"mpath","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"0f64fd6d7936ce2f0578e6c6399457cc4b13ab11","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"fix wwn not found if mapth comes across too fast"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In a situation where the mpath is generate too fast, the iscsi devices"},{"line_number":10,"context_line":"are not listed anymore by udev in /dev/disk/by-id/* meaning that the"},{"line_number":11,"context_line":"current workflow which is to find one of them falldown in a infiny"},{"line_number":12,"context_line":"loop until the timeout occurs."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_2ff6e590","line":9,"range":{"start_line":9,"start_character":34,"end_line":9,"end_character":42},"updated":"2019-02-26 16:22:35.000000000","message":"generated","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"0f64fd6d7936ce2f0578e6c6399457cc4b13ab11","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In a situation where the mpath is generate too fast, the iscsi devices"},{"line_number":10,"context_line":"are not listed anymore by udev in /dev/disk/by-id/* meaning that the"},{"line_number":11,"context_line":"current workflow which is to find one of them falldown in a infiny"},{"line_number":12,"context_line":"loop until the timeout occurs."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"In this commit we ensure that even if we are not able to find wwid, we"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_efebfda0","line":11,"range":{"start_line":11,"start_character":60,"end_line":11,"end_character":66},"updated":"2019-02-26 16:22:35.000000000","message":"infinite","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e7e70d9e83da0d807b312a65b17d18922e3afce","unresolved":false,"context_lines":[{"line_number":17,"context_line":"infinite loop until the timeout occurs. In this commit we ensure that"},{"line_number":18,"context_line":"even if we are not able to find wwid, we try to find mpath. If the"},{"line_number":19,"context_line":"condition is satsfied, we generate wwid by using iscsi_id from the"},{"line_number":20,"context_line":"mpath."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I94b82cc051ce81cfa0e17911eb51a98fc4a7017b"},{"line_number":23,"context_line":"Closes-Bug: #1815844"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_a120aaad","line":20,"updated":"2019-03-13 14:28:39.000000000","message":"-1: This commit message doesn\u0027t seem right.\n\nWhile refactoring the iSCSI code I did extensive testing setting \"find_multipaths\u003dyes\", even dropping more than 10% of the network traffic, and this was NEVER an issue.\n\nThis may be related to something else, and the patch may be fixing it, but we definitely need a better explanation of WHY it is not generating the right links.\n\nudev rules for the generation of by-id links should be independent of multipath.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"968d0854b54168d83c335d4efa74c84c8568561d","unresolved":false,"context_lines":[{"line_number":17,"context_line":"infinite loop until the timeout occurs. In this commit we ensure that"},{"line_number":18,"context_line":"even if we are not able to find wwid, we try to find mpath. If the"},{"line_number":19,"context_line":"condition is satsfied, we generate wwid by using iscsi_id from the"},{"line_number":20,"context_line":"mpath."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I94b82cc051ce81cfa0e17911eb51a98fc4a7017b"},{"line_number":23,"context_line":"Closes-Bug: #1815844"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_cf4334af","line":20,"in_reply_to":"5fc1f717_3997aa3d","updated":"2019-03-19 12:38:12.000000000","message":"I am not suffering it, I used the log extracts from the bug report to forcefully get into the same situation.\nThe fact that it didn\u0027t happen for *you* with find_multipaths set to no is not significant.  In the same way that the bug   never happening to me in all my extensive testing of the os-brick iSCSI code is not significant either.\n\nI added a comment on the LP bug on how find_multipaths actually work, because you are mistaken.  Truth be told, the choosing of the configuration option and what it does when set to \"no\" is terrible, and you are not the first one getting confused by it.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"7e7a93e541f10b1ba4ef630c7efdedd897d835f2","unresolved":false,"context_lines":[{"line_number":17,"context_line":"infinite loop until the timeout occurs. In this commit we ensure that"},{"line_number":18,"context_line":"even if we are not able to find wwid, we try to find mpath. If the"},{"line_number":19,"context_line":"condition is satsfied, we generate wwid by using iscsi_id from the"},{"line_number":20,"context_line":"mpath."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I94b82cc051ce81cfa0e17911eb51a98fc4a7017b"},{"line_number":23,"context_line":"Closes-Bug: #1815844"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_3997aa3d","line":20,"in_reply_to":"5fc1f717_860a09cf","updated":"2019-03-19 11:44:18.000000000","message":"As indicated in the bug report I think you are suffering a different issue. The system is not loaded and this does not happen with find_multipaths no.\n\nAlso your statement is not correct, if \u0027find_multipaths\u0027 is set to \u0027no\u0027, we can\u0027t be in a situation where a multipath device is created before the individual volumes appear under sysfs \u0027/dev/disk/by-id/scsi-*\u0027. Since multipathd will *never* create one for us automatically. That will be done by os-brick [0]\n\n[0] https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/iscsi.py#L751","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e106ea3b0c09e6c982ca69a015befc2ef3931492","unresolved":false,"context_lines":[{"line_number":17,"context_line":"infinite loop until the timeout occurs. In this commit we ensure that"},{"line_number":18,"context_line":"even if we are not able to find wwid, we try to find mpath. If the"},{"line_number":19,"context_line":"condition is satsfied, we generate wwid by using iscsi_id from the"},{"line_number":20,"context_line":"mpath."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I94b82cc051ce81cfa0e17911eb51a98fc4a7017b"},{"line_number":23,"context_line":"Closes-Bug: #1815844"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_e7ea42d6","line":20,"in_reply_to":"5fc1f717_a120aaad","updated":"2019-03-13 15:12:38.000000000","message":"Hello Gorka, Thanks a lot for your inputs on this patch.\n\nI sent a email to the mailing list [0] with reproducing steps (really easy to reproduce), also you can find the bug reported here [1] which basically looks to really follow what the behavior is when find_multipath is set to yes.\n\nAn other important point. During my tests I noticed that to have the changes in option find_multipath well took into account I had to reboot the server. Restarting the service was not enough (You might know the reasons).\n\nBut basically the first time you attach the volume the process is working well since multipathd did not have in /etc/multipath/wwid registered the wwid, then the second time the issue occurs since the wiid is in the whitelist, multipathd automatically creates the mpath.\n\n[0] http://lists.openstack.org/pipermail/openstack-discuss/2019-March/003618.html\n[1] https://bugs.launchpad.net/charm-nova-compute/+bug/1815844","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a9307fc6a7ab659dac932cd23208768c1266e3ea","unresolved":false,"context_lines":[{"line_number":17,"context_line":"infinite loop until the timeout occurs. In this commit we ensure that"},{"line_number":18,"context_line":"even if we are not able to find wwid, we try to find mpath. If the"},{"line_number":19,"context_line":"condition is satsfied, we generate wwid by using iscsi_id from the"},{"line_number":20,"context_line":"mpath."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I94b82cc051ce81cfa0e17911eb51a98fc4a7017b"},{"line_number":23,"context_line":"Closes-Bug: #1815844"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5fc1f717_860a09cf","line":20,"in_reply_to":"5fc1f717_e7ea42d6","updated":"2019-03-19 11:07:25.000000000","message":"I read the bug report before reviewing the code, and it doesn\u0027t sound too reasonable.  udev rules should always get triggered, regardless of the multipath part, and symlinks should be added not only on the first time the device is added.\n\nIf you had to reboot the system for multipathd to use the options there are 3 possibilities:\n- There is a bug in the multipathd binary from you distro (it works fine with Fedora, Centos, and RHEL).\n- The service didn\u0027t actually restart\n- The service restarted, the config changes took effect, but you missed it (this has happened to me before with other services).\n\nI have updated the bug with an RCA that explains why this is happening.  In summary, it is a race condition between the detection and the udev triggering that happens on backends that have multiple designators on page 0x83 when the CPU usage is high.  It will happen with \"find_multipaths no\" as well.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"}],"os_brick/initiator/connectors/iscsi.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"d974c9c48595c7f254a3304e8be25ed94621957d","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":738,"context_line":"            if wwn is None and found:"},{"line_number":739,"context_line":"                # We want pass here only if wwn is None. In a"},{"line_number":740,"context_line":"                # situation where the dm path comes accross too fast,"},{"line_number":741,"context_line":"                # get_sysfs_wwn() will always returns \u0027\u0027 that\u0027s"},{"line_number":742,"context_line":"                # because the devices are not anymore registered in"},{"line_number":743,"context_line":"                # /dev/disk/by-id/."}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_ded124ce","line":740,"range":{"start_line":740,"start_character":52,"end_line":740,"end_character":59},"updated":"2019-02-25 17:18:35.000000000","message":"across\n\nAlthough, \"where the dm path comes across too fast\" is a little confusing to me. That makes it sound like there\u0027s some stream being parsed and we miss it. Is there a better way you can reword this to state the condition?","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":20634,"name":"Chris MacNaughton","email":"chris.macnaughton@canonical.com","username":"Chris.MacNaughton"},"change_message_id":"370f68af7a074f287ca62266c85129c4de9dd15c","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":738,"context_line":"            if wwn is None and found:"},{"line_number":739,"context_line":"                # We want pass here only if wwn is None. In a"},{"line_number":740,"context_line":"                # situation where the dm path comes accross too fast,"},{"line_number":741,"context_line":"                # get_sysfs_wwn() will always returns \u0027\u0027 that\u0027s"},{"line_number":742,"context_line":"                # because the devices are not anymore registered in"},{"line_number":743,"context_line":"                # /dev/disk/by-id/."}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_ca8b191c","line":740,"range":{"start_line":740,"start_character":52,"end_line":740,"end_character":59},"in_reply_to":"9fdfeff1_aa138cb7","updated":"2019-03-04 07:59:58.000000000","message":"I still think a better description is needed here as to what the problem looks like, and what we\u0027re doing about it.","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"da2aeeef9bf702a7a08183f6bbdd0eb97f155745","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":738,"context_line":"            if wwn is None and found:"},{"line_number":739,"context_line":"                # We want pass here only if wwn is None. In a"},{"line_number":740,"context_line":"                # situation where the dm path comes accross too fast,"},{"line_number":741,"context_line":"                # get_sysfs_wwn() will always returns \u0027\u0027 that\u0027s"},{"line_number":742,"context_line":"                # because the devices are not anymore registered in"},{"line_number":743,"context_line":"                # /dev/disk/by-id/."}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_aa138cb7","line":740,"range":{"start_line":740,"start_character":52,"end_line":740,"end_character":59},"in_reply_to":"9fdfeff1_ded124ce","updated":"2019-02-27 15:38:56.000000000","message":"Basically this patch is fixing the issue where multipathd is automatically finding multipaths. I don\u0027t know whether we really want to fix it but at least we should document somewhere that we don\u0027t support this option.\n\n    find_multipaths \"yes\"","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"d974c9c48595c7f254a3304e8be25ed94621957d","unresolved":false,"context_lines":[{"line_number":738,"context_line":"            if wwn is None and found:"},{"line_number":739,"context_line":"                # We want pass here only if wwn is None. In a"},{"line_number":740,"context_line":"                # situation where the dm path comes accross too fast,"},{"line_number":741,"context_line":"                # get_sysfs_wwn() will always returns \u0027\u0027 that\u0027s"},{"line_number":742,"context_line":"                # because the devices are not anymore registered in"},{"line_number":743,"context_line":"                # /dev/disk/by-id/."},{"line_number":744,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_3ee07097","line":741,"range":{"start_line":741,"start_character":46,"end_line":741,"end_character":53},"updated":"2019-02-25 17:18:35.000000000","message":"return","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"0f64fd6d7936ce2f0578e6c6399457cc4b13ab11","unresolved":false,"context_lines":[{"line_number":739,"context_line":"                # We want pass here only if wwn is None. In a"},{"line_number":740,"context_line":"                # situation where the dm path comes accross too fast,"},{"line_number":741,"context_line":"                # get_sysfs_wwn() will always returns \u0027\u0027 that\u0027s"},{"line_number":742,"context_line":"                # because the devices are not anymore registered in"},{"line_number":743,"context_line":"                # /dev/disk/by-id/."},{"line_number":744,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":745,"context_line":"            # We have the wwn but not a multipath"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_cf4399af","line":742,"range":{"start_line":742,"start_character":42,"end_line":742,"end_character":53},"updated":"2019-02-26 16:22:35.000000000","message":"no longer","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"d974c9c48595c7f254a3304e8be25ed94621957d","unresolved":false,"context_lines":[{"line_number":744,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":745,"context_line":"            # We have the wwn but not a multipath"},{"line_number":746,"context_line":"            if wwn is not None and not mpath:"},{"line_number":747,"context_line":"                # At this step wwn should be something else that None"},{"line_number":748,"context_line":"                # and we have found devices."},{"line_number":749,"context_line":"                mpath \u003d self._linuxscsi.find_sysfs_multipath_dm(found)"},{"line_number":750,"context_line":"                if wwn \u003d\u003d \u0027\u0027 and mpath:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_1ef56c57","line":747,"range":{"start_line":747,"start_character":60,"end_line":747,"end_character":64},"updated":"2019-02-25 17:18:35.000000000","message":"than","commit_id":"4bc4a792e221a1aa666d23932f6cd067fee87b62"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e7e70d9e83da0d807b312a65b17d18922e3afce","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_243fe8b4","line":740,"updated":"2019-03-13 14:28:39.000000000","message":"-1: If there is a bug, it is most likely in \u0027get_sysfs_wwn\u0027, so please fix it there.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"754727eea9cbd2d0344811b442229b262dbb43b2","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_fe681f88","line":740,"in_reply_to":"5fc1f717_05759dcc","updated":"2019-03-19 15:49:14.000000000","message":"Breaking the abstraction levels does not mean that the code is buggy or doesn\u0027t work.\n\nWe should have functions that do 1 thing and do it well, if possible they should be small, but we should not get too focused on that. In the case of the `get_sysfs_wwn` it should return the wwn, so if that method return \"\", it must mean that we don\u0027t know, or can\u0027t, get it, not that we have to try to find it some other way here.\n\nI saw your comment, but it\u0027s also incorrect, and I didn\u0027t want to keep piling negative comments, because it can be very demotivating and frustrating.  Using `get_dm_name` may or may NOT return the wwn. As the method says it returns the name, it doesn\u0027t say it returns the wwn, so you may get something like \"mpathb\" instead.\n\nThe iSCSI code is non-trivial, and takes into consideration many different multipathd configurations as well as possible iSCSI issues, so changes must be carefully thought so we don\u0027t introduce issues we are already aware of, so I\u0027m ok with you looking for an alternative that pleases everyone, after all this is your patch, and I will not submit my code changes in an alternative patch to compete with yours.\n\nMaybe you prefer to modify the `get_sysfs_wwn` to also accept a mpath device (that can be None), and to try to get the mpath here as soon as we have found some devices.  This is possible most efficient, but will require more changes to the code and a more careful review and thorough testing.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9ecb115b97be0e2b669b2e0ee788be569bd88da7","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_98c91827","line":740,"in_reply_to":"5fc1f717_243fe8b4","updated":"2019-03-18 08:59:32.000000000","message":"I\u0027m not sure it\u0027s where we should fix this issue. From what I understand the usage of that function is really about to find a wwn based on devices discovered by _connect_vol().","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"64a4e0d99b0ce4e23cc9e5ad85d600d14f315844","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_05759dcc","line":740,"in_reply_to":"5fc1f717_2fe8b87a","updated":"2019-03-19 13:58:37.000000000","message":"\u003e I have tested the code on a system where I manually forced the situation to happen. You mentioned that the bug was really easy to reproduce. I suggest you try the code or try to find a code path that will make it fail.\n\nYou can be sure I tested it. Really easy to follow both of the paths just set find_multipath on/off.\n\n\u003e Breaking our abstraction levels by bringing the algorithm to find the WWN to this level.\n\nWell I don\u0027t think I\u0027m breaking anything I imagine it\u0027s just a divergence in POV. I think we should have small functions which are doing one thing. And reflecting usage of them in a general algorithm, we want to avoid magic I guess. And what is the purpose of passing L754 if we know that we have a viable mpath.\n\nLet me a chance to think of an algorithm which pleas both of us.\n\n\u003e You are using iscsiadm, which is problematic, and the reason why I wrote the \"get_sysfs_wwn\" code in the first place.\n\nI guess you did not see that I commented line 751. About to use get_dm_name() instead of scsi_id.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"7e7a93e541f10b1ba4ef630c7efdedd897d835f2","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_39332a65","line":740,"in_reply_to":"5fc1f717_3681f9c4","updated":"2019-03-19 11:44:18.000000000","message":"Basically what you are suggesting is exactly what I do propose. But you want put all of that code in get_sysfs_wwn(). as indicated that does not look right to update this method since its signature seems clear.\n\nWe are suffering a specific condition in the general algorithm. We don\u0027t what hide such behavior by doing magic. I hope that makes sense.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5e9878eca06085e88344f586260b5e662b4f68be","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_ac9036ef","line":740,"in_reply_to":"5fc1f717_39332a65","updated":"2019-03-19 12:02:52.000000000","message":"Also that. If we update the way you want, the condition L754 will pass, and it\u0027s not what we want since in the fact we already have the mpath.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a9307fc6a7ab659dac932cd23208768c1266e3ea","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_3681f9c4","line":740,"in_reply_to":"5fc1f717_98c91827","updated":"2019-03-19 11:07:25.000000000","message":"After doing the RCA, I can confirm that the right place to fix this bug is in \"get_sysfs_wwn\".\n\nWe just need to add a check for the dm as well.  Replacing \"get_sysfs_wwn\" with this code fixes the issue:\n\n        wwid \u003d self.get_sysfs_wwid(device_names)\n        glob_str \u003d \u0027/dev/disk/by-id/scsi-\u0027\n        wwn_paths \u003d glob.glob(glob_str + \u0027*\u0027)\n        # If we don\u0027t have multiple designators on page 0x83\n        if wwid and glob_str + wwid in wwn_paths:\n            return wwid\n\n        dev_to_wwn \u003d {}\n        # If we have multiple designators follow the symlinks\n        for wwn_path in wwn_paths:\n            try:\n                if os.path.islink(wwn_path) and os.stat(wwn_path):\n                    path \u003d os.path.realpath(wwn_path)\n                    if path.startswith(\u0027/dev/\u0027):\n                        dev_name \u003d path[5:]\n                        wwn \u003d wwn_path[len(glob_str):]\n                        if dev_name in device_names:\n                            return wwn\n                        dev_to_wwn[dev_name] \u003d wwn\n            except OSError:\n                continue\n\n        mpath \u003d self.find_sysfs_multipath_dm(device_names)\n        return dev_to_wwn.get(mpath, \u0027\u0027)","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"968d0854b54168d83c335d4efa74c84c8568561d","unresolved":false,"context_lines":[{"line_number":737,"context_line":"            LOG.debug(\"Connections threads info: %s\", data)"},{"line_number":738,"context_line":"            # We have devices but we don\u0027t know the wwn yet"},{"line_number":739,"context_line":"            if not wwn and found:"},{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_2fe8b87a","line":740,"in_reply_to":"5fc1f717_ac9036ef","updated":"2019-03-19 12:38:12.000000000","message":"That is not correct, \"get_sysfs_wwn\" must look for the wwn without using iscsiadm, and we definitely shouldn\u0027t try to find the wwn in here.\n\nL754 passing is fine, because mpath will be found on L755, just like it did inside the \"get_sysfs_wwn\".\n\nI have tested the code on a system where I manually forced the situation to happen.  You mentioned that the bug was really easy to reproduce.  I suggest you try the code or try to find a code path that will make it fail.\n\nAlso, my code is not exactly what you are doing in this code, since you are:\n\n- Breaking our abstraction levels by bringing the algorithm to find the WWN to this level.\n- You are using iscsiadm, which is problematic, and the reason why I wrote the \"get_sysfs_wwn\" code in the first place.\n\nI am not saying you have to use my code, you can write something that is more efficient, or more readable, or whatever, but I\u0027m pretty sure that my code fixes the issue, is in the right place, and doesn\u0027t call iscsiadm.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"af55026114957932ee7bf8b3bf1d1f86abcc7c00","unresolved":false,"context_lines":[{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""},{"line_number":744,"context_line":"                              \"unable to get any correct wwn. This probably \""},{"line_number":745,"context_line":"                              \"means that multipathd is configured with \""},{"line_number":746,"context_line":"                              \"\u0027find_multipaths\u0027 set to \u0027yes\u0027. A mpath should \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_a3baf306","line":743,"updated":"2019-03-13 11:08:09.000000000","message":"This is a bit wordy - maybe just \"Failed to find WWN from found devices, attempting to proceed\"","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9ecb115b97be0e2b669b2e0ee788be569bd88da7","unresolved":false,"context_lines":[{"line_number":740,"context_line":"                wwn \u003d self._linuxscsi.get_sysfs_wwn(found)"},{"line_number":741,"context_line":"                if wwn \u003d\u003d \u0027\u0027:"},{"line_number":742,"context_line":"                    # Unable to find any correct wwn from devices found"},{"line_number":743,"context_line":"                    LOG.debug(\"Even with the devices found \u0027%s\u0027, we are \""},{"line_number":744,"context_line":"                              \"unable to get any correct wwn. This probably \""},{"line_number":745,"context_line":"                              \"means that multipathd is configured with \""},{"line_number":746,"context_line":"                              \"\u0027find_multipaths\u0027 set to \u0027yes\u0027. A mpath should \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_98f2786f","line":743,"in_reply_to":"5fc1f717_a3baf306","updated":"2019-03-18 08:59:32.000000000","message":"Thanks, I will rewrote that part.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9e7e70d9e83da0d807b312a65b17d18922e3afce","unresolved":false,"context_lines":[{"line_number":748,"context_line":"                              \"available.\", found)"},{"line_number":749,"context_line":"                    mpath \u003d self._linuxscsi.find_sysfs_multipath_dm(found)"},{"line_number":750,"context_line":"                    if mpath:"},{"line_number":751,"context_line":"                        wwn \u003d self._linuxscsi.get_scsi_wwn(\"/dev/\" + mpath)"},{"line_number":752,"context_line":"                    LOG.debug(\"mpath\u003d%s, wwn\u003d%s\", mpath, wwn)"},{"line_number":753,"context_line":"            # We have the wwn but not a multipath"},{"line_number":754,"context_line":"            if wwn and not mpath:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_44a234fe","line":751,"updated":"2019-03-13 14:28:39.000000000","message":"-1: Don\u0027t use `/lib/udev/scsi_id` to find the WWN, as we know this introduces lots of issues as soon as the network is not working as expected.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9ecb115b97be0e2b669b2e0ee788be569bd88da7","unresolved":false,"context_lines":[{"line_number":748,"context_line":"                              \"available.\", found)"},{"line_number":749,"context_line":"                    mpath \u003d self._linuxscsi.find_sysfs_multipath_dm(found)"},{"line_number":750,"context_line":"                    if mpath:"},{"line_number":751,"context_line":"                        wwn \u003d self._linuxscsi.get_scsi_wwn(\"/dev/\" + mpath)"},{"line_number":752,"context_line":"                    LOG.debug(\"mpath\u003d%s, wwn\u003d%s\", mpath, wwn)"},{"line_number":753,"context_line":"            # We have the wwn but not a multipath"},{"line_number":754,"context_line":"            if wwn and not mpath:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_38d824ed","line":751,"in_reply_to":"5fc1f717_44a234fe","updated":"2019-03-18 08:59:32.000000000","message":"Thanks I was not aware of the issues related to scsi_id. I could probably use \u0027linuxscsi.get_dm_name()\u0027. Sounds reasonable?","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"}],"os_brick/tests/initiator/connectors/test_iscsi.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"37d87b40b62ce700d121da2401abea4717f68733","unresolved":false,"context_lines":[{"line_number":1366,"context_line":"        result \u003d list(get_wwn_mock.call_args[0][0])"},{"line_number":1367,"context_line":"        result.sort()"},{"line_number":1368,"context_line":"        self.assertEqual([\u0027sda\u0027, \u0027sdb\u0027, \u0027sdc\u0027, \u0027sdd\u0027], result)"},{"line_number":1369,"context_line":"        self.assertEqual(0, add_wwid_mock.call_count)"},{"line_number":1370,"context_line":"        self.assertEqual(0, add_path_mock.call_count)"},{"line_number":1371,"context_line":"        self.assertEqual(2, find_dm_mock.call_count)"},{"line_number":1372,"context_line":"        self.assertEqual(4, connect_mock.call_count)"},{"line_number":1373,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_b0516011","line":1370,"range":{"start_line":1369,"start_character":0,"end_line":1370,"end_character":53},"updated":"2019-03-06 13:52:59.000000000","message":"This is ensuring that no need to call multipath to create mpath. It will be generated automatically by multipathd.","commit_id":"65fa4b56ae36c6078b57320101a1b16ce7e99c9b"}]}