)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c52d8fc0e45515a0288709f41eac9c28d5d3a618","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the volume status to be \u0027available\u0027, \u0027in-use\u0027 or \u0027backing-up\u0027,"},{"line_number":12,"context_line":"while in this situation the volume status would be \u0027deleting\u0027."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Trace: http://paste.openstack.org/raw/606034/"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Most of those drivers apply locks to the volume/snapshot deletion"},{"line_number":17,"context_line":"methods, so it should be safe to just add \u0027deleting\u0027 to the list of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bff0334d_b372357a","line":14,"updated":"2017-04-10 15:32:39.000000000","message":"Don\u0027t really need to link to this in the commit message.","commit_id":"f31cbc6eed9fc78d671e2156848e963f17b09e77"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"f6f44cac08bab890b60c190193346822836b1898","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the volume status to be \u0027available\u0027, \u0027in-use\u0027 or \u0027backing-up\u0027,"},{"line_number":12,"context_line":"while in this situation the volume status would be \u0027deleting\u0027."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Trace: http://paste.openstack.org/raw/606034/"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Most of those drivers apply locks to the volume/snapshot deletion"},{"line_number":17,"context_line":"methods, so it should be safe to just add \u0027deleting\u0027 to the list of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bff0334d_534e4997","line":14,"in_reply_to":"bff0334d_b372357a","updated":"2017-04-10 15:45:58.000000000","message":"Done","commit_id":"f31cbc6eed9fc78d671e2156848e963f17b09e77"}],"cinder/volume/drivers/remotefs.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c52d8fc0e45515a0288709f41eac9c28d5d3a618","unresolved":false,"context_lines":[{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027 or \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"},{"line_number":1020,"context_line":""},{"line_number":1021,"context_line":"            raise exception.InvalidVolume(msg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_1320c19a","line":1018,"updated":"2017-04-10 15:32:39.000000000","message":"Need to fix this message to match the change.","commit_id":"f31cbc6eed9fc78d671e2156848e963f17b09e77"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"f6f44cac08bab890b60c190193346822836b1898","unresolved":false,"context_lines":[{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027 or \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"},{"line_number":1020,"context_line":""},{"line_number":1021,"context_line":"            raise exception.InvalidVolume(msg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_13b6818b","line":1018,"in_reply_to":"bff0334d_1320c19a","updated":"2017-04-10 15:45:58.000000000","message":"My bad, thanks for pointing this out.","commit_id":"f31cbc6eed9fc78d671e2156848e963f17b09e77"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ae1293e9a72dd831da743c24ac7378959bc6a4d9","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_ff5cad4b","line":1016,"updated":"2017-04-12 16:07:57.000000000","message":"The only real concern here is that previously this check was protecting against a deletion happening on top of another deletion that was ongoing, or stuck in a weird state.\n\nThis probably works, but we need to make really sure that we aren\u0027t opening a window for dangerous behavior here.","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"c933a8f1669f6052894d9ba031c8d28e0b481517","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_797b72bd","line":1016,"in_reply_to":"7ffa3b31_b586af12","updated":"2017-04-20 08:57:04.000000000","message":"I think you\u0027re referring to this, right? https://docs.openstack.org/developer/cinder/devref/api_conditional_updates.html\n\nThose conditional updates are there for some time now so I\u0027d say it\u0027s safe now. Anyway, thanks for bringing this up.","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"35d1e0d39d022aa0d18636cab0f1724a21ec0f7f","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_b586af12","line":1016,"in_reply_to":"9ff52f3f_fcc82075","updated":"2017-04-19 18:02:00.000000000","message":"Have we closed off that race in the API now? It certainly was not a safe assumption that the API would have blocked this prior to the api-race-fixing patch series.","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"20bbe870bcbc3d3aeb1970fda2881675a61071a5","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_f0217ab5","line":1016,"in_reply_to":"bff0334d_8a78c18f","updated":"2017-04-12 17:34:15.000000000","message":"The Ceph driver doesn\u0027t use remotefs, this would have been a consideration for glusterfs. :)","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"54add977cbc22a4a8f51e14248c19024e0f136b9","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_5d195ed4","line":1016,"in_reply_to":"bff0334d_f0217ab5","updated":"2017-04-12 18:31:12.000000000","message":"Oh, of course. :)","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c815c6c25bfb428aebde457256b6080857e29c58","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_8a78c18f","line":1016,"in_reply_to":"bff0334d_ff5cad4b","updated":"2017-04-12 16:11:36.000000000","message":"Eric, was that an issue for the Ceph driver? And if so, would this expose it again? I seem to remember conflicts with that, but I think it had more to do with snapshots not actually being gone before the volume delete came in.","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"ed18703634b6beec9af155c3971643b270d24e32","unresolved":false,"context_lines":[{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        volume_status \u003d snapshot.volume.status"},{"line_number":1015,"context_line":"        if volume_status not in [\u0027available\u0027, \u0027in-use\u0027,"},{"line_number":1016,"context_line":"                                 \u0027backing-up\u0027, \u0027deleting\u0027]:"},{"line_number":1017,"context_line":"            msg \u003d _(\"Volume status must be \u0027available\u0027, \u0027in-use\u0027, \""},{"line_number":1018,"context_line":"                    \"\u0027backing-up\u0027 or \u0027deleting\u0027 but is: \""},{"line_number":1019,"context_line":"                    \"%(status)s.\") % {\u0027status\u0027: volume_status}"}],"source_content_type":"text/x-python","patch_set":2,"id":"9ff52f3f_fcc82075","line":1016,"in_reply_to":"bff0334d_ff5cad4b","updated":"2017-04-13 09:09:54.000000000","message":"Since those methods are synchronized, that shouldn\u0027t be an issue.\n\nI was considering the case in which the volume would be stuck in a \u0027deleting\u0027 state. Well, the volume deletion request would be rejected at the API level (considering we have snapshots), unless the cascade flag is set. That as well will be rejected if the volume status is \u0027deleting\u0027, unless the \u0027force\u0027 flag is set.\n\nhttps://github.com/openstack/cinder/blob/f32b98dd233a5d773ec895856d698ee7340ee8eb/cinder/volume/api.py#L408-L422\n\nConsidering this, IMO we\u0027re covered.","commit_id":"78a532cdd672307f78397b5a1afa5386f47c5a07"}]}