)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2e80b437d2d68e12df8d41ab14f5cc4bbf09a0ec","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the some of the libvirt volume drivers to using"},{"line_number":11,"context_line":"the os-brick pypi library.  os-brick is now"},{"line_number":12,"context_line":"being used by Cinder officially, and has been"},{"line_number":13,"context_line":"incubated inside cinder for 3 OpenStack releases."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I400db60fcc29c2d5e2d3b9dabc055649138468eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a016987_803b009c","line":13,"updated":"2015-05-18 10:16:02.000000000","message":"I think we\u0027re going to need to have a blueprint and spec for such a major architectural change in the code.","commit_id":"e6a728cfd35d3ee299b0322f90ea3df926b73d02"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"c0a4019c0204f6237a6a1324327ddef84baac71f","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the some of the libvirt volume drivers to using"},{"line_number":11,"context_line":"the os-brick pypi library.  os-brick is now"},{"line_number":12,"context_line":"being used by Cinder officially, and has been"},{"line_number":13,"context_line":"incubated inside cinder for 3 OpenStack releases."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I400db60fcc29c2d5e2d3b9dabc055649138468eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a016987_4712a10a","line":13,"in_reply_to":"7a016987_803b009c","updated":"2015-05-20 22:46:25.000000000","message":"yah I just haven\u0027t gotten to it yet.   I\u0027ll do that next.  Thanks for reminding me Daniel.","commit_id":"e6a728cfd35d3ee299b0322f90ea3df926b73d02"},{"author":{"_account_id":1849,"name":"Joe Gordon","email":"joe.gordon0@gmail.com","username":"jogo"},"change_message_id":"7563c27adbca239721afa7eb9c0c449d60f3e7f6","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Walter A. Boring IV \u003cwalter.boring@hp.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-12 19:38:18 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP switch to using os-brick"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a Work in progress patch for switching"},{"line_number":10,"context_line":"the some of the libvirt volume drivers to using"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"fa32b979_e49012ef","line":7,"updated":"2015-06-15 07:11:54.000000000","message":"how far are we away from dropping the \u0027WIP\u0027 here?  I am looking forward to reviewing this once it is ready.","commit_id":"094b88de0de4f9e4b3a5277f7b7f4490fda5dd9e"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"203ffc0ad82c5d2327f99a6a2638c55a5efc4e1c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Walter A. Boring IV \u003cwalter.boring@hp.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-12 19:38:18 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP switch to using os-brick"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is a Work in progress patch for switching"},{"line_number":10,"context_line":"the some of the libvirt volume drivers to using"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"fa32b979_6b53ea6b","line":7,"in_reply_to":"fa32b979_e49012ef","updated":"2015-06-17 17:43:31.000000000","message":"Hi Joe,\n   I\u0027ve been out for a few days, I\u0027m going to work on getting this out of WIP asap","commit_id":"094b88de0de4f9e4b3a5277f7b7f4490fda5dd9e"},{"author":{"_account_id":1849,"name":"Joe Gordon","email":"joe.gordon0@gmail.com","username":"jogo"},"change_message_id":"cc7e52e828eccd6a6550455f79cabab0cc2f32fb","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the os-brick pypi library.  os-brick is now"},{"line_number":12,"context_line":"being used by Cinder officially, and has been"},{"line_number":13,"context_line":"incubated inside cinder for 3 OpenStack releases."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I400db60fcc29c2d5e2d3b9dabc055649138468eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"fa32b979_8463461a","line":14,"updated":"2015-06-15 07:12:38.000000000","message":"This should reference the blueprint\n\nhttps://blueprints.launchpad.net/nova/+spec/use-os-brick-library","commit_id":"094b88de0de4f9e4b3a5277f7b7f4490fda5dd9e"},{"author":{"_account_id":15903,"name":"Earle F. Philhower, III","email":"earle.philhower.iii@hgst.com","username":"earlephilhower"},"change_message_id":"638c591132af36a405d3b8e442bd517940278da5","unresolved":false,"context_lines":[{"line_number":24,"context_line":"libvirt volume drivers."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"This patch also fetches the initiator side information"},{"line_number":27,"context_line":"from os-bick.  This replaces the internals of the"},{"line_number":28,"context_line":"libvirt driver\u0027s get_volume_connector"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Also updated the rootwrap filters to consolidate them under"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"fa32b979_8a38baef","line":27,"updated":"2015-06-23 21:27:20.000000000","message":"s/os-bick/os-brick/","commit_id":"5d3e8f6b1f076c3ccfd8a3659cca8fa158db0f48"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"bce2bf236921d7361065cf949f99d7c21aac2ce5","unresolved":false,"context_lines":[{"line_number":24,"context_line":"libvirt volume drivers."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"This patch also fetches the initiator side information"},{"line_number":27,"context_line":"from os-bick.  This replaces the internals of the"},{"line_number":28,"context_line":"libvirt driver\u0027s get_volume_connector"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Also updated the rootwrap filters to consolidate them under"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"fa32b979_fd9cb26b","line":27,"in_reply_to":"fa32b979_8a38baef","updated":"2015-06-23 21:39:38.000000000","message":"Done","commit_id":"5d3e8f6b1f076c3ccfd8a3659cca8fa158db0f48"}],"etc/nova/rootwrap.d/compute.filters":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":204,"context_line":"read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi"},{"line_number":205,"context_line":"multipath: CommandFilter, multipath, root"},{"line_number":206,"context_line":"# multipathd show status"},{"line_number":207,"context_line":"multipathd: CommandFilter, multipathd, root"},{"line_number":208,"context_line":"systool: CommandFilter, systool, root"},{"line_number":209,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":210,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":19,"id":"fa32b979_1d4c4f75","line":207,"updated":"2015-06-27 01:58:47.000000000","message":"ack: http://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/initiator/connector.py#n59","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":204,"context_line":"read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi"},{"line_number":205,"context_line":"multipath: CommandFilter, multipath, root"},{"line_number":206,"context_line":"# multipathd show status"},{"line_number":207,"context_line":"multipathd: CommandFilter, multipathd, root"},{"line_number":208,"context_line":"systool: CommandFilter, systool, root"},{"line_number":209,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":210,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":19,"id":"9a41bdd9_be5d037b","line":207,"in_reply_to":"ba3cc151_6d8ffe23","updated":"2015-07-13 19:19:31.000000000","message":"More or less just a note to myself to acknowledge this is a thing that os-brick needs, which is what the comment on 203 says.  We probably don\u0027t need to explicitly list it in here since that could change in os-brick over time based on different versions used.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":204,"context_line":"read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi"},{"line_number":205,"context_line":"multipath: CommandFilter, multipath, root"},{"line_number":206,"context_line":"# multipathd show status"},{"line_number":207,"context_line":"multipathd: CommandFilter, multipathd, root"},{"line_number":208,"context_line":"systool: CommandFilter, systool, root"},{"line_number":209,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":210,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":19,"id":"ba3cc151_6d8ffe23","line":207,"in_reply_to":"fa32b979_1d4c4f75","updated":"2015-06-30 16:08:13.000000000","message":"?  \n\nDo you want me to paste that line in the comment ?  I\u0027m not sure what you mean by \"ack: ....\"","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":204,"context_line":"read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi"},{"line_number":205,"context_line":"multipath: CommandFilter, multipath, root"},{"line_number":206,"context_line":"# multipathd show status"},{"line_number":207,"context_line":"multipathd: CommandFilter, multipathd, root"},{"line_number":208,"context_line":"systool: CommandFilter, systool, root"},{"line_number":209,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":210,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":24,"id":"9a41bdd9_3278478f","line":207,"updated":"2015-07-13 19:53:31.000000000","message":"Given:\n\nhttp://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/initiator/connector.py?id\u003d0.2.0#n57\n\nCan this just be specifically:\n\nmultipathd: CommandFilter, multipathd, root, multipathd, show, status\n\n?\n\nOr is that too brittle?","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":204,"context_line":"read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi"},{"line_number":205,"context_line":"multipath: CommandFilter, multipath, root"},{"line_number":206,"context_line":"# multipathd show status"},{"line_number":207,"context_line":"multipathd: CommandFilter, multipathd, root"},{"line_number":208,"context_line":"systool: CommandFilter, systool, root"},{"line_number":209,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":210,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":24,"id":"9a41bdd9_d5fea56a","line":207,"in_reply_to":"9a41bdd9_3278478f","updated":"2015-07-13 21:39:11.000000000","message":"I\u0027m concerned that could change, and I don\u0027t want to break nova in the case that check changes. \n\nThis touches on the idea of how we can possibly get nova to pull in library needed filters automatically eventually ?  \n\nI can easily see new vendor specific connectors being added to brick, which might add/change the needed filters.   future feature/idea I guess.  For the time being, I\u0027d feel safer leaving the show status filter as is.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a20c988fc11fe4398e1ad90463499bc547d8497a","unresolved":false,"context_lines":[{"line_number":204,"context_line":"read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi"},{"line_number":205,"context_line":"multipath: CommandFilter, multipath, root"},{"line_number":206,"context_line":"# multipathd show status"},{"line_number":207,"context_line":"multipathd: CommandFilter, multipathd, root"},{"line_number":208,"context_line":"systool: CommandFilter, systool, root"},{"line_number":209,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":210,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":24,"id":"9a41bdd9_6263e642","line":207,"in_reply_to":"9a41bdd9_d5fea56a","updated":"2015-07-15 19:43:56.000000000","message":"Sure, agreed.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"}],"nova/tests/unit/virt/libvirt/fake_libvirt_utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"529793e4ac5d4e4f4094e30d6a39787c0106b481","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    return \"fake.initiator.iqn\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def get_fc_hbas(self):"},{"line_number":33,"context_line":"    \"\"\"self is to mimick the os-brick object method.\"\"\""},{"line_number":34,"context_line":"    return [{\u0027ClassDevice\u0027: \u0027host1\u0027,"},{"line_number":35,"context_line":"             \u0027ClassDevicePath\u0027: \u0027/sys/devices/pci0000:00/0000:00:03.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_0fa72d0c","line":32,"updated":"2015-06-27 13:20:23.000000000","message":"Are these even used in tests anymore?  They probably shouldn\u0027t be b/c I\u0027d think that\u0027s then mocking out the internals of os-brick rather than just mocking what os-brick\u0027s initiator interface is returning.\n\nSeems this should be a fixture in os-brick now and then the nova unit tests could just use that fixture.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    return \"fake.initiator.iqn\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def get_fc_hbas(self):"},{"line_number":33,"context_line":"    \"\"\"self is to mimick the os-brick object method.\"\"\""},{"line_number":34,"context_line":"    return [{\u0027ClassDevice\u0027: \u0027host1\u0027,"},{"line_number":35,"context_line":"             \u0027ClassDevicePath\u0027: \u0027/sys/devices/pci0000:00/0000:00:03.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_3ef81349","line":32,"in_reply_to":"ba3cc151_ed89cecb","updated":"2015-07-13 19:19:31.000000000","message":"If we\u0027re replacing code that\u0027s now in os-brick and it\u0027s tested there, then we should remove stale testing from nova since like I said, this is just mocking the internals of os-brick for nova unit tests, which isn\u0027t good - the only things we should be mocking out are the interfaces to os-brick that nova is talking to.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    return \"fake.initiator.iqn\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def get_fc_hbas(self):"},{"line_number":33,"context_line":"    \"\"\"self is to mimick the os-brick object method.\"\"\""},{"line_number":34,"context_line":"    return [{\u0027ClassDevice\u0027: \u0027host1\u0027,"},{"line_number":35,"context_line":"             \u0027ClassDevicePath\u0027: \u0027/sys/devices/pci0000:00/0000:00:03.0\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_ed89cecb","line":32,"in_reply_to":"fa32b979_0fa72d0c","updated":"2015-06-30 16:08:13.000000000","message":"I think the only place this is used is in the test_driver.py code.  The purpose of that is to test the fetching of the initiator connector, which is tested in os-brick now.   I\u0027d be ok with removing this completely.\n\nthoughts?","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":24,"context_line":"disk_type \u003d \"qcow2\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_iscsi_initiator(self):"},{"line_number":28,"context_line":"    \"\"\"self is to mimick the os-brick object method.\"\"\""},{"line_number":29,"context_line":"    return \"fake.initiator.iqn\""},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_f27f3f64","line":27,"updated":"2015-07-13 19:53:31.000000000","message":"As noted in patch set 19, if this is just being used as a monkey patch fixture for nova unit tests which are now mocking out the *internals* of os-brick, and those internals are tested in os-brick itself, which you said they were, then let\u0027s just remove this code because nova shouldn\u0027t be mocking out the internals of a library, only the interfaces, otherwise we\u0027ll end up screwing ourselves in the gate with changes to the internals of os-brick which nova unit tests aren\u0027t able to handle - unless of course we want to gate os-brick changes on gate-nova-python27, but we probably don\u0027t.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":24,"context_line":"disk_type \u003d \"qcow2\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def get_iscsi_initiator(self):"},{"line_number":28,"context_line":"    \"\"\"self is to mimick the os-brick object method.\"\"\""},{"line_number":29,"context_line":"    return \"fake.initiator.iqn\""},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_88dec46b","line":27,"in_reply_to":"9a41bdd9_f27f3f64","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"}],"nova/tests/unit/virt/libvirt/fake_os_brick_connector.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"396e61f55c1a98a1db6e0a2dcb6dcc71fb7020ce","unresolved":false,"context_lines":[{"line_number":15,"context_line":"                             host\u003dNone):"},{"line_number":16,"context_line":"    \"\"\"Fake os-brick.\"\"\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    props \u003d {}"},{"line_number":19,"context_line":"    props[\u0027ip\u0027] \u003d my_ip"},{"line_number":20,"context_line":"    props[\u0027host\u0027] \u003d host"},{"line_number":21,"context_line":"    iscsi \u003d ISCSIConnector(\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a41bdd9_58e31dd9","line":18,"updated":"2015-07-16 06:22:27.000000000","message":"why not:\n\n    return {\u0027ip\u0027: my_ip,\n            \u0027host\u0027: host,\n            \u0027initiator\u0027: ISCSIConnector(\u0027\u0027).get_initiator(),\n            \u0027wwpns\u0027: \u0027100010604b019419\u0027],\n            \u0027wwnns\u0027: [\u0027200010604b019419\u0027],\n            \u0027multipath\u0027: multipath,\n            \u0027platform\u0027: \u0027x86_64\u0027,\n            \u0027os_type\u0027: \u0027linux2\u0027}","commit_id":"b2212bd7a864ae0e84d089b92a9fb955d75f36b9"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":526,"context_line":"            \u0027nova.virt.libvirt.imagebackend.libvirt_utils\u0027,"},{"line_number":527,"context_line":"            fake_libvirt_utils))"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Stub out the os_brick initiator code"},{"line_number":530,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":531,"context_line":"            \u0027os_brick.initiator.connector.ISCSIConnector.get_initiator\u0027,"},{"line_number":532,"context_line":"            fake_libvirt_utils.get_iscsi_initiator))"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_9ddcdf75","line":529,"updated":"2015-06-27 01:58:47.000000000","message":"Could we throw these into a separate fixture like we have for the FakeLibvirtFixture so it\u0027s all self-contained in fake_libvirt_utils?\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/tests/unit/virt/libvirt/fakelibvirt.py#n1298","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":526,"context_line":"            \u0027nova.virt.libvirt.imagebackend.libvirt_utils\u0027,"},{"line_number":527,"context_line":"            fake_libvirt_utils))"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Stub out the os_brick initiator code"},{"line_number":530,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":531,"context_line":"            \u0027os_brick.initiator.connector.ISCSIConnector.get_initiator\u0027,"},{"line_number":532,"context_line":"            fake_libvirt_utils.get_iscsi_initiator))"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_fe986b1d","line":529,"in_reply_to":"ba3cc151_0dd992f3","updated":"2015-07-13 19:19:31.000000000","message":"OK, agreed, let\u0027s remove.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":526,"context_line":"            \u0027nova.virt.libvirt.imagebackend.libvirt_utils\u0027,"},{"line_number":527,"context_line":"            fake_libvirt_utils))"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Stub out the os_brick initiator code"},{"line_number":530,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":531,"context_line":"            \u0027os_brick.initiator.connector.ISCSIConnector.get_initiator\u0027,"},{"line_number":532,"context_line":"            fake_libvirt_utils.get_iscsi_initiator))"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_0dd992f3","line":529,"in_reply_to":"fa32b979_9ddcdf75","updated":"2015-06-30 16:08:13.000000000","message":"I can simply remove these tests associated with testing fetching the initiator connector all together.   The os-brick code does this stuff now.\n\nhttps://github.com/openstack/os-brick/blob/master/os_brick/tests/initiator/test_connector.py#L43","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":526,"context_line":"            \u0027nova.virt.libvirt.imagebackend.libvirt_utils\u0027,"},{"line_number":527,"context_line":"            fake_libvirt_utils))"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Stub out the os_brick initiator code"},{"line_number":530,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":531,"context_line":"            \u0027os_brick.initiator.connector.ISCSIConnector.get_initiator\u0027,"},{"line_number":532,"context_line":"            fake_libvirt_utils.get_iscsi_initiator))"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_b221b7b6","line":529,"updated":"2015-07-13 19:53:31.000000000","message":"I thought we talked about dropping this in patch set 19?","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":526,"context_line":"            \u0027nova.virt.libvirt.imagebackend.libvirt_utils\u0027,"},{"line_number":527,"context_line":"            fake_libvirt_utils))"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Stub out the os_brick initiator code"},{"line_number":530,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":531,"context_line":"            \u0027os_brick.initiator.connector.ISCSIConnector.get_initiator\u0027,"},{"line_number":532,"context_line":"            fake_libvirt_utils.get_iscsi_initiator))"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_a808682e","line":529,"in_reply_to":"9a41bdd9_b221b7b6","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":905,"context_line":"            \u0027id\u0027: \u0027fake\u0027"},{"line_number":906,"context_line":"        }"},{"line_number":907,"context_line":"        result \u003d drvr.get_volume_connector(volume)"},{"line_number":908,"context_line":"        print(result)"},{"line_number":909,"context_line":"        self.assertThat(expected, matchers.DictMatches(result))"},{"line_number":910,"context_line":""},{"line_number":911,"context_line":"    def test_get_connector_storage_ip(self):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_a847081c","line":908,"updated":"2015-07-13 21:39:11.000000000","message":"need to remove this.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a1e0df05334399209d6d5b93888d4549fd7bc72","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        self.assertRaises(exception.PciDeviceDetachFailed,"},{"line_number":865,"context_line":"                          drvr._detach_pci_devices, guest, pci_devices)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"    def test_get_connector(self):"},{"line_number":868,"context_line":"        initiator \u003d \u0027fake.initiator.iqn\u0027"},{"line_number":869,"context_line":"        ip \u003d \u0027fakeip\u0027"},{"line_number":870,"context_line":"        host \u003d \u0027fakehost\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a41bdd9_8d6bbb84","side":"PARENT","line":867,"updated":"2015-07-15 19:48:34.000000000","message":"We should leave this test in but mock out the calls to os-brick.","commit_id":"761b9a37e1e55e652a49546f57b5412e4eb07e31"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"1ff6268d607c758741a9ecd675d71c63095363ae","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        self.assertRaises(exception.PciDeviceDetachFailed,"},{"line_number":865,"context_line":"                          drvr._detach_pci_devices, guest, pci_devices)"},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"    def test_get_connector(self):"},{"line_number":868,"context_line":"        initiator \u003d \u0027fake.initiator.iqn\u0027"},{"line_number":869,"context_line":"        ip \u003d \u0027fakeip\u0027"},{"line_number":870,"context_line":"        host \u003d \u0027fakehost\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a41bdd9_a8b8ed11","side":"PARENT","line":867,"in_reply_to":"9a41bdd9_8d6bbb84","updated":"2015-07-15 20:15:01.000000000","message":"Done","commit_id":"761b9a37e1e55e652a49546f57b5412e4eb07e31"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"396e61f55c1a98a1db6e0a2dcb6dcc71fb7020ce","unresolved":false,"context_lines":[{"line_number":106,"context_line":"CONF.import_opt(\u0027my_ip\u0027, \u0027nova.netconf\u0027)"},{"line_number":107,"context_line":"CONF.import_opt(\u0027image_cache_subdirectory_name\u0027, \u0027nova.virt.imagecache\u0027)"},{"line_number":108,"context_line":"CONF.import_opt(\u0027instances_path\u0027, \u0027nova.compute.manager\u0027)"},{"line_number":109,"context_line":"CONF.import_opt(\u0027iscsi_use_multipath\u0027, \u0027nova.virt.libvirt.volume\u0027, \u0027libvirt\u0027)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"_fake_network_info \u003d fake_network.fake_get_instance_nw_info"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a41bdd9_f8fc7173","line":109,"updated":"2015-07-16 06:22:27.000000000","message":"why do we need this?","commit_id":"b2212bd7a864ae0e84d089b92a9fb955d75f36b9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fdfcc8dfd25997ac1b4aaedd36aae2f83da2060b","unresolved":false,"context_lines":[{"line_number":106,"context_line":"CONF.import_opt(\u0027my_ip\u0027, \u0027nova.netconf\u0027)"},{"line_number":107,"context_line":"CONF.import_opt(\u0027image_cache_subdirectory_name\u0027, \u0027nova.virt.imagecache\u0027)"},{"line_number":108,"context_line":"CONF.import_opt(\u0027instances_path\u0027, \u0027nova.compute.manager\u0027)"},{"line_number":109,"context_line":"CONF.import_opt(\u0027iscsi_use_multipath\u0027, \u0027nova.virt.libvirt.volume\u0027, \u0027libvirt\u0027)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"_fake_network_info \u003d fake_network.fake_get_instance_nw_info"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_1e236557","line":109,"updated":"2015-07-29 00:17:59.000000000","message":"This isn\u0027t used.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"}],"nova/tests/unit/virt/libvirt/test_volume.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"869a0f00f4d31958635012464c8c7606f9c5f827","unresolved":false,"context_lines":[{"line_number":328,"context_line":"%s %s"},{"line_number":329,"context_line":"\"\"\" % (targets[0][0], targets[0][1], targets[1][0], targets[1][1])"},{"line_number":330,"context_line":"        driver \u003d volume.LibvirtISCSIVolumeDriver(\"none\")"},{"line_number":331,"context_line":"        out \u003d driver.connector._get_target_portals_from_iscsiadm_output("},{"line_number":332,"context_line":"            sample_input)"},{"line_number":333,"context_line":"        self.assertEqual(out, targets)"},{"line_number":334,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_4f8a151c","line":331,"updated":"2015-06-27 13:54:37.000000000","message":"If we need to do this, shouldn\u0027t the test live in os-brick instead?","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":328,"context_line":"%s %s"},{"line_number":329,"context_line":"\"\"\" % (targets[0][0], targets[0][1], targets[1][0], targets[1][1])"},{"line_number":330,"context_line":"        driver \u003d volume.LibvirtISCSIVolumeDriver(\"none\")"},{"line_number":331,"context_line":"        out \u003d driver.connector._get_target_portals_from_iscsiadm_output("},{"line_number":332,"context_line":"            sample_input)"},{"line_number":333,"context_line":"        self.assertEqual(out, targets)"},{"line_number":334,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_3ea6d353","line":331,"in_reply_to":"ba3cc151_7f632f1b","updated":"2015-07-13 19:19:31.000000000","message":"As noted in other places in this review, I prefer that we only mock calls to os-brick.  If we\u0027re mocking out anything that\u0027s internal to os-brick, then I think the test is doing too much and it\u0027ll be brittle and subject to failures when there are changes to the internals of os-brick that are backwards incompatible with what nova unit tests are testing, which will then break nova in the gate when those new releases of os-brick come out (we\u0027ve had this problem with other libraries in oslo).","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":328,"context_line":"%s %s"},{"line_number":329,"context_line":"\"\"\" % (targets[0][0], targets[0][1], targets[1][0], targets[1][1])"},{"line_number":330,"context_line":"        driver \u003d volume.LibvirtISCSIVolumeDriver(\"none\")"},{"line_number":331,"context_line":"        out \u003d driver.connector._get_target_portals_from_iscsiadm_output("},{"line_number":332,"context_line":"            sample_input)"},{"line_number":333,"context_line":"        self.assertEqual(out, targets)"},{"line_number":334,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_7f632f1b","line":331,"in_reply_to":"fa32b979_4f8a151c","updated":"2015-06-30 16:08:13.000000000","message":"The tests do live in os-brick.   I was having a hard time deciding what we should actually test in nova.   I can remove more of these tests if you like.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"869a0f00f4d31958635012464c8c7606f9c5f827","unresolved":false,"context_lines":[{"line_number":348,"context_line":"            return (None, None)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        libvirt_driver \u003d volume.LibvirtISCSIVolumeDriver(self.fake_conn)"},{"line_number":351,"context_line":"        libvirt_driver.connector.set_execute(fake_execute)"},{"line_number":352,"context_line":"        connection_info \u003d self.iscsi_connection(self.vol, self.location,"},{"line_number":353,"context_line":"                                                self.iqn)"},{"line_number":354,"context_line":"        iscsi_properties \u003d connection_info[\u0027data\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_0f840d09","line":351,"updated":"2015-06-27 13:54:37.000000000","message":"Seems this test should live in os-brick, especially if we\u0027re mocking out connector.LOG.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"869a0f00f4d31958635012464c8c7606f9c5f827","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        }"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"    @mock.patch(\u0027os.path.exists\u0027, return_value\u003dTrue)"},{"line_number":661,"context_line":"    def test_libvirt_aoe_driver(self, exists):"},{"line_number":662,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        # Mock out the os-brick internal execute"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_2f8eb127","line":661,"updated":"2015-06-27 13:54:37.000000000","message":"This test is basically already in os-brick:\n\nhttp://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/tests/initiator/test_connector.py#n1038\n\nIn nova we should just test that we are calling the proper connector and mock out the calls to os-brick.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        }"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"    @mock.patch(\u0027os.path.exists\u0027, return_value\u003dTrue)"},{"line_number":661,"context_line":"    def test_libvirt_aoe_driver(self, exists):"},{"line_number":662,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        # Mock out the os-brick internal execute"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_3f44c772","line":661,"in_reply_to":"fa32b979_2f8eb127","updated":"2015-06-30 16:08:13.000000000","message":"Done","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"869a0f00f4d31958635012464c8c7606f9c5f827","unresolved":false,"context_lines":[{"line_number":679,"context_line":"    def test_libvirt_aoe_driver_get_config(self):"},{"line_number":680,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"        # Mock out the os-brick internal execute"},{"line_number":683,"context_line":"        mock_execute \u003d mock.Mock()"},{"line_number":684,"context_line":"        mock_execute.return_value \u003d (\u0027out\u0027, None)"},{"line_number":685,"context_line":"        libvirt_driver.connector._execute \u003d mock_execute"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_4f91b545","line":682,"updated":"2015-06-27 13:54:37.000000000","message":"ditto same as above","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"869a0f00f4d31958635012464c8c7606f9c5f827","unresolved":false,"context_lines":[{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390)"},{"line_number":847,"context_line":"    def test_libvirt_fibrechan_driver_s390(self, mock_machine):"},{"line_number":848,"context_line":"            self._test_libvirt_fibrechan_driver_s390()"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390X)"},{"line_number":851,"context_line":"    def test_libvirt_fibrechan_driver_s390x(self, mock_machine):"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_ef976938","line":848,"updated":"2015-06-27 13:54:37.000000000","message":"Unnecessary indent here.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390)"},{"line_number":847,"context_line":"    def test_libvirt_fibrechan_driver_s390(self, mock_machine):"},{"line_number":848,"context_line":"            self._test_libvirt_fibrechan_driver_s390()"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390X)"},{"line_number":851,"context_line":"    def test_libvirt_fibrechan_driver_s390x(self, mock_machine):"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_9f6873f7","line":848,"in_reply_to":"fa32b979_ef976938","updated":"2015-06-30 16:08:13.000000000","message":"Done","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":661,"context_line":"    def test_libvirt_aoe_driver(self, exists):"},{"line_number":662,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        # Mock out the os-brick internal execute"},{"line_number":665,"context_line":"        mock_execute \u003d mock.Mock()"},{"line_number":666,"context_line":"        mock_execute.return_value \u003d (\u0027out\u0027, None)"},{"line_number":667,"context_line":"        libvirt_driver.connector._execute \u003d mock_execute"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_352a717f","line":664,"updated":"2015-07-13 19:53:31.000000000","message":"eww why?  Can\u0027t we just mock out os-brick altogether?  If this test is only testing internals of os-brick now, and this is already tested in os-brick, let\u0027s just remove it.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":661,"context_line":"    def test_libvirt_aoe_driver(self, exists):"},{"line_number":662,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"        # Mock out the os-brick internal execute"},{"line_number":665,"context_line":"        mock_execute \u003d mock.Mock()"},{"line_number":666,"context_line":"        mock_execute.return_value \u003d (\u0027out\u0027, None)"},{"line_number":667,"context_line":"        libvirt_driver.connector._execute \u003d mock_execute"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_6b025a0d","line":664,"in_reply_to":"9a41bdd9_352a717f","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":679,"context_line":"    def test_libvirt_aoe_driver_get_config(self):"},{"line_number":680,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"        # Mock out the os-brick internal execute"},{"line_number":683,"context_line":"        mock_execute \u003d mock.Mock()"},{"line_number":684,"context_line":"        mock_execute.return_value \u003d (\u0027out\u0027, None)"},{"line_number":685,"context_line":"        libvirt_driver.connector._execute \u003d mock_execute"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_f523a9a5","line":682,"updated":"2015-07-13 19:53:31.000000000","message":"Same as above.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":679,"context_line":"    def test_libvirt_aoe_driver_get_config(self):"},{"line_number":680,"context_line":"        libvirt_driver \u003d volume.LibvirtAOEVolumeDriver(self.fake_conn)"},{"line_number":681,"context_line":""},{"line_number":682,"context_line":"        # Mock out the os-brick internal execute"},{"line_number":683,"context_line":"        mock_execute \u003d mock.Mock()"},{"line_number":684,"context_line":"        mock_execute.return_value \u003d (\u0027out\u0027, None)"},{"line_number":685,"context_line":"        libvirt_driver.connector._execute \u003d mock_execute"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_06006500","line":682,"in_reply_to":"9a41bdd9_f523a9a5","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390)"},{"line_number":847,"context_line":"    def test_libvirt_fibrechan_driver_s390(self, mock_machine):"},{"line_number":848,"context_line":"            self._test_libvirt_fibrechan_driver_s390()"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390X)"},{"line_number":851,"context_line":"    def test_libvirt_fibrechan_driver_s390x(self, mock_machine):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_7530f9e8","line":848,"updated":"2015-07-13 19:53:31.000000000","message":"Unnecessary indent.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390)"},{"line_number":847,"context_line":"    def test_libvirt_fibrechan_driver_s390(self, mock_machine):"},{"line_number":848,"context_line":"            self._test_libvirt_fibrechan_driver_s390()"},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"    @mock.patch.object(platform, \u0027machine\u0027, return_value\u003darch.S390X)"},{"line_number":851,"context_line":"    def test_libvirt_fibrechan_driver_s390x(self, mock_machine):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_66a9317b","line":848,"in_reply_to":"9a41bdd9_7530f9e8","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d3ba390e06b20d75e25e395f944d19271765ef81","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            ret[\u0027data\u0027][\u0027auth_username\u0027] \u003d \u0027foo\u0027"},{"line_number":278,"context_line":"            ret[\u0027data\u0027][\u0027auth_password\u0027] \u003d \u0027bar\u0027"},{"line_number":279,"context_line":"        return ret"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"    def iscsi_connection_discovery_chap_enable(self, volume, location, iqn):"},{"line_number":282,"context_line":"        dev_name \u003d \u0027ip-%s-iscsi-%s-lun-1\u0027 % (location, iqn)"},{"line_number":283,"context_line":"        dev_path \u003d \u0027/dev/disk/by-path/%s\u0027 % (dev_name)"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_583fc586","line":280,"updated":"2015-07-28 23:48:43.000000000","message":"No longer used.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d3ba390e06b20d75e25e395f944d19271765ef81","unresolved":false,"context_lines":[{"line_number":299,"context_line":"                }"},{"line_number":300,"context_line":"        }"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def generate_device(self, transport\u003dNone, lun\u003d1, short\u003dFalse):"},{"line_number":303,"context_line":"        dev_format \u003d \"ip-%s-iscsi-%s-lun-%s\" % (self.location, self.iqn, lun)"},{"line_number":304,"context_line":"        if transport:"},{"line_number":305,"context_line":"            dev_format \u003d \"pci-0000:00:00.0-\" + dev_format"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_9839cda6","line":302,"updated":"2015-07-28 23:48:43.000000000","message":"This isn\u0027t used anymore.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fb64d251d010ff205d699f4b541a4f98d14d7820","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        fake_dev_path \u003d \"/dev/disk/by-path/\" + dev_format"},{"line_number":309,"context_line":"        return fake_dev_path"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"    def test_iscsiadm_discover_parsing(self):"},{"line_number":312,"context_line":"        # Ensure that parsing iscsiadm discover ignores cruft."},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        targets \u003d ["}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_536b26f1","line":311,"updated":"2015-07-29 00:02:26.000000000","message":"This should live in os-brick.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"696e70b9f18c33c0942eae9284e13275e7b65852","unresolved":false,"context_lines":[{"line_number":337,"context_line":"        self.assertIsInstance(libvirt_driver.connector,"},{"line_number":338,"context_line":"                              connector.ISCSIConnector)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    def test_sanitize_log_run_iscsiadm(self):"},{"line_number":341,"context_line":"        # Tests that the parameters to the os-brick connector\u0027s"},{"line_number":342,"context_line":"        # _run_iscsiadm function are sanitized for passwords when logged."},{"line_number":343,"context_line":"        def fake_debug(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_d3e5f637","line":340,"updated":"2015-07-28 23:52:55.000000000","message":"This test should be moved to os-brick.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"55a242bf1199a1c996a086bbaf75fed2fd9d4f3a","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            # our stub method is called which asserts the password is scrubbed"},{"line_number":362,"context_line":"            self.assertTrue(debug_mock.called)"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    def iser_connection(self, volume, location, iqn):"},{"line_number":365,"context_line":"        return {"},{"line_number":366,"context_line":"                \u0027driver_volume_type\u0027: \u0027iser\u0027,"},{"line_number":367,"context_line":"                \u0027data\u0027: {"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_383f59b5","line":364,"updated":"2015-07-28 23:04:33.000000000","message":"This is no longer used from what I can tell.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"}],"nova/tests/unit/virt/test_virt_drivers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":97,"context_line":"            \u0027nova.virt.libvirt.driver.libvirt\u0027,"},{"line_number":98,"context_line":"            fakelibvirt))"},{"line_number":99,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":100,"context_line":"            \u0027nova.virt.libvirt.driver.libvirt_utils\u0027,"},{"line_number":101,"context_line":"            fake_libvirt_utils))"},{"line_number":102,"context_line":"        self.useFixture(fixtures.MonkeyPatch("}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_5de65745","line":99,"updated":"2015-06-27 01:58:47.000000000","message":"This is similar to the monkey patches put in nova.tests.unit.virt.libvirt.test_driver, can we throw those into a fixture and just call that?  Then we don\u0027t need the mocks below.\n\nI had a similar thing here:\n\nhttps://review.openstack.org/#/c/193317/4/nova/tests/unit/virt/test_virt_drivers.py","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":97,"context_line":"            \u0027nova.virt.libvirt.driver.libvirt\u0027,"},{"line_number":98,"context_line":"            fakelibvirt))"},{"line_number":99,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":100,"context_line":"            \u0027nova.virt.libvirt.driver.libvirt_utils\u0027,"},{"line_number":101,"context_line":"            fake_libvirt_utils))"},{"line_number":102,"context_line":"        self.useFixture(fixtures.MonkeyPatch("}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_91ddb844","line":99,"in_reply_to":"fa32b979_5de65745","updated":"2015-06-30 16:08:13.000000000","message":"Done","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    @catch_notimplementederror"},{"line_number":439,"context_line":"    def test_get_volume_connector(self):"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"        result \u003d self.connection.get_volume_connector({\u0027id\u0027: \u0027fake\u0027})"},{"line_number":442,"context_line":"        self.assertIn(\u0027ip\u0027, result)"},{"line_number":443,"context_line":"        self.assertIn(\u0027initiator\u0027, result)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_b26e9794","line":440,"updated":"2015-07-13 19:53:31.000000000","message":"nit: unrelated whitespace change","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    @catch_notimplementederror"},{"line_number":439,"context_line":"    def test_get_volume_connector(self):"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"        result \u003d self.connection.get_volume_connector({\u0027id\u0027: \u0027fake\u0027})"},{"line_number":442,"context_line":"        self.assertIn(\u0027ip\u0027, result)"},{"line_number":443,"context_line":"        self.assertIn(\u0027initiator\u0027, result)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_e60f8186","line":440,"in_reply_to":"9a41bdd9_b26e9794","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":467,"context_line":"                                          \u0027/dev/sda\u0027))"},{"line_number":468,"context_line":"        self.assertIsNone("},{"line_number":469,"context_line":"            self.connection.detach_volume(connection_info, instance_ref,"},{"line_number":470,"context_line":"                                          \u0027/dev/sda\u0027))"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    @catch_notimplementederror"},{"line_number":473,"context_line":"    def test_swap_volume(self):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_320927de","line":470,"updated":"2015-07-13 19:53:31.000000000","message":"nit: probably unnecessary","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"396e61f55c1a98a1db6e0a2dcb6dcc71fb7020ce","unresolved":false,"context_lines":[{"line_number":466,"context_line":"                                          \u0027/dev/sda\u0027))"},{"line_number":467,"context_line":"        self.assertIsNone("},{"line_number":468,"context_line":"            self.connection.detach_volume(connection_info, instance_ref,"},{"line_number":469,"context_line":"                                          \u0027/dev/sda\u0027))"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    @catch_notimplementederror"},{"line_number":472,"context_line":"    def test_swap_volume(self):"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a41bdd9_382e59fb","line":469,"updated":"2015-07-16 06:22:27.000000000","message":"not really related","commit_id":"b2212bd7a864ae0e84d089b92a9fb955d75f36b9"}],"nova/tests/unit/virt/test_volumeutils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Tests fot virt volumeutils."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"from oslo_concurrency import processutils as putils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova import test"},{"line_number":23,"context_line":"from nova.virt import volumeutils"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_bd452316","line":20,"updated":"2015-06-27 01:58:47.000000000","message":"Why don\u0027t we just mock os-brick calls in here?  What you\u0027re doing below is essentially unit testing the os-brick code, we should be unit testing the nova code talking to os-brick.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Tests fot virt volumeutils."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"from oslo_concurrency import processutils as putils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova import test"},{"line_number":23,"context_line":"from nova.virt import volumeutils"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_1e82ef94","line":20,"in_reply_to":"ba3cc151_be15074a","updated":"2015-07-13 19:19:31.000000000","message":"Cool.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Tests fot virt volumeutils."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"from oslo_concurrency import processutils as putils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova import test"},{"line_number":23,"context_line":"from nova.virt import volumeutils"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_be15074a","line":20,"in_reply_to":"fa32b979_bd452316","updated":"2015-06-30 16:08:13.000000000","message":"with my change to the test_virt_drivers, which also does these same tests, I think we can just remove this file completely.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from nova.virt import volumeutils"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class VolumeUtilsTestCase(test.NoDBTestCase):"},{"line_number":27,"context_line":"    def test_get_iscsi_initiator(self):"},{"line_number":28,"context_line":"        self.mox.StubOutWithMock(utils, \u0027execute\u0027)"},{"line_number":29,"context_line":"        initiator \u003d \u0027fake.initiator.iqn\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_1232e310","side":"PARENT","line":26,"updated":"2015-07-13 19:53:31.000000000","message":"Not sure why this module is deleted since the actual code it was testing it still in tree, it\u0027s just that it\u0027s calling os-brick now - we should still have that code tested but mock out any calls to os-brick.","commit_id":"4bfb094a6fc5e92a2e8329e32be5e47f09cd84be"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from nova.virt import volumeutils"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class VolumeUtilsTestCase(test.NoDBTestCase):"},{"line_number":27,"context_line":"    def test_get_iscsi_initiator(self):"},{"line_number":28,"context_line":"        self.mox.StubOutWithMock(utils, \u0027execute\u0027)"},{"line_number":29,"context_line":"        initiator \u003d \u0027fake.initiator.iqn\u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_c17ca774","side":"PARENT","line":26,"in_reply_to":"9a41bdd9_1232e310","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"4bfb094a6fc5e92a2e8329e32be5e47f09cd84be"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":955,"context_line":"        return []"},{"line_number":956,"context_line":""},{"line_number":957,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":958,"context_line":"        if self._initiator is None:"},{"line_number":959,"context_line":"            self._initiator \u003d libvirt_utils.get_iscsi_initiator()"},{"line_number":960,"context_line":"            if not self._initiator:"},{"line_number":961,"context_line":"                LOG.debug(\u0027Could not determine iscsi initiator name\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_9dae1fe6","side":"PARENT","line":958,"updated":"2015-06-27 01:58:47.000000000","message":"So I\u0027m guessing these variables are cached so we don\u0027t have to do this lookup every time this is called, so can we also cache the connector value as just self._connector?","commit_id":"ca59f81ec179bdffc145a7a39f1fe33c37913aba"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":958,"context_line":"        return []"},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":961,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":962,"context_line":"        return connector.get_connector_properties("},{"line_number":963,"context_line":"            root_helper, CONF.my_block_storage_ip,"},{"line_number":964,"context_line":"            CONF.libvirt.iscsi_use_multipath, True,"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_bda9a3df","line":961,"updated":"2015-06-27 01:58:47.000000000","message":"We need to make get_root_helper() a public method.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":958,"context_line":"        return []"},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":961,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":962,"context_line":"        return connector.get_connector_properties("},{"line_number":963,"context_line":"            root_helper, CONF.my_block_storage_ip,"},{"line_number":964,"context_line":"            CONF.libvirt.iscsi_use_multipath, True,"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_be474376","line":961,"in_reply_to":"ba3cc151_99093d2f","updated":"2015-07-13 19:19:31.000000000","message":"Can you make it a dependent change?","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":958,"context_line":"        return []"},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":961,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":962,"context_line":"        return connector.get_connector_properties("},{"line_number":963,"context_line":"            root_helper, CONF.my_block_storage_ip,"},{"line_number":964,"context_line":"            CONF.libvirt.iscsi_use_multipath, True,"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_99093d2f","line":961,"in_reply_to":"fa32b979_bda9a3df","updated":"2015-06-30 16:08:13.000000000","message":"I think that\u0027s outside the scope of this patch.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":961,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":962,"context_line":"        return connector.get_connector_properties("},{"line_number":963,"context_line":"            root_helper, CONF.my_block_storage_ip,"},{"line_number":964,"context_line":"            CONF.libvirt.iscsi_use_multipath, True,"},{"line_number":965,"context_line":"            host\u003dCONF.host)"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"    def _cleanup_resize(self, instance, network_info):"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_fd6f2bc2","line":964,"updated":"2015-06-27 01:58:47.000000000","message":"nit: pass enforce_multipath\u003dTrue for clarity of what the True is here.\n\nAlso, what about iser_use_multipath?\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/volume.py#n89\n\nHow do we know the connector here is just using iscsi?","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":961,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":962,"context_line":"        return connector.get_connector_properties("},{"line_number":963,"context_line":"            root_helper, CONF.my_block_storage_ip,"},{"line_number":964,"context_line":"            CONF.libvirt.iscsi_use_multipath, True,"},{"line_number":965,"context_line":"            host\u003dCONF.host)"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"    def _cleanup_resize(self, instance, network_info):"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_3e54931f","line":964,"in_reply_to":"ba3cc151_5921754c","updated":"2015-07-13 19:19:31.000000000","message":"OK, good.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":961,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":962,"context_line":"        return connector.get_connector_properties("},{"line_number":963,"context_line":"            root_helper, CONF.my_block_storage_ip,"},{"line_number":964,"context_line":"            CONF.libvirt.iscsi_use_multipath, True,"},{"line_number":965,"context_line":"            host\u003dCONF.host)"},{"line_number":966,"context_line":""},{"line_number":967,"context_line":"    def _cleanup_resize(self, instance, network_info):"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_5921754c","line":964,"in_reply_to":"fa32b979_fd6f2bc2","updated":"2015-06-30 16:08:13.000000000","message":"I\u0027ll update the enforce_multipath param\n\nthe iser Connector is going away in os-brick and will be part of the ISCSIConnector.  the iser_user_multipath should be deprecated.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":961,"context_line":"            return disks"},{"line_number":962,"context_line":"        return []"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":965,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":966,"context_line":"        return connector.get_connector_properties("},{"line_number":967,"context_line":"            root_helper, CONF.my_block_storage_ip,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_d541c50c","line":964,"updated":"2015-07-13 19:53:31.000000000","message":"The results of this method used to be cached, so we should probably cache the results of connector.get_connector_properties.\n\nI don\u0027t know if fc wwpns or wwnns can change between calls, and I guess multipathd could stop for some reason, so there is potentially an argument for not caching the results of this, but if we don\u0027t cache here we should be smarter about not looping over this in the compute manager, like here:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/compute/manager.py?id\u003d12.0.0.0b1#n2239","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a20c988fc11fe4398e1ad90463499bc547d8497a","unresolved":false,"context_lines":[{"line_number":961,"context_line":"            return disks"},{"line_number":962,"context_line":"        return []"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":965,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":966,"context_line":"        return connector.get_connector_properties("},{"line_number":967,"context_line":"            root_helper, CONF.my_block_storage_ip,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_229e3e07","line":964,"in_reply_to":"9a41bdd9_a147231c","updated":"2015-07-15 19:43:56.000000000","message":"OK, so not caching is fine.  As noted we could be smarter in the compute manager about this, like get this before looping over BDMs, but that\u0027s a separate change.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":961,"context_line":"            return disks"},{"line_number":962,"context_line":"        return []"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"    def get_volume_connector(self, instance):"},{"line_number":965,"context_line":"        root_helper \u003d utils._get_root_helper()"},{"line_number":966,"context_line":"        return connector.get_connector_properties("},{"line_number":967,"context_line":"            root_helper, CONF.my_block_storage_ip,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_a147231c","line":964,"in_reply_to":"9a41bdd9_d541c50c","updated":"2015-07-13 21:39:11.000000000","message":"They can actually change.   If the admin enables a new FC HBA or plugs in a new cable, a port that used to be offline is now online and will suddenly show up in the results.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"}],"nova/virt/libvirt/volume.py":[{"author":{"_account_id":15903,"name":"Earle F. Philhower, III","email":"earle.philhower.iii@hgst.com","username":"earlephilhower"},"change_message_id":"145452a28a4acb2ec24d72f7446f64529cce305d","unresolved":false,"context_lines":[{"line_number":504,"context_line":"        # Call the factory here so we can support"},{"line_number":505,"context_line":"        # more than x86 architectures."},{"line_number":506,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":507,"context_line":"            \u0027AOE\u0027, utils._get_root_helper(),"},{"line_number":508,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries)"},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    def get_config(self, connection_info, disk_info):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_c1117553","line":507,"updated":"2015-06-18 19:19:36.000000000","message":"If the factory\u0027s 1st parameter could be an input, this LibvirtAOEVolumeDriver would make a perfect \"generic block attachment\" base class for drivers who give a local block device (not via iscsi/fc) and have no other work than calling os_brick.  Thinking of my own HGSTConnector here, for starters, but maybe other 3rd party drivers as well.","commit_id":"5e1e5bcf281cfe7b3a26360a41c6d77e440a7b72"},{"author":{"_account_id":15903,"name":"Earle F. Philhower, III","email":"earle.philhower.iii@hgst.com","username":"earlephilhower"},"change_message_id":"8f14460f2f04d1ff6b7e8a0b7dbf8fa45e1013c2","unresolved":false,"context_lines":[{"line_number":504,"context_line":"        # Call the factory here so we can support"},{"line_number":505,"context_line":"        # more than x86 architectures."},{"line_number":506,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":507,"context_line":"            \u0027AOE\u0027, utils._get_root_helper(),"},{"line_number":508,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries)"},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    def get_config(self, connection_info, disk_info):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_74570658","line":507,"in_reply_to":"fa32b979_32667151","updated":"2015-06-18 20:59:32.000000000","message":"Mybad, I don\u0027t know how it\u0027d be handled in Python, so I\u0027m a little obtuse here.\n\nThe only difference between LibvirtAOEVolumeDriver and LibvirtHGSTVolumeDriver will be s/\u0027AOE\u0027/\u0027HGST\u0027/ on line 507.  I\u0027d love to not have to cut-n-paste the same code, and maintain those copies, just to replace 3 characters.\n\nSo in C++ I\u0027d do something like a generic LibvirtBlockVolumeDriver() class which calls a virtual _get_proto_name().  The AOE volume driver, HGST volume driver, etc. subclass this and implements just the _get_proto_name() and return \"AOE\" or \"HGST.\"  Could probably fit it in one line, even.","commit_id":"5e1e5bcf281cfe7b3a26360a41c6d77e440a7b72"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"ae4c900cb3ede384e7bf0da28d35c58a8a8adfd3","unresolved":false,"context_lines":[{"line_number":504,"context_line":"        # Call the factory here so we can support"},{"line_number":505,"context_line":"        # more than x86 architectures."},{"line_number":506,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":507,"context_line":"            \u0027AOE\u0027, utils._get_root_helper(),"},{"line_number":508,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries)"},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    def get_config(self, connection_info, disk_info):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_32667151","line":507,"in_reply_to":"fa32b979_c1117553","updated":"2015-06-18 20:45:56.000000000","message":"I\u0027m not following you here.   The first param to the factory is an input.  ?","commit_id":"5e1e5bcf281cfe7b3a26360a41c6d77e440a7b72"},{"author":{"_account_id":15903,"name":"Earle F. Philhower, III","email":"earle.philhower.iii@hgst.com","username":"earlephilhower"},"change_message_id":"145452a28a4acb2ec24d72f7446f64529cce305d","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":676,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"        super(LibvirtISCSIVolumeDriver,"},{"line_number":679,"context_line":"              self).disconnect_volume(connection_info, disk_dev)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_06bf7361","line":678,"updated":"2015-06-18 19:19:36.000000000","message":"Should this be \"super(LibvirtFibreChannelVolumeDriver)\" since the base class is not iscsi here in the FC connector?","commit_id":"5e1e5bcf281cfe7b3a26360a41c6d77e440a7b72"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"ae4c900cb3ede384e7bf0da28d35c58a8a8adfd3","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":676,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"        super(LibvirtISCSIVolumeDriver,"},{"line_number":679,"context_line":"              self).disconnect_volume(connection_info, disk_dev)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"fa32b979_9242ddac","line":678,"in_reply_to":"fa32b979_06bf7361","updated":"2015-06-18 20:45:56.000000000","message":"You are right.  nice catch.","commit_id":"5e1e5bcf281cfe7b3a26360a41c6d77e440a7b72"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"6a45b28129d7a6e91820b577309017c9abca6f40","unresolved":false,"context_lines":[{"line_number":661,"context_line":"        LOG.debug(\"Calling os-brick to attach FC Volume %s\","},{"line_number":662,"context_line":"                  connection_info)"},{"line_number":663,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":664,"context_line":"        LOG.debug(\"Attached iSCSI volume %s\", device_info)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        connection_info[\u0027data\u0027][\u0027device_path\u0027] \u003d device_info[\u0027path\u0027]"},{"line_number":667,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_251c74c0","line":664,"updated":"2015-06-22 23:13:22.000000000","message":"iSCSI -\u003e FC","commit_id":"fa6fb973f684f9c626cb53cbdd2a7c5f082354fb"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"6a45b28129d7a6e91820b577309017c9abca6f40","unresolved":false,"context_lines":[{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume %s\","},{"line_number":672,"context_line":"                  connection_info)"},{"line_number":673,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":674,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_c57908fe","line":673,"updated":"2015-06-22 23:13:22.000000000","message":"2nd param can\u0027t be None for FC.","commit_id":"fa6fb973f684f9c626cb53cbdd2a7c5f082354fb"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"6a45b28129d7a6e91820b577309017c9abca6f40","unresolved":false,"context_lines":[{"line_number":671,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume %s\","},{"line_number":672,"context_line":"                  connection_info)"},{"line_number":673,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":674,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"},{"line_number":677,"context_line":"              self).disconnect_volume(connection_info, disk_dev)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_858300cc","line":674,"updated":"2015-06-22 23:13:22.000000000","message":"iSCSI -\u003e FC","commit_id":"fa6fb973f684f9c626cb53cbdd2a7c5f082354fb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        # Call the factory here so we can support"},{"line_number":303,"context_line":"        # more than x86 architectures."},{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_3d6033a4","line":305,"updated":"2015-06-27 01:58:47.000000000","message":"Given that the protocol value is case-sensitive in os-brick:\n\nhttp://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/initiator/connector.py#n142\n\nWe should really make these enums/constants in os-brick and then use those here so we don\u0027t screw it up at some point and pass in \u0027iscsi\u0027 and it breaks.  Obviously that would have to be added to os-brick and then a release would need to happen that nova picks up as a new minimum required version, but it\u0027s something to maybe add a TODO in the code for.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        # Call the factory here so we can support"},{"line_number":303,"context_line":"        # more than x86 architectures."},{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_5e0dd73c","line":305,"in_reply_to":"ba3cc151_99829dff","updated":"2015-07-13 19:19:31.000000000","message":"OK, yeah I see \"protocol \u003d protocol.upper()\" now - but agreed, constants would be goodness.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        # Call the factory here so we can support"},{"line_number":303,"context_line":"        # more than x86 architectures."},{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_99829dff","line":305,"in_reply_to":"fa32b979_3d6033a4","updated":"2015-06-30 16:08:13.000000000","message":"The protocols are case insensitive.  Look at like 141.\n\nhttp://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/initiator/connector.py#n141\n\nBut I do like the idea of adding protocol defines.  I\u0027ll add that in a follow up patch to os-brick.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        else:"},{"line_number":319,"context_line":"            return \u0027default\u0027"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"    def _validate_transport(self, transport_iface):"},{"line_number":322,"context_line":"        \"\"\"Check that given iscsi_iface uses only supported transports"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        Accepted transport names for provided iface param are"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_b2fe9736","side":"PARENT","line":321,"updated":"2015-07-13 19:53:31.000000000","message":"Where does this validation happen now?","commit_id":"4bfb094a6fc5e92a2e8329e32be5e47f09cd84be"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    def _get_transport(self):"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_b273f7e6","line":307,"updated":"2015-07-13 19:53:31.000000000","message":"In the case of the LibvirtISERVolumeDriver class this will be using the wrong config option, that uses num_iser_scan_tries and iser_use_multipath.  So either we need to do the self.connector in the __init__ method for LibvirtISERVolumeDriver or you have to make the option a getter method on this class so LibvirtISERVolumeDriver can override it and provide it to the base class.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        \"\"\"Attach the volume to instance_name.\"\"\""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        LOG.debug(\"Calling os-brick to attach iSCSI Volume %s\","},{"line_number":330,"context_line":"                  connection_info)"},{"line_number":331,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":332,"context_line":"        LOG.debug(\"Attached iSCSI volume %s\", device_info)"},{"line_number":333,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_b291b754","line":330,"updated":"2015-07-13 19:53:31.000000000","message":"connection_info can have admin_username and admin_password in it, so we shouldn\u0027t log the whole connection_info dict - can we just log the connection_info.get(\u0027serial\u0027, connection_info.get(\u0027volume_id\u0027))?  Something like that - last I checked cinder drivers aren\u0027t consistent about always returning the volume_id key in the connection_info dict but I could be mistaken.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        \"\"\"Attach the volume to instance_name.\"\"\""},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        LOG.debug(\"Calling os-brick to attach iSCSI Volume %s\","},{"line_number":330,"context_line":"                  connection_info)"},{"line_number":331,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":332,"context_line":"        LOG.debug(\"Attached iSCSI volume %s\", device_info)"},{"line_number":333,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_c126e767","line":330,"in_reply_to":"9a41bdd9_b291b754","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":337,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        LOG.debug(\"calling os-brick to detach iSCSI Volume %s\","},{"line_number":340,"context_line":"                  connection_info)"},{"line_number":341,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":342,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":343,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_72724f4b","line":340,"updated":"2015-07-13 19:53:31.000000000","message":"ditto on logging credentials","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        LOG.debug(\"calling os-brick to detach iSCSI Volume %s\","},{"line_number":340,"context_line":"                  connection_info)"},{"line_number":341,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":342,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        super(LibvirtISCSIVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_d2e79be2","line":341,"updated":"2015-07-13 19:53:31.000000000","message":"Why don\u0027t we pass disk_dev in for device_info here rather than None?","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a20c988fc11fe4398e1ad90463499bc547d8497a","unresolved":false,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        LOG.debug(\"calling os-brick to detach iSCSI Volume %s\","},{"line_number":340,"context_line":"                  connection_info)"},{"line_number":341,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":342,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        super(LibvirtISCSIVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_4246e27b","line":341,"in_reply_to":"9a41bdd9_c1b4a7f3","updated":"2015-07-15 19:43:56.000000000","message":"OK, seems we should make device_info\u003dNone by default in os-brick then.  Obviously that\u0027s a later change.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        LOG.debug(\"calling os-brick to detach iSCSI Volume %s\","},{"line_number":340,"context_line":"                  connection_info)"},{"line_number":341,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":342,"context_line":"        LOG.debug(\"Disconnected iSCSI Volume %s\", disk_dev)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        super(LibvirtISCSIVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_c1b4a7f3","line":341,"in_reply_to":"9a41bdd9_d2e79be2","updated":"2015-07-13 21:39:11.000000000","message":"It\u0027s not used for ISCSI, everything iSCSI uses is in the \u0027data\u0027 dict.  The data where this comes from is slightly different when it\u0027s in the context of Cinder vs. Nova.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":505,"context_line":"        # more than x86 architectures."},{"line_number":506,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":507,"context_line":"            \u0027AOE\u0027, utils._get_root_helper(),"},{"line_number":508,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries)"},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":511,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_72a9ef7a","line":508,"updated":"2015-07-13 19:53:31.000000000","message":"This should be:\n\nCONF.libvirt.num_aoe_discover_tries","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":505,"context_line":"        # more than x86 architectures."},{"line_number":506,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":507,"context_line":"            \u0027AOE\u0027, utils._get_root_helper(),"},{"line_number":508,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries)"},{"line_number":509,"context_line":""},{"line_number":510,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":511,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_a1916354","line":508,"in_reply_to":"9a41bdd9_72a9ef7a","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":518,"context_line":""},{"line_number":519,"context_line":"    def connect_volume(self, connection_info, mount_device):"},{"line_number":520,"context_line":"        LOG.debug(\"Calling os-brick to attach AoE Volume %s\","},{"line_number":521,"context_line":"                  connection_info)"},{"line_number":522,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":523,"context_line":"        LOG.debug(\"Attached AoE volume %s\", device_info)"},{"line_number":524,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_92d273ff","line":521,"updated":"2015-07-13 19:53:31.000000000","message":"ditto on logging credentials","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":518,"context_line":""},{"line_number":519,"context_line":"    def connect_volume(self, connection_info, mount_device):"},{"line_number":520,"context_line":"        LOG.debug(\"Calling os-brick to attach AoE Volume %s\","},{"line_number":521,"context_line":"                  connection_info)"},{"line_number":522,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":523,"context_line":"        LOG.debug(\"Attached AoE volume %s\", device_info)"},{"line_number":524,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_619b5b73","line":521,"in_reply_to":"9a41bdd9_92d273ff","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":529,"context_line":""},{"line_number":530,"context_line":"        LOG.debug(\"calling os-brick to detach AoE Volume %s\","},{"line_number":531,"context_line":"                  connection_info)"},{"line_number":532,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":533,"context_line":"        LOG.debug(\"Disconnected AoE Volume %s\", disk_dev)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        super(LibvirtAOEVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_72d7afed","line":532,"updated":"2015-07-13 19:53:31.000000000","message":"Why isn\u0027t disk_dev passed in for device_info?","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":529,"context_line":""},{"line_number":530,"context_line":"        LOG.debug(\"calling os-brick to detach AoE Volume %s\","},{"line_number":531,"context_line":"                  connection_info)"},{"line_number":532,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":533,"context_line":"        LOG.debug(\"Disconnected AoE Volume %s\", disk_dev)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        super(LibvirtAOEVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_817d3f1e","line":532,"in_reply_to":"9a41bdd9_72d7afed","updated":"2015-07-13 21:39:11.000000000","message":"It\u0027s not used for AoE.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":644,"context_line":"            \u0027FIBRE_CHANNEL\u0027, utils._get_root_helper(),"},{"line_number":645,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":646,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":647,"context_line":"            arch\u003dplatform.machine())"},{"line_number":648,"context_line":""},{"line_number":649,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":650,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_52f38b9e","line":647,"updated":"2015-07-13 19:53:31.000000000","message":"Is this necessary? It\u0027s already handled in os-brick\u0027s factory() method:\n\nhttp://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/initiator/connector.py?id\u003d0.2.0#n128","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a20c988fc11fe4398e1ad90463499bc547d8497a","unresolved":false,"context_lines":[{"line_number":644,"context_line":"            \u0027FIBRE_CHANNEL\u0027, utils._get_root_helper(),"},{"line_number":645,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":646,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":647,"context_line":"            arch\u003dplatform.machine())"},{"line_number":648,"context_line":""},{"line_number":649,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":650,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_4d02930b","line":647,"in_reply_to":"9a41bdd9_01904f42","updated":"2015-07-15 19:43:56.000000000","message":"OK, hmm, I guess we could add a TODO to remove that when a new enough version of os-brick is available.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":644,"context_line":"            \u0027FIBRE_CHANNEL\u0027, utils._get_root_helper(),"},{"line_number":645,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":646,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":647,"context_line":"            arch\u003dplatform.machine())"},{"line_number":648,"context_line":""},{"line_number":649,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":650,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_01904f42","line":647,"in_reply_to":"9a41bdd9_52f38b9e","updated":"2015-07-13 21:39:11.000000000","message":"That code hasn\u0027t shipped to pypi yet.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        \"\"\"Attach the volume to instance_name.\"\"\""},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        LOG.debug(\"Calling os-brick to attach FC Volume %s\","},{"line_number":662,"context_line":"                  connection_info)"},{"line_number":663,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":664,"context_line":"        LOG.debug(\"Attached FC volume %s\", device_info)"},{"line_number":665,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_f2389f3f","line":662,"updated":"2015-07-13 19:53:31.000000000","message":"credentials would be logged","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        \"\"\"Attach the volume to instance_name.\"\"\""},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        LOG.debug(\"Calling os-brick to attach FC Volume %s\","},{"line_number":662,"context_line":"                  connection_info)"},{"line_number":663,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":664,"context_line":"        LOG.debug(\"Attached FC volume %s\", device_info)"},{"line_number":665,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_61a91b7b","line":662,"in_reply_to":"9a41bdd9_f2389f3f","updated":"2015-07-13 21:39:11.000000000","message":"Done","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":673,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume %s\","},{"line_number":676,"context_line":"                  connection_info)"},{"line_number":677,"context_line":"        self.connector.disconnect_volume(connection_info,"},{"line_number":678,"context_line":"                                         connection_info[\u0027data\u0027])"},{"line_number":679,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_72456fb1","line":676,"updated":"2015-07-13 19:53:31.000000000","message":"logging credentials","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume %s\","},{"line_number":676,"context_line":"                  connection_info)"},{"line_number":677,"context_line":"        self.connector.disconnect_volume(connection_info,"},{"line_number":678,"context_line":"                                         connection_info[\u0027data\u0027])"},{"line_number":679,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_d2d1dbce","line":678,"updated":"2015-07-13 19:53:31.000000000","message":"In other volume drivers you\u0027re passing None for device_info, but here you pass connection_info[\u0027data\u0027], why?  What\u0027s the difference and why wouldn\u0027t disk_dev be used?","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a20c988fc11fe4398e1ad90463499bc547d8497a","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume %s\","},{"line_number":676,"context_line":"                  connection_info)"},{"line_number":677,"context_line":"        self.connector.disconnect_volume(connection_info,"},{"line_number":678,"context_line":"                                         connection_info[\u0027data\u0027])"},{"line_number":679,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_ed3847d5","line":678,"in_reply_to":"9a41bdd9_81d21fff","updated":"2015-07-15 19:43:56.000000000","message":"Yeah, we should make this better / more clear in os-brick since it\u0027s pretty flaky otherwise, i.e. as the caller I have to know about the specific internal implementation of os-brick for the type of driver I\u0027m using, which sucks.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume %s\","},{"line_number":676,"context_line":"                  connection_info)"},{"line_number":677,"context_line":"        self.connector.disconnect_volume(connection_info,"},{"line_number":678,"context_line":"                                         connection_info[\u0027data\u0027])"},{"line_number":679,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":680,"context_line":""},{"line_number":681,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_81d21fff","line":678,"in_reply_to":"9a41bdd9_d2d1dbce","updated":"2015-07-13 21:39:11.000000000","message":"This is just a difference in how Cinder stores it\u0027s data vs Nova.  I know, it\u0027s odd.  most connectors don\u0027t use the 2nd param, but we wanted to keep the API consistent for all connectors.   The FC driver doesn\u0027t look into the connection_info for \u0027data\u0027.  It\u0027s something we can look at changing later, and should be easy to do.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a1e0df05334399209d6d5b93888d4549fd7bc72","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    def _get_transport(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a41bdd9_edc28784","line":307,"updated":"2015-07-15 19:48:34.000000000","message":"In the case of the LibvirtISERVolumeDriver class this will be using the wrong config option, that uses num_iser_scan_tries and iser_use_multipath. So either we need to do the self.connector in the __init__ method for LibvirtISERVolumeDriver or you have to make the option a getter method on this class so LibvirtISERVolumeDriver can override it and provide it to the base class.","commit_id":"54448ab6b59a16ef84720f8780672148f6291cf4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d44a91f0f8aaf69f60eca73c14f01aa4cfc0c2ef","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    def _get_transport(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a41bdd9_42dc9d45","line":307,"in_reply_to":"9a41bdd9_6d3757ff","updated":"2015-07-15 21:15:58.000000000","message":"Since nova has the config options for iser why wouldn\u0027t we just use them?  Otherwise we\u0027re breaking existing deployments using the iser option.","commit_id":"54448ab6b59a16ef84720f8780672148f6291cf4"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"1ff6268d607c758741a9ecd675d71c63095363ae","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":305,"context_line":"            \u0027ISCSI\u0027, utils._get_root_helper(),"},{"line_number":306,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":307,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":308,"context_line":"            transport\u003dself._get_transport())"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    def _get_transport(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a41bdd9_6d3757ff","line":307,"in_reply_to":"9a41bdd9_edc28784","updated":"2015-07-15 20:15:01.000000000","message":"ok I\u0027ll add the __init__ for the ISER volume driver.  It will reuse the CONF.libvirt.iscsi_use_multipath for the use_multipath param, because ultimately in os-brick the ISER connector is nothing more than an ISCSIConnector.","commit_id":"54448ab6b59a16ef84720f8780672148f6291cf4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"15ed079e4e6794909ad505d33e8eb83c5db118a0","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        # more than x86 architectures."},{"line_number":354,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":355,"context_line":"            \u0027ISER\u0027, utils._get_root_helper(),"},{"line_number":356,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":357,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iser_scan_tries,"},{"line_number":358,"context_line":"            transport\u003dself._get_transport())"},{"line_number":359,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9a41bdd9_0299b57b","line":356,"updated":"2015-07-15 21:18:17.000000000","message":"This should be CONF.libvirt.iser_use_multipath otherwise we\u0027re breaking existing deployments using the iser driver and setting that option to something that\u0027s not the default, and iscsi_use_multipath isn\u0027t used.\n\nWe can deprecate/consolidate options later.","commit_id":"ea92146ba2d34d61d00ec5fe15d1d8f83212d875"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"eaebbb9c84772cab6099d9f742f00ee0c0c18642","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        # more than x86 architectures."},{"line_number":354,"context_line":"        self.connector \u003d connector.InitiatorConnector.factory("},{"line_number":355,"context_line":"            \u0027ISER\u0027, utils._get_root_helper(),"},{"line_number":356,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":357,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iser_scan_tries,"},{"line_number":358,"context_line":"            transport\u003dself._get_transport())"},{"line_number":359,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"9a41bdd9_c2c3cd36","line":356,"in_reply_to":"9a41bdd9_0299b57b","updated":"2015-07-15 21:20:52.000000000","message":"Done","commit_id":"ea92146ba2d34d61d00ec5fe15d1d8f83212d875"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f05c104496d77dc2ba78fc7caeb264418e58f7e6","unresolved":false,"context_lines":[{"line_number":652,"context_line":"            \u0027FIBRE_CHANNEL\u0027, utils._get_root_helper(),"},{"line_number":653,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":654,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":655,"context_line":"            arch\u003dplatform.machine())"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":658,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":26,"id":"9a41bdd9_629be166","line":655,"updated":"2015-07-15 21:20:24.000000000","message":"We could also add a TODO to remove this once we require at least os-brick\u003e\u003d0.3.0.","commit_id":"ea92146ba2d34d61d00ec5fe15d1d8f83212d875"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"68064664b6d3a84ad492c45e1cff8687357d184f","unresolved":false,"context_lines":[{"line_number":652,"context_line":"            \u0027FIBRE_CHANNEL\u0027, utils._get_root_helper(),"},{"line_number":653,"context_line":"            use_multipath\u003dCONF.libvirt.iscsi_use_multipath,"},{"line_number":654,"context_line":"            device_scan_attempts\u003dCONF.libvirt.num_iscsi_scan_tries,"},{"line_number":655,"context_line":"            arch\u003dplatform.machine())"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"    def get_config(self, connection_info, disk_info):"},{"line_number":658,"context_line":"        \"\"\"Returns xml for libvirt.\"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"9a41bdd9_23b9ed79","line":655,"updated":"2015-07-17 18:16:47.000000000","message":"Can we remove this line since this is in os-brick 0.3.0?","commit_id":"963d423587cd0c7185ef786d5bee5121872a44e4"},{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"d29b0c468a2fcfc02ea149f54399def2358e5bb6","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":678,"context_line":""},{"line_number":679,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume\")"},{"line_number":680,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":681,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a50d1a3_c2f634d7","line":680,"updated":"2015-07-20 06:47:01.000000000","message":"I am not sure this could work if we pass \u0027None\u0027 to connector.py. Becasue I see in connector\u0027s disconnect_volume, the \u0027device_info\u0027 is directly referred:\n\n    @synchronized(\u0027connect_volume\u0027)\n    def disconnect_volume(self, connection_properties, device_info):\n        \"\"\"Detach the volume from instance_name.\n\n        connection_properties for Fibre Channel must include:\n        target_wwn - iSCSI Qualified Name\n        target_lun - LUN id of the volume\n        \"\"\"\n\n        # If this is a multipath device, we need to search again\n        # and make sure we remove all the devices. Some of them\n        # might not have shown up at attach time.\n        if self.use_multipath and \u0027multipath_id\u0027 in device_info:\n            multipath_id \u003d device_info[\u0027multipath_id\u0027]\n            mdev_info \u003d self._linuxscsi.find_multipath_device(multipath_id)\n            devices \u003d mdev_info[\u0027devices\u0027]\n            LOG.debug(\"devices to remove \u003d %s\", devices)\n            self._linuxscsi.flush_multipath_device(multipath_id)\n        else:\n            device \u003d device_info[\u0027path\u0027]\n            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~","commit_id":"fd3263a1f7bb8970f7ee6d1ffacc2df61889e484"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"f5af6cf270ef55cb80ac48ace8bfb0cbfcf89ad2","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":678,"context_line":""},{"line_number":679,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume\")"},{"line_number":680,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":681,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a50d1a3_0bbd3001","line":680,"in_reply_to":"3a50d1a3_3ff5f1eb","updated":"2015-07-23 18:57:35.000000000","message":"For now, I\u0027ll add back the passing of connection_info[\u0027data\u0027] \nas the 2nd param to the connector\u0027s disconnect_volume.\n\nAlso adding a TODO note comment above the call about this.\nI\u0027ll have a follow up patch for nova to revert this back to None, as\npart of the patch to eliminate the need to save the multipath_id at all.  I have found a mechanism that allows me to get the multipath_id without needing to keep it around, that works for all volumes.   This will also clean up more code in Nova. \n\nFirst, I have to update os-brick to not need the multipath_id in the connection_info, then I\u0027ll post a new nova patch to eliminate multipath_id entirely.","commit_id":"fd3263a1f7bb8970f7ee6d1ffacc2df61889e484"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"2e345503b1ebe3ffd4f8b2f64bf4186efd741325","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":678,"context_line":""},{"line_number":679,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume\")"},{"line_number":680,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":681,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a50d1a3_75493130","line":680,"in_reply_to":"3a50d1a3_3ff5f1eb","updated":"2015-07-23 18:22:59.000000000","message":"brick is going to move away from needing the multipath_id at all.  I\u0027ve found a way to reliably get it from the volume itself using the 0x83 scsi page information you can get from the /lib/udev/scsi_id utility.","commit_id":"fd3263a1f7bb8970f7ee6d1ffacc2df61889e484"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"d03217bbe4d5127f94c8a0fae6ba6291b8ac57ba","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":678,"context_line":""},{"line_number":679,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume\")"},{"line_number":680,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":681,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a50d1a3_d1bb470d","line":680,"in_reply_to":"3a50d1a3_c2f634d7","updated":"2015-07-20 15:47:48.000000000","message":"yah, I have a fix os-brick for this already here:\n\nhttps://review.openstack.org/#/c/202826/","commit_id":"fd3263a1f7bb8970f7ee6d1ffacc2df61889e484"},{"author":{"_account_id":9988,"name":"Tina Tang","email":"tina.tang@emc.com","username":"Tina"},"change_message_id":"5acf5215c7218f7d19e4622bbe65bd557adba528","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":678,"context_line":""},{"line_number":679,"context_line":"        LOG.debug(\"calling os-brick to detach FC Volume\")"},{"line_number":680,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027], None)"},{"line_number":681,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        super(LibvirtFibreChannelVolumeDriver,"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a50d1a3_3ff5f1eb","line":680,"in_reply_to":"3a50d1a3_d1bb470d","updated":"2015-07-23 07:02:02.000000000","message":"Seems the line \u0027if self.use_multipath and \u0027multipath_id\u0027 in device_info:\u0027   in disconnect_volume will throw exception directly when the device_info is None. Seems the change 202826 hasn\u0027t addressed it yet.\n\n\u003e\u003e\u003e a \u003d None\n\u003e\u003e\u003e if \u0027b\u0027 in a:\n...   print(\u0027Not failed.\u0027)\n...\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nTypeError: argument of type \u0027NoneType\u0027 is not iterable","commit_id":"fd3263a1f7bb8970f7ee6d1ffacc2df61889e484"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2e690cf44b6d2c676813ef82c123e80e63e205c","unresolved":false,"context_lines":[{"line_number":527,"context_line":"        conf.source_path \u003d connection_info[\u0027data\u0027][\u0027device_path\u0027]"},{"line_number":528,"context_line":"        return conf"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    def connect_volume(self, connection_info, mount_device):"},{"line_number":531,"context_line":"        LOG.debug(\"Calling os-brick to attach AoE Volume\")"},{"line_number":532,"context_line":"        device_info \u003d self.connector.connect_volume(connection_info[\u0027data\u0027])"},{"line_number":533,"context_line":"        LOG.debug(\"Attached AoE volume %s\", device_info)"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_ce56b65d","line":530,"updated":"2015-07-28 21:07:45.000000000","message":"We don\u0027t have any unit test coverage for this anymore.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2e690cf44b6d2c676813ef82c123e80e63e205c","unresolved":false,"context_lines":[{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        connection_info[\u0027data\u0027][\u0027device_path\u0027] \u003d device_info[\u0027path\u0027]"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"    def disconnect_volume(self, connection_info, disk_dev):"},{"line_number":538,"context_line":"        \"\"\"Detach the volume from instance_name.\"\"\""},{"line_number":539,"context_line":""},{"line_number":540,"context_line":"        LOG.debug(\"calling os-brick to detach AoE Volume %s\","}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_ee5b7284","line":537,"updated":"2015-07-28 21:07:45.000000000","message":"ditto","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd80b563a26566026e3ebc3b2300331209ff1628","unresolved":false,"context_lines":[{"line_number":664,"context_line":"        conf.source_path \u003d connection_info[\u0027data\u0027][\u0027device_path\u0027]"},{"line_number":665,"context_line":"        return conf"},{"line_number":666,"context_line":""},{"line_number":667,"context_line":"    def connect_volume(self, connection_info, disk_info):"},{"line_number":668,"context_line":"        \"\"\"Attach the volume to instance_name.\"\"\""},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"        LOG.debug(\"Calling os-brick to attach FC Volume\")"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_e2ace989","line":667,"updated":"2015-07-28 19:25:45.000000000","message":"I just realized this method isn\u0027t unit tested in nova now.","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dd80b563a26566026e3ebc3b2300331209ff1628","unresolved":false,"context_lines":[{"line_number":685,"context_line":"        # This will then eliminate the need to pass anything in"},{"line_number":686,"context_line":"        # the 2nd param of disconnect_volume and be consistent"},{"line_number":687,"context_line":"        # with the rest of the connectors."},{"line_number":688,"context_line":"        self.connector.disconnect_volume(connection_info[\u0027data\u0027],"},{"line_number":689,"context_line":"                                         connection_info[\u0027data\u0027])"},{"line_number":690,"context_line":"        LOG.debug(\"Disconnected FC Volume %s\", disk_dev)"},{"line_number":691,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"3a50d1a3_a2cb4152","line":688,"updated":"2015-07-28 19:25:45.000000000","message":"ditto","commit_id":"e6cdd1693ba843e1c8dcccbde3af20f62eb0b5a2"}],"nova/virt/volumeutils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    for l in contents.split(\u0027\\n\u0027):"},{"line_number":33,"context_line":"        if l.startswith(\u0027InitiatorName\u003d\u0027):"},{"line_number":34,"context_line":"            return l[l.index(\u0027\u003d\u0027) + 1:].strip()"},{"line_number":35,"context_line":"    return \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_bd0d2337","side":"PARENT","line":35,"updated":"2015-06-27 01:58:47.000000000","message":"One difference to note is that the os-brick version will return None in case there is a ProcessExecutionError, like if the file isn\u0027t found, whereas nova\u0027s version would return \u0027\u0027:\n\nhttp://git.openstack.org/cgit/openstack/os-brick/tree/os_brick/initiator/connector.py#n494","commit_id":"ca59f81ec179bdffc145a7a39f1fe33c37913aba"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    for l in contents.split(\u0027\\n\u0027):"},{"line_number":33,"context_line":"        if l.startswith(\u0027InitiatorName\u003d\u0027):"},{"line_number":34,"context_line":"            return l[l.index(\u0027\u003d\u0027) + 1:].strip()"},{"line_number":35,"context_line":"    return \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_d0902898","side":"PARENT","line":35,"in_reply_to":"fa32b979_bd0d2337","updated":"2015-06-30 16:08:13.000000000","message":"yup, as designed, because an iSCSI initiator of \u0027\u0027 is completely useless.","commit_id":"ca59f81ec179bdffc145a7a39f1fe33c37913aba"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":16,"context_line":"Volume utilities for virt drivers."},{"line_number":17,"context_line":"\"\"\""},{"line_number":18,"context_line":"from nova import utils"},{"line_number":19,"context_line":"from os_brick.initiator import connector"},{"line_number":20,"context_line":"from oslo_concurrency import processutils as putils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_ba58c958","line":19,"updated":"2015-06-27 01:58:47.000000000","message":"These are third party imports so they should be in a separate group above the nova ones (lib imports, then 3rd party lib imports, then nova imports).","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":23,"context_line":"def get_iscsi_initiator(execute\u003dNone):"},{"line_number":24,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":27,"context_line":"    # so we can mock out the"},{"line_number":28,"context_line":"    if not execute:"},{"line_number":29,"context_line":"        execute \u003d putils.execute"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_da390df4","line":26,"updated":"2015-06-27 01:58:47.000000000","message":"Hrm, we should probably make this method public if it\u0027s going to be used outside of nova.utils:\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/utils.py#n217","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cee7bb978c25687ef78e72ae9d64d2e747c14f8c","unresolved":false,"context_lines":[{"line_number":23,"context_line":"def get_iscsi_initiator(execute\u003dNone):"},{"line_number":24,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":27,"context_line":"    # so we can mock out the"},{"line_number":28,"context_line":"    if not execute:"},{"line_number":29,"context_line":"        execute \u003d putils.execute"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a41bdd9_d209db2d","line":26,"in_reply_to":"ba3cc151_0dcc3288","updated":"2015-07-13 19:19:31.000000000","message":"Let\u0027s do it in a dependent change.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":23,"context_line":"def get_iscsi_initiator(execute\u003dNone):"},{"line_number":24,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":27,"context_line":"    # so we can mock out the"},{"line_number":28,"context_line":"    if not execute:"},{"line_number":29,"context_line":"        execute \u003d putils.execute"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_0dcc3288","line":26,"in_reply_to":"fa32b979_da390df4","updated":"2015-06-30 16:08:13.000000000","message":"seems a bit out of scope for this patch.   changing _get_root_helper to get_root_helper would require touching lots of files that aren\u0027t related to this review.  \n\nIf you really want me to go that route I can.  It will just make this review quite a bit bigger.","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"587b49f67414d0096e78b309299f6cbfb12b5a22","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":27,"context_line":"    # so we can mock out the"},{"line_number":28,"context_line":"    if not execute:"},{"line_number":29,"context_line":"        execute \u003d putils.execute"},{"line_number":30,"context_line":"    iscsi \u003d connector.ISCSIConnector(root_helper\u003droot_helper,"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa32b979_5a573d26","line":27,"updated":"2015-06-27 01:58:47.000000000","message":"Incomplete thought?","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"7f66f430b19f752efd8db85b4b93a4a7e00c7b7d","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":27,"context_line":"    # so we can mock out the"},{"line_number":28,"context_line":"    if not execute:"},{"line_number":29,"context_line":"        execute \u003d putils.execute"},{"line_number":30,"context_line":"    iscsi \u003d connector.ISCSIConnector(root_helper\u003droot_helper,"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba3cc151_f0e44c3c","line":27,"in_reply_to":"fa32b979_5a573d26","updated":"2015-06-30 16:08:13.000000000","message":"Done","commit_id":"0bd0fd415824b1ae028601d80952cc739cab9008"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"342d1af68a12a5e177b703e6fe31a3224a280d5b","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":28,"context_line":"    # so we can mock out the execute itself"},{"line_number":29,"context_line":"    # in unit tests."},{"line_number":30,"context_line":"    if not execute:"},{"line_number":31,"context_line":"        execute \u003d putils.execute"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_f2473f75","line":28,"updated":"2015-07-13 19:53:31.000000000","message":"Since you delete test_volumeutils, we actually don\u0027t have any unit tests for this code now.  We should have unit tests on this method.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"469dd4e78f0ec4c4cf6c515b749998c297fd8948","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    \"\"\"Get iscsi initiator name for this machine.\"\"\""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    root_helper \u003d utils._get_root_helper()"},{"line_number":28,"context_line":"    # so we can mock out the execute itself"},{"line_number":29,"context_line":"    # in unit tests."},{"line_number":30,"context_line":"    if not execute:"},{"line_number":31,"context_line":"        execute \u003d putils.execute"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a41bdd9_c1fee76a","line":28,"in_reply_to":"9a41bdd9_f2473f75","updated":"2015-07-13 21:39:11.000000000","message":"I added it back in, mocking out os-brick.","commit_id":"97df7f537ad3b2ebd6bb1da97b7c0b766f720b95"}],"requirements.txt":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"515a9dcd3811ac74b0a48d816bab1cc220924742","unresolved":false,"context_lines":[{"line_number":48,"context_line":"psutil\u003c2.0.0,\u003e\u003d1.1.1"},{"line_number":49,"context_line":"oslo.versionedobjects!\u003d0.5.0,\u003e\u003d0.3.0"},{"line_number":50,"context_line":"alembic\u003e\u003d0.7.2"},{"line_number":51,"context_line":"os-brick"}],"source_content_type":"text/plain","patch_set":30,"id":"3a50d1a3_063337ac","line":51,"updated":"2015-07-22 15:01:16.000000000","message":"This would need to be updated to match whatever is in https://review.openstack.org/#/c/203193/ but that\u0027s going to be updated to 0.3.1 I think once it\u0027s released since that has the FC fix.","commit_id":"fd3263a1f7bb8970f7ee6d1ffacc2df61889e484"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"166927562e693998a53a8855455ef395c1d45f11","unresolved":false,"context_lines":[{"line_number":51,"context_line":"psutil\u003c2.0.0,\u003e\u003d1.1.1"},{"line_number":52,"context_line":"oslo.versionedobjects!\u003d0.5.0,\u003e\u003d0.3.0"},{"line_number":53,"context_line":"alembic\u003e\u003d0.7.2"},{"line_number":54,"context_line":"os-brick"}],"source_content_type":"text/plain","patch_set":31,"id":"3a50d1a3_0f8d007b","line":54,"updated":"2015-07-24 19:39:22.000000000","message":"This should be this now:\n\nos-brick\u003e\u003d0.3.2                         # Apache-2.0","commit_id":"67d175c385bd07cf46fea286b85e2f68c7463d4e"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"9f286c2852e0f7b660207e574720870c5b1d78c3","unresolved":false,"context_lines":[{"line_number":51,"context_line":"psutil\u003c2.0.0,\u003e\u003d1.1.1"},{"line_number":52,"context_line":"oslo.versionedobjects!\u003d0.5.0,\u003e\u003d0.3.0"},{"line_number":53,"context_line":"alembic\u003e\u003d0.7.2"},{"line_number":54,"context_line":"os-brick"}],"source_content_type":"text/plain","patch_set":31,"id":"3a50d1a3_fb1b721a","line":54,"in_reply_to":"3a50d1a3_0f8d007b","updated":"2015-07-27 17:55:47.000000000","message":"Done","commit_id":"67d175c385bd07cf46fea286b85e2f68c7463d4e"}]}