)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"596a0a7e2038caf79a1ddc1e94cf3efd86fb990a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"ee5abf5b_4cea5764","updated":"2021-11-11 15:45:50.000000000","message":"Before I forget, this needs to be targeted to the current development cycle (yoga).","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fbb8bdb3e3a9fa9d949c8e581bc480c6fb0f414d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"641b4c69_6eccd823","updated":"2021-11-11 15:48:50.000000000","message":"Ignore my previous content, i was looking at an outdated patch set.","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"7f9088e80dd4a85e7be19188d8f82be29ac16606","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"49bd485b_2a5b2297","updated":"2021-12-09 06:59:46.000000000","message":"LGTM!\nmaybe we can consider use cinder schedler to check and filter backends that do not support this feature","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"f4be1c07_6e20fec2","updated":"2021-12-09 13:37:14.000000000","message":"Some requests for clarifications inline.\n\nOne issue I\u0027d like you to address is this: Suppose a vendor supports some kind of efficient backend snapshot, but does *not* support revert-to-any on the backend.  A user has snapshots s1, s2, s3 (with s3 most recent).  User requests revert to s1.  Since the driver doesn\u0027t support this, we do the generic revert you describe in the spec.  What implications does that have for s2 and s3?","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"d22c8e8c_ad9ad3cf","in_reply_to":"f4be1c07_6e20fec2","updated":"2021-12-10 02:37:29.000000000","message":"This has no effect, Because it is equivalent to writing data to the volume, the snapshot data will not be lost and disordered.\n\nAccording to the snapshot principle, when the original volume data changes, the data at the snapshot time will be copied to the snapshot for saving.","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"25716b39_1622ed56","updated":"2021-12-19 18:06:48.000000000","message":"Some more questions noted inline.  Most are minor, but there\u0027s a large end user impact I\u0027d like you to address (or explain why it\u0027s not an issue).\n\n","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"fb0c261f7187ac4382e87c59423fd86f6e2bf238","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"febff82a_08b3447b","updated":"2022-01-06 20:27:22.000000000","message":"I don\u0027t see how this can be progressed when the revert-to-snapshot feature does not amend the volume creation date to match the snapshot creation date after a reversion.\n","commit_id":"bdc17270c80b337bf1914a3c3c26a8bceaf81c14"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"d8eb09d1d6db8b8560543cdcb49109052b6338a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"4418cac6_4c037420","updated":"2021-12-24 14:10:13.000000000","message":"Pretty close to a +1, but I need someone to read my comment and confirm...","commit_id":"bdc17270c80b337bf1914a3c3c26a8bceaf81c14"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"1a7f42b8f541d5de08708195eb7910e82bec0966","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"255b0a85_82edca3b","updated":"2022-03-03 17:39:27.000000000","message":"Will need to move to Zed if this is going to be moved forward","commit_id":"bdc17270c80b337bf1914a3c3c26a8bceaf81c14"}],"specs/victoria/support-revert-any-snapshot.rst":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"78cb7b66d04ff4c496e9f34d7c19f7d04de6d39d","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Problem description"},{"line_number":14,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":15,"context_line":"Currently, the end user can only revert the latest snapshot to volume,"},{"line_number":16,"context_line":"but they cann\u0027t revert other snapshot in the snapshot chain. If they"},{"line_number":17,"context_line":"want to revert the *middle* snapshot, they must delete all snapshots"},{"line_number":18,"context_line":"after this snapshot, and keep it up to the latest."},{"line_number":19,"context_line":"This brings great inconvenience to users."}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_8c117e07","line":16,"range":{"start_line":16,"start_character":9,"end_line":16,"end_character":15},"updated":"2020-06-24 15:49:33.000000000","message":"can\u0027t","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9655547e77af9c23325dd3311207b89bd55f614c","unresolved":false,"context_lines":[{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Vendor-specific changes"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_4c9be616","line":29,"updated":"2020-06-24 15:18:07.000000000","message":"We need more detail in the spec, this is what we discussed in the mid-cycle:\n\nSome drivers cannot do it because if they do it they lose all other snapshots that were created after the middle snapshot we are reverting to.\n\nFor example if we have: snap1, snap2, and snap3 and we revert to snap1, then some drivers delete the actual data in snap2 and snap3.\n\nSo we need a new flag in the drivers so they can report that they don\u0027t lose data when reverting to middle snapshots.  The default for this flag should be that they lose data.\n\nThen the code in cinder/volume/manager.py has to check if the revert is to a middle snapshot and the driver doesn\u0027t support it, and in that case the manager.py needs to do a generic revert, which consists of:\n\n- Create temporary volume from snapshot\n- Attach temporary volume and destination volume\n- Copy data between the volumes\n- Detach volumes\n- Delete the temporary volume\n\nThat way we don\u0027t limit the snapshot we can revert to at the API level, and it will work for all drivers.\n\nIt will be a fast operation for some drivers, and slow for the others.","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e8623eaf1ed3344851cd8a918bb8d928a405bb25","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Vendor-specific changes"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_ec22ba51","line":29,"range":{"start_line":28,"start_character":60,"end_line":29,"end_character":10},"updated":"2020-06-24 14:58:14.000000000","message":"is this the real intent? from the above problem description i think we want to revert to the same volume\u0027s snapshot just not the latest one but any.","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"12fa521572772bdacb7209d58678c05bc4bad5f9","unresolved":false,"context_lines":[{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Vendor-specific changes"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_5f9a4ab8","line":29,"in_reply_to":"bf51134e_4c9be616","updated":"2020-06-26 00:43:48.000000000","message":"\u003e Some drivers cannot do it because if they do it they lose all other\n \u003e snapshots that were created after the middle snapshot we are\n \u003e reverting to.\n \u003e \n\nPerhaps we can declare through the configuration file, which drivers support revert *any* snapshot, which drivers are not supported temporarily (or not sure).","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fc526387312c3a235069819b78efedfb01c4faa6","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Vendor-specific changes"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_262b1f83","line":29,"range":{"start_line":28,"start_character":60,"end_line":29,"end_character":10},"in_reply_to":"bf51134e_ec22ba51","updated":"2020-06-25 11:59:56.000000000","message":"In addition to Rajat\u0027s point (that is, to make clear that you are proposing to change the revert-to-snapshot REST API to allow a volume to be reverted to *any* snapshot), you should also emphasize the second item mentioned in your use case, that is, what will happen to those snapshots more recent than the one reverted to.  (Gorka talks about the issues related to this in the next comment.)","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e8623eaf1ed3344851cd8a918bb8d928a405bb25","unresolved":false,"context_lines":[{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Vendor-specific changes"},{"line_number":33,"context_line":"-----------------------"},{"line_number":34,"context_line":"None"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_cce41610","line":31,"updated":"2020-06-24 14:58:14.000000000","message":"The proposed change require more details to describe the feature properly","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"186b26cd7765f72b66c14eeb6d0bad88fdc72768","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Vendor-specific changes"},{"line_number":33,"context_line":"-----------------------"},{"line_number":34,"context_line":"None"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Alternatives"},{"line_number":37,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_969b4b64","line":34,"updated":"2020-06-24 13:48:29.000000000","message":"Drivers must implement an efficient way to do this.","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"d47c703b99fac5ae926098144b0129ef68d99efa","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Alternatives"},{"line_number":37,"context_line":"------------"},{"line_number":38,"context_line":"None"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Data model impact"},{"line_number":41,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_f1d1f1fc","line":38,"range":{"start_line":38,"start_character":0,"end_line":38,"end_character":4},"updated":"2020-06-24 14:27:05.000000000","message":"User can delete a volume and create a new one from the specified snapshot","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"186b26cd7765f72b66c14eeb6d0bad88fdc72768","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Performance Impact"},{"line_number":61,"context_line":"------------------"},{"line_number":62,"context_line":"None"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"Other deployer impact"},{"line_number":65,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bf51134e_5690934a","line":62,"updated":"2020-06-24 13:48:29.000000000","message":"Serious performance impact for drivers that can\u0027t efficiently do this.","commit_id":"0377dd0ca9d46c0dae50df0c6e7790f962d415b4"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c936dd3595a32299fe68d1eb9910253a6a3d139e","unresolved":false,"context_lines":[{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots, For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. when revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"}],"source_content_type":"text/x-rst","patch_set":4,"id":"bf51134e_74abb374","line":32,"range":{"start_line":32,"start_character":50,"end_line":32,"end_character":51},"updated":"2020-07-01 11:01:57.000000000","message":"/,/./","commit_id":"dbca0bcbf96d2f571c19061f061d7ef09ac44031"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c936dd3595a32299fe68d1eb9910253a6a3d139e","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots, For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. when revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise limit"}],"source_content_type":"text/x-rst","patch_set":4,"id":"bf51134e_54a82f76","line":33,"range":{"start_line":33,"start_character":57,"end_line":33,"end_character":58},"updated":"2020-07-01 11:01:57.000000000","message":"/w/W/","commit_id":"dbca0bcbf96d2f571c19061f061d7ef09ac44031"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c936dd3595a32299fe68d1eb9910253a6a3d139e","unresolved":false,"context_lines":[{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. when revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise limit"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Vendor-specific changes"},{"line_number":39,"context_line":"-----------------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"bf51134e_b4db8b1f","line":36,"range":{"start_line":36,"start_character":2,"end_line":36,"end_character":17},"updated":"2020-07-01 11:01:57.000000000","message":"I think you need to better explain what \"otherwise limit\" means here.\n\nAs the points mentioned in the previous patchsets touched on, we need consistent behavior from Cinder. So we either need to have an optimized path where the driver and backend can handle reverting to any snapshot, or we need a less optimal way to get the same end state without losing data or providing unexpected behavior from one cloud to another.\n\nThat last part is the tricky part, and why things were limited to just the last snapshot as it has been so far. This just isn\u0027t universal storage functionality, so it is hard to make it behave like it is through Cinder.","commit_id":"dbca0bcbf96d2f571c19061f061d7ef09ac44031"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c936dd3595a32299fe68d1eb9910253a6a3d139e","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"REST API impact"},{"line_number":51,"context_line":"---------------"},{"line_number":52,"context_line":"Microversion bump is required for this change."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"Security impact"},{"line_number":55,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"bf51134e_34cf7b58","line":52,"range":{"start_line":52,"start_character":0,"end_line":52,"end_character":46},"updated":"2020-07-01 11:01:57.000000000","message":"This spec should also state what the required change is in the API that will require a microversion. There are no details here about how the API changes with this.","commit_id":"dbca0bcbf96d2f571c19061f061d7ef09ac44031"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c936dd3595a32299fe68d1eb9910253a6a3d139e","unresolved":false,"context_lines":[{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Documentation Impact"},{"line_number":98,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":99,"context_line":"Release note should point out that revert to other snapshot is supported."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"References"},{"line_number":102,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"bf51134e_94092797","line":99,"updated":"2020-07-01 11:01:57.000000000","message":"Will also need to update the driver support matrix to list which drivers have optimized revert functionality.","commit_id":"dbca0bcbf96d2f571c19061f061d7ef09ac44031"}],"specs/wallaby/remove-me.rst":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"61cb94a60036d5938e6c3e9baff1906f32220045","unresolved":false,"context_lines":[{"line_number":1,"context_line":".. This file is a place holder.  It should be removed by"},{"line_number":2,"context_line":"   any patch proposing a spec for the Wallaby release"},{"line_number":3,"context_line":""},{"line_number":4,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f65232a_d0b60fda","line":1,"range":{"start_line":1,"start_character":46,"end_line":1,"end_character":53},"updated":"2020-10-22 12:19:26.000000000","message":"You don\u0027t need to add this file.  By the time you read this, it should have been removed by the merging of https://review.opendev.org/#/c/747475/ .  So I think just delete it from your local branch and then rebase on master and you should be good to go.","commit_id":"09ae3c6180ba1c4eae071a6f8606a3e633799770"}],"specs/wallaby/support-revert-any-snapshot.rst":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c207eddd01720beeb982daaf2f0fcd91dac3b5e8","unresolved":false,"context_lines":[{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise limit, terminate the rollback process and restore the volume state"},{"line_number":37,"context_line":"  and the snapshot state."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"Vendor-specific changes"},{"line_number":40,"context_line":"-----------------------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"3f65232a_2d557051","line":37,"range":{"start_line":36,"start_character":2,"end_line":37,"end_character":25},"updated":"2020-10-22 11:14:49.000000000","message":"-1: In Cinder we have a policy of implementing generic mechanisms, when possible, for features that not all drivers support.\n\nIn this case the generic mechanism for drivers with \u0027safe_revert_middle_snapshot\u0027 set to the default False would be to:\n\n- Log info message stating that a slow revert to snapshot is starting (I would even go as far as adding a user message for this)\n- Either attach the snapshot if driver supports it, or create temp volume from snapshot and attach it\n- Attach the destination volume\n- dd the contents\n- Detach snapshot and volumes\n- Delete temp volume if one was created","commit_id":"09ae3c6180ba1c4eae071a6f8606a3e633799770"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f22d5bb8284b464e92baa4c0079270b4a809891c","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_5794b044","line":29,"range":{"start_line":28,"start_character":2,"end_line":29,"end_character":35},"updated":"2020-10-30 02:31:39.000000000","message":"Technically, you are modifying the \u0027revert\u0027 action in the Volume Actions API to allow a volume to be reverted to any snapshot of that volume.","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f22d5bb8284b464e92baa4c0079270b4a809891c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_9d4f87bc","line":33,"range":{"start_line":32,"start_character":52,"end_line":33,"end_character":55},"updated":"2020-10-30 02:31:39.000000000","message":"I\u0027m assuming the default value will be \u0027false\u0027?","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f22d5bb8284b464e92baa4c0079270b4a809891c","unresolved":false,"context_lines":[{"line_number":36,"context_line":"  otherwise, perform the following process"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  * Log info message stating that a slow revert to snapshot is starting"},{"line_number":39,"context_line":"  * Either attach the snapshot if driver supports it, or create temp volume from snapshot and attach it"},{"line_number":40,"context_line":"  * Attach the destination volume"},{"line_number":41,"context_line":"  * dd the contents"},{"line_number":42,"context_line":"  * Detach snapshot and volumes"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_5dda0fe9","line":39,"updated":"2020-10-30 02:31:39.000000000","message":"Nit: please observe the 79 char per line where possible.","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f22d5bb8284b464e92baa4c0079270b4a809891c","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"Performance Impact"},{"line_number":76,"context_line":"------------------"},{"line_number":77,"context_line":"Serious performance impact for drivers that can\u0027t efficiently do this."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"Other deployer impact"},{"line_number":80,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_37913454","line":77,"updated":"2020-10-30 02:31:39.000000000","message":"Question: is this performance impact so bad that an operator with such a driver might want to not allow this feature?  Eric mentioned this on PS4, and I think it is something we need to worry about.","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f22d5bb8284b464e92baa4c0079270b4a809891c","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"Other deployer impact"},{"line_number":80,"context_line":"---------------------"},{"line_number":81,"context_line":"None"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Developer impact"},{"line_number":84,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_3dd753c2","line":81,"updated":"2020-10-30 02:31:39.000000000","message":"I don\u0027t know about this -- if your backend doesn\u0027t support it and you have to use the default method, you may need to have beefier nodes for running cinder.","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"0b9124799055a640b50c8efbfc1f965b11bee37d","unresolved":false,"context_lines":[{"line_number":101,"context_line":"Testing"},{"line_number":102,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":103,"context_line":"* Add related unit tests"},{"line_number":104,"context_line":"* Add related functional test"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Documentation Impact"},{"line_number":107,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_f6f21506","line":104,"updated":"2020-10-30 14:40:14.000000000","message":"Can you please clarify what you mean by functional test? This feature requires to be tested against a real deployment as well (so higher level of testing).","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"e614ea3c4196b2725848c375fa9fa36c90ef05ec","unresolved":false,"context_lines":[{"line_number":101,"context_line":"Testing"},{"line_number":102,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":103,"context_line":"* Add related unit tests"},{"line_number":104,"context_line":"* Add related functional test"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Documentation Impact"},{"line_number":107,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":9,"id":"1f621f24_18ad1c31","line":104,"in_reply_to":"1f621f24_f6f21506","updated":"2020-11-03 07:21:08.000000000","message":"If there is necessary to add the functional test, I think we can, otherwise, we can remove this.","commit_id":"1025808c70fbd186b550e1bfe350c82cb4a57d69"}],"specs/xena/support-revert-any-snapshot.rst":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"a025da37ca05d9b0c8d133cb33f2dbaf61ae8247","unresolved":true,"context_lines":[{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"}],"source_content_type":"text/x-rst","patch_set":12,"id":"05f92f8c_83109560","line":31,"updated":"2021-04-23 15:31:44.000000000","message":"This should be flagged as a capability of a driver, such as QoS. multiattach or replication. Something like `revert_any_snapshot \u003d True/False`\nThere should be a standard LCD method to perform the revert from any snapshot and this can be overridden by a driver specific method if the vendor can do this in a more efficient way.","commit_id":"a3175954112fb1c4d49225fd21b44c559259eaad"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"6a522a7f6d1a7243f2191a280155396142aaf814","unresolved":true,"context_lines":[{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"}],"source_content_type":"text/x-rst","patch_set":12,"id":"396df277_14177772","line":31,"in_reply_to":"05f92f8c_83109560","updated":"2021-06-28 06:40:21.000000000","message":"This feature define \u0027safe_revert_middle_snapshot\u0027, and explained below, thanks.","commit_id":"a3175954112fb1c4d49225fd21b44c559259eaad"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"a025da37ca05d9b0c8d133cb33f2dbaf61ae8247","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Alternatives"},{"line_number":59,"context_line":"------------"},{"line_number":60,"context_line":"User can delete a volume and create a new one from the specified snapshot"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"Data model impact"},{"line_number":63,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":12,"id":"454a57b2_0f9406b7","line":60,"updated":"2021-04-23 15:31:44.000000000","message":"I assume you don\u0027t mean the parent volume as this is not supported","commit_id":"a3175954112fb1c4d49225fd21b44c559259eaad"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"6a522a7f6d1a7243f2191a280155396142aaf814","unresolved":true,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Alternatives"},{"line_number":59,"context_line":"------------"},{"line_number":60,"context_line":"User can delete a volume and create a new one from the specified snapshot"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"Data model impact"},{"line_number":63,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":12,"id":"0f619b60_7442c24b","line":60,"in_reply_to":"454a57b2_0f9406b7","updated":"2021-06-28 06:40:21.000000000","message":"yes, was not the parent volume.","commit_id":"a3175954112fb1c4d49225fd21b44c559259eaad"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"f8a86e9cc0b06259120b0f11ef1da02444da4426","unresolved":true,"context_lines":[{"line_number":44,"context_line":"  * Delete temp volume if one was created"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":47,"context_line":"* If there is a chain of 10 snapshots and revert to 3, they can not revert to 7"},{"line_number":48,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":49,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":50,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."}],"source_content_type":"text/x-rst","patch_set":13,"id":"3aad8378_18cfabf8","line":47,"range":{"start_line":47,"start_character":0,"end_line":47,"end_character":79},"updated":"2021-06-16 08:55:57.000000000","message":"As your design, we can revert to any snapshots if the storage vendors support rolling back middle snapshots by setting safe_revert_middle_snapshot to true.\nSo for example, the volume has 10 snapshots, named snap01, snap02 ... snap10. And the snap10 is the latest snapshot. Do you mean we can first to revert to snap03, but at next we can not revert to snap07?\nWhy? I think if the safe_revert_middle_snapshot of storage vendors is true, we can revert to any snapshots from snap01 to snap10.","commit_id":"9accb245b5c7c3c9f9aca40cf96f289dca4c4e7f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e154c819ccae82be9bb72c3245f7bc24a788c421","unresolved":true,"context_lines":[{"line_number":44,"context_line":"  * Delete temp volume if one was created"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":47,"context_line":"* If there is a chain of 10 snapshots and revert to 3, they can not revert to 7"},{"line_number":48,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":49,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":50,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."}],"source_content_type":"text/x-rst","patch_set":13,"id":"922b3755_303d7f3d","line":47,"updated":"2021-06-04 13:14:11.000000000","message":"Please explain more clearly what you mean here.","commit_id":"9accb245b5c7c3c9f9aca40cf96f289dca4c4e7f"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"6a522a7f6d1a7243f2191a280155396142aaf814","unresolved":true,"context_lines":[{"line_number":44,"context_line":"  * Delete temp volume if one was created"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":47,"context_line":"* If there is a chain of 10 snapshots and revert to 3, they can not revert to 7"},{"line_number":48,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":49,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":50,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."}],"source_content_type":"text/x-rst","patch_set":13,"id":"f4a777ff_aed9c40f","line":47,"range":{"start_line":47,"start_character":0,"end_line":47,"end_character":79},"in_reply_to":"3aad8378_18cfabf8","updated":"2021-06-28 06:40:21.000000000","message":"thanks for review, this mean\u0027s that, when reverting to 3, they can not revert to 7, it the reverting finished, they can revert to 7.","commit_id":"9accb245b5c7c3c9f9aca40cf96f289dca4c4e7f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"}],"source_content_type":"text/x-rst","patch_set":15,"id":"e6128b97_3e443542","line":33,"range":{"start_line":33,"start_character":3,"end_line":33,"end_character":30},"updated":"2021-06-30 11:38:20.000000000","message":"-1: I believe what Simon was referring to in patchset #12 is that this feature should be reported to the scheduler as a capability, for example \"efficient_revert_middle_snap\".  That way volumes types can be created to place volumes in a backend supporting this.\n\nI agree with him.","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"}],"source_content_type":"text/x-rst","patch_set":15,"id":"dcaefc56_92fe3e1b","line":33,"range":{"start_line":33,"start_character":3,"end_line":33,"end_character":30},"in_reply_to":"e6128b97_3e443542","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"a07eb27a_e87f51c3","line":34,"updated":"2021-06-30 11:38:20.000000000","message":"-1: Please add to the spec that the default value for a driver will be \u0027false\u0027, since that\u0027s the safe default (mentioned by Brian on patchset #9).","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"fd4b5ddf_0fce345d","line":34,"in_reply_to":"a07eb27a_e87f51c3","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  * Log info message stating that a slow revert to snapshot is starting"}],"source_content_type":"text/x-rst","patch_set":15,"id":"7db0cf49_225abb4f","line":35,"range":{"start_line":35,"start_character":49,"end_line":35,"end_character":68},"updated":"2021-06-30 11:38:20.000000000","message":"nit: \"continue with the existing execution path, calling existing ``revert_to_snapshot`` method\"","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027. When revert to"},{"line_number":34,"context_line":"  middle snapshot, the volume manager layer check the feature, if"},{"line_number":35,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 feature is true, continue to execute,"},{"line_number":36,"context_line":"  otherwise, perform the following process"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"  * Log info message stating that a slow revert to snapshot is starting"}],"source_content_type":"text/x-rst","patch_set":15,"id":"b15f766a_10955724","line":35,"range":{"start_line":35,"start_character":49,"end_line":35,"end_character":68},"in_reply_to":"7db0cf49_225abb4f","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":44,"context_line":"  * Delete temp volume if one was created"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":47,"context_line":"* If there is a chain of 10 snapshots and revert to 3, they can not revert to 7 when the"},{"line_number":48,"context_line":"  reverting to 3 is not completed."},{"line_number":49,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":50,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":51,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."}],"source_content_type":"text/x-rst","patch_set":15,"id":"c995ffc6_d18ae837","line":48,"range":{"start_line":47,"start_character":2,"end_line":48,"end_character":34},"updated":"2021-06-30 11:38:20.000000000","message":"nit: Easier to explain with something like: \"Only one revert operation can be performed at a time and users must wait until one completes before reverting to another one.\"","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":44,"context_line":"  * Delete temp volume if one was created"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":47,"context_line":"* If there is a chain of 10 snapshots and revert to 3, they can not revert to 7 when the"},{"line_number":48,"context_line":"  reverting to 3 is not completed."},{"line_number":49,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":50,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":51,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."}],"source_content_type":"text/x-rst","patch_set":15,"id":"f4cda91a_fcbb986d","line":48,"range":{"start_line":47,"start_character":2,"end_line":48,"end_character":34},"in_reply_to":"c995ffc6_d18ae837","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":51,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":52,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":53,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Vendor-specific changes"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":"Drivers must implement an efficient way to do this."}],"source_content_type":"text/x-rst","patch_set":15,"id":"8a65bff0_882eb42c","line":54,"updated":"2021-06-30 11:38:20.000000000","message":"-1: I agree with Eric\u0027s comment on patchset #4 and Brian\u0027s comment on patchset #9, we need a mechanism to disable the generic revert to snapshot feature, since the performance can be very poor and an administrator may want to disable this feature for its users.","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":51,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":52,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":53,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Vendor-specific changes"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":"Drivers must implement an efficient way to do this."}],"source_content_type":"text/x-rst","patch_set":15,"id":"8dc32fad_77b3fbc1","line":54,"in_reply_to":"8a65bff0_882eb42c","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Vendor-specific changes"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":"Drivers must implement an efficient way to do this."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"Alternatives"},{"line_number":60,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":15,"id":"acdb20af_2bf5ea49","line":57,"range":{"start_line":57,"start_character":0,"end_line":57,"end_character":51},"updated":"2021-06-30 11:38:20.000000000","message":"-1: As I understand it, they would need to modify the existing ``revert_to_snapshot`` method, if that\u0027s the case please say it here.","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"Vendor-specific changes"},{"line_number":56,"context_line":"-----------------------"},{"line_number":57,"context_line":"Drivers must implement an efficient way to do this."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"Alternatives"},{"line_number":60,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":15,"id":"bbe652da_127b94af","line":57,"range":{"start_line":57,"start_character":0,"end_line":57,"end_character":51},"in_reply_to":"acdb20af_2bf5ea49","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"Alternatives"},{"line_number":60,"context_line":"------------"},{"line_number":61,"context_line":"User can create a new one from the specified snapshot"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Data model impact"},{"line_number":64,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":15,"id":"9a10681c_5f49f8d2","line":61,"range":{"start_line":61,"start_character":22,"end_line":61,"end_character":25},"updated":"2021-06-30 11:38:20.000000000","message":"nit: replace \"one\" with \"volume\"","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"Alternatives"},{"line_number":60,"context_line":"------------"},{"line_number":61,"context_line":"User can create a new one from the specified snapshot"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Data model impact"},{"line_number":64,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":15,"id":"32237790_1e5ebd20","line":61,"range":{"start_line":61,"start_character":22,"end_line":61,"end_character":25},"in_reply_to":"9a10681c_5f49f8d2","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":110,"context_line":"Testing"},{"line_number":111,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":112,"context_line":"* Add related unit tests"},{"line_number":113,"context_line":"* Add related functional test"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"Documentation Impact"},{"line_number":116,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"7f9caeeb_ad0a4619","line":113,"updated":"2021-06-30 11:38:20.000000000","message":"-1: I agree with Luigi\u0027s comment on patchset #9, this needs a tempest test (integration test) more than a functional test.","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":110,"context_line":"Testing"},{"line_number":111,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":112,"context_line":"* Add related unit tests"},{"line_number":113,"context_line":"* Add related functional test"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"Documentation Impact"},{"line_number":116,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"dd7a8d29_999b8c29","line":113,"in_reply_to":"7f9caeeb_ad0a4619","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b219c66e4e8b985907b82533fa05b5a287136b09","unresolved":true,"context_lines":[{"line_number":116,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":117,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":118,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":119,"context_line":"functionality."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"References"},{"line_number":122,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"691b9845_12b8b830","line":119,"updated":"2021-06-30 11:38:20.000000000","message":"nit: documentation should be updated to briefly describe the generic revert mechanism so administrators are aware of the performance implications if their driver doesn\u0027t support the feature.","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"c9f4378f8e3d2e031266aca7a0f7be3042ca8aaf","unresolved":false,"context_lines":[{"line_number":116,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":117,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":118,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":119,"context_line":"functionality."},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"References"},{"line_number":122,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":15,"id":"2ffd4534_7e1e93dd","line":119,"in_reply_to":"691b9845_12b8b830","updated":"2021-07-02 06:28:03.000000000","message":"Done","commit_id":"4b93ec863b0a3d725532c21e3e6295d915f82696"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"aa4bbdf735a0b04180aa5947cc448f7bfd2961b0","unresolved":true,"context_lines":[{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027, the default"},{"line_number":34,"context_line":"  value is \u0027false\u0027 for a driver. When revert to middle snapshot,"},{"line_number":35,"context_line":"  the volume manager layer check the feature, if \u0027safe_revert_middle_snapshot\u0027"},{"line_number":36,"context_line":"  feature is true, continue with the existing execution path, calling existing"},{"line_number":37,"context_line":"  ``revert_to_snapshot`` method, otherwise, perform the following process"}],"source_content_type":"text/x-rst","patch_set":18,"id":"c0fc5ff1_39a29fb8","line":34,"range":{"start_line":34,"start_character":48,"end_line":34,"end_character":63},"updated":"2021-07-06 13:36:48.000000000","message":"you keeping mentioning middle snapshot. Really you are talking about *any* available snapshot for the volume, including the first one.\nIt would be better to make it clearer throughout the document that this change to to support any snapshot as well as the latest snapshot","commit_id":"408d008e39ef49e38f361a76a4ce3d46da371d8c"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"7f2285655444722587bdd115cc4de7a68b823c60","unresolved":false,"context_lines":[{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027, the default"},{"line_number":34,"context_line":"  value is \u0027false\u0027 for a driver. When revert to middle snapshot,"},{"line_number":35,"context_line":"  the volume manager layer check the feature, if \u0027safe_revert_middle_snapshot\u0027"},{"line_number":36,"context_line":"  feature is true, continue with the existing execution path, calling existing"},{"line_number":37,"context_line":"  ``revert_to_snapshot`` method, otherwise, perform the following process"}],"source_content_type":"text/x-rst","patch_set":18,"id":"277954c0_698fcf5d","line":34,"range":{"start_line":34,"start_character":48,"end_line":34,"end_character":63},"in_reply_to":"c0fc5ff1_39a29fb8","updated":"2021-07-07 03:23:03.000000000","message":"Done","commit_id":"408d008e39ef49e38f361a76a4ce3d46da371d8c"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"aa4bbdf735a0b04180aa5947cc448f7bfd2961b0","unresolved":true,"context_lines":[{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027, the default"},{"line_number":34,"context_line":"  value is \u0027false\u0027 for a driver. When revert to middle snapshot,"},{"line_number":35,"context_line":"  the volume manager layer check the feature, if \u0027safe_revert_middle_snapshot\u0027"},{"line_number":36,"context_line":"  feature is true, continue with the existing execution path, calling existing"},{"line_number":37,"context_line":"  ``revert_to_snapshot`` method, otherwise, perform the following process"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-rst","patch_set":18,"id":"3167cf8f_6709642f","line":35,"range":{"start_line":35,"start_character":50,"end_line":35,"end_character":77},"updated":"2021-07-06 13:36:48.000000000","message":"Maybe this feature name should not be \u0027middle\u0027 but \u0027any\u0027 or \u0027all\u0027","commit_id":"408d008e39ef49e38f361a76a4ce3d46da371d8c"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"7f2285655444722587bdd115cc4de7a68b823c60","unresolved":false,"context_lines":[{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"},{"line_number":33,"context_line":"  \u0027safe_revert_middle_snapshot\u0027 value \u0027true\u0027 or \u0027false\u0027, the default"},{"line_number":34,"context_line":"  value is \u0027false\u0027 for a driver. When revert to middle snapshot,"},{"line_number":35,"context_line":"  the volume manager layer check the feature, if \u0027safe_revert_middle_snapshot\u0027"},{"line_number":36,"context_line":"  feature is true, continue with the existing execution path, calling existing"},{"line_number":37,"context_line":"  ``revert_to_snapshot`` method, otherwise, perform the following process"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-rst","patch_set":18,"id":"0fa601bd_e546ae34","line":35,"range":{"start_line":35,"start_character":50,"end_line":35,"end_character":77},"in_reply_to":"3167cf8f_6709642f","updated":"2021-07-07 03:23:03.000000000","message":"Done","commit_id":"408d008e39ef49e38f361a76a4ce3d46da371d8c"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"aa4bbdf735a0b04180aa5947cc448f7bfd2961b0","unresolved":true,"context_lines":[{"line_number":124,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":125,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":126,"context_line":"functionality."},{"line_number":127,"context_line":"Update the document, describe e generic revert mechanism so administrators are"},{"line_number":128,"context_line":"aware of the performance implications if their driver doesn\u0027t support the feature."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"References"}],"source_content_type":"text/x-rst","patch_set":18,"id":"933e684a_34e8fbfc","line":127,"updated":"2021-07-06 13:36:48.000000000","message":"nit: replace `e` with `a`","commit_id":"408d008e39ef49e38f361a76a4ce3d46da371d8c"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"7f2285655444722587bdd115cc4de7a68b823c60","unresolved":false,"context_lines":[{"line_number":124,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":125,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":126,"context_line":"functionality."},{"line_number":127,"context_line":"Update the document, describe e generic revert mechanism so administrators are"},{"line_number":128,"context_line":"aware of the performance implications if their driver doesn\u0027t support the feature."},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"References"}],"source_content_type":"text/x-rst","patch_set":18,"id":"abafc4e3_de7713f2","line":127,"in_reply_to":"933e684a_34e8fbfc","updated":"2021-07-07 03:23:03.000000000","message":"Done","commit_id":"408d008e39ef49e38f361a76a4ce3d46da371d8c"}],"specs/yoga/support-revert-any-snapshot.rst":[{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"e55dfb14fc73606b7351a7809dbf26b3104b9d76","unresolved":true,"context_lines":[{"line_number":45,"context_line":"  * Detach snapshot and volumes"},{"line_number":46,"context_line":"  * Delete temp volume if one was created"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":49,"context_line":"* Only one revert operation can be performed at a time and users must wait until one completes"},{"line_number":50,"context_line":"  before reverting to another one."},{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"}],"source_content_type":"text/x-rst","patch_set":20,"id":"cc9dcee0_423e85f7","line":48,"updated":"2021-09-21 07:15:29.000000000","message":"temp volume","commit_id":"7873ce47e23fadb32f32018259f6aa9c32ee4efb"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"390521275a3e334ae1028b695b66241a45bc99bd","unresolved":true,"context_lines":[{"line_number":45,"context_line":"  * Detach snapshot and volumes"},{"line_number":46,"context_line":"  * Delete temp volume if one was created"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"* Now there is a general rollback progess use tmp volume, in the _revert_to_snapshot_generic"},{"line_number":49,"context_line":"* Only one revert operation can be performed at a time and users must wait until one completes"},{"line_number":50,"context_line":"  before reverting to another one."},{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"}],"source_content_type":"text/x-rst","patch_set":20,"id":"3d79442f_291f0987","line":48,"in_reply_to":"cc9dcee0_423e85f7","updated":"2021-10-19 01:30:52.000000000","message":"OK, thanks for review, please review again.","commit_id":"7873ce47e23fadb32f32018259f6aa9c32ee4efb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":13,"context_line":"Problem description"},{"line_number":14,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":15,"context_line":"Currently, the end user can only revert the latest snapshot to volume,"},{"line_number":16,"context_line":"but they can\u0027t revert other snapshot in the snapshot chain. If they"},{"line_number":17,"context_line":"want to revert the *middle* snapshot, they must delete all snapshots"},{"line_number":18,"context_line":"after this snapshot, and keep it up to the latest."},{"line_number":19,"context_line":"This brings great inconvenience to users."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":21,"id":"edcd2303_52fea26d","line":18,"range":{"start_line":16,"start_character":60,"end_line":18,"end_character":50},"updated":"2021-12-09 13:37:14.000000000","message":"I think Simon made a good point on an earlier patch set that speaking of the \"middle\" snapshot is imprecise.  So maybe explain your point like this:\n\nSuppose, for example, that a user has snapshots s1, s2, s3, s4 of some volume v, where these are ordered so that s1 is the oldest and s4 is the most recent.  If a user wants to revert v to s2, they must first delete s3 and s4 in order to make s2 the most recent snapshot.","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Problem description"},{"line_number":14,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":15,"context_line":"Currently, the end user can only revert the latest snapshot to volume,"},{"line_number":16,"context_line":"but they can\u0027t revert other snapshot in the snapshot chain. If they"},{"line_number":17,"context_line":"want to revert the *middle* snapshot, they must delete all snapshots"},{"line_number":18,"context_line":"after this snapshot, and keep it up to the latest."},{"line_number":19,"context_line":"This brings great inconvenience to users."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":21,"id":"382b2d7a_bef325ac","line":18,"range":{"start_line":16,"start_character":60,"end_line":18,"end_character":50},"in_reply_to":"edcd2303_52fea26d","updated":"2021-12-19 18:06:48.000000000","message":"Done","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"}],"source_content_type":"text/x-rst","patch_set":21,"id":"4922e9c1_dea9d4f0","line":29,"range":{"start_line":28,"start_character":2,"end_line":29,"end_character":35},"updated":"2021-12-09 13:37:14.000000000","message":"Let\u0027s be more precise here:\n\n  * Modify the \u0027revert\u0027 action in the Volume Actions API to allow a volume to be\n    reverted to any snapshot of that volume.  (The current functionality allows\n    reverting only to the most recent snapshot.)","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Proposed change"},{"line_number":27,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":28,"context_line":"* Change the revert-to-snapshot rest api, support revert to other volume\u0027s"},{"line_number":29,"context_line":"  snapshot not only the latest one."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"* In the volume driver layer, Storage vendors need to define features,"},{"line_number":32,"context_line":"  whether to support rolling back middle snapshots. For example define"}],"source_content_type":"text/x-rst","patch_set":21,"id":"c5721a38_b79bb570","line":29,"range":{"start_line":28,"start_character":2,"end_line":29,"end_character":35},"in_reply_to":"4922e9c1_dea9d4f0","updated":"2021-12-19 18:06:48.000000000","message":"Done","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":34,"context_line":"  value is \u0027false\u0027 for a driver. When revert to any snapshot,"},{"line_number":35,"context_line":"  the volume manager layer check the feature, if \u0027safe_revert_any_snapshot\u0027"},{"line_number":36,"context_line":"  feature is true, continue with the existing execution path, calling existing"},{"line_number":37,"context_line":"  ``revert_to_snapshot`` method, otherwise, and the snapshot is not the latest one"},{"line_number":38,"context_line":"  perform the following process"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"  * Log info message stating that a slow revert to snapshot is starting"}],"source_content_type":"text/x-rst","patch_set":21,"id":"b2d9a7ef_3c9aceac","line":37,"range":{"start_line":37,"start_character":44,"end_line":37,"end_character":47},"updated":"2021-12-09 13:37:14.000000000","message":"nit: when","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":false,"context_lines":[{"line_number":34,"context_line":"  value is \u0027false\u0027 for a driver. When revert to any snapshot,"},{"line_number":35,"context_line":"  the volume manager layer check the feature, if \u0027safe_revert_any_snapshot\u0027"},{"line_number":36,"context_line":"  feature is true, continue with the existing execution path, calling existing"},{"line_number":37,"context_line":"  ``revert_to_snapshot`` method, otherwise, and the snapshot is not the latest one"},{"line_number":38,"context_line":"  perform the following process"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"  * Log info message stating that a slow revert to snapshot is starting"}],"source_content_type":"text/x-rst","patch_set":21,"id":"8e0447d1_411ce35f","line":37,"range":{"start_line":37,"start_character":44,"end_line":37,"end_character":47},"in_reply_to":"b2d9a7ef_3c9aceac","updated":"2021-12-10 02:37:29.000000000","message":"Done","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":48,"context_line":"* Now there is a general rollback progess use temp volume, in the _revert_to_snapshot_generic"},{"line_number":49,"context_line":"* Only one revert operation can be performed at a time and users must wait until one completes"},{"line_number":50,"context_line":"  before reverting to another one."},{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":52,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":53,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"}],"source_content_type":"text/x-rst","patch_set":21,"id":"dbecf23d_3522a458","line":53,"range":{"start_line":51,"start_character":2,"end_line":53,"end_character":77},"updated":"2021-12-09 13:37:14.000000000","message":"I\u0027m not clear on what you\u0027re saying here.  Could you re-phrase it?","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":true,"context_lines":[{"line_number":48,"context_line":"* Now there is a general rollback progess use temp volume, in the _revert_to_snapshot_generic"},{"line_number":49,"context_line":"* Only one revert operation can be performed at a time and users must wait until one completes"},{"line_number":50,"context_line":"  before reverting to another one."},{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":52,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":53,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"}],"source_content_type":"text/x-rst","patch_set":21,"id":"fe4e2079_074ee838","line":53,"range":{"start_line":51,"start_character":2,"end_line":53,"end_character":77},"in_reply_to":"dbecf23d_3522a458","updated":"2021-12-10 02:37:29.000000000","message":"On, This is a reply to some review questions and should not be placed in here, removed delete it.\n\nWhat I want to express is that snapshots in the snapshot chain can be deleted safely. For example, deleting previous snapshots does not affect the data of the last snapshot.","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":false,"context_lines":[{"line_number":48,"context_line":"* Now there is a general rollback progess use temp volume, in the _revert_to_snapshot_generic"},{"line_number":49,"context_line":"* Only one revert operation can be performed at a time and users must wait until one completes"},{"line_number":50,"context_line":"  before reverting to another one."},{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":52,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":53,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"}],"source_content_type":"text/x-rst","patch_set":21,"id":"cbca31bd_d06f08a7","line":53,"range":{"start_line":51,"start_character":2,"end_line":53,"end_character":77},"in_reply_to":"fe4e2079_074ee838","updated":"2021-12-19 18:06:48.000000000","message":"Ack","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":52,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":53,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"}],"source_content_type":"text/x-rst","patch_set":21,"id":"af4dafa3_c1af695d","line":54,"range":{"start_line":54,"start_character":45,"end_line":54,"end_character":51},"updated":"2021-12-09 13:37:14.000000000","message":"any","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":false,"context_lines":[{"line_number":51,"context_line":"* Not all the storage vendors design snapshots as tree, perhaps the internal tree,"},{"line_number":52,"context_line":"  but there are no restrictions on operation. Like this, there are three snapshots of"},{"line_number":53,"context_line":"  volume: snap1, snap2, snap3, we can delete snap1 in the storage management."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_middle_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"}],"source_content_type":"text/x-rst","patch_set":21,"id":"790ccc6b_3b86f24a","line":54,"range":{"start_line":54,"start_character":45,"end_line":54,"end_character":51},"in_reply_to":"af4dafa3_c1af695d","updated":"2021-12-10 02:37:29.000000000","message":"Done","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":74,"context_line":"REST API impact"},{"line_number":75,"context_line":"---------------"},{"line_number":76,"context_line":"This changes also need to bump the microversion of API to keep forward"},{"line_number":77,"context_line":"compatibility."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"Security impact"},{"line_number":80,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":21,"id":"a4e0c25f_27e95732","line":77,"updated":"2021-12-09 13:37:14.000000000","message":"The current \u0027revert\u0027 action is defined to take a snapshot_id, so the JSON request won\u0027t change at all.  So my question is, how do you see this working?  What I mean is:\n\n- suppose that your microversion for this feature is 3.68\n- suppose a user has volume v with snapshots s1, s2, s3 (where s3 is most recent)\n- suppose the user makes the request POST /v3/{project_id}/volumes/{v}/action with request body\n\n  {\"revert\": {\"snapshot_id\": s2}}\n\nIf the user passes the header \"OpenStack-API-Version: volume 3.65\", will this request fail?  In other words, does the user need to specify \"OpenStack-API-Version: volume 3.68\" in order to revert the volume to s2?\n\nWhat\u0027s at issue here is whether the old behavior is desirable, that is, whether a user might *want* a revert-to-snapshot request to fail if they pass a middle shapshot in the request.  If that\u0027s desirable, then they can just use mv \u003c 3.68 to get the behavior they want.\n\nIf you\u0027re not sure about this, that\u0027s OK, I won\u0027t hold up the spec over this.  (We can discuss it at a cinder meeting and update the spec later.)","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":true,"context_lines":[{"line_number":74,"context_line":"REST API impact"},{"line_number":75,"context_line":"---------------"},{"line_number":76,"context_line":"This changes also need to bump the microversion of API to keep forward"},{"line_number":77,"context_line":"compatibility."},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"Security impact"},{"line_number":80,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":21,"id":"8ece5c1d_e45299d3","line":77,"in_reply_to":"a4e0c25f_27e95732","updated":"2021-12-10 02:37:29.000000000","message":"I\u0027m really not sure if it is controlled by version number.","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"Other end user impact"},{"line_number":88,"context_line":"---------------------"},{"line_number":89,"context_line":"None"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Performance Impact"},{"line_number":92,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":21,"id":"11e50789_0b86bc72","line":89,"updated":"2021-12-09 13:37:14.000000000","message":"Because this is controlled by a config option, it\u0027s going to be difficult for a user to tell whether this functionality is available or not.  I guess the best we can do is to modify the error message when mv 3.68 or later is requested and the config option is False to indicate that reverting to any snapshot is not supported in this cloud (if mv \u003c 3.68 is requested, just use the current error message).","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Work Items"},{"line_number":110,"context_line":"----------"},{"line_number":111,"context_line":"* Remove the recent snapshot revert limit in the revert-to-snapshot interface"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"Dependencies"},{"line_number":114,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":21,"id":"22b4ec1c_19c1f985","line":111,"updated":"2021-12-09 13:37:14.000000000","message":"add:\n\n* update the help text for revert-to-snapshot in the python-cinderclient to indicate that since mv 3.68, you\u0027re not restricted to the most recent snapshot (unless the functionality has been disabled by the operator).","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Work Items"},{"line_number":110,"context_line":"----------"},{"line_number":111,"context_line":"* Remove the recent snapshot revert limit in the revert-to-snapshot interface"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"Dependencies"},{"line_number":114,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":21,"id":"aae7757a_2c789adc","line":111,"in_reply_to":"22b4ec1c_19c1f985","updated":"2021-12-10 02:37:29.000000000","message":"Done","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d9c44832141b728d6bcd764405bd720fe68704bb","unresolved":true,"context_lines":[{"line_number":124,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":125,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":126,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":127,"context_line":"functionality."},{"line_number":128,"context_line":"Update the document, describe a generic revert mechanism so administrators are"},{"line_number":129,"context_line":"aware of the performance implications if their driver doesn\u0027t support the feature."},{"line_number":130,"context_line":""}],"source_content_type":"text/x-rst","patch_set":21,"id":"0db1c9eb_64465f8e","line":127,"updated":"2021-12-09 13:37:14.000000000","message":"nit: add a space after this line","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"49b226bc5d2c558f7fe1669e4718a59f5d6ac8cc","unresolved":false,"context_lines":[{"line_number":124,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":125,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":126,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":127,"context_line":"functionality."},{"line_number":128,"context_line":"Update the document, describe a generic revert mechanism so administrators are"},{"line_number":129,"context_line":"aware of the performance implications if their driver doesn\u0027t support the feature."},{"line_number":130,"context_line":""}],"source_content_type":"text/x-rst","patch_set":21,"id":"86d11c36_da283191","line":127,"in_reply_to":"0db1c9eb_64465f8e","updated":"2021-12-10 02:37:29.000000000","message":"Done","commit_id":"6b422dd91b1070cee65e63cbfdb40bb7ba4143e5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":51,"context_line":"* Now there is a general rollback progess use temp volume, in the _revert_to_snapshot_generic"},{"line_number":52,"context_line":"* Only one revert operation can be performed at a time and users must wait until one completes"},{"line_number":53,"context_line":"  before reverting to another one."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_any_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"}],"source_content_type":"text/x-rst","patch_set":23,"id":"7978d7e0_81e3d742","line":54,"range":{"start_line":54,"start_character":2,"end_line":54,"end_character":58},"updated":"2021-12-19 18:06:48.000000000","message":"I think you can be more specific about this:\n\n  * Add a \u0027revert_to_any_snapshot\u0027 function to the VolumeSnapshotRevertDriver interface.  The implementation of this function in the VolumeDriver class will raise a NotImplementedError.  In the case of a NotImplementedError, we use ...","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":53,"context_line":"  before reverting to another one."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_any_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"},{"line_number":58,"context_line":"* Reported it to the scheduler as a capability, for example \"efficient_revert_middle_snap\"."},{"line_number":59,"context_line":"  That way volumes types can be created to place volumes in a backend supporting this."}],"source_content_type":"text/x-rst","patch_set":23,"id":"6f9bf80a_1dc55158","line":56,"range":{"start_line":56,"start_character":20,"end_line":56,"end_character":35},"updated":"2021-12-19 18:06:48.000000000","message":"option named \u0027allow_revert_to_any_snapshot\u0027 to control","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"a2f43dac1f674fee8ebae982081ccb535edab04a","unresolved":false,"context_lines":[{"line_number":53,"context_line":"  before reverting to another one."},{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_any_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"},{"line_number":58,"context_line":"* Reported it to the scheduler as a capability, for example \"efficient_revert_middle_snap\"."},{"line_number":59,"context_line":"  That way volumes types can be created to place volumes in a backend supporting this."}],"source_content_type":"text/x-rst","patch_set":23,"id":"69b94e4b_80a3cfa8","line":56,"range":{"start_line":56,"start_character":20,"end_line":56,"end_character":35},"in_reply_to":"6f9bf80a_1dc55158","updated":"2021-12-24 02:08:08.000000000","message":"Done","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_any_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"},{"line_number":58,"context_line":"* Reported it to the scheduler as a capability, for example \"efficient_revert_middle_snap\"."},{"line_number":59,"context_line":"  That way volumes types can be created to place volumes in a backend supporting this."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":23,"id":"09c24c46_c7f3677d","line":57,"updated":"2021-12-19 18:06:48.000000000","message":"A question about this option: does it disable the feature completely, or just disable the generic revert-to-any operation?  Also, what will the default value of this option be?","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"a2f43dac1f674fee8ebae982081ccb535edab04a","unresolved":true,"context_lines":[{"line_number":54,"context_line":"* We can also design an interface \u0027revert_to_any_snapshot\u0027, if driver is not implemented"},{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"},{"line_number":58,"context_line":"* Reported it to the scheduler as a capability, for example \"efficient_revert_middle_snap\"."},{"line_number":59,"context_line":"  That way volumes types can be created to place volumes in a backend supporting this."},{"line_number":60,"context_line":""}],"source_content_type":"text/x-rst","patch_set":23,"id":"20d52e93_a1f9c3ea","line":57,"in_reply_to":"09c24c46_c7f3677d","updated":"2021-12-24 02:08:08.000000000","message":"The default value is true，\nbecause the generic revert is copy the all data to the volume, so the completion time depends on the IO network condition。if io network is poor， the user can set it false.\nif false and the allow_revert_to_any_snapshot is false, the user can not revert any snapshot.","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"},{"line_number":58,"context_line":"* Reported it to the scheduler as a capability, for example \"efficient_revert_middle_snap\"."},{"line_number":59,"context_line":"  That way volumes types can be created to place volumes in a backend supporting this."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"Vendor-specific changes"}],"source_content_type":"text/x-rst","patch_set":23,"id":"a3492333_50ecdea1","line":58,"range":{"start_line":58,"start_character":78,"end_line":58,"end_character":84},"updated":"2021-12-19 18:06:48.000000000","message":"any","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"a2f43dac1f674fee8ebae982081ccb535edab04a","unresolved":false,"context_lines":[{"line_number":55,"context_line":"  we use the \u0027_revert_to_snapshot_generic\u0027 function to revert"},{"line_number":56,"context_line":"* Add configuration item to control whether to use \u0027_revert_to_snapshot_generic\u0027 method, admin"},{"line_number":57,"context_line":"  can disable this feature when the performance is very poor"},{"line_number":58,"context_line":"* Reported it to the scheduler as a capability, for example \"efficient_revert_middle_snap\"."},{"line_number":59,"context_line":"  That way volumes types can be created to place volumes in a backend supporting this."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"Vendor-specific changes"}],"source_content_type":"text/x-rst","patch_set":23,"id":"25745b5b_94e5efd8","line":58,"range":{"start_line":58,"start_character":78,"end_line":58,"end_character":84},"in_reply_to":"a3492333_50ecdea1","updated":"2021-12-24 02:08:08.000000000","message":"Done","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"Other end user impact"},{"line_number":88,"context_line":"---------------------"},{"line_number":89,"context_line":"When mv 3.68 or later is requested and the config option is False to indicate"},{"line_number":90,"context_line":"that reverting to any snapshot is not supported in this cloud (if mv \u003c 3.68"},{"line_number":91,"context_line":"is requested, just use the current error message)."},{"line_number":92,"context_line":""}],"source_content_type":"text/x-rst","patch_set":23,"id":"a3db50ca_26ac32d1","line":89,"range":{"start_line":89,"start_character":35,"end_line":89,"end_character":65},"updated":"2021-12-19 18:06:48.000000000","message":"I guess this depends on how the config option behaves (see my question above).","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":88,"context_line":"---------------------"},{"line_number":89,"context_line":"When mv 3.68 or later is requested and the config option is False to indicate"},{"line_number":90,"context_line":"that reverting to any snapshot is not supported in this cloud (if mv \u003c 3.68"},{"line_number":91,"context_line":"is requested, just use the current error message)."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"Performance Impact"},{"line_number":94,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":23,"id":"1699fd6c_109443ed","line":91,"updated":"2021-12-19 18:06:48.000000000","message":"One other impact is that the end user is somehow going to have to keep track of what\u0027s in their snapshots.  What I mean is this:\n\nSuppose you have volume V, and you do the following:\nmonday: create snap s1\ntuesday: create snap s2\nwednesday: create snap s3\nthursday: revert V to s1 and then create s4\nfriday, etc., continue working\n\nWhen the user lists snapshots, most recent first, you get:\n  s4, s3, s2, s1\n\nBut what you actually have is two chains:\n  s1, s4\n  s1, s2, s3\n\nThe current cinder model doesn\u0027t allow this, because to get back to s1, you would have to have deleted s2 and s3, so when you listed snapshots, all you would see is\n  s4, s1\n\nIn other words, it just so happens that all the snapshot chains available for a particular volume can currently be revealed by listing the snapshots in chronological order.  But once your proposal is implemented, this will no longer be the case.  This is going make things difficult for users unless you have some scheme for adding appropriate info to the snapshots so that users can distinguish chains.  (This is the problem about shapshots forming a tree that Eric brought up a while back, but I don\u0027t think you ever addressed it.)","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"a2f43dac1f674fee8ebae982081ccb535edab04a","unresolved":true,"context_lines":[{"line_number":88,"context_line":"---------------------"},{"line_number":89,"context_line":"When mv 3.68 or later is requested and the config option is False to indicate"},{"line_number":90,"context_line":"that reverting to any snapshot is not supported in this cloud (if mv \u003c 3.68"},{"line_number":91,"context_line":"is requested, just use the current error message)."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"Performance Impact"},{"line_number":94,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":23,"id":"88d32390_b226870a","line":91,"in_reply_to":"1699fd6c_109443ed","updated":"2021-12-24 02:08:08.000000000","message":"thanks for explain very much。\nI thought about it. If we restore and creation snapshot frequently，the tree will be many，and difficult to manage in the cinder layer， i realy have no good idea about this，\nI think above problem should managed by the storage system。","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"d8eb09d1d6db8b8560543cdcb49109052b6338a9","unresolved":true,"context_lines":[{"line_number":88,"context_line":"---------------------"},{"line_number":89,"context_line":"When mv 3.68 or later is requested and the config option is False to indicate"},{"line_number":90,"context_line":"that reverting to any snapshot is not supported in this cloud (if mv \u003c 3.68"},{"line_number":91,"context_line":"is requested, just use the current error message)."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"Performance Impact"},{"line_number":94,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":23,"id":"a936716b_c02f081e","line":91,"in_reply_to":"88d32390_b226870a","updated":"2021-12-24 14:10:13.000000000","message":"I tend to agree that this complexity should be managed by the storage backend, but the operator should also be aware of what has transpired on their volume. The timestamps should allow them to see the current date of the volume, as that should match the last snapshot restore that was performed - if it doesn\u0027t then it should, otherwise there is no way to track any snapshot recvoery has occured. Can someone check this (I can\u0027t currently as I don\u0027t have access to a system [due to time of year[) and confirm if the timestamp changes on the volume after a snapshot restore - this is the way a backend generally exposes that a snapshot recovery has occured.\nIt should be clearly stated in the user documentation that where there are multiple snapshots it is the responsibilty of the operator to manage the recovery actions that have previously been employed on a volume.","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"fb0c261f7187ac4382e87c59423fd86f6e2bf238","unresolved":true,"context_lines":[{"line_number":88,"context_line":"---------------------"},{"line_number":89,"context_line":"When mv 3.68 or later is requested and the config option is False to indicate"},{"line_number":90,"context_line":"that reverting to any snapshot is not supported in this cloud (if mv \u003c 3.68"},{"line_number":91,"context_line":"is requested, just use the current error message)."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"Performance Impact"},{"line_number":94,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":23,"id":"78283c09_b7e8a127","line":91,"in_reply_to":"a936716b_c02f081e","updated":"2022-01-06 20:27:22.000000000","message":"OK - I had a chance to check the revert-to-snapshot functionality and a revert does not change the created timestamp for the volume. \nWithout this timestamp changing to match the timestamp of the reverted snapshot there is no way to check if a volume has ever been reverted to. \nThis, IMHO, is a bug with the revert-to-snapshot implementation.\nI see no way forward for this spec until this is resolved.","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"bafc95bc0ee27d8da84ab1487de4dd27683881d9","unresolved":true,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Documentation Impact"},{"line_number":129,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":130,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":131,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":132,"context_line":"functionality."},{"line_number":133,"context_line":""}],"source_content_type":"text/x-rst","patch_set":23,"id":"dcd9ea49_a36324ba","line":130,"range":{"start_line":130,"start_character":45,"end_line":130,"end_character":50},"updated":"2021-12-19 18:06:48.000000000","message":"any","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"},{"author":{"_account_id":30092,"name":"Xuan Yandong","email":"xuanyd@outlook.com","username":"xuanyandong"},"change_message_id":"a2f43dac1f674fee8ebae982081ccb535edab04a","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Documentation Impact"},{"line_number":129,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":130,"context_line":"Release note should point out that revert to other snapshot is supported and"},{"line_number":131,"context_line":"update the driver support matrix to list which drivers have optimized revert"},{"line_number":132,"context_line":"functionality."},{"line_number":133,"context_line":""}],"source_content_type":"text/x-rst","patch_set":23,"id":"b2719f34_43571716","line":130,"range":{"start_line":130,"start_character":45,"end_line":130,"end_character":50},"in_reply_to":"dcd9ea49_a36324ba","updated":"2021-12-24 02:08:08.000000000","message":"Done","commit_id":"eb166a072b00a97bb3069a00179265c8f5155e37"}]}