)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"46568971_0e345259","updated":"2022-11-16 08:49:55.000000000","message":"Thanks for proposing the spec, few comments inline.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e97c122b_ce2a4d8d","updated":"2022-11-23 14:57:17.000000000","message":"Reviewers on the Nova spec again brought up concerns regarding this being a synchronous solution, and I kinda agree with them.\n\nI\u0027m currently considering submitting an alternative spec for assisted volume extend that proposes a callback volume action similar to os-migrate-volume-completion.\n\nIs this a good idea or would it just be confusing to have two competing specs?","commit_id":"6b36f6e0f13d65452553222487a4a12a6789dc7f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c6ec54c1_f181e336","updated":"2022-12-05 04:08:55.000000000","message":"Thanks Konrad for the update. The spec looks good to me.","commit_id":"6b36f6e0f13d65452553222487a4a12a6789dc7f"}],"specs/2023.1/use-assisted-volume-extend-api.rst":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This blueprint proposes to use the assisted volume extend API [1]_ that has"},{"line_number":14,"context_line":"been proposed for Nova, to support online extend for remotefs-based volume"},{"line_number":15,"context_line":"drivers such as NFS, NetApp NFS, and Quobyte."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Problem description"},{"line_number":18,"context_line":"\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":2,"id":"878f95bc_175ee304","line":15,"range":{"start_line":15,"start_character":16,"end_line":15,"end_character":44},"updated":"2022-11-16 08:49:55.000000000","message":"we also need to consider the powerstore nfs driver added recently\n\nhttps://github.com/openstack/cinder/blob/dcec2f6f01ffc63dc058f641370e9f5a0bad07e4/cinder/volume/drivers/dell_emc/powerstore/nfs.py#L130-L133","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This blueprint proposes to use the assisted volume extend API [1]_ that has"},{"line_number":14,"context_line":"been proposed for Nova, to support online extend for remotefs-based volume"},{"line_number":15,"context_line":"drivers such as NFS, NetApp NFS, and Quobyte."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Problem description"},{"line_number":18,"context_line":"\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":2,"id":"78de66b0_e9fb0986","line":15,"range":{"start_line":15,"start_character":16,"end_line":15,"end_character":44},"in_reply_to":"878f95bc_175ee304","updated":"2022-11-23 14:57:17.000000000","message":"Done","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Nova already uses this command to notify QEMU of a change in volume size when"},{"line_number":30,"context_line":"handling a ``volume-extended`` external server event."},{"line_number":31,"context_line":"A previous change tried to use this mechanism to support online extend for the"},{"line_number":32,"context_line":"NFS driver [2]_, but was abandoned."},{"line_number":33,"context_line":"Problems with this approach were the lack of feedback from Nova on the success"},{"line_number":34,"context_line":"of the operation, and the risk of leaving the volume with an inconsistent size"},{"line_number":35,"context_line":"and state on a failure."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"The blueprint at [1]_ proposes a new synchronous API for assisted volume"},{"line_number":38,"context_line":"extend to fix those issues."}],"source_content_type":"text/x-rst","patch_set":2,"id":"7c447a42_0c018c35","line":35,"range":{"start_line":31,"start_character":0,"end_line":35,"end_character":23},"updated":"2022-11-16 08:49:55.000000000","message":"Although having drawbacks, this is better suited in the alternatives section","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Nova already uses this command to notify QEMU of a change in volume size when"},{"line_number":30,"context_line":"handling a ``volume-extended`` external server event."},{"line_number":31,"context_line":"A previous change tried to use this mechanism to support online extend for the"},{"line_number":32,"context_line":"NFS driver [2]_, but was abandoned."},{"line_number":33,"context_line":"Problems with this approach were the lack of feedback from Nova on the success"},{"line_number":34,"context_line":"of the operation, and the risk of leaving the volume with an inconsistent size"},{"line_number":35,"context_line":"and state on a failure."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"The blueprint at [1]_ proposes a new synchronous API for assisted volume"},{"line_number":38,"context_line":"extend to fix those issues."}],"source_content_type":"text/x-rst","patch_set":2,"id":"631764d4_00750cf6","line":35,"range":{"start_line":31,"start_character":0,"end_line":35,"end_character":23},"in_reply_to":"7c447a42_0c018c35","updated":"2022-11-23 14:57:17.000000000","message":"Done","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":55,"context_line":"will be available as a mixin for the remotefs driver, similar to the mixins"},{"line_number":56,"context_line":"used by the remotefs-based smbfs driver."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"The mixin will provide one private method to call the assisted extend API, and"},{"line_number":59,"context_line":"another one that wraps ``cinder.image.image_utils.resize_image()`` and serves"},{"line_number":60,"context_line":"as a drop-in replacement."},{"line_number":61,"context_line":"The second method will call ``image_utils.resize_image()``, catch"},{"line_number":62,"context_line":"``oslo_concurrency.processutils.ProcessExecutionError`` exceptions and check"},{"line_number":63,"context_line":"the stored ``stderr`` for a message indicating an active lock on the file (see"},{"line_number":64,"context_line":"the proposed change to ``nfs.py`` in [2]_ for an example on how to do that)."},{"line_number":65,"context_line":"If such a message is found, it will use the first method to call the assisted"},{"line_number":66,"context_line":"extend API."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"}],"source_content_type":"text/x-rst","patch_set":2,"id":"c759c985_a156eb30","line":66,"range":{"start_line":58,"start_character":0,"end_line":66,"end_character":11},"updated":"2022-11-16 08:49:55.000000000","message":"I don\u0027t mind the extra low level details but we could have rephrased it as,\n\nWe will update the current resize method for the following cases:\n1) Volume in available state: do what we currently do i.e. execute qemu-img command to resize the volume file\n2) Volume is in-use state (attached): We handle the case when lock is acquired by nova qemu process and call the assisted extend API\n\nI don\u0027t have strong opinion on the rephrase but if you plan to do feel free to change the wordings or add details as necessary, I\u0027m just providing a structure for rephrasing","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":false,"context_lines":[{"line_number":55,"context_line":"will be available as a mixin for the remotefs driver, similar to the mixins"},{"line_number":56,"context_line":"used by the remotefs-based smbfs driver."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"The mixin will provide one private method to call the assisted extend API, and"},{"line_number":59,"context_line":"another one that wraps ``cinder.image.image_utils.resize_image()`` and serves"},{"line_number":60,"context_line":"as a drop-in replacement."},{"line_number":61,"context_line":"The second method will call ``image_utils.resize_image()``, catch"},{"line_number":62,"context_line":"``oslo_concurrency.processutils.ProcessExecutionError`` exceptions and check"},{"line_number":63,"context_line":"the stored ``stderr`` for a message indicating an active lock on the file (see"},{"line_number":64,"context_line":"the proposed change to ``nfs.py`` in [2]_ for an example on how to do that)."},{"line_number":65,"context_line":"If such a message is found, it will use the first method to call the assisted"},{"line_number":66,"context_line":"extend API."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"}],"source_content_type":"text/x-rst","patch_set":2,"id":"27904763_4ead23d2","line":66,"range":{"start_line":58,"start_character":0,"end_line":66,"end_character":11},"in_reply_to":"c759c985_a156eb30","updated":"2022-11-23 14:57:17.000000000","message":"I removed most of the low level details and tried to formalize better what we are trying to do here.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":65,"context_line":"If such a message is found, it will use the first method to call the assisted"},{"line_number":66,"context_line":"extend API."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"},{"line_number":70,"context_line":"``SHELVED_OFFLOADED``."},{"line_number":71,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"c9310930_05736da3","line":68,"range":{"start_line":68,"start_character":51,"end_line":68,"end_character":57},"updated":"2022-11-16 08:49:55.000000000","message":"volumes?","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":false,"context_lines":[{"line_number":65,"context_line":"If such a message is found, it will use the first method to call the assisted"},{"line_number":66,"context_line":"extend API."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"},{"line_number":70,"context_line":"``SHELVED_OFFLOADED``."},{"line_number":71,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"f0e1320f_7b621aac","line":68,"range":{"start_line":68,"start_character":51,"end_line":68,"end_character":57},"in_reply_to":"c9310930_05736da3","updated":"2022-11-23 14:57:17.000000000","message":"Ack","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"},{"line_number":70,"context_line":"``SHELVED_OFFLOADED``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The NFS, Netapp NFS, and Quobyte drivers all use ``image_utils.resize_image``,"},{"line_number":73,"context_line":"so support for assisted extend will be added by inheriting from the new mixin"}],"source_content_type":"text/x-rst","patch_set":2,"id":"2e2b05b9_541a3d8c","line":70,"range":{"start_line":70,"start_character":0,"end_line":70,"end_character":22},"updated":"2022-11-16 08:49:55.000000000","message":"In this case, how do we plan on notifying nova about the new size?","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"},{"line_number":70,"context_line":"``SHELVED_OFFLOADED``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The NFS, Netapp NFS, and Quobyte drivers all use ``image_utils.resize_image``,"},{"line_number":73,"context_line":"so support for assisted extend will be added by inheriting from the new mixin"}],"source_content_type":"text/x-rst","patch_set":2,"id":"c461a397_b2be3f87","line":70,"range":{"start_line":70,"start_character":0,"end_line":70,"end_character":22},"in_reply_to":"2e2b05b9_541a3d8c","updated":"2022-11-23 14:57:17.000000000","message":"Sorry, that detail was missing.\nCurrently, the spec proposes using the new API from within the driver, in a way that is oblivious to the volume manager.\nSo the volume manager would always send the external server event on attached resize, whether it used assisted extend or not.\n\nIt would be on Nova then to gracefully handle events for volumes that it already knows the new size of, because of assisted extend.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"Trying ``image_utils.resize_image()`` for attached images is done in case"},{"line_number":69,"context_line":"the instance is in a state where the volume is no longer locked, such as"},{"line_number":70,"context_line":"``SHELVED_OFFLOADED``."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The NFS, Netapp NFS, and Quobyte drivers all use ``image_utils.resize_image``,"},{"line_number":73,"context_line":"so support for assisted extend will be added by inheriting from the new mixin"}],"source_content_type":"text/x-rst","patch_set":2,"id":"4cfdc9a5_934dca4a","line":70,"range":{"start_line":70,"start_character":0,"end_line":70,"end_character":22},"in_reply_to":"c461a397_b2be3f87","updated":"2022-12-05 04:08:55.000000000","message":"Ack but we need to be sure nova does handle this.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":78,"context_line":"------------"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This spec assumes the synchronous API described in [1]_."},{"line_number":81,"context_line":"Instead of handling the new feature in the remotefs driver, it could also be"},{"line_number":82,"context_line":"integrated into the base volume driver or the cinder-volume handler, to make"},{"line_number":83,"context_line":"it available to other drivers."},{"line_number":84,"context_line":"However, since currently only remotefs-based drivers have use for the feature,"},{"line_number":85,"context_line":"and the related assisted snapshot feature is handled in the remotefs driver as"},{"line_number":86,"context_line":"well, this seems like the appropriate place to include it."}],"source_content_type":"text/x-rst","patch_set":2,"id":"c064d5e9_0ca5d189","line":83,"range":{"start_line":81,"start_character":0,"end_line":83,"end_character":30},"updated":"2022-11-16 08:49:55.000000000","message":"we need to highlight that after implementing this change, we will have 2 different ways of extending volumes. one for FS drivers and one for others.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":false,"context_lines":[{"line_number":78,"context_line":"------------"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This spec assumes the synchronous API described in [1]_."},{"line_number":81,"context_line":"Instead of handling the new feature in the remotefs driver, it could also be"},{"line_number":82,"context_line":"integrated into the base volume driver or the cinder-volume handler, to make"},{"line_number":83,"context_line":"it available to other drivers."},{"line_number":84,"context_line":"However, since currently only remotefs-based drivers have use for the feature,"},{"line_number":85,"context_line":"and the related assisted snapshot feature is handled in the remotefs driver as"},{"line_number":86,"context_line":"well, this seems like the appropriate place to include it."}],"source_content_type":"text/x-rst","patch_set":2,"id":"758a4dcb_236db7ba","line":83,"range":{"start_line":81,"start_character":0,"end_line":83,"end_character":30},"in_reply_to":"1317d2ba_38e21c91","updated":"2022-12-05 04:08:55.000000000","message":"Yeah that makes sense. I was somehow under the assumption that we are changing the manager code as well. looks good then.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":true,"context_lines":[{"line_number":78,"context_line":"------------"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This spec assumes the synchronous API described in [1]_."},{"line_number":81,"context_line":"Instead of handling the new feature in the remotefs driver, it could also be"},{"line_number":82,"context_line":"integrated into the base volume driver or the cinder-volume handler, to make"},{"line_number":83,"context_line":"it available to other drivers."},{"line_number":84,"context_line":"However, since currently only remotefs-based drivers have use for the feature,"},{"line_number":85,"context_line":"and the related assisted snapshot feature is handled in the remotefs driver as"},{"line_number":86,"context_line":"well, this seems like the appropriate place to include it."}],"source_content_type":"text/x-rst","patch_set":2,"id":"1317d2ba_38e21c91","line":83,"range":{"start_line":81,"start_character":0,"end_line":83,"end_character":30},"in_reply_to":"c064d5e9_0ca5d189","updated":"2022-11-23 14:57:17.000000000","message":"The way a driver extends volumes is usually different between drivers.\nThis is more like a solution for a special case that only occurs in some FS drivers and does not impact the offline extend support that was already there.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"Alternatives"},{"line_number":78,"context_line":"------------"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This spec assumes the synchronous API described in [1]_."},{"line_number":81,"context_line":"Instead of handling the new feature in the remotefs driver, it could also be"},{"line_number":82,"context_line":"integrated into the base volume driver or the cinder-volume handler, to make"},{"line_number":83,"context_line":"it available to other drivers."},{"line_number":84,"context_line":"However, since currently only remotefs-based drivers have use for the feature,"},{"line_number":85,"context_line":"and the related assisted snapshot feature is handled in the remotefs driver as"},{"line_number":86,"context_line":"well, this seems like the appropriate place to include it."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The alternatives to a synchronous API listed in [1]_ require either polling or"},{"line_number":89,"context_line":"a new callback in the Cinder API."},{"line_number":90,"context_line":"Both are more complex to implement than a synchronous call, but also have"},{"line_number":91,"context_line":"precedents in the Cinder:"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"The existing assisted snapshot feature combines a callback (to change the"},{"line_number":94,"context_line":"snapshot status) and polling (for the changed snapshot status)."},{"line_number":95,"context_line":"This allows setup and cleanup of the operation to be handled by the same"},{"line_number":96,"context_line":"thread."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"The assisted volume migration feature uses only a callback, which requires"},{"line_number":99,"context_line":"the cleanup to be split into a different method, which may be called by a"},{"line_number":100,"context_line":"different thread handling the callback."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"For an asynchronous assisted volume extend, both would be possible but the"},{"line_number":103,"context_line":"pure callback option would require some data, such as the reference to the"},{"line_number":104,"context_line":"preliminary quota reservation, to be stored persistently in the volume"},{"line_number":105,"context_line":"metadata, so that the callback can either finalize or rollback the change."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Data model impact"}],"source_content_type":"text/x-rst","patch_set":2,"id":"7de5846a_50f474df","line":105,"range":{"start_line":79,"start_character":0,"end_line":105,"end_character":74},"updated":"2022-11-16 08:49:55.000000000","message":"It\u0027s hard to understand which one is an alternative and which one is a sub point under an alternative, can you make it point wise as to how many alternatives we have?","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":true,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"Alternatives"},{"line_number":78,"context_line":"------------"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This spec assumes the synchronous API described in [1]_."},{"line_number":81,"context_line":"Instead of handling the new feature in the remotefs driver, it could also be"},{"line_number":82,"context_line":"integrated into the base volume driver or the cinder-volume handler, to make"},{"line_number":83,"context_line":"it available to other drivers."},{"line_number":84,"context_line":"However, since currently only remotefs-based drivers have use for the feature,"},{"line_number":85,"context_line":"and the related assisted snapshot feature is handled in the remotefs driver as"},{"line_number":86,"context_line":"well, this seems like the appropriate place to include it."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The alternatives to a synchronous API listed in [1]_ require either polling or"},{"line_number":89,"context_line":"a new callback in the Cinder API."},{"line_number":90,"context_line":"Both are more complex to implement than a synchronous call, but also have"},{"line_number":91,"context_line":"precedents in the Cinder:"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"The existing assisted snapshot feature combines a callback (to change the"},{"line_number":94,"context_line":"snapshot status) and polling (for the changed snapshot status)."},{"line_number":95,"context_line":"This allows setup and cleanup of the operation to be handled by the same"},{"line_number":96,"context_line":"thread."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"The assisted volume migration feature uses only a callback, which requires"},{"line_number":99,"context_line":"the cleanup to be split into a different method, which may be called by a"},{"line_number":100,"context_line":"different thread handling the callback."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"For an asynchronous assisted volume extend, both would be possible but the"},{"line_number":103,"context_line":"pure callback option would require some data, such as the reference to the"},{"line_number":104,"context_line":"preliminary quota reservation, to be stored persistently in the volume"},{"line_number":105,"context_line":"metadata, so that the callback can either finalize or rollback the change."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Data model impact"}],"source_content_type":"text/x-rst","patch_set":2,"id":"831a464e_c4a49626","line":105,"range":{"start_line":79,"start_character":0,"end_line":105,"end_character":74},"in_reply_to":"7de5846a_50f474df","updated":"2022-11-23 14:57:17.000000000","message":"How many total alternatives we have is difficult to show, because in the case of an asynchronous solution we have independent alternatives for calling Nova and getting feedback from Nova.\n\nI tried to structure it a bit better, though.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"Alternatives"},{"line_number":78,"context_line":"------------"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This spec assumes the synchronous API described in [1]_."},{"line_number":81,"context_line":"Instead of handling the new feature in the remotefs driver, it could also be"},{"line_number":82,"context_line":"integrated into the base volume driver or the cinder-volume handler, to make"},{"line_number":83,"context_line":"it available to other drivers."},{"line_number":84,"context_line":"However, since currently only remotefs-based drivers have use for the feature,"},{"line_number":85,"context_line":"and the related assisted snapshot feature is handled in the remotefs driver as"},{"line_number":86,"context_line":"well, this seems like the appropriate place to include it."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The alternatives to a synchronous API listed in [1]_ require either polling or"},{"line_number":89,"context_line":"a new callback in the Cinder API."},{"line_number":90,"context_line":"Both are more complex to implement than a synchronous call, but also have"},{"line_number":91,"context_line":"precedents in the Cinder:"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"The existing assisted snapshot feature combines a callback (to change the"},{"line_number":94,"context_line":"snapshot status) and polling (for the changed snapshot status)."},{"line_number":95,"context_line":"This allows setup and cleanup of the operation to be handled by the same"},{"line_number":96,"context_line":"thread."},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"The assisted volume migration feature uses only a callback, which requires"},{"line_number":99,"context_line":"the cleanup to be split into a different method, which may be called by a"},{"line_number":100,"context_line":"different thread handling the callback."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"For an asynchronous assisted volume extend, both would be possible but the"},{"line_number":103,"context_line":"pure callback option would require some data, such as the reference to the"},{"line_number":104,"context_line":"preliminary quota reservation, to be stored persistently in the volume"},{"line_number":105,"context_line":"metadata, so that the callback can either finalize or rollback the change."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Data model impact"}],"source_content_type":"text/x-rst","patch_set":2,"id":"c5071cbe_fcd6cba6","line":105,"range":{"start_line":79,"start_character":0,"end_line":105,"end_character":74},"in_reply_to":"831a464e_c4a49626","updated":"2022-12-05 04:08:55.000000000","message":"Looks good thanks","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa7d37e012dba8ad849953b104eacc11bb018b28","unresolved":true,"context_lines":[{"line_number":113,"context_line":"REST API impact"},{"line_number":114,"context_line":"---------------"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"Calling ``os-extend`` on an attached volume managed by one of the affected"},{"line_number":117,"context_line":"drivers will no longer automatically result in an ``error_extending`` status."},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"Instead, an assisted extend operation will be attempted and the volume status"},{"line_number":120,"context_line":"will be updated depending on the success of the operation."}],"source_content_type":"text/x-rst","patch_set":2,"id":"7c6c1e37_dd1dfc9a","line":117,"range":{"start_line":116,"start_character":0,"end_line":117,"end_character":77},"updated":"2022-11-16 08:49:55.000000000","message":"do we get an async or sync error in response? If we get a synchronous error from API then from a REST API perspective, i think it\u0027s better to also tell which HTTP response we will get.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":34859,"name":"Konrad Gube","email":"konrad.gube@cloudandheat.com","username":"kgube"},"change_message_id":"d8012d08e44b7c0022e34af440a7cc86541d7e04","unresolved":false,"context_lines":[{"line_number":113,"context_line":"REST API impact"},{"line_number":114,"context_line":"---------------"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"Calling ``os-extend`` on an attached volume managed by one of the affected"},{"line_number":117,"context_line":"drivers will no longer automatically result in an ``error_extending`` status."},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"Instead, an assisted extend operation will be attempted and the volume status"},{"line_number":120,"context_line":"will be updated depending on the success of the operation."}],"source_content_type":"text/x-rst","patch_set":2,"id":"ea9ced47_f20f0bbe","line":117,"range":{"start_line":116,"start_character":0,"end_line":117,"end_character":77},"in_reply_to":"7c6c1e37_dd1dfc9a","updated":"2022-11-23 14:57:17.000000000","message":"os-extend is an asynchronous volume action so it always responds 202, but changes the volume status depending on success/failure.","commit_id":"075b4e2c14d7b00bb1b2be49a2d06cf35df2998e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":true,"context_lines":[{"line_number":40,"context_line":"Proposed change"},{"line_number":41,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"We will update the extend_volume method of selected volume drivers to no"},{"line_number":44,"context_line":"longer fail when the volume to be extended is attached."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"The extend_volume method of these drivers will also be updated to catch"}],"source_content_type":"text/x-rst","patch_set":3,"id":"6ec1ce60_d18eedf5","line":43,"range":{"start_line":43,"start_character":43,"end_line":43,"end_character":51},"updated":"2022-12-05 04:08:55.000000000","message":"nit: I think we will only update File system type drivers so could\u0027ve mentioned that","commit_id":"6b36f6e0f13d65452553222487a4a12a6789dc7f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":true,"context_lines":[{"line_number":69,"context_line":"manager will continue to update the volume object and finalize the quota"},{"line_number":70,"context_line":"change."},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The volume manager will also continue to send the ``volume-extended`` external"},{"line_number":73,"context_line":"server event to Nova, regardless of whether the extend operation was assisted"},{"line_number":74,"context_line":"or not."},{"line_number":75,"context_line":"Nova has the responsibility of dealing correctly with this event for a volume"},{"line_number":76,"context_line":"which, by assisted extend, it already knows has been extended."},{"line_number":77,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"e7df2b69_17358c00","line":74,"range":{"start_line":72,"start_character":0,"end_line":74,"end_character":7},"updated":"2022-12-05 04:08:55.000000000","message":"nit: We will only send these requests when volume is attached. I would like this to be explicitly mentioned so people don\u0027t assume we will be sending these events for available volumes (which we never do).\n\nthis can be rephrased as,\n\n... external server event to Nova when volume is attached, regardless ...","commit_id":"6b36f6e0f13d65452553222487a4a12a6789dc7f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8c2272a5100c1ac38a8c46147faa5850324e7973","unresolved":false,"context_lines":[{"line_number":110,"context_line":"  Both introduce more complexity than using a synchronous API, but support"},{"line_number":111,"context_line":"  longer running operations and have precedents in Cinder."},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"  * The existing assisted snapshot feature combines a callback (to change the"},{"line_number":114,"context_line":"    snapshot status) and polling (for the changed snapshot status)."},{"line_number":115,"context_line":"    This allows setup and cleanup of the operation to be handled by the same"},{"line_number":116,"context_line":"    thread."},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"  * The assisted volume migration feature uses only a callback, which requires"},{"line_number":119,"context_line":"    the cleanup to be split into a different method, which may be called by a"}],"source_content_type":"text/x-rst","patch_set":3,"id":"6fac2410_1d019867","line":116,"range":{"start_line":113,"start_character":0,"end_line":116,"end_character":11},"updated":"2022-12-05 04:08:55.000000000","message":"I think there was some work ongoing to change this and use external events here instead. this is clearly not what we promote for new features.","commit_id":"6b36f6e0f13d65452553222487a4a12a6789dc7f"}]}
