)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e92f13e7bdf2e2e8d79d3feb93bd4bb373cc70c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003cliyong.qiao@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-21 01:13:13 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Catch Exception when detach encryptied volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an attached encryptied volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_18276eb7","line":7,"updated":"2015-08-28 01:16:58.000000000","message":"s/encryptied/encrypted/\n\nAlso, should prefix this commit message title with \"libvirt:\"","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e92f13e7bdf2e2e8d79d3feb93bd4bb373cc70c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Catch Exception when detach encryptied volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an attached encryptied volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch adds exception handler when exception happened to avoid blocking"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_d820169e","line":9,"updated":"2015-08-28 01:16:58.000000000","message":"s/encryptied/encrypted/","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"7e476bf271de7e589acc99f84008f050c802ce2e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003cliyong.qiao@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-10-13 15:38:29 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ignore errorcode\u003d4 when executing cryptsetup remove command in cryptsetup"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an attached encrypted volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7a2fa921_2341fb39","line":7,"updated":"2015-10-13 07:51:57.000000000","message":"Matt recommended to add the prefix \"libvirt: \" on PS1 review.","commit_id":"144bc77efbe4864fb607aa9aa46332b4a291ef76"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a97c8a598e28b05185c59670da0d23402739df85","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003cliyong.qiao@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-10-13 16:03:53 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libivrt: Ignore errorcode\u003d4 when executing `cryptsetup remove` command"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an attached encrypted volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5a2ca52d_3105e5c1","line":7,"updated":"2015-10-13 19:12:11.000000000","message":"Sorry for the confusion, you don\u0027t actually need the libvirt: prefix here now since you\u0027re not changing anything in the nova/virt/libvirt subtree.","commit_id":"ade3e2f83dd25561cc2f97a6371717f67fbe589d"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"a1b62c049942d21f4536a09461fdf75c7a82ab1e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003cliyong.qiao@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-10-13 16:03:53 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libivrt: Ignore errorcode\u003d4 when executing `cryptsetup remove` command"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an attached encrypted volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5a2ca52d_38cffcb1","line":7,"in_reply_to":"5a2ca52d_3105e5c1","updated":"2015-10-14 01:55:47.000000000","message":"Done","commit_id":"ade3e2f83dd25561cc2f97a6371717f67fbe589d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a97c8a598e28b05185c59670da0d23402739df85","unresolved":false,"context_lines":[{"line_number":9,"context_line":"If an attached encrypted volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch adds 4 in check_exit_code when executing `cryptsetup remove`"},{"line_number":13,"context_line":"command to eat that exception."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-Bug: #1482066"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5a2ca52d_b1f9f5ca","line":12,"updated":"2015-10-13 19:12:11.000000000","message":"You should probably explain what 4 means in this context.","commit_id":"ade3e2f83dd25561cc2f97a6371717f67fbe589d"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"a1b62c049942d21f4536a09461fdf75c7a82ab1e","unresolved":false,"context_lines":[{"line_number":9,"context_line":"If an attached encrypted volume is failed to detach from the instance"},{"line_number":10,"context_line":"when deleting that instance, user can not delete that instance at all."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch adds 4 in check_exit_code when executing `cryptsetup remove`"},{"line_number":13,"context_line":"command to eat that exception."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-Bug: #1482066"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5a2ca52d_d8c4c893","line":12,"in_reply_to":"5a2ca52d_b1f9f5ca","updated":"2015-10-14 01:55:47.000000000","message":"Done","commit_id":"ade3e2f83dd25561cc2f97a6371717f67fbe589d"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e92f13e7bdf2e2e8d79d3feb93bd4bb373cc70c","unresolved":false,"context_lines":[{"line_number":912,"context_line":"                volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":913,"context_line":"                encryption \u003d encryptors.get_encryption_metadata("},{"line_number":914,"context_line":"                    context, self._volume_api, volume_id, connection_info)"},{"line_number":915,"context_line":""},{"line_number":916,"context_line":"                if encryption:"},{"line_number":917,"context_line":"                    # The volume must be detached from the VM before"},{"line_number":918,"context_line":"                    # disconnecting it from its encryptor. Otherwise, the"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_d809f62d","side":"PARENT","line":915,"updated":"2015-08-28 01:16:58.000000000","message":"nit: unnecessary whitespace change","commit_id":"36198e4c443059a6f7e8b36545cb5f9b796236fd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e92f13e7bdf2e2e8d79d3feb93bd4bb373cc70c","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        # encryptor may report that the volume is still in use."},{"line_number":919,"context_line":"                        encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":920,"context_line":"                                                               encryption)"},{"line_number":921,"context_line":"                        encryptor.detach_volume(**encryption)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"                self._disconnect_volume(connection_info, disk_dev)"},{"line_number":924,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_984cde43","line":921,"updated":"2015-08-28 01:16:58.000000000","message":"I\u0027m not entirely sure we should be ignoring this error...are there potential leaks/security issues with leaving this encrypted volume sitting around after the instance is deleted?\n\nThe bug says the volume is not active - could we detect that in the detach method of the encryptor and activate the volume then try to detach it?  Or at the very least detect it and return a more useful error / fault for the instance delete?\n\nMaybe I\u0027m over-thinking this.","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"a925bc1243facc6b1606ab2b8cc506ef29616e9d","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        # encryptor may report that the volume is still in use."},{"line_number":919,"context_line":"                        encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":920,"context_line":"                                                               encryption)"},{"line_number":921,"context_line":"                        encryptor.detach_volume(**encryption)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"                self._disconnect_volume(connection_info, disk_dev)"},{"line_number":924,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_6ceb0d79","line":921,"in_reply_to":"da20952f_06d11b23","updated":"2015-08-29 08:59:30.000000000","message":"IIANM \u0027is not active\u0027 is returned if the device mapping doesn\u0027t exist in /dev/mapper. If it exists (or something other exists there with the same name), but wrong, cryptsetup returns empty error message with the same 4 exit code.\n\nThus it looks like that in the case of the error the mapping either was not created, or had been deleted by another delete instance request (which probably had failed later, moving the instance to error state). Unfortunatly the LP bug doesn\u0027t contain steps to reproduce.\n\nThis error looks for me similar to a recent bug with 400 http error at volume_api.detach if a volume is not attached. That bug has been fixed in cinder to do nothing with the volume. It would be consistent to fix this bug inside corresponding custom code as well.\n\nAlso in ths extended try-catch statement if encryptor.detach_volume fails, _disconnect_volume is bypassed, which doesn\u0027t look good for me.","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"b882a59b9f6de6f98f7e7479e2e00df3da3764a4","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        # encryptor may report that the volume is still in use."},{"line_number":919,"context_line":"                        encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":920,"context_line":"                                                               encryption)"},{"line_number":921,"context_line":"                        encryptor.detach_volume(**encryption)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"                self._disconnect_volume(connection_info, disk_dev)"},{"line_number":924,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_dd7f6da9","line":921,"in_reply_to":"da20952f_36851a59","updated":"2015-09-16 16:07:36.000000000","message":"I agree with the above comments: the place to make this change is in the encryptors where specific conditions (e.g., exit status 4 from cryptsetup) can be detected and handled.\n\nThis change would follow similar handling elsewhere in Nova -- e.g., http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/storage/dmcrypt.py#n85","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b450494984f5ecc614431e97f08266ee1f9727b4","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        # encryptor may report that the volume is still in use."},{"line_number":919,"context_line":"                        encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":920,"context_line":"                                                               encryption)"},{"line_number":921,"context_line":"                        encryptor.detach_volume(**encryption)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"                self._disconnect_volume(connection_info, disk_dev)"},{"line_number":924,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_92051712","line":921,"in_reply_to":"da20952f_6ceb0d79","updated":"2015-08-31 22:43:37.000000000","message":"If this is a case of a thing that\u0027s already gone, I\u0027d be in favor of detecting and ignoring that in the encryptor itself rather than doing it here.","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"43344168c0235c82e59ab1f87e5176c5156453bc","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        # encryptor may report that the volume is still in use."},{"line_number":919,"context_line":"                        encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":920,"context_line":"                                                               encryption)"},{"line_number":921,"context_line":"                        encryptor.detach_volume(**encryption)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"                self._disconnect_volume(connection_info, disk_dev)"},{"line_number":924,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_36851a59","line":921,"in_reply_to":"da20952f_92051712","updated":"2015-09-01 01:57:28.000000000","message":"hi Matt, if you prefer do it in encryptor, then we need to handle all kinds of encryptor.","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"ad2605531f34acb6b76ae731778754f7c79ac2ec","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        # encryptor may report that the volume is still in use."},{"line_number":919,"context_line":"                        encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":920,"context_line":"                                                               encryption)"},{"line_number":921,"context_line":"                        encryptor.detach_volume(**encryption)"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"                self._disconnect_volume(connection_info, disk_dev)"},{"line_number":924,"context_line":"            except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_06d11b23","line":921,"in_reply_to":"da20952f_984cde43","updated":"2015-08-28 21:43:31.000000000","message":"The failure could mean that the data remains accessible on the compute host indefinitely -- that\u0027s a security concern!","commit_id":"abb97131f01c99966c73d96bbab420139347fe1c"}]}
