)]}'
{"cinder/api/v2/volumes.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9498a7981f5c44000bdce84a771bd18db385cd9d","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        context \u003d req.environ[\u0027cinder.context\u0027]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        cascade \u003d False"},{"line_number":203,"context_line":"        if \u0027cascade\u0027 in req.params:"},{"line_number":204,"context_line":"            cascade \u003d utils.get_bool_param(\u0027cascade\u0027, req.params)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        LOG.info(_LI(\"Delete volume with id: %s\"), id, context\u003dcontext)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_07043f13","line":203,"updated":"2016-02-15 10:36:41.000000000","message":"-1: We don\u0027t need these 2 lines, get_bool_param already returns False if \u0027cascade\u0027 doesn\u0027t exist.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3a026ca98bef23072b2d5f09b7e9ca46eb5effda","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        context \u003d req.environ[\u0027cinder.context\u0027]"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        cascade \u003d False"},{"line_number":203,"context_line":"        if \u0027cascade\u0027 in req.params:"},{"line_number":204,"context_line":"            cascade \u003d utils.get_bool_param(\u0027cascade\u0027, req.params)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        LOG.info(_LI(\"Delete volume with id: %s\"), id, context\u003dcontext)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_c891ed48","line":203,"in_reply_to":"dae33548_07043f13","updated":"2016-02-22 20:46:33.000000000","message":"Done","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"e2b9e0c98a6dfda8d14465945a89bfeffe8b0b2a","unresolved":false,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        cascade \u003d utils.get_bool_param(\u0027cascade\u0027, req.params)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        LOG.info(_LI(\"Delete volume with id: %s\"), id, context\u003dcontext)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        try:"},{"line_number":207,"context_line":"            volume \u003d self.volume_api.get(context, id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_dc24450e","line":204,"updated":"2016-02-23 22:04:47.000000000","message":"If you end up doing another update - it might be useful to log the value of cascade here as well.","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"fd6ecb69a4ac7af72eb95b90c36f2b701cc44e4a","unresolved":false,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        cascade \u003d utils.get_bool_param(\u0027cascade\u0027, req.params)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        LOG.info(_LI(\"Delete volume with id: %s\"), id, context\u003dcontext)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        try:"},{"line_number":207,"context_line":"            volume \u003d self.volume_api.get(context, id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_738ff174","line":204,"in_reply_to":"bae84128_00dc4d44","updated":"2016-02-24 16:17:50.000000000","message":"Ah, makes sense. Thanks!","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4267e5f00989f73562922fdcf43711ec42ae7a8b","unresolved":false,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        cascade \u003d utils.get_bool_param(\u0027cascade\u0027, req.params)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        LOG.info(_LI(\"Delete volume with id: %s\"), id, context\u003dcontext)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        try:"},{"line_number":207,"context_line":"            volume \u003d self.volume_api.get(context, id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_00dc4d44","line":204,"in_reply_to":"bae84128_dc24450e","updated":"2016-02-24 16:07:05.000000000","message":"You should already be able to see the cascade parameter in the call at debug level, so I think this is mostly ok.  I can add it if desired, though.","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c88d47a07bd34c6b6b4cb831b7760fbe91f8df39","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027in-use\u0027, \u0027available\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_53d58de2","line":1821,"updated":"2016-02-24 16:24:19.000000000","message":"-1: deletable_status should be \u0027available\u0027, \u0027error\u0027, \u0027error_deleting\u0027","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6fa8cf54c3aec710db24f274a586d555261b5a3c","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027in-use\u0027, \u0027available\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_3699d7c4","line":1821,"in_reply_to":"bae84128_135ca549","updated":"2016-02-24 16:30:29.000000000","message":"Then you also have to update it on the api, since those are the only statuses you are expecting when doing the conditional update of the snapshots.","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"8b28dd6ebdb2a54334f91f1dd607ebff8f46548c","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027in-use\u0027, \u0027available\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_b6068783","line":1821,"in_reply_to":"bae84128_3699d7c4","updated":"2016-02-24 16:34:43.000000000","message":"Sorry, error_restoring and error_extending was for the volume status -- for snaps it should just be available and error.","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e9b6cea669a0f9e29e7bc0dc66b21a49893dbd13","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027in-use\u0027, \u0027available\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_135ca549","line":1821,"in_reply_to":"bae84128_53d58de2","updated":"2016-02-24 16:27:27.000000000","message":"I don\u0027t think error_deleting is correct, you have to do a reset-state/force-delete for that.\n\n\"error\" should be there.\n\nAccording to the volume delete API code, so should \"error_restoring\" and \"error_extending\".","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9365920d857e1bde2e9037a6beafecd99f5054fe","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027in-use\u0027, \u0027available\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_515c6518","line":1821,"in_reply_to":"bae84128_b6068783","updated":"2016-02-24 18:54:59.000000000","message":"Done","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c88d47a07bd34c6b6b4cb831b7760fbe91f8df39","unresolved":false,"context_lines":[{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"},{"line_number":1825,"context_line":"             models.Snapshot.status.notin_(deletable_statuses)))"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":""},{"line_number":1828,"context_line":"def volume_has_attachments_filter():"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_33fc49b7","line":1825,"updated":"2016-02-24 16:24:19.000000000","message":"-1: This is missing the cgsnapshot_id check","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9365920d857e1bde2e9037a6beafecd99f5054fe","unresolved":false,"context_lines":[{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"},{"line_number":1825,"context_line":"             models.Snapshot.status.notin_(deletable_statuses)))"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":""},{"line_number":1828,"context_line":"def volume_has_attachments_filter():"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_b19df1bd","line":1825,"in_reply_to":"bae84128_33fc49b7","updated":"2016-02-24 18:54:59.000000000","message":"Done","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":14084,"name":"Einst Crazy","email":"yu.changcai@99cloud.net","username":"Einst"},"change_message_id":"5b16abfdc6fd2ea9483166e6653341ba555f4b90","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027available\u0027, \u0027error\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_1c8c9834","line":1821,"updated":"2016-02-25 01:53:46.000000000","message":"I think error status may come when the volume is in-use, and snapshot or other action failed. If in this situation, I think error is not deletable","commit_id":"fc74cbb3cceed24b06e4d57fbdd8d4bfc618f371"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ce2b880a5cf54a52c1adf354a45badecfc5c634d","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":""},{"line_number":1820,"context_line":"def volume_has_undeletable_snapshots_filter():"},{"line_number":1821,"context_line":"    deletable_statuses \u003d [\u0027available\u0027, \u0027error\u0027]"},{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_886df0ce","line":1821,"in_reply_to":"bae84128_1c8c9834","updated":"2016-02-25 11:15:14.000000000","message":"Why? You should be easily able to remove a snapshot that\u0027s in \"error\" state.","commit_id":"fc74cbb3cceed24b06e4d57fbdd8d4bfc618f371"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ce2b880a5cf54a52c1adf354a45badecfc5c634d","unresolved":false,"context_lines":[{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"},{"line_number":1825,"context_line":"             models.Snapshot.cgsnapshot_id !\u003d None,  # noqa: !\u003d None"},{"line_number":1826,"context_line":"             models.Snapshot.status.notin_(deletable_statuses)))"},{"line_number":1827,"context_line":""},{"line_number":1828,"context_line":""},{"line_number":1829,"context_line":"def volume_has_attachments_filter():"}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_88c970af","line":1826,"range":{"start_line":1825,"start_character":0,"end_line":1826,"end_character":64},"updated":"2016-02-25 11:15:14.000000000","message":"Shouldn\u0027t we have or_ between these two?","commit_id":"fc74cbb3cceed24b06e4d57fbdd8d4bfc618f371"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"abe69fa655cb4e8e795b1ddfe9f05800967494dc","unresolved":false,"context_lines":[{"line_number":1822,"context_line":"    return sql.exists().where("},{"line_number":1823,"context_line":"        and_(models.Volume.id \u003d\u003d models.Snapshot.volume_id,"},{"line_number":1824,"context_line":"             ~models.Snapshot.deleted,"},{"line_number":1825,"context_line":"             models.Snapshot.cgsnapshot_id !\u003d None,  # noqa: !\u003d None"},{"line_number":1826,"context_line":"             models.Snapshot.status.notin_(deletable_statuses)))"},{"line_number":1827,"context_line":""},{"line_number":1828,"context_line":""},{"line_number":1829,"context_line":"def volume_has_attachments_filter():"}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_9b8b6768","line":1826,"range":{"start_line":1825,"start_character":0,"end_line":1826,"end_character":64},"in_reply_to":"bae84128_88c970af","updated":"2016-02-25 15:42:13.000000000","message":"Yes, good call.","commit_id":"fc74cbb3cceed24b06e4d57fbdd8d4bfc618f371"}],"cinder/tests/unit/test_volume_rpcapi.py":[{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"1ed1c4820302ea86e6fa2645ea0aae737ff98e7f","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                              unmanage_only\u003dFalse,"},{"line_number":293,"context_line":"                              cascade\u003dFalse,"},{"line_number":294,"context_line":"                              version\u003d\u00271.39\u0027)"},{"line_number":295,"context_line":"        can_send_version.assert_any_call(\u00271.39\u0027)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    @mock.patch(\u0027oslo_messaging.RPCClient.can_send_version\u0027,"},{"line_number":298,"context_line":"                return_value\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_2b94d788","line":295,"range":{"start_line":295,"start_character":0,"end_line":295,"end_character":48},"updated":"2016-02-24 15:45:31.000000000","message":"ohhh noooooz, I\u0027m about to have another RPC version stolen from me :(","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"}],"cinder/volume/api.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12507d660949001dc362cf86b75738c481e4ccdc","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if not cascade:"},{"line_number":404,"context_line":"            # Don\u0027t allow deletion of volume with snapshots unless"},{"line_number":405,"context_line":"            # cascade was requested."},{"line_number":406,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"},{"line_number":407,"context_line":"        values \u003d {\u0027status\u0027: \u0027deleting\u0027, \u0027terminated_at\u0027: timeutils.utcnow()}"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        result \u003d volume.conditional_update(values, expected, filters)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_5dd9d855","line":406,"range":{"start_line":403,"start_character":0,"end_line":406,"end_character":57},"updated":"2016-02-01 22:22:16.000000000","message":"I believe we should also set the snapshots in \"deleting\" state in the API to make sure they won\u0027t be used for example to create a new volume. Otherwise we get a race condition here.\n\nSo first - we should change the volume status (to make sure no new snapshots will be created for a volume). Then we can switch snapshots status. This may be even better if we\u0027ve managed to do that in one call or transaction, so we could rollback everything.\n\nGorka is more knowledgeable on these kinds of problems, so I\u0027ll add him to the review.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9498a7981f5c44000bdce84a771bd18db385cd9d","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if not cascade:"},{"line_number":404,"context_line":"            # Don\u0027t allow deletion of volume with snapshots unless"},{"line_number":405,"context_line":"            # cascade was requested."},{"line_number":406,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"},{"line_number":407,"context_line":"        values \u003d {\u0027status\u0027: \u0027deleting\u0027, \u0027terminated_at\u0027: timeutils.utcnow()}"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        result \u003d volume.conditional_update(values, expected, filters)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_e79f9b5d","line":406,"range":{"start_line":403,"start_character":0,"end_line":406,"end_character":57},"in_reply_to":"7a5de9d1_5dd9d855","updated":"2016-02-15 10:36:41.000000000","message":"Michal is correct, since PostgreSQL doesn\u0027t support multi-table updates, we need to do 2 queries inside a transaction.\nAnd I would also add a filter to the volume conditional update when doing cascade that ensures that all snapshots have the right status, otherwise you would need a third request to check that all snapshots are set to deleting status.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3a026ca98bef23072b2d5f09b7e9ca46eb5effda","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if not cascade:"},{"line_number":404,"context_line":"            # Don\u0027t allow deletion of volume with snapshots unless"},{"line_number":405,"context_line":"            # cascade was requested."},{"line_number":406,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"},{"line_number":407,"context_line":"        values \u003d {\u0027status\u0027: \u0027deleting\u0027, \u0027terminated_at\u0027: timeutils.utcnow()}"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"        result \u003d volume.conditional_update(values, expected, filters)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_99afaf29","line":406,"range":{"start_line":403,"start_character":0,"end_line":406,"end_character":57},"in_reply_to":"dae33548_e79f9b5d","updated":"2016-02-22 20:46:33.000000000","message":"Done","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"bd900c0f9afd107725868442a45a2fbbdd34bd52","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            # Don\u0027t allow deletion of volume with snapshots"},{"line_number":408,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_626bfd85","line":405,"range":{"start_line":403,"start_character":0,"end_line":405,"end_character":69},"updated":"2016-02-23 09:48:19.000000000","message":"This doesn\u0027t protect us from the situation described by me in line 431. Snapshot state change may still happen between UPDATE queries if you don\u0027t do that in a single transaction.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"49a6dfaca5fa4373fe645e299b372e6dd4b16d98","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            # Don\u0027t allow deletion of volume with snapshots"},{"line_number":408,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_74043749","line":405,"range":{"start_line":403,"start_character":0,"end_line":405,"end_character":69},"in_reply_to":"bae84128_3ec8d0a7","updated":"2016-02-23 20:09:46.000000000","message":"Done","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b2c07a066aceafee37c60fc36ab5e6f5d226a5d4","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            # Don\u0027t allow deletion of volume with snapshots"},{"line_number":408,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_7b438a42","line":405,"range":{"start_line":403,"start_character":0,"end_line":405,"end_character":69},"in_reply_to":"bae84128_626bfd85","updated":"2016-02-23 18:18:41.000000000","message":"Is it true that such a transition can happen when the volume is already marked as \"deleting\"?  I\u0027m not sure what the case is here that you are referring to.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"1502e29bb0496ea087c6664a1e581500df9fa4b2","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            # Don\u0027t allow deletion of volume with snapshots"},{"line_number":408,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_db1e166f","line":405,"range":{"start_line":403,"start_character":0,"end_line":405,"end_character":69},"in_reply_to":"bae84128_7b438a42","updated":"2016-02-23 18:38:10.000000000","message":"Ah, yes, you\u0027re probably right. Snapshots don\u0027t have a lot of states - something like creating, available, deleting, deleted and error (+error_deleting?). In case something is being created *from* snapshot - it doesn\u0027t change it\u0027s state.\n\nSo the only strange situation would be that between line #411 and one of the executions of #427 a snapshot would begin to be deleted. I don\u0027t know if we want to ignore an error when line #427 failed due to status being \"deleted\"… I don\u0027t even know if we can?","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b02f1c70233a15bd22224f3582a78c7d184c3c78","unresolved":false,"context_lines":[{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"},{"line_number":406,"context_line":"        else:"},{"line_number":407,"context_line":"            # Don\u0027t allow deletion of volume with snapshots"},{"line_number":408,"context_line":"            filters \u003d [~db.volume_has_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_3ec8d0a7","line":405,"range":{"start_line":403,"start_character":0,"end_line":405,"end_character":69},"in_reply_to":"bae84128_db1e166f","updated":"2016-02-23 18:42:18.000000000","message":"I wonder if the right answer isn\u0027t to allow \u0027deleted\u0027 there anyway?  That doesn\u0027t change the contract with drivers AFAIK.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"4ad2c203560babd938aad68098f3f681efa36c70","unresolved":false,"context_lines":[{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        if cascade:"},{"line_number":422,"context_line":"            values \u003d {\u0027status\u0027: \u0027deleting\u0027}"},{"line_number":423,"context_line":"            expected \u003d {\u0027status\u0027: (\u0027available\u0027, \u0027error\u0027)}"},{"line_number":424,"context_line":"            snapshots \u003d objects.snapshot.SnapshotList.get_all_for_volume("},{"line_number":425,"context_line":"                context, volume.id)"},{"line_number":426,"context_line":"            for s in snapshots:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_1cd7fbed","line":423,"range":{"start_line":423,"start_character":12,"end_line":423,"end_character":57},"updated":"2016-02-23 07:12:00.000000000","message":"Should \"expected\" here includes {\u0027cgsnapshot_id\u0027:None}? I saw it is needed at the top of delete_snapshot() method in cinder/volume/api.py.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"bd900c0f9afd107725868442a45a2fbbdd34bd52","unresolved":false,"context_lines":[{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        if cascade:"},{"line_number":422,"context_line":"            values \u003d {\u0027status\u0027: \u0027deleting\u0027}"},{"line_number":423,"context_line":"            expected \u003d {\u0027status\u0027: (\u0027available\u0027, \u0027error\u0027)}"},{"line_number":424,"context_line":"            snapshots \u003d objects.snapshot.SnapshotList.get_all_for_volume("},{"line_number":425,"context_line":"                context, volume.id)"},{"line_number":426,"context_line":"            for s in snapshots:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_a2918549","line":423,"range":{"start_line":423,"start_character":12,"end_line":423,"end_character":57},"in_reply_to":"bae84128_1cd7fbed","updated":"2016-02-23 09:48:19.000000000","message":"+1, this is fair concern.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"49a6dfaca5fa4373fe645e299b372e6dd4b16d98","unresolved":false,"context_lines":[{"line_number":420,"context_line":""},{"line_number":421,"context_line":"        if cascade:"},{"line_number":422,"context_line":"            values \u003d {\u0027status\u0027: \u0027deleting\u0027}"},{"line_number":423,"context_line":"            expected \u003d {\u0027status\u0027: (\u0027available\u0027, \u0027error\u0027)}"},{"line_number":424,"context_line":"            snapshots \u003d objects.snapshot.SnapshotList.get_all_for_volume("},{"line_number":425,"context_line":"                context, volume.id)"},{"line_number":426,"context_line":"            for s in snapshots:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_34a90f91","line":423,"range":{"start_line":423,"start_character":12,"end_line":423,"end_character":57},"in_reply_to":"bae84128_a2918549","updated":"2016-02-23 20:09:46.000000000","message":"Done","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"bd900c0f9afd107725868442a45a2fbbdd34bd52","unresolved":false,"context_lines":[{"line_number":426,"context_line":"            for s in snapshots:"},{"line_number":427,"context_line":"                result \u003d s.conditional_update(values, expected, filters)"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"                if not result:"},{"line_number":430,"context_line":"                    msg \u003d _(\u0027Failed to update snapshot.\u0027)"},{"line_number":431,"context_line":"                    raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        cache \u003d image_cache.ImageVolumeCache(self.db, self)"},{"line_number":434,"context_line":"        entry \u003d cache.get_by_image_volume(context, volume.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_62a93d24","line":431,"range":{"start_line":429,"start_character":0,"end_line":431,"end_character":61},"updated":"2016-02-23 09:48:19.000000000","message":"So it may very easily happen that we end up with volume in deleting (line 411), but some of it\u0027s snapshots left? Shouldn\u0027t we revert volume here to error_deleting?","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"49a6dfaca5fa4373fe645e299b372e6dd4b16d98","unresolved":false,"context_lines":[{"line_number":426,"context_line":"            for s in snapshots:"},{"line_number":427,"context_line":"                result \u003d s.conditional_update(values, expected, filters)"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"                if not result:"},{"line_number":430,"context_line":"                    msg \u003d _(\u0027Failed to update snapshot.\u0027)"},{"line_number":431,"context_line":"                    raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        cache \u003d image_cache.ImageVolumeCache(self.db, self)"},{"line_number":434,"context_line":"        entry \u003d cache.get_by_image_volume(context, volume.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_d4f4235c","line":431,"range":{"start_line":429,"start_character":0,"end_line":431,"end_character":61},"in_reply_to":"bae84128_28153a48","updated":"2016-02-23 20:09:46.000000000","message":"Done","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b2c07a066aceafee37c60fc36ab5e6f5d226a5d4","unresolved":false,"context_lines":[{"line_number":426,"context_line":"            for s in snapshots:"},{"line_number":427,"context_line":"                result \u003d s.conditional_update(values, expected, filters)"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"                if not result:"},{"line_number":430,"context_line":"                    msg \u003d _(\u0027Failed to update snapshot.\u0027)"},{"line_number":431,"context_line":"                    raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        cache \u003d image_cache.ImageVolumeCache(self.db, self)"},{"line_number":434,"context_line":"        entry \u003d cache.get_by_image_volume(context, volume.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_28153a48","line":431,"range":{"start_line":429,"start_character":0,"end_line":431,"end_character":61},"in_reply_to":"bae84128_62a93d24","updated":"2016-02-23 18:18:41.000000000","message":"Yes, I think we should.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c88d47a07bd34c6b6b4cb831b7760fbe91f8df39","unresolved":false,"context_lines":[{"line_number":399,"context_line":"            expected[\u0027status\u0027] \u003d (\u0027available\u0027, \u0027error\u0027, \u0027error_restoring\u0027,"},{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_c04c25a0","line":402,"updated":"2016-02-24 16:24:19.000000000","message":"nit: We don\u0027t need to initialize filters, it is going to get a value in any case.","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"9365920d857e1bde2e9037a6beafecd99f5054fe","unresolved":false,"context_lines":[{"line_number":399,"context_line":"            expected[\u0027status\u0027] \u003d (\u0027available\u0027, \u0027error\u0027, \u0027error_restoring\u0027,"},{"line_number":400,"context_line":"                                  \u0027error_extending\u0027)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        filters \u003d []"},{"line_number":403,"context_line":"        if cascade:"},{"line_number":404,"context_line":"            # Allow deletion if all snapshots are in an expected state"},{"line_number":405,"context_line":"            filters \u003d [~db.volume_has_undeletable_snapshots_filter()]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bae84128_91870d60","line":402,"in_reply_to":"bae84128_c04c25a0","updated":"2016-02-24 18:54:59.000000000","message":"Done","commit_id":"2268309d47558930df6e3e23566f47a0737bb98b"}],"cinder/volume/manager.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12507d660949001dc362cf86b75738c481e4ccdc","unresolved":false,"context_lines":[{"line_number":635,"context_line":"        if unmanage_only and cascade:"},{"line_number":636,"context_line":"            # This could be done, but is ruled out for now just"},{"line_number":637,"context_line":"            # for simplicity."},{"line_number":638,"context_line":"            raise exception.InvalidVolume("},{"line_number":639,"context_line":"                reason\u003d_(\"Unmanage and cascade delete options \""},{"line_number":640,"context_line":"                         \"are mutually exclusive.\"))"},{"line_number":641,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_222e37fb","line":638,"range":{"start_line":638,"start_character":28,"end_line":638,"end_character":42},"updated":"2016-02-01 22:22:16.000000000","message":"Is this a proper exception raised?","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d83f9faa96db055efadad0aee7cfbbe38054cb77","unresolved":false,"context_lines":[{"line_number":635,"context_line":"        if unmanage_only and cascade:"},{"line_number":636,"context_line":"            # This could be done, but is ruled out for now just"},{"line_number":637,"context_line":"            # for simplicity."},{"line_number":638,"context_line":"            raise exception.InvalidVolume("},{"line_number":639,"context_line":"                reason\u003d_(\"Unmanage and cascade delete options \""},{"line_number":640,"context_line":"                         \"are mutually exclusive.\"))"},{"line_number":641,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a57f1b5_1e4bba2b","line":638,"range":{"start_line":638,"start_character":28,"end_line":638,"end_character":42},"in_reply_to":"7a5de9d1_222e37fb","updated":"2016-02-16 16:41:35.000000000","message":"This is used for a handful of similar failures, so I think so.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9498a7981f5c44000bdce84a771bd18db385cd9d","unresolved":false,"context_lines":[{"line_number":658,"context_line":"            if unmanage_only:"},{"line_number":659,"context_line":"                self.driver.unmanage(volume)"},{"line_number":660,"context_line":"            elif cascade:"},{"line_number":661,"context_line":"                LOG.debug(\u0027Performing cascade delete.\u0027)"},{"line_number":662,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":663,"context_line":"                                                                    volume.id)"},{"line_number":664,"context_line":"                for s in snapshots:"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_0709df95","line":661,"updated":"2016-02-15 10:36:41.000000000","message":"nit: Maybe add the volume id","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12507d660949001dc362cf86b75738c481e4ccdc","unresolved":false,"context_lines":[{"line_number":662,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":663,"context_line":"                                                                    volume.id)"},{"line_number":664,"context_line":"                for s in snapshots:"},{"line_number":665,"context_line":"                    s.status \u003d \u0027deleting\u0027"},{"line_number":666,"context_line":"                    s.save()"},{"line_number":667,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_c23de32b","line":666,"range":{"start_line":665,"start_character":0,"end_line":666,"end_character":28},"updated":"2016-02-01 22:22:16.000000000","message":"This should be done using compare-and-swap technique to avoid race conditions. I personally think that status change should happen in the API layer. More in the inline comment there.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9498a7981f5c44000bdce84a771bd18db385cd9d","unresolved":false,"context_lines":[{"line_number":662,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":663,"context_line":"                                                                    volume.id)"},{"line_number":664,"context_line":"                for s in snapshots:"},{"line_number":665,"context_line":"                    s.status \u003d \u0027deleting\u0027"},{"line_number":666,"context_line":"                    s.save()"},{"line_number":667,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_e7495bd2","line":666,"range":{"start_line":665,"start_character":0,"end_line":666,"end_character":28},"in_reply_to":"7a5de9d1_c23de32b","updated":"2016-02-15 10:36:41.000000000","message":"I agree, this should be done in the API layer.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3a026ca98bef23072b2d5f09b7e9ca46eb5effda","unresolved":false,"context_lines":[{"line_number":662,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":663,"context_line":"                                                                    volume.id)"},{"line_number":664,"context_line":"                for s in snapshots:"},{"line_number":665,"context_line":"                    s.status \u003d \u0027deleting\u0027"},{"line_number":666,"context_line":"                    s.save()"},{"line_number":667,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_d9a5b747","line":666,"range":{"start_line":665,"start_character":0,"end_line":666,"end_character":28},"in_reply_to":"dae33548_e7495bd2","updated":"2016-02-22 20:46:33.000000000","message":"Done","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12507d660949001dc362cf86b75738c481e4ccdc","unresolved":false,"context_lines":[{"line_number":664,"context_line":"                for s in snapshots:"},{"line_number":665,"context_line":"                    s.status \u003d \u0027deleting\u0027"},{"line_number":666,"context_line":"                    s.save()"},{"line_number":667,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":670,"context_line":"                                                                    volume.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_c2d963f4","line":667,"range":{"start_line":667,"start_character":0,"end_line":667,"end_character":52},"updated":"2016-02-01 22:22:16.000000000","message":"This may hang for a while if for example a new volume is being created from the snapshot. I don\u0027t think that\u0027s a problem but want to signal it.\n\nShouldn\u0027t we also catch and repack exceptions from delete_snapshot here?","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3a026ca98bef23072b2d5f09b7e9ca46eb5effda","unresolved":false,"context_lines":[{"line_number":664,"context_line":"                for s in snapshots:"},{"line_number":665,"context_line":"                    s.status \u003d \u0027deleting\u0027"},{"line_number":666,"context_line":"                    s.save()"},{"line_number":667,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":670,"context_line":"                                                                    volume.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_3965c31d","line":667,"range":{"start_line":667,"start_character":0,"end_line":667,"end_character":52},"in_reply_to":"7a5de9d1_c2d963f4","updated":"2016-02-22 20:46:33.000000000","message":"We don\u0027t for delete_volume, so I\u0027m not sure why we should.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12507d660949001dc362cf86b75738c481e4ccdc","unresolved":false,"context_lines":[{"line_number":673,"context_line":"                        LOG.error(_LE(\"A snapshot failed to delete during \""},{"line_number":674,"context_line":"                                      \"cascade delete for volume %s.\"),"},{"line_number":675,"context_line":"                                  volume.id)"},{"line_number":676,"context_line":"                        raise exception.VolumeIsBusy(volume_name\u003dvolume.name)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"                LOG.debug(\u0027Snapshots deleted, issuing volume delete\u0027)"},{"line_number":679,"context_line":"                self.driver.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_622e6fa6","line":676,"range":{"start_line":676,"start_character":0,"end_line":676,"end_character":77},"updated":"2016-02-01 22:22:16.000000000","message":"Shouldn\u0027t we reset volume\u0027s state to error_deleting in that case?","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9498a7981f5c44000bdce84a771bd18db385cd9d","unresolved":false,"context_lines":[{"line_number":673,"context_line":"                        LOG.error(_LE(\"A snapshot failed to delete during \""},{"line_number":674,"context_line":"                                      \"cascade delete for volume %s.\"),"},{"line_number":675,"context_line":"                                  volume.id)"},{"line_number":676,"context_line":"                        raise exception.VolumeIsBusy(volume_name\u003dvolume.name)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"                LOG.debug(\u0027Snapshots deleted, issuing volume delete\u0027)"},{"line_number":679,"context_line":"                self.driver.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_c7e2579f","line":676,"range":{"start_line":676,"start_character":0,"end_line":676,"end_character":77},"in_reply_to":"7a5de9d1_622e6fa6","updated":"2016-02-15 10:36:41.000000000","message":"We should either set it to \u0027error_deleting\u0027 or store the previous volume status in the DB and then recover it here.  I think \u0027error_deleting\u0027 is more appropriate.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3a026ca98bef23072b2d5f09b7e9ca46eb5effda","unresolved":false,"context_lines":[{"line_number":673,"context_line":"                        LOG.error(_LE(\"A snapshot failed to delete during \""},{"line_number":674,"context_line":"                                      \"cascade delete for volume %s.\"),"},{"line_number":675,"context_line":"                                  volume.id)"},{"line_number":676,"context_line":"                        raise exception.VolumeIsBusy(volume_name\u003dvolume.name)"},{"line_number":677,"context_line":""},{"line_number":678,"context_line":"                LOG.debug(\u0027Snapshots deleted, issuing volume delete\u0027)"},{"line_number":679,"context_line":"                self.driver.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_041cd668","line":676,"range":{"start_line":676,"start_character":0,"end_line":676,"end_character":77},"in_reply_to":"dae33548_c7e2579f","updated":"2016-02-22 20:46:33.000000000","message":"Done","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"bd900c0f9afd107725868442a45a2fbbdd34bd52","unresolved":false,"context_lines":[{"line_number":649,"context_line":"        if unmanage_only and cascade:"},{"line_number":650,"context_line":"            # This could be done, but is ruled out for now just"},{"line_number":651,"context_line":"            # for simplicity."},{"line_number":652,"context_line":"            raise exception.InvalidVolume("},{"line_number":653,"context_line":"                reason\u003d_(\"Unmanage and cascade delete options \""},{"line_number":654,"context_line":"                         \"are mutually exclusive.\"))"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        # The status \u0027deleting\u0027 is not included, because it only applies to"},{"line_number":657,"context_line":"        # the source volume to be deleted after a migration. No quota"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_fd4a0e25","line":654,"range":{"start_line":652,"start_character":0,"end_line":654,"end_character":52},"updated":"2016-02-23 09:48:19.000000000","message":"Repeating the concern from previous patchset.\n\nIn this module InvalidVolume is raised only in case some of the properties of the volume are blocking the operation. And this is clearly an error caused by passing wrong arguments combination.\n\nI understand this shouldn\u0027t ever happen with the REST API changes you\u0027re providing, but still this doesn\u0027t seem the right exception to me.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"49a6dfaca5fa4373fe645e299b372e6dd4b16d98","unresolved":false,"context_lines":[{"line_number":649,"context_line":"        if unmanage_only and cascade:"},{"line_number":650,"context_line":"            # This could be done, but is ruled out for now just"},{"line_number":651,"context_line":"            # for simplicity."},{"line_number":652,"context_line":"            raise exception.InvalidVolume("},{"line_number":653,"context_line":"                reason\u003d_(\"Unmanage and cascade delete options \""},{"line_number":654,"context_line":"                         \"are mutually exclusive.\"))"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        # The status \u0027deleting\u0027 is not included, because it only applies to"},{"line_number":657,"context_line":"        # the source volume to be deleted after a migration. No quota"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_d4c0a352","line":654,"range":{"start_line":652,"start_character":0,"end_line":654,"end_character":52},"in_reply_to":"bae84128_52e5ced9","updated":"2016-02-23 20:09:46.000000000","message":"Done","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b2c07a066aceafee37c60fc36ab5e6f5d226a5d4","unresolved":false,"context_lines":[{"line_number":649,"context_line":"        if unmanage_only and cascade:"},{"line_number":650,"context_line":"            # This could be done, but is ruled out for now just"},{"line_number":651,"context_line":"            # for simplicity."},{"line_number":652,"context_line":"            raise exception.InvalidVolume("},{"line_number":653,"context_line":"                reason\u003d_(\"Unmanage and cascade delete options \""},{"line_number":654,"context_line":"                         \"are mutually exclusive.\"))"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        # The status \u0027deleting\u0027 is not included, because it only applies to"},{"line_number":657,"context_line":"        # the source volume to be deleted after a migration. No quota"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_52e5ced9","line":654,"range":{"start_line":652,"start_character":0,"end_line":654,"end_character":52},"in_reply_to":"bae84128_fd4a0e25","updated":"2016-02-23 18:18:41.000000000","message":"Sounds right, I think I\u0027ll change this to just exception.Invalid.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"bd900c0f9afd107725868442a45a2fbbdd34bd52","unresolved":false,"context_lines":[{"line_number":688,"context_line":""},{"line_number":689,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":692,"context_line":"                                                                    volume.id)"},{"line_number":693,"context_line":"                for s in snapshots:"},{"line_number":694,"context_line":"                    if s.status \u003d\u003d \u0027available\u0027 or s.status \u003d\u003d \u0027error_deleting\u0027:"},{"line_number":695,"context_line":"                        LOG.error(_LE(\"A snapshot failed to delete during \""},{"line_number":696,"context_line":"                                      \"cascade delete for volume %s.\"),"},{"line_number":697,"context_line":"                                  volume.id)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"                        self._clear_db(context, is_migrating_dest, volume,"},{"line_number":700,"context_line":"                                       \u0027error_deleting\u0027)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"                        raise exception.VolumeIsBusy(volume_name\u003dvolume.name)"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"                LOG.debug(\u0027Snapshots deleted, issuing volume delete\u0027)"},{"line_number":705,"context_line":"                self.driver.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_7dc1de53","line":702,"range":{"start_line":691,"start_character":0,"end_line":702,"end_character":77},"updated":"2016-02-23 09:48:19.000000000","message":"Why are we doing this second time…? No new snapshots should be allowed to be created as we have already set the volume to \"deleting\" in the API, which should block creating new snaps.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b2c07a066aceafee37c60fc36ab5e6f5d226a5d4","unresolved":false,"context_lines":[{"line_number":688,"context_line":""},{"line_number":689,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":692,"context_line":"                                                                    volume.id)"},{"line_number":693,"context_line":"                for s in snapshots:"},{"line_number":694,"context_line":"                    if s.status \u003d\u003d \u0027available\u0027 or s.status \u003d\u003d \u0027error_deleting\u0027:"},{"line_number":695,"context_line":"                        LOG.error(_LE(\"A snapshot failed to delete during \""},{"line_number":696,"context_line":"                                      \"cascade delete for volume %s.\"),"},{"line_number":697,"context_line":"                                  volume.id)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"                        self._clear_db(context, is_migrating_dest, volume,"},{"line_number":700,"context_line":"                                       \u0027error_deleting\u0027)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"                        raise exception.VolumeIsBusy(volume_name\u003dvolume.name)"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"                LOG.debug(\u0027Snapshots deleted, issuing volume delete\u0027)"},{"line_number":705,"context_line":"                self.driver.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_b21b7ac9","line":702,"range":{"start_line":691,"start_character":0,"end_line":702,"end_character":77},"in_reply_to":"bae84128_7dc1de53","updated":"2016-02-23 18:18:41.000000000","message":"I believe I was thinking in the wrong context here, and thought self.delete_snapshot would not raise exceptions in all cases.\n\nYou\u0027re right, this can be removed.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"49a6dfaca5fa4373fe645e299b372e6dd4b16d98","unresolved":false,"context_lines":[{"line_number":688,"context_line":""},{"line_number":689,"context_line":"                    self.delete_snapshot(context, s)"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"                snapshots \u003d objects.SnapshotList.get_all_for_volume(context,"},{"line_number":692,"context_line":"                                                                    volume.id)"},{"line_number":693,"context_line":"                for s in snapshots:"},{"line_number":694,"context_line":"                    if s.status \u003d\u003d \u0027available\u0027 or s.status \u003d\u003d \u0027error_deleting\u0027:"},{"line_number":695,"context_line":"                        LOG.error(_LE(\"A snapshot failed to delete during \""},{"line_number":696,"context_line":"                                      \"cascade delete for volume %s.\"),"},{"line_number":697,"context_line":"                                  volume.id)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"                        self._clear_db(context, is_migrating_dest, volume,"},{"line_number":700,"context_line":"                                       \u0027error_deleting\u0027)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"                        raise exception.VolumeIsBusy(volume_name\u003dvolume.name)"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"                LOG.debug(\u0027Snapshots deleted, issuing volume delete\u0027)"},{"line_number":705,"context_line":"                self.driver.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_94ec5bf4","line":702,"range":{"start_line":691,"start_character":0,"end_line":702,"end_character":77},"in_reply_to":"bae84128_b21b7ac9","updated":"2016-02-23 20:09:46.000000000","message":"Done","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"}],"cinder/volume/rpcapi.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"fd94f36a6d28d2e9dfab202bad6601e47e19f9bc","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                    \u0027unmanage_only\u0027: unmanage_only,"},{"line_number":165,"context_line":"                    \u0027cascade\u0027: cascade}"},{"line_number":166,"context_line":"        if self.client.can_send_version(\u00271.33\u0027):"},{"line_number":167,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":168,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":169,"context_line":"        else:"},{"line_number":170,"context_line":"            version \u003d \u00271.15\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_83f466cf","line":167,"range":{"start_line":167,"start_character":23,"end_line":167,"end_character":27},"updated":"2016-01-08 09:33:45.000000000","message":"We shouldn\u0027t avoid a version bump here and the backward compatibility code. The reason is that RPC server (manager.py) is unable to accept additional (unknown for him) keyword parameters.\n\nIn case of an upgrade we can have a new c-api service and an old c-vol. Old c-vol will fail requests with cascade arg, as it doesn\u0027t understand it. As requests with cascade parameter won\u0027t have any sense without passing it, I think we should raise an exception here if can_send_version will return False.\n\nAnother idea would be to simply pin API to a microversion without delete_cascade, but I\u0027m not sure on microversions status.","commit_id":"9537b5654065a2a7253796810c239a1aa696c095"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"487e4b41a76919d4ee04caddea36f80a87025109","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                    \u0027unmanage_only\u0027: unmanage_only,"},{"line_number":165,"context_line":"                    \u0027cascade\u0027: cascade}"},{"line_number":166,"context_line":"        if self.client.can_send_version(\u00271.33\u0027):"},{"line_number":167,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":168,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":169,"context_line":"        else:"},{"line_number":170,"context_line":"            version \u003d \u00271.15\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_35f59bca","line":167,"range":{"start_line":167,"start_character":23,"end_line":167,"end_character":27},"in_reply_to":"fa69d971_83f466cf","updated":"2016-01-08 15:17:33.000000000","message":"Thanks for the explanation, I wasn\u0027t sure yet if it would cause a failure or not.","commit_id":"9537b5654065a2a7253796810c239a1aa696c095"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"12507d660949001dc362cf86b75738c481e4ccdc","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":159,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        if self.client.can_send_version(\u00271.38\u0027):"},{"line_number":162,"context_line":"            version \u003d \u00271.38\u0027"},{"line_number":163,"context_line":"            msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        new_host \u003d utils.extract_host(volume.host)"},{"line_number":166,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_a228e79a","line":163,"range":{"start_line":161,"start_character":0,"end_line":163,"end_character":41},"updated":"2016-02-01 22:22:16.000000000","message":"What if client cannot send it? User\u0027s request will be quietly ignored (although volume will be in deleting state already). I wonder if raising if we cannot fulfill the request isn\u0027t more appropriate.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"d1ba7566070129a0f8dabf21bb182d95a982dc77","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":159,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        if self.client.can_send_version(\u00271.38\u0027):"},{"line_number":162,"context_line":"            version \u003d \u00271.38\u0027"},{"line_number":163,"context_line":"            msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        new_host \u003d utils.extract_host(volume.host)"},{"line_number":166,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_764b56b1","line":163,"range":{"start_line":161,"start_character":0,"end_line":163,"end_character":41},"in_reply_to":"7a5de9d1_2f4cb075","updated":"2016-02-14 10:25:24.000000000","message":"Except that we\u0027d have caught it in the API layer before, but you\u0027ve removed that check.\n\nI suppose you could do the can_send_... check in the API and say the cascade isn\u0027t supported by this server? Microversion is the better solution though","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"124eafdd6d6ae6a08e620815d6eb6440775b2315","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":159,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        if self.client.can_send_version(\u00271.38\u0027):"},{"line_number":162,"context_line":"            version \u003d \u00271.38\u0027"},{"line_number":163,"context_line":"            msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        new_host \u003d utils.extract_host(volume.host)"},{"line_number":166,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_2f4cb075","line":163,"range":{"start_line":161,"start_character":0,"end_line":163,"end_character":41},"in_reply_to":"7a5de9d1_a228e79a","updated":"2016-02-11 19:17:24.000000000","message":"I think this is ok, since they will send the old-style request, which will fail like it does w/o this feature.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"3a026ca98bef23072b2d5f09b7e9ca46eb5effda","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":159,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        if self.client.can_send_version(\u00271.38\u0027):"},{"line_number":162,"context_line":"            version \u003d \u00271.38\u0027"},{"line_number":163,"context_line":"            msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        new_host \u003d utils.extract_host(volume.host)"},{"line_number":166,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bae84128_32195f89","line":163,"range":{"start_line":161,"start_character":0,"end_line":163,"end_character":41},"in_reply_to":"dae33548_40e57635","updated":"2016-02-22 20:46:33.000000000","message":"This is handled smoothly in the next patchset by catching it in the API layer.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"81a86781b2c5a92b4b5dd9a9b11d738017ed18da","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":159,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        if self.client.can_send_version(\u00271.38\u0027):"},{"line_number":162,"context_line":"            version \u003d \u00271.38\u0027"},{"line_number":163,"context_line":"            msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        new_host \u003d utils.extract_host(volume.host)"},{"line_number":166,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dae33548_40e57635","line":163,"range":{"start_line":161,"start_character":0,"end_line":163,"end_character":41},"in_reply_to":"dae33548_764b56b1","updated":"2016-02-15 19:15:13.000000000","message":"Pinning to a N-1 microversion until upgrade is completed would be certainly the best solution.","commit_id":"b33dae365ce7f23316d8cf8b79b067a70c943039"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"bd900c0f9afd107725868442a45a2fbbdd34bd52","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":162,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if cascade:"},{"line_number":165,"context_line":"            if self.client.can_send_version(\u00271.39\u0027):"},{"line_number":166,"context_line":"                version \u003d \u00271.39\u0027"},{"line_number":167,"context_line":"                msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                msg \u003d _(\u0027Cascade option is not supported.\u0027)"},{"line_number":170,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, version)"},{"line_number":173,"context_line":"        cctxt.cast(ctxt, \u0027delete_volume\u0027, **msg_args)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_1dcd3266","line":170,"range":{"start_line":164,"start_character":0,"end_line":170,"end_character":57},"updated":"2016-02-23 09:48:19.000000000","message":"Interesting idea. I think we would prefer to send the highest supported version anyway, even when not setting cascade. This will make it easier to remove compatibility code when bumping major RPC API version.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b2c07a066aceafee37c60fc36ab5e6f5d226a5d4","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":162,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if cascade:"},{"line_number":165,"context_line":"            if self.client.can_send_version(\u00271.39\u0027):"},{"line_number":166,"context_line":"                version \u003d \u00271.39\u0027"},{"line_number":167,"context_line":"                msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                msg \u003d _(\u0027Cascade option is not supported.\u0027)"},{"line_number":170,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, version)"},{"line_number":173,"context_line":"        cctxt.cast(ctxt, \u0027delete_volume\u0027, **msg_args)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_92e916e9","line":170,"range":{"start_line":164,"start_character":0,"end_line":170,"end_character":57},"in_reply_to":"bae84128_1dcd3266","updated":"2016-02-23 18:18:41.000000000","message":"Makes sense.","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"49a6dfaca5fa4373fe645e299b372e6dd4b16d98","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            version \u003d \u00271.33\u0027"},{"line_number":162,"context_line":"            msg_args[\u0027volume\u0027] \u003d volume"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if cascade:"},{"line_number":165,"context_line":"            if self.client.can_send_version(\u00271.39\u0027):"},{"line_number":166,"context_line":"                version \u003d \u00271.39\u0027"},{"line_number":167,"context_line":"                msg_args[\u0027cascade\u0027] \u003d cascade"},{"line_number":168,"context_line":"            else:"},{"line_number":169,"context_line":"                msg \u003d _(\u0027Cascade option is not supported.\u0027)"},{"line_number":170,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, version)"},{"line_number":173,"context_line":"        cctxt.cast(ctxt, \u0027delete_volume\u0027, **msg_args)"}],"source_content_type":"text/x-python","patch_set":3,"id":"bae84128_547653d7","line":170,"range":{"start_line":164,"start_character":0,"end_line":170,"end_character":57},"in_reply_to":"bae84128_92e916e9","updated":"2016-02-23 20:09:46.000000000","message":"Done","commit_id":"e5f23538c51501106b0bd8e19cb35d719254bee6"}]}