)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"6f249d3b608e6fb6564aa460ce15bad9d4e67c28","unresolved":false,"context_lines":[{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need follow two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo. {\u0027data\u0027:\u0027device_path\u0027}"},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to the real device."},{"line_number":12,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":13,"context_line":"   attach_volume finds the real device from the symlink, then create a encrypted partition from the real device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f044301_e8eeea86","line":10,"updated":"2017-05-05 13:14:25.000000000","message":"Please limit line length to 72 characters.\n\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"33b50e6fcfec9ba9bd754f1088525a20af6ea574"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     cheng li \u003cshcli@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-05-05 15:32:43 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_0d6fcc92","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":1},"updated":"2017-05-07 22:32:09.000000000","message":"nit - C","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1c67146c4d1b7c5b7b674d736b7089fdf741b622","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     cheng li \u003cshcli@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-05-05 15:32:43 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_26faa574","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":1},"in_reply_to":"3f044301_0d6fcc92","updated":"2017-05-08 06:05:53.000000000","message":"Done","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_ed6dc098","line":10,"range":{"start_line":10,"start_character":41,"end_line":10,"end_character":55},"updated":"2017-05-07 22:32:09.000000000","message":"nit - connection_info","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1c67146c4d1b7c5b7b674d736b7089fdf741b622","unresolved":false,"context_lines":[{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_06f56983","line":10,"range":{"start_line":10,"start_character":41,"end_line":10,"end_character":55},"in_reply_to":"3f044301_ed6dc098","updated":"2017-05-08 06:05:53.000000000","message":"Done","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":14,"context_line":"   attach_volume finds the real device from the symlink,"},{"line_number":15,"context_line":"   then create a encrypted partition from the real device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_0d46ac22","line":12,"range":{"start_line":10,"start_character":0,"end_line":12,"end_character":19},"updated":"2017-05-07 22:32:09.000000000","message":"Well, we don\u0027t create the symlink until after the cryptsetup create call.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1c67146c4d1b7c5b7b674d736b7089fdf741b622","unresolved":false,"context_lines":[{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":14,"context_line":"   attach_volume finds the real device from the symlink,"},{"line_number":15,"context_line":"   then create a encrypted partition from the real device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_c68d41d4","line":12,"range":{"start_line":10,"start_character":0,"end_line":12,"end_character":19},"in_reply_to":"3f044301_0d46ac22","updated":"2017-05-08 06:05:53.000000000","message":"yes, we do.\nBefore \u0027cryptsetup create\u0027, the symlink points to the rbd device(e.g /dev/rbd0). \u0027rbd map\u0027creates the symlink.\nAfter \u0027cryptsetup create\u0027, os_brick points it to /dev/mapper/xxx.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5176a4b8ae1ad5f2ae8e945ebb24734b35887b27","unresolved":false,"context_lines":[{"line_number":7,"context_line":"check volume status before open/close volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"To encrypt a volume with luks, we need following two steps:[1]"},{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":14,"context_line":"   attach_volume finds the real device from the symlink,"},{"line_number":15,"context_line":"   then create a encrypted partition from the real device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f013ff3_36a30ea1","line":12,"range":{"start_line":10,"start_character":0,"end_line":12,"end_character":19},"in_reply_to":"3f044301_c68d41d4","updated":"2017-05-16 13:19:56.000000000","message":"udev is creating these links right? That\u0027s outside of our control, so when I say _we_ don\u0027t create the symlink until after the cryptsetup call I mean the actual call to `ln` from os-brick.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":14,"context_line":"   attach_volume finds the real device from the symlink,"},{"line_number":15,"context_line":"   then create a encrypted partition from the real device."},{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_ed54a0cc","line":13,"range":{"start_line":13,"start_character":35,"end_line":13,"end_character":42},"updated":"2017-05-07 22:32:09.000000000","message":"nit - decrypt","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1c67146c4d1b7c5b7b674d736b7089fdf741b622","unresolved":false,"context_lines":[{"line_number":10,"context_line":"1. create the luks encryptor with volume connectioninfo."},{"line_number":11,"context_line":"   connectioninfo[\u0027data\u0027][\u0027device_path\u0027] is the symlink to"},{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":14,"context_line":"   attach_volume finds the real device from the symlink,"},{"line_number":15,"context_line":"   then create a encrypted partition from the real device."},{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_86c179e7","line":13,"range":{"start_line":13,"start_character":35,"end_line":13,"end_character":42},"in_reply_to":"3f044301_ed54a0cc","updated":"2017-05-08 06:05:53.000000000","message":"Done","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":12,"context_line":"   the real device."},{"line_number":13,"context_line":"2. call encryptor.attach_volume to encrypt the volume."},{"line_number":14,"context_line":"   attach_volume finds the real device from the symlink,"},{"line_number":15,"context_line":"   then create a encrypted partition from the real device."},{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_cd68c488","line":15,"range":{"start_line":15,"start_character":27,"end_line":15,"end_character":36},"updated":"2017-05-07 22:32:09.000000000","message":"nit - device","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_ad60d8a6","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":42},"updated":"2017-05-07 22:32:09.000000000","message":"I don\u0027t see the use case for Nova to ever do this, I\u0027m not sure about Cinder. When would we want to call encryptor.attach_volume after it has already been called? Shouldn\u0027t we just raise an exception if the volume is already decrypted?","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1c67146c4d1b7c5b7b674d736b7089fdf741b622","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_c15e4b11","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":42},"in_reply_to":"3f044301_ad60d8a6","updated":"2017-05-08 06:05:53.000000000","message":"nova doesn\u0027t call encryptor.attach_volume when VM soft/hard reboot because this condition. https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L5051-L5060\n\nThis condition could prevent opening encryption device after hypervisor host reboot.\n\nIn fact, I would like to tolerant calling encryptor.attach_volume multi-times in os_brick project, instead of take care of nova/cinder code to not call it twice.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5176a4b8ae1ad5f2ae8e945ebb24734b35887b27","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f013ff3_36cc2e69","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":42},"in_reply_to":"3f044301_c15e4b11","updated":"2017-05-16 13:19:56.000000000","message":"That\u0027s a bug in Nova, IMHO we should call attach_volume in this situation but would need the check from this change to do it safely.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"opens the volume."},{"line_number":28,"context_line":"Also do the check before closing volume."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://github.com/openstack/nova/blob/16.0.0.0b1/"},{"line_number":31,"context_line":"    nova/virt/libvirt/driver.py#L1241-L1244"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_2d74c867","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":43},"updated":"2017-05-07 22:32:09.000000000","message":"As this code has been removed from Nova it might be better to point to os-brick.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"1c67146c4d1b7c5b7b674d736b7089fdf741b622","unresolved":false,"context_lines":[{"line_number":27,"context_line":"opens the volume."},{"line_number":28,"context_line":"Also do the check before closing volume."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://github.com/openstack/nova/blob/16.0.0.0b1/"},{"line_number":31,"context_line":"    nova/virt/libvirt/driver.py#L1241-L1244"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f044301_c14f6b32","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":43},"in_reply_to":"3f044301_2d74c867","updated":"2017-05-08 06:05:53.000000000","message":"https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1229-L1232","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5176a4b8ae1ad5f2ae8e945ebb24734b35887b27","unresolved":false,"context_lines":[{"line_number":27,"context_line":"opens the volume."},{"line_number":28,"context_line":"Also do the check before closing volume."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://github.com/openstack/nova/blob/16.0.0.0b1/"},{"line_number":31,"context_line":"    nova/virt/libvirt/driver.py#L1241-L1244"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f013ff3_9609c296","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":43},"in_reply_to":"3f044301_c14f6b32","updated":"2017-05-16 13:19:56.000000000","message":"Yeah sorry I think I assumed you were pointing at the old encryptor code for some reason.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e328ea8f74e61a06f4a376eb52423af95d8af7a9","unresolved":false,"context_lines":[{"line_number":23,"context_line":"   Because it had pointed the symlink to the encrypted partition"},{"line_number":24,"context_line":"   in last attach_volume."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"This patch is to check if volume is already opened before attach_volume"},{"line_number":27,"context_line":"opens the volume."},{"line_number":28,"context_line":"Also do the check before closing volume."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://github.com/openstack/nova/blob/master/nova/virt/"},{"line_number":31,"context_line":"    libvirt/driver.py#L1229-L1232"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f013ff3_193cc3bd","line":28,"range":{"start_line":26,"start_character":0,"end_line":28,"end_character":40},"updated":"2017-05-16 13:41:03.000000000","message":"super nit - this can be a single paragraph.","commit_id":"6dd30522086ca98b8d1ff00644e317e722f8e822"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e328ea8f74e61a06f4a376eb52423af95d8af7a9","unresolved":false,"context_lines":[{"line_number":27,"context_line":"opens the volume."},{"line_number":28,"context_line":"Also do the check before closing volume."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://github.com/openstack/nova/blob/master/nova/virt/"},{"line_number":31,"context_line":"    libvirt/driver.py#L1229-L1232"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f013ff3_d9309beb","line":30,"range":{"start_line":30,"start_character":43,"end_line":30,"end_character":49},"updated":"2017-05-16 13:41:03.000000000","message":"nit - Can you point to a specific commit so these line numbers remain valid?","commit_id":"6dd30522086ca98b8d1ff00644e317e722f8e822"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_824911b8","line":19,"updated":"2017-09-07 13:44:47.000000000","message":"?: I can\u0027t think of a case where we would be attaching twice the same volume on the same node...  Could you provide an example, please?","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"39f6506b426e858bdb16bc9dee812539f63d7c84","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_6a2825e1","line":19,"in_reply_to":"7f287b81_309bb673","updated":"2017-09-08 08:32:56.000000000","message":"I don\u0027t think those would trigger the call to attach again, and if they did the problem is that the caller should have closed it before or should have just reused the device that was already attached, so the bug would be in the caller that doesn\u0027t do something sensible...","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"87970b3ab4f0da95f443011695e91c13ed50314f","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_a59bde14","line":19,"in_reply_to":"7f287b81_6a2825e1","updated":"2017-09-08 09:25:11.000000000","message":"Without this patch, the caller(nova/cinder) has to keep it in mind to not call attach_volume twice. It\u0027s not good for the caller, that\u0027s why I create this commit to tolerance multi times calling of attach_volume.\n\nWhen the caller call attach_volume, they want to encrypt the device. So if the device is already encrypted, we should prevent encrypting again, and return immediately after printing the debug message.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"ed938e450972ef0367b5dcce843a1e4ef140f1ed","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_309bb673","line":19,"in_reply_to":"7f287b81_824911b8","updated":"2017-09-08 01:39:47.000000000","message":"It\u0027s not a short time since I committed the first patch. I don\u0027t exactly remember how to trigger it. It could be \"soft/hard reboot VM\" or \"reboot the compute host\"","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"52f54d92fc00cd7fcccbdf46dd89e3c348df09fa","unresolved":false,"context_lines":[{"line_number":16,"context_line":"   At last, it points the symlink to the encrypted partition"},{"line_number":17,"context_line":"   instead of the device."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Thinking that we call attach_volume again."},{"line_number":20,"context_line":"1. create luks encryptor with connectioninfo."},{"line_number":21,"context_line":"2. Now attach_volume wants to find the real device from the symlink."},{"line_number":22,"context_line":"   But it will get the partition instead of the device."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_bac1bc56","line":19,"in_reply_to":"7f287b81_a59bde14","updated":"2017-09-08 10:54:58.000000000","message":"This all caused by a terrible, terrible flow, as I explained in this patch\u0027s [1] commit message [2].  If we had a normal flow all this could be easily fixed.\n\nKeeping track of having called attach is *basic*, if you are not keeping track that means that you will leave devices behind in the system.\n\nI understand how this fixes the attach, I wanted to have a reason why this was necessary, and all I hear is \"because Nova is broken and it\u0027s not going to get fixed\".\n\n[1] https://review.openstack.org/#/c/483069/\n[2] https://review.openstack.org/#/c/483069/2//COMMIT_MSG","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"0162cd8761762b9a0bde4a56ffbab01273d58ded","unresolved":false,"context_lines":[{"line_number":26,"context_line":"This patch is to check if volume is already opened before attach_volume"},{"line_number":27,"context_line":"opens the volume. Also do the check before closing volume."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[1] https://github.com/openstack/nova/blob/37438e21efbf3877e5f84dd791a"},{"line_number":30,"context_line":"    0497af7af66cf/nova/virt/libvirt/driver.py#L1232-L1235"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_9c9c4e5e","line":30,"range":{"start_line":29,"start_character":4,"end_line":30,"end_character":57},"updated":"2017-09-02 16:18:56.000000000","message":"Can we not split this up so that it will actually work as a link please?\n\nThanks!","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"e6a8d6ea6a412b753914e22d4ada099409db5671","unresolved":false,"context_lines":[{"line_number":26,"context_line":"This patch is to check if volume is already opened before attach_volume"},{"line_number":27,"context_line":"opens the volume. Also do the check before closing volume."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[1] https://github.com/openstack/nova/blob/37438e21efbf3877e5f84dd791a"},{"line_number":30,"context_line":"    0497af7af66cf/nova/virt/libvirt/driver.py#L1232-L1235"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_a8ea20a9","line":30,"range":{"start_line":29,"start_character":4,"end_line":30,"end_character":57},"in_reply_to":"7f287b81_9c9c4e5e","updated":"2017-09-03 05:57:28.000000000","message":"It would be better to follow the rule of limiting each line 72 characters\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":26,"context_line":"This patch is to check if volume is already opened before attach_volume"},{"line_number":27,"context_line":"opens the volume. Also do the check before closing volume."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[1] https://github.com/openstack/nova/blob/37438e21efbf3877e5f84dd791a"},{"line_number":30,"context_line":"    0497af7af66cf/nova/virt/libvirt/driver.py#L1232-L1235"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Change-Id: I10fae64613cd1f6562e43b495c956894be134632"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7f287b81_c2cef958","line":30,"range":{"start_line":29,"start_character":4,"end_line":30,"end_character":57},"in_reply_to":"7f287b81_a8ea20a9","updated":"2017-09-07 13:44:47.000000000","message":"No, it won\u0027t be better, I agree with Jay, urls may be one of the only exceptions to the rule.  And the link to the steps should be the OS-Brick code in my opinion: https://github.com/openstack/os-brick/blob/1d617b2adb2d72c49c59816f60d0eb751ec8a1f1/os_brick/encryptors/luks.py#L151-L176","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"}],"os_brick/encryptors/luks.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"6f249d3b608e6fb6564aa460ce15bad9d4e67c28","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _is_opened(self):"},{"line_number":106,"context_line":"        \"\"\"Check if the the volume is opened"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        \"\"\""},{"line_number":109,"context_line":"        try:"},{"line_number":110,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027status\u0027, self.dev_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_c888cec7","line":107,"updated":"2017-05-05 13:14:25.000000000","message":"Single line docstrings should all be on one line with an ending period.\n\n\"\"\"Check if the volume is opened.\"\"\"\n\nAlso, two \"the\"s in the sentence.","commit_id":"33b50e6fcfec9ba9bd754f1088525a20af6ea574"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"9290a6ab6716b84b568c78dd1c04018c10cc02f5","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _is_opened(self):"},{"line_number":106,"context_line":"        \"\"\"Check if the the volume is opened"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        \"\"\""},{"line_number":109,"context_line":"        try:"},{"line_number":110,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027status\u0027, self.dev_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_057c4db3","line":107,"in_reply_to":"3f044301_c888cec7","updated":"2017-05-06 11:54:16.000000000","message":"Done","commit_id":"33b50e6fcfec9ba9bd754f1088525a20af6ea574"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"6f249d3b608e6fb6564aa460ce15bad9d4e67c28","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":205,"context_line":"                          root_helper\u003dself._root_helper,"},{"line_number":206,"context_line":"                          attempts\u003d3)"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            LOG.info(\"volume %s is already closed\", self.dev_path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_28c3b2ef","line":208,"range":{"start_line":207,"start_character":8,"end_line":208,"end_character":21},"updated":"2017-05-05 13:14:25.000000000","message":"This should just be removed, or at least changed to LOG.debug.","commit_id":"33b50e6fcfec9ba9bd754f1088525a20af6ea574"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"9290a6ab6716b84b568c78dd1c04018c10cc02f5","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":205,"context_line":"                          root_helper\u003dself._root_helper,"},{"line_number":206,"context_line":"                          attempts\u003d3)"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            LOG.info(\"volume %s is already closed\", self.dev_path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f044301_a546b97b","line":208,"range":{"start_line":207,"start_character":8,"end_line":208,"end_character":21},"in_reply_to":"3f044301_28c3b2ef","updated":"2017-05-06 11:54:16.000000000","message":"I would like to change it to LOG.debug, as this could be important msg for troubleshooting.","commit_id":"33b50e6fcfec9ba9bd754f1088525a20af6ea574"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                      root_helper\u003dself._root_helper,"},{"line_number":87,"context_line":"                      attempts\u003d3)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def _is_opened(self):"},{"line_number":90,"context_line":"        \"\"\"Check if the volume is opened\"\"\""},{"line_number":91,"context_line":"        try:"},{"line_number":92,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027status\u0027, self.dev_name,"},{"line_number":93,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":94,"context_line":"                          root_helper\u003dself._root_helper)"},{"line_number":95,"context_line":"        except putils.ProcessExecutionError:"},{"line_number":96,"context_line":"            return False"},{"line_number":97,"context_line":"        return True"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def _open_volume(self, passphrase, **kwargs):"},{"line_number":100,"context_line":"        \"\"\"Opens the LUKS partition on the volume using the specified"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f044301_8d087cd9","line":97,"range":{"start_line":89,"start_character":0,"end_line":97,"end_character":19},"updated":"2017-05-07 22:32:09.000000000","message":"Could we just use self._is_crypt_device_available(dev_name) to see if the device has already been opened?","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"9295c63bdf12900760a9e5fb44ceeaab46096c23","unresolved":false,"context_lines":[{"line_number":86,"context_line":"                      root_helper\u003dself._root_helper,"},{"line_number":87,"context_line":"                      attempts\u003d3)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def _is_opened(self):"},{"line_number":90,"context_line":"        \"\"\"Check if the volume is opened\"\"\""},{"line_number":91,"context_line":"        try:"},{"line_number":92,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027status\u0027, self.dev_name,"},{"line_number":93,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":94,"context_line":"                          root_helper\u003dself._root_helper)"},{"line_number":95,"context_line":"        except putils.ProcessExecutionError:"},{"line_number":96,"context_line":"            return False"},{"line_number":97,"context_line":"        return True"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def _open_volume(self, passphrase, **kwargs):"},{"line_number":100,"context_line":"        \"\"\"Opens the LUKS partition on the volume using the specified"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f044301_6d9f7003","line":97,"range":{"start_line":89,"start_character":0,"end_line":97,"end_character":19},"in_reply_to":"3f044301_8d087cd9","updated":"2017-05-08 01:41:39.000000000","message":"Sure, we can. I just didn\u0027t notice that method in parent class.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3571951aaeb73c3a403d7c6680a4cd6e5aa1921a","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                          process_input\u003dpassphrase,"},{"line_number":110,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":111,"context_line":"                          root_helper\u003dself._root_helper)"},{"line_number":112,"context_line":"        else:"},{"line_number":113,"context_line":"            LOG.debug(\"volume %s is already opened\", self.dev_path)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def _unmangle_volume(self, key, passphrase, **kwargs):"},{"line_number":116,"context_line":"        \"\"\"Workaround for bug#1633518"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f044301_ad0578c4","line":113,"range":{"start_line":112,"start_character":0,"end_line":113,"end_character":67},"updated":"2017-05-07 22:32:09.000000000","message":"I\u0027d rather we throw something back up to Nova/Cinder at this point to indicate that the volume is already decrypted, suggesting something has gone wrong.","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"9295c63bdf12900760a9e5fb44ceeaab46096c23","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                          process_input\u003dpassphrase,"},{"line_number":110,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":111,"context_line":"                          root_helper\u003dself._root_helper)"},{"line_number":112,"context_line":"        else:"},{"line_number":113,"context_line":"            LOG.debug(\"volume %s is already opened\", self.dev_path)"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def _unmangle_volume(self, key, passphrase, **kwargs):"},{"line_number":116,"context_line":"        \"\"\"Workaround for bug#1633518"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f044301_30af7be4","line":113,"range":{"start_line":112,"start_character":0,"end_line":113,"end_character":67},"in_reply_to":"3f044301_ad0578c4","updated":"2017-05-08 01:41:39.000000000","message":"I think nova/cinder doesn\u0027t need to know this. What nova/cinder wants is to create a encrypted partition,\nno matter the encrypted partition was created before or now.\nThe LOG.debug message will be good information for debug.\n\nVM reboot may meet this case(encrypted partition exists when nova tries to create it).","commit_id":"4f34785726105965a135b75d8aaba6b9e43923ab"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"29d7fc733c358d3151f6cc7c80d70561ba9b2f01","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        :param passphrase: the passphrase used to access the volume"},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":"        LOG.debug(\"opening encrypted volume %s\", self.dev_path)"},{"line_number":96,"context_line":"        if not self._is_crypt_device_available(self.dev_name):"},{"line_number":97,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksOpen\u0027, \u0027--key-file\u003d-\u0027,"},{"line_number":98,"context_line":"                          self.dev_path, self.dev_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_16cbae72","line":95,"updated":"2017-09-01 12:42:49.000000000","message":"not: should we move it inside \u0027if\u0027 statement?","commit_id":"c485561df1a762276c9074c9a724a54f43ea4124"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"66654745f484d418eb5be2126b7229e8df05ea9b","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        :param passphrase: the passphrase used to access the volume"},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":"        LOG.debug(\"opening encrypted volume %s\", self.dev_path)"},{"line_number":96,"context_line":"        if not self._is_crypt_device_available(self.dev_name):"},{"line_number":97,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksOpen\u0027, \u0027--key-file\u003d-\u0027,"},{"line_number":98,"context_line":"                          self.dev_path, self.dev_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_ac759bce","line":95,"in_reply_to":"7f287b81_16cbae72","updated":"2017-09-02 07:10:57.000000000","message":"good catch, will fix it in next commit","commit_id":"c485561df1a762276c9074c9a724a54f43ea4124"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"29d7fc733c358d3151f6cc7c80d70561ba9b2f01","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def _close_volume(self, **kwargs):"},{"line_number":188,"context_line":"        \"\"\"Closes the device (effectively removes the dm-crypt mapping).\"\"\""},{"line_number":189,"context_line":"        LOG.debug(\"closing encrypted volume %s\", self.dev_path)"},{"line_number":190,"context_line":"        if self._is_crypt_device_available(self.dev_name):"},{"line_number":191,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":192,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_96a43ebb","line":189,"updated":"2017-09-01 12:42:49.000000000","message":"The same, as above","commit_id":"c485561df1a762276c9074c9a724a54f43ea4124"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"},{"line_number":101,"context_line":"                          root_helper\u003dself._root_helper)"},{"line_number":102,"context_line":"        else:"},{"line_number":103,"context_line":"            LOG.debug(\"volume %s is already opened\", self.dev_path)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def _unmangle_volume(self, key, passphrase, **kwargs):"},{"line_number":106,"context_line":"        \"\"\"Workaround for bug#1633518"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_6232a512","line":103,"updated":"2017-09-07 13:44:47.000000000","message":"nit: I think we can improve the message, something in the lines of \"LUKS partition already open at %s\"","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def _close_volume(self, **kwargs):"},{"line_number":188,"context_line":"        \"\"\"Closes the device (effectively removes the dm-crypt mapping).\"\"\""},{"line_number":189,"context_line":"        if self._is_crypt_device_available(self.dev_name):"},{"line_number":190,"context_line":"            LOG.debug(\"closing encrypted volume %s\", self.dev_path)"},{"line_number":191,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":192,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_82011109","line":189,"updated":"2017-09-07 13:44:47.000000000","message":"-1: If we\u0027ve attached it twice, detaching the first volume will effectively break the second one.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"39f6506b426e858bdb16bc9dee812539f63d7c84","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def _close_volume(self, **kwargs):"},{"line_number":188,"context_line":"        \"\"\"Closes the device (effectively removes the dm-crypt mapping).\"\"\""},{"line_number":189,"context_line":"        if self._is_crypt_device_available(self.dev_name):"},{"line_number":190,"context_line":"            LOG.debug(\"closing encrypted volume %s\", self.dev_path)"},{"line_number":191,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":192,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_2af0dd90","line":189,"in_reply_to":"7f287b81_10d43ae8","updated":"2017-09-08 08:32:56.000000000","message":"Well, if I understood it correctly the fix is for cases were you call _open_volume twice, right?  and that would mean that there are 2 different callers, and at one point one of them could decide to detach while the other doesn\u0027t want to detach yet, so that means that the first one would close luks and leave the other to fail.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"87970b3ab4f0da95f443011695e91c13ed50314f","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def _close_volume(self, **kwargs):"},{"line_number":188,"context_line":"        \"\"\"Closes the device (effectively removes the dm-crypt mapping).\"\"\""},{"line_number":189,"context_line":"        if self._is_crypt_device_available(self.dev_name):"},{"line_number":190,"context_line":"            LOG.debug(\"closing encrypted volume %s\", self.dev_path)"},{"line_number":191,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":192,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_a5ae3e7e","line":189,"in_reply_to":"7f287b81_2af0dd90","updated":"2017-09-08 09:25:11.000000000","message":"Let me explain my understanding of attach/detach:\nAssume there is a device(it may be lvm or rbd mapped device) on compute host. attach_volume/_open_volume is to encrypt the disk and then create an encrypted partition from the disk.\n\nNow, if we call attach twice, it won\u0027t create a new encrypted partition again with this patch. And the disk path and encrypted partition path are saved as attributions of LuksEncryptor class instance. There is only one instance, no matter how many time _open_volume is called.\n\nThen when you call detach/_close_volume, there is only one instance you can detach/close.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":20403,"name":"Cheng Li","email":"shcli@cn.ibm.com","username":"Cheng"},"change_message_id":"ed938e450972ef0367b5dcce843a1e4ef140f1ed","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def _close_volume(self, **kwargs):"},{"line_number":188,"context_line":"        \"\"\"Closes the device (effectively removes the dm-crypt mapping).\"\"\""},{"line_number":189,"context_line":"        if self._is_crypt_device_available(self.dev_name):"},{"line_number":190,"context_line":"            LOG.debug(\"closing encrypted volume %s\", self.dev_path)"},{"line_number":191,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":192,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_10d43ae8","line":189,"in_reply_to":"7f287b81_82011109","updated":"2017-09-08 01:39:47.000000000","message":"When you say \u0027attached it twice\u0027, do you mean run the command twice \"cryptsetup luksOpen\"?\nCould you explain more about it? Thanks.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"52f54d92fc00cd7fcccbdf46dd89e3c348df09fa","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def _close_volume(self, **kwargs):"},{"line_number":188,"context_line":"        \"\"\"Closes the device (effectively removes the dm-crypt mapping).\"\"\""},{"line_number":189,"context_line":"        if self._is_crypt_device_available(self.dev_name):"},{"line_number":190,"context_line":"            LOG.debug(\"closing encrypted volume %s\", self.dev_path)"},{"line_number":191,"context_line":"            self._execute(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":192,"context_line":"                          run_as_root\u003dTrue, check_exit_code\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_3acdcc84","line":189,"in_reply_to":"7f287b81_a5ae3e7e","updated":"2017-09-08 10:54:58.000000000","message":"I know it fixes that case, but what about the case were you attach the device to multiple VMs?, that\u0027s what I was referring to when I said you have 2 different callers.\n\nOn multi attach when one of the VMs detaches the volume, you\u0027ll be breaking the other attachment.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":193,"context_line":"                          root_helper\u003dself._root_helper,"},{"line_number":194,"context_line":"                          attempts\u003d3)"},{"line_number":195,"context_line":"        else:"},{"line_number":196,"context_line":"            LOG.debug(\"volume %s is already closed\", self.dev_path)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_1d1fc65b","line":196,"updated":"2017-09-07 13:44:47.000000000","message":"-1: I don\u0027t think this should be silently ignored...","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"}],"os_brick/tests/encryptors/test_luks.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    @mock.patch(\u0027os_brick.executor.Executor._execute\u0027)"},{"line_number":71,"context_line":"    @mock.patch(\u0027os_brick.encryptors.cryptsetup.CryptsetupEncryptor\u0027"},{"line_number":72,"context_line":"                \u0027._is_crypt_device_available\u0027, return_value\u003dFalse)"},{"line_number":73,"context_line":"    def test__open_volume(self, mock_is_opened, mock_execute):"},{"line_number":74,"context_line":"        self.encryptor._open_volume(\"passphrase\")"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_3d95e233","line":72,"updated":"2017-09-07 13:44:47.000000000","message":"nit: It\u0027s usually best to add it to the top so that the new method parameter is added at the end.","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                \u0027._is_crypt_device_available\u0027, return_value\u003dTrue)"},{"line_number":86,"context_line":"    def test_open_volume_exist(self, mock_is_opened, mock_execute):"},{"line_number":87,"context_line":"        self.encryptor._open_volume(\"passphrase\")"},{"line_number":88,"context_line":"        mock_call \u003d mock.call(\u0027cryptsetup\u0027, \u0027luksOpen\u0027, \u0027--key-file\u003d-\u0027,"},{"line_number":89,"context_line":"                              self.dev_path, self.dev_name,"},{"line_number":90,"context_line":"                              process_input\u003d\u0027passphrase\u0027,"},{"line_number":91,"context_line":"                              root_helper\u003dself.root_helper,"},{"line_number":92,"context_line":"                              run_as_root\u003dTrue, check_exit_code\u003dTrue),"},{"line_number":93,"context_line":"        self.assertNotIn(mock_call, mock_execute.mock_calls)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    @mock.patch(\u0027os_brick.executor.Executor._execute\u0027)"},{"line_number":96,"context_line":"    @mock.patch(\u0027os_brick.encryptors.cryptsetup.CryptsetupEncryptor\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_3da722c0","line":93,"range":{"start_line":88,"start_character":0,"end_line":93,"end_character":60},"updated":"2017-09-07 13:44:47.000000000","message":"-1: Use mock\u0027s method instead:\n\n mock_execute.assert_any_call(\u0027cryptsetup\u0027, \u0027luksOpen\u0027, ...)","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    @mock.patch(\u0027os_brick.executor.Executor._execute\u0027)"},{"line_number":96,"context_line":"    @mock.patch(\u0027os_brick.encryptors.cryptsetup.CryptsetupEncryptor\u0027"},{"line_number":97,"context_line":"                \u0027._is_crypt_device_available\u0027, return_value\u003dFalse)"},{"line_number":98,"context_line":"    def test_attach_volume(self, mock_is_opened, mock_execute):"},{"line_number":99,"context_line":"        fake_key \u003d \u00270c84146034e747639b698368807286df\u0027"},{"line_number":100,"context_line":"        self.encryptor._get_key \u003d mock.MagicMock()"},{"line_number":101,"context_line":"        self.encryptor._get_key.return_value \u003d ("}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_ddcc9e6b","line":98,"range":{"start_line":98,"start_character":33,"end_line":98,"end_character":47},"updated":"2017-09-07 13:44:47.000000000","message":"nit: mock_is_open","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5f1e124897cc5cdfd576879163adef4210c5b0d6","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                \u0027._is_crypt_device_available\u0027, return_value\u003dFalse)"},{"line_number":200,"context_line":"    def test_close_volume_closed(self, mock_is_opened, mock_execute):"},{"line_number":201,"context_line":"        self.encryptor._close_volume()"},{"line_number":202,"context_line":"        mock_call \u003d mock.call(\u0027cryptsetup\u0027, \u0027luksClose\u0027, self.dev_name,"},{"line_number":203,"context_line":"                              root_helper\u003dself.root_helper, attempts\u003d3,"},{"line_number":204,"context_line":"                              run_as_root\u003dTrue, check_exit_code\u003dTrue)"},{"line_number":205,"context_line":"        self.assertNotIn(mock_call, mock_execute.mock_calls)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    @mock.patch(\u0027os_brick.executor.Executor._execute\u0027)"},{"line_number":208,"context_line":"    @mock.patch(\u0027os_brick.encryptors.cryptsetup.CryptsetupEncryptor\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_3d0cc2a3","line":205,"range":{"start_line":202,"start_character":0,"end_line":205,"end_character":60},"updated":"2017-09-07 13:44:47.000000000","message":"-1: Same as above, use `assert_any_call` method","commit_id":"e3d03792ba6b94a3f7b9487ea12c14a77eb74e83"}]}