)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ba082db035a6771be5aa870397cb826943b3020b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7de79d5b_8e7c2046","updated":"2024-12-11 15:23:30.000000000","message":"FWIW, the current policy can be found here : \nhttps://docs.openstack.org/nova/latest/configuration/policy.html\n\n\n```\nos_compute_api:os-volumes-attachments:show\n\n    Default:\n\n        rule:project_reader_or_admin\n    Operations:\n\n            GET /servers/{server_id}/os-volume_attachments/{volume_id}\n\n    Scope Types:\n\n            project\n\n    Show details of a volume attachment\n```","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3d8572237945167084482a8632ff54b061e04bfe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"806b14c3_a7c60734","updated":"2024-12-11 15:20:55.000000000","message":"I have some concerns about the security issue if someone directly calls the API.","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7dae178e03bef2d54d2f68023f530a6e2366d5a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"172d6a8f_6efade26","updated":"2024-12-13 00:17:43.000000000","message":"I\u0027m not asking for an immediate change to the spec but rather would like to discuss a little bit about how to approach this. The feature itself I think is reasonable to be able to expose a volume swap progress for API callers.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"010a374f0eced86351edd581b41b9d2fa2d9a51d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f876c467_9482f955","updated":"2025-03-10 18:58:55.000000000","message":"I would like advice on how to move this project along. This spec has had eyes on it and I responded to comments. The corresponding cinder spec [1] was approved, but the cinder feature relies on nova\u0027s.\n\nMeanwhile, I forged ahead and proposed the code for nova [2] and cinder [3], and have a tempest patch [4] that shows the new tempest.scenario.test_volume_migrate_attached.TestVolumeMigrationProgress test passes [5]\n\n[1] https://review.opendev.org/c/openstack/cinder-specs/+/937808\n[2] https://review.opendev.org/c/openstack/nova/+/941476\n[3] https://review.opendev.org/c/openstack/cinder/+/942716\n[4] https://review.opendev.org/c/openstack/tempest/+/943614\n[5] https://5e7e7be1ea42142e6d74-f918abb3dcdfb01ed824790c230ea283.ssl.cf2.rackcdn.com/943614/1/check/tempest-slow-py3/86d4ffb/testr_results.html","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ddf5cb14914e4d22d45b7be82163ffa5b7e7cd6f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a311dbc4_a3f8f643","updated":"2025-01-10 00:06:24.000000000","message":"Just some thoughts inline. I\u0027m interested to know what other reviewers think since there\u0027s a fair bit to consider here.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a36929b5_2c34b812","updated":"2025-12-04 19:26:05.000000000","message":"Overall I am good with the proposal but only one thing blocking me to +2 on this spec and that is something I would like this spec to include. I am not going for implementation detail and ok if that is achieved in other way:\n\n- Nova should only make compute RPC call to calculate the swap progress only if swap is in progress.\n\nThe current plan of calling swap progress RPC call only if call is from cinder and cinder will only call this API when volume is in migration, does not solve the Nova concern of unnecessary RPC call. Cinder or any other service can call this API for other use case also and then swap progress RPC call will be unnecessary.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8fd592e32d669d223ea98cb8be558a0505dd5d95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b7f41350_e6eee2d6","updated":"2025-12-04 17:34:22.000000000","message":"Thank you, everyone, for the feedback. Is there any chance the spec is sufficient, and remaining unresolved questions could be addressed during the implementation phase?","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f90abceeb0bc02a74a11c2b23bfa77cfa63c2de7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d8b8ad18_7384640e","updated":"2025-12-04 17:57:49.000000000","message":"i think the salient point are captured and the main design is captured in the spec.\n\nbut to proceed we need to ensure dan and gansham agree that there concerns have been addressed or can be based on the over all direction.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"293599d7cd4fe581307d6fea51996ae54eeac192","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0bb0299f_3f503462","updated":"2025-12-03 18:31:35.000000000","message":"some comment inline but over all im OK with this directionally.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"}],"specs/2025.1/approved/swap-progress-in-attachment-details.rst":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3d8572237945167084482a8632ff54b061e04bfe","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            \"bdm_uuid\": \"c088db45-92b8-49e8-81e2-a1b77a144b3b\""},{"line_number":104,"context_line":"            \"swap_progress\": \"42\""},{"line_number":105,"context_line":"        }"},{"line_number":106,"context_line":"    }"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Security impact"},{"line_number":109,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"1ec88237_1bc1f0ed","line":106,"updated":"2024-12-11 15:20:55.000000000","message":"I guess you would need to provide some specific API policy for it, right?\nFor the moment, AFAIK, any user can call the ``GET /servers/{server_id}/os-volume_attachments/{volume_id}`` API : \n\nhttps://docs.openstack.org/api-ref/compute/#show-a-detail-of-a-volume-attachment","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"e7e41ef8ddc4a49b8a1c41cc4f654a338c46d2df","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            \"bdm_uuid\": \"c088db45-92b8-49e8-81e2-a1b77a144b3b\""},{"line_number":104,"context_line":"            \"swap_progress\": \"42\""},{"line_number":105,"context_line":"        }"},{"line_number":106,"context_line":"    }"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Security impact"},{"line_number":109,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"89b45b3f_60ab6217","line":106,"in_reply_to":"1ec88237_1bc1f0ed","updated":"2024-12-11 15:39:24.000000000","message":"ack, and will provide a more complete response below.","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8d24dc81df81854735cbe151ecba9b4e1e752ed3","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            \"bdm_uuid\": \"c088db45-92b8-49e8-81e2-a1b77a144b3b\""},{"line_number":104,"context_line":"            \"swap_progress\": \"42\""},{"line_number":105,"context_line":"        }"},{"line_number":106,"context_line":"    }"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Security impact"},{"line_number":109,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"c98f6c8e_f3fde1ce","line":106,"in_reply_to":"89b45b3f_60ab6217","updated":"2025-02-26 00:01:22.000000000","message":"to be cleare i dont see any reason why this would require andything more then the reader role to retrive.\n\n\ni dont think this should be admin only or require the service role\n\nmy understanding is that swap volume can be called in responce to 2? diffent cinder api actuions.  a volume retrype which i belive someone with the member role can request or a volume migration which im unsure if its admin only or member.\n\nif any of the cinder optration that can triger swap volume to be called  can be  be done by someone with the member role   then thsi should be assisble by reader","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"89ee06b16f738d9a4455b5bd4a798de337017cf7","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            \"bdm_uuid\": \"c088db45-92b8-49e8-81e2-a1b77a144b3b\""},{"line_number":104,"context_line":"            \"swap_progress\": \"42\""},{"line_number":105,"context_line":"        }"},{"line_number":106,"context_line":"    }"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"Security impact"},{"line_number":109,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"eab8c1c8_7fdaf6f1","line":106,"in_reply_to":"c98f6c8e_f3fde1ce","updated":"2025-02-26 18:04:08.000000000","message":"In an earlier patchset, Sylvain raised a security concern that a bad actor could abuse the API to an extent that it impacts the compute service. A couple of options were highlighted, and I went with the service role because the motivation for the feature is driven by a request to add a similar feature to cinder. I do not intend to expose the \u0027swap_progress\u0027 directly to users. Also, bear in mind that cinder\u0027s migration status (and the corresponding feature to add support for reporting the \u0027migration_progress\u0027 [1],[2]) are restricted to Admins. (Almost) all information on how volumes are managed on cinder\u0027s backends are Admin-only.\n\nI\u0027m happy to adjust whatever rules the nova team feel will address the security concern. For reference, you can see how I use the service role on L371 of [3].\n\n[1] Ibd1f9ed2639ae98a56d6faed81fa558f604f95e3\n[2] Ib574ee5a6ca2d4da06140ff9f2d7abe36e8afade\n[3] https://review.opendev.org/c/openstack/nova/+/941476/2/nova/api/openstack/compute/volumes.py","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3d8572237945167084482a8632ff54b061e04bfe","unresolved":true,"context_lines":[{"line_number":108,"context_line":"Security impact"},{"line_number":109,"context_line":"---------------"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"None."},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"Notifications impact"},{"line_number":114,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"ac0816f8_ecddc0be","line":111,"updated":"2024-12-11 15:20:55.000000000","message":"Really ?","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"e7e41ef8ddc4a49b8a1c41cc4f654a338c46d2df","unresolved":true,"context_lines":[{"line_number":108,"context_line":"Security impact"},{"line_number":109,"context_line":"---------------"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"None."},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"Notifications impact"},{"line_number":114,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"7fe98d5d_9c023408","line":111,"in_reply_to":"ac0816f8_ecddc0be","updated":"2024-12-11 15:39:24.000000000","message":"I\u0027ll respond in the comment below.","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3d8572237945167084482a8632ff54b061e04bfe","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":137,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":138,"context_line":"attachment details."},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"Other deployer impact"},{"line_number":141,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"63159c15_aacbfa43","line":138,"updated":"2024-12-11 15:20:55.000000000","message":"are you sure ? If I\u0027m a bad user, I could call the API every second that would then hit the compute service by the RPC, if we don\u0027t have any specific policy for saying no.","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"e7e41ef8ddc4a49b8a1c41cc4f654a338c46d2df","unresolved":true,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":137,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":138,"context_line":"attachment details."},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"Other deployer impact"},{"line_number":141,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":1,"id":"10005af7_05f18beb","line":138,"in_reply_to":"63159c15_aacbfa43","updated":"2024-12-11 15:39:24.000000000","message":"If I understand your concern correctly, the potential security issue is the impact on the compute RPC if a bad actor hammers the API. They would need to be a project member to hit the attachment details API at all, so the bad actor would have to be in the project.\n\nI have no problem adding a new policy to restrict the feature to admins.","commit_id":"ea226fa4776ca91cbf328049d6ee68a5c3125ece"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4db2c1a824cca11a55b97522d20d5985618f4200","unresolved":true,"context_lines":[{"line_number":110,"context_line":"Security impact"},{"line_number":111,"context_line":"---------------"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"A new policy will be added in order to ensure the RPC call from the API to"},{"line_number":114,"context_line":"compute layer isn\u0027t abused. The default policy will be restricted to admins,"},{"line_number":115,"context_line":"and the API won\u0027t make the RPC call unless the policy permits. Cloud admins"},{"line_number":116,"context_line":"who trust their users have the option of lossening the policy in order to"}],"source_content_type":"text/x-rst","patch_set":2,"id":"326a81b6_1bdf64a2","line":113,"range":{"start_line":113,"start_character":0,"end_line":113,"end_character":26},"updated":"2024-12-16 17:40:01.000000000","message":"Update: As I dig further into the cinder side of the feature, I learned that cinder restricts all migration status to just admins. Project members don\u0027t see any migration info, and this is hard-coded: there\u0027s no API policy that could be modified to allow project members to see migration data.\n\nWith this in mind, I think nova\u0027s side of the feature should behave the same. Would it be acceptable to nova community if *this* feature is limited to admins? I\u0027d post another update to the spec to say it\u0027s admin-only, which hopefully would alleviate any security concerns.\n\nThoughts?","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7dae178e03bef2d54d2f68023f530a6e2366d5a2","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"63496cde_b8b7d67d","line":145,"updated":"2024-12-13 00:17:43.000000000","message":"I\u0027m not sure I would characterize the performance impact as \"minimal\" considering what the GET volume attachment API does today [1] vs what this is proposing. Currently it is just database query for the volume BDM record and shows fields from it. This proposal would add a synchronous RPC call to nova-compute to call into the libvirt driver, which is quite a lot more than one database query.\n\nThe progress info is only relevant when a volume swap is in progress, which is really specific and not something we want to be making an RPC every time (i.e. how often will it not be a waste to call it?).\n\nSo I\u0027m wondering if it might be better to just add a swap_volume_progress column to the block_device_mapping table which defaults to NULL, and then nova-compute will periodically update the field each time it calls is_job_complete in _swap_volume. And NULL it back out when the swap is done (to indicate no swap in progress). It would keep nova-compute decoupled from nova-api for GET\u0027ing volume attachment info and seems more self-contained IMHO for nova-compute to sort of passively update the progress and let nova-api look in the database to check it.\n\nI\u0027m interested to hear others\u0027 opinions/ideas about it.\n\n[1] https://github.com/openstack/nova/blob/728337f2005bae0a5a631839dc2de1a38ec5c297/nova/api/openstack/compute/volumes.py#L331-L363","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8ec3e9874c74da1128b7aee385c8ce4b985c375f","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3c91eeae_0135275d","line":145,"in_reply_to":"086723c9_8dd87c8a","updated":"2025-01-08 19:41:25.000000000","message":"I\u0027ve got an update to post that takes the \"check for a service role\" approach. I\u0027ll also look into the task state idea, which is something I\u0027m not familiar with.\n\nI will say the \"checking for a service role\" PoC proved to be very light weight. I also think it\u0027s reasonable (based on past experience) to assume admins will be looking at cinder when they\u0027re migrating a volume, so it should be sufficient to provide the swap progress only when cinder is asking for it.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"680c9cdff8c7a2c4a29dd80952a6931ab2f2ed88","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5bfc5f97_3b0dc10e","line":145,"in_reply_to":"15dd798a_07e1fc0f","updated":"2025-12-04 19:51:37.000000000","message":"wait this is v2 and the file has been renamed so this did not show up when i looke at this before on v5\n\nthree is defintly somethign stragne happening with how comment render in gerrit at the moment","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"94256381a08a9ddf6b8ab5d666dec8c812a0108d","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"086723c9_8dd87c8a","line":145,"in_reply_to":"16de0bfe_5389b90c","updated":"2025-01-08 19:08:55.000000000","message":"\u003e If we are worrying about RPC load then this proposal might not help much, as each DB operation from the nova-compute also means an RPC round trip to the cell conductor to access the DB.\n\nThis is a good point however I think rather than pure RPC load, I was most concerned about the idea of a synchronous (blocking) RPC call going from the API to libvirt, especially if it would happen every time GET /servers/{server_id}/os-volume_attachments/{volume_id} is called. Generally I think to try to avoid blocking RPC calls into libvirt as it adds quite a bit of burden to what would otherwise be a quick and consistent API call that doesn\u0027t have to wait for a reply from libvirt before returning.\n\n\u003e Do we have any state in the DB already like VM state, task state, or the fields of the BDM, we can use to decide if a swap is in progress and therefore the RPC call make sense?\n\nI hadn\u0027t thought about adding a new task state to represent \"volume swap in progress\" but that is a much better idea than what I said 😂 I had been thinking about how live migration monitor writes status into the DB periodically and doing similar for this as the volume swap is already looping and checking status [1]. But obviously that would add RPC + DB whereas task state would only add two DB calls: one to set the task state when the swap begins and one to set the task state back to None when the swap is finished.\n\nHere\u0027s our current task states and I think a new one for volume swap would fit in well:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/task_states.py\n\nAs for the PoC with checking for the service role, that would also be a big improvement over the current proposal.\n\nI think I like the task state idea better though -- it\u0027s the lightest weight and there\u0027s the additional benefit of users being able to see \"volume swap in progress\" when listing or showing servers. It also exactly fits into the purpose of task state IMHO.\n\n[1] https://github.com/openstack/nova/blob/634be5191e0fde60aac774fb7917868de9a2b29c/nova/virt/libvirt/driver.py#L2375-L2376","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"fccdf131_681a766b","line":145,"in_reply_to":"2f533b15_8bd47716","updated":"2025-12-04 19:26:05.000000000","message":"While reading the RPC call proposal, I also had concerns about why to make an RPC call when no swap is going on.\n\nI know the current use case is Cinder call the GET attachment API only when migration is in progress and to know swap progress, but if I see from Nova perspective, that is not the only use case for the GET volume attachment API. In future, Cinder might be calling this API for a non-migration case, and then there will be unnecessary RPC call to compute to know swap progress.\n\nI think that is something we need to fix/agree here that Nova only makes RPC call whenthe  swap volume is in progress (I commented on it in the performance section also).\n\nI like the melwitt idea of a new task state, but I am not sure if we agreed on that or not.\n\nAnother way to achieve it is to check the volume migration status (I hope this will give us real infomration if swap is in progress):\n\nIn GET volume attachment API:\n\n         \n        for bdm in bdms:\n            if bdm.volume_id:\n            try:\n                volume \u003d self.volume_api.get(context, volume_id)\n            except exception.VolumeNotFound as e:\n                raise exc.HTTPNotFound(explanation\u003de.format_message())\n            if (\u0027migration_status\u0027 in volume or\n                old_volume[\u0027migration_status\u0027] in not (None, \u0027\u0027)):\n                -\u003e make RPC call to compute for swap-progress \n        \nThis is kind of what we are doing in the swap volume API:\n - https://github.com/openstack/nova/blob/7bd20e52d1ac134109a48bf151ac16154a97f246/nova/api/openstack/compute/volume_attachments.py#L261","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ddf5cb14914e4d22d45b7be82163ffa5b7e7cd6f","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"c1339a53_36527ddd","line":145,"in_reply_to":"3c91eeae_0135275d","updated":"2025-01-10 00:06:24.000000000","message":"Thanks for testing that.\n\nI realized I brain lapsed (I blame PTO brain) and was thinking about the RPC call and task_state as either/or because I forgot you are wanting to provide the progress %, not just a boolean whether a swap is in progress or not. So task_state alone would not do it.\n\nI understand your point about the service role given how the PUT /servers/{server_id}/os-volume_attachments/{volume_id} API is only meant to be used by another service as part of an orchestrated action like volume migrate or retype.\n\nI wondered however, what kind of users are permitted by default policy to initiate the actual migrate or retype? And it looks like retype allows system admin and project member [1] (migrate is admin-only by default).\n\nSo I\u0027m thinking about if a project member initiates a retype through Cinder API, they may be interested to see the swap progress (and they would be allowed to call the Nova GET /servers/{server_id}/os-volume_attachments/{volume_id} API which is admin or project reader [2]).\n\nI wonder if this could be reason enough not to limit the swap progress call to only service role and instead decide based on the task_state. I guess the argument could be that only Cinder needs to know the swap progress and that it should be of no interest to the retype API caller. Or, perhaps Cinder would prefer to expose the swap progress through one of its APIs so the caller gets it from there instead. Just thinking aloud.\n\nEither way, for informational purposes, to add a new task_state you would:\n\n1. Add a new enum value to `InstanceTaskState` [1] for example something like `VOLUME_SWAPPING`\n2. Set the `instance.task_state \u003d VOLUME_SWAPPING` when the job begins and `instance.save()` it to the DB\n3. In the GET /servers/{server_id}/os-volume_attachments/{volume_id} API check if `instance.task_state \u003d\u003d VOLUME_SWAPPING` to decide whether to query the driver for the swap progress\n4. After the job completes, set the `instance.task_state \u003d None` and `instance.save()` it to the DB\n\nThe finest grained place to do it is in the libvirt driver [2] but we could also consider setting the task_state begin/end in compute/manager.py or set the task_state begin in compute/api.py and set the task_state end in compute/manager.py. It seems it would be simplest to do both in the libvirt driver, I\u0027m not sure if there would be any drawback to that.\n\n[1] https://github.com/openstack/cinder/blob/962fe29e778c58a8e90c78602e7249cb4b06e450/cinder/policies/volume_actions.py#L163\n[2] https://github.com/openstack/nova/blob/634be5191e0fde60aac774fb7917868de9a2b29c/nova/policies/volumes_attachments.py#L48\n[1] https://github.com/openstack/nova/blob/634be5191e0fde60aac774fb7917868de9a2b29c/nova/objects/fields.py#L1018\n[2] https://github.com/openstack/nova/blob/634be5191e0fde60aac774fb7917868de9a2b29c/nova/virt/libvirt/driver.py#L2340","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3635fbf81a3e4305f7125875f2ba71573a5cc160","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"bebac065_6744084d","line":145,"in_reply_to":"5bfc5f97_3b0dc10e","updated":"2025-12-04 20:05:06.000000000","message":"From my past research (and fading memory) I don\u0027t recall the API layer \"knew\" a swap was in progress, or I would have included that constraint in the spec already. It feels weird for nova to ask cinder for the volume\u0027s migration_status (to know if a swap is in progress) when cinder is asking for the swap_progress only when it knows the volume is migrating. This is one reason why I though it would be OK to constrain the RPC call to only when it\u0027s cinder that\u0027s making the request.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"24d8df5acc645ab540d7ecea63a1e006f239c63b","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"d221c752_b355b4b1","line":145,"in_reply_to":"63496cde_b8b7d67d","updated":"2024-12-13 04:30:54.000000000","message":"I also would like to avoid the RPC, and will take a closer look to see if there\u0027s any way for the API layer to know if a swap is in progress. I hoped to avoid adding something to the DB but am happy to do that if needed. I think having the compute periodically updating a progress field would result in hammering the DB more often than the API would call the RPC. But here\u0027s another thought: add a \u0027swapping\u0027 (or \u0027swap_in_progress\u0027) flag that would be set once at the start of the swap and cleared at the end. Just 2 DB updates. Then the API could test the flag to determine if the RPC is warranted.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"ac61822a5cbb69f9af285ca7341ce04848a51d93","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"16de0bfe_5389b90c","line":145,"in_reply_to":"955b72b2_a0958b6a","updated":"2025-01-08 13:57:38.000000000","message":"I have a new proposal, and have a working PoC. I still cannot find an existing DB entry that can be checked in order to determine whether a swap operation is in progress, so I prototyped adding one. Conceptually it works, but breaks a lot of unit tests and it affects a large number of files.\n\nThe alternative I am now proposing is to restrict the API layer from invoking the RPC to API requests coming from a service (e.g. cinder). I never expected admins to directly use the \u0027swap_progress\u0027 feature in nova. Instead they\u0027d be using the corresponding cinder feature (see https://review.opendev.org/c/openstack/cinder-specs/+/937808). In my PoC, the nova code checks whether the context includes a service role, which cinder provides when it makes the call to the nova API. In the cinder code, only admins can access migration information, so regular users cannot abuse the nova interface via cinder.\n\nI will update this spec if this new approach is favorable.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"0e562ebd2908c8580b9942822c2739a8c2c1e53c","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"91836dc4_059153d5","line":145,"in_reply_to":"bebac065_6744084d","updated":"2025-12-04 21:13:27.000000000","message":"What do you think of the idea of add a boolean \u0027swapping\u0027 to the BDM, which the compute would be responsible for setting and clearing at the beginning and end? The API\u0027s already fetched the BDM and could use it as another constraint before calling the RPM.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"bb792f4f09bf6af33620a804f7f5cff5383a9339","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"2f533b15_8bd47716","line":145,"in_reply_to":"c1339a53_36527ddd","updated":"2025-01-10 06:04:29.000000000","message":"\u003e I wonder if this could be reason enough not to limit the swap progress call to only service role and instead decide based on the task_state. I guess the argument could be that only Cinder needs to know the swap progress and that it should be of no interest to the retype API caller. Or, perhaps Cinder would prefer to expose the swap progress through one of its APIs so the caller gets it from there instead. Just thinking aloud.\n\nI agree with this ^^ view. In cinder, explicitly migrating a volume is admin-only, but users can retype a volume that can trigger a migration (retype with migration_policy \u003d on-demand). However, only admins can see that a volume is actually migrating, and this is because with few exceptions cinder explicitly hides any back-end details from non-admins.\n\nIf only admins see a volume\u0027s migration_status then cinder would similarly limit showing a migration_progress (nova\u0027s swap_progress would be presented in cinder as migration_progress)\n\nThis is why I propose including the swap_progress in the nova API response only when the request is coming from cinder. Cinder would implement a consistent policy of only allowing admins to see the migration status/progress, mainly to preserve the long standing goal of hiding details about the storage backends from users.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"66d2bdef7c28b1ace607f9a9ecc2be27a77259fd","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"955b72b2_a0958b6a","line":145,"in_reply_to":"d221c752_b355b4b1","updated":"2025-01-08 13:27:01.000000000","message":"@melwittt@gmail.com I agree that the change is not minimal. \nI suggest to simply drop \"Minimal.\"\n\n\u003e The progress info is only relevant when a volume swap is in progress, which is really specific and not something we want to be making an RPC every time (i.e. how often will it not be a waste to call it?).\n\nI agree that we should avoid the RPC is there is no swap operation in progress. \n\nDo we have any state in the DB already like VM state, task state, or the fields of the BDM, we can use to decide if a swap is in progress and therefore the RPC call make sense?\n\nIf not can we add some new task state maybe?\n\n\u003e So I\u0027m wondering if it might be better to just add a swap_volume_progress column to the block_device_mapping table which defaults to NULL, and then nova-compute will periodically update the field each time it calls is_job_complete in _swap_volume. And NULL it back out when the swap is done (to indicate no swap in progress). It would keep nova-compute decoupled from nova-api for GET\u0027ing volume attachment info and seems more self-contained IMHO for nova-compute to sort of passively update the progress and let nova-api look in the database to check it.\n\nIf we are worrying about RPC load then this proposal might not help much, as each DB operation from the nova-compute also means an RPC round trip to the cell conductor to access the DB. \n\n---\n\nI see Alan is thinking about the same direction. I think instead of a new flag this can be a new task state value on the instance.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6ff7063eb107a5903bfd20f1a12c478329feff6c","unresolved":true,"context_lines":[{"line_number":142,"context_line":"Minimal. The RPC call to fetch libVirt job info (whether or not a job is"},{"line_number":143,"context_line":"running) is triggered only when something (e.g. cinder) requests a volume\u0027s"},{"line_number":144,"context_line":"attachment details. The new policy will prevent bad actors from abusing the"},{"line_number":145,"context_line":"API to an extent that it impacts performance."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Other deployer impact"},{"line_number":148,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"15dd798a_07e1fc0f","line":145,"in_reply_to":"fccdf131_681a766b","updated":"2025-12-04 19:49:09.000000000","message":"ah that a valid point that i missed.\n\ni wonder if we shoudl add a new query arg and only include it if its requested\n\nsimialry how you can ask for the server to be included in the hypervior details list\n\nso we would only include the swap progress (and do the rpc) if with_swap\u003dtrue was set.","commit_id":"6798fb949c9fa3d2baeff6980917a1ebc0a6ae93"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ddf5cb14914e4d22d45b7be82163ffa5b7e7cd6f","unresolved":true,"context_lines":[{"line_number":42,"context_line":"the API response only when permitted by the microversion, and when the API"},{"line_number":43,"context_line":"request is coming from another OpenStack service (e.g. cinder). The field will"},{"line_number":44,"context_line":"not be included in the response if the API request is coming directly from a"},{"line_number":45,"context_line":"user (including admins). The value would be obtained via an RPC call that"},{"line_number":46,"context_line":"fetches the data from libVirt on the compute host."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"The n-cpu manager code that handles the RPC request will call into the"},{"line_number":49,"context_line":"virt driver. When the driver is libVirt, the code will return:"}],"source_content_type":"text/x-rst","patch_set":3,"id":"176e7a96_6e9f0858","line":46,"range":{"start_line":45,"start_character":25,"end_line":46,"end_character":50},"updated":"2025-01-10 00:06:24.000000000","message":"I know we kind of glossed over what this RPC API addition might look like. I assume something like, get_volume_swap_progress()?\n\nI had started thinking about how for libvirt it\u0027s more generic than that in that we would be pulling block job info (which doesn\u0027t necessarily have to be about swapping volumes) ... but the RPC API must be driver agnostic, so maybe we do have to make it very specific like \"get swap volume progress\". I was just thinking if it would be worthwhile to make it generic i.e. usable for other future potential uses but that might be overcomplicating matters for unknown gain.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"bb792f4f09bf6af33620a804f7f5cff5383a9339","unresolved":true,"context_lines":[{"line_number":42,"context_line":"the API response only when permitted by the microversion, and when the API"},{"line_number":43,"context_line":"request is coming from another OpenStack service (e.g. cinder). The field will"},{"line_number":44,"context_line":"not be included in the response if the API request is coming directly from a"},{"line_number":45,"context_line":"user (including admins). The value would be obtained via an RPC call that"},{"line_number":46,"context_line":"fetches the data from libVirt on the compute host."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"The n-cpu manager code that handles the RPC request will call into the"},{"line_number":49,"context_line":"virt driver. When the driver is libVirt, the code will return:"}],"source_content_type":"text/x-rst","patch_set":3,"id":"689b357e_243c0fb1","line":46,"range":{"start_line":45,"start_character":25,"end_line":46,"end_character":50},"in_reply_to":"176e7a96_6e9f0858","updated":"2025-01-10 06:04:29.000000000","message":"Yes, that\u0027s exactly what I have in the PoC. I added a get_volume_swap_progress() to the virt layer that defaults to raising NotImplementedError, then an actual implementation in libVirt. This parallels the existing swap_volume() RPC.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8d24dc81df81854735cbe151ecba9b4e1e752ed3","unresolved":true,"context_lines":[{"line_number":42,"context_line":"the API response only when permitted by the microversion, and when the API"},{"line_number":43,"context_line":"request is coming from another OpenStack service (e.g. cinder). The field will"},{"line_number":44,"context_line":"not be included in the response if the API request is coming directly from a"},{"line_number":45,"context_line":"user (including admins). The value would be obtained via an RPC call that"},{"line_number":46,"context_line":"fetches the data from libVirt on the compute host."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"The n-cpu manager code that handles the RPC request will call into the"},{"line_number":49,"context_line":"virt driver. When the driver is libVirt, the code will return:"}],"source_content_type":"text/x-rst","patch_set":3,"id":"bfe168d8_c5deca28","line":46,"range":{"start_line":45,"start_character":25,"end_line":46,"end_character":50},"in_reply_to":"689b357e_243c0fb1","updated":"2025-02-26 00:01:22.000000000","message":"if we add this it should be implementabel for any driver like ironic\n\nso its a new compute manager rpc + a change to the virt dirver api + potentually a diver speicici implemation fo that new menthod.\n\nto be this is very much like the server disagnostics api we allready have.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"89ee06b16f738d9a4455b5bd4a798de337017cf7","unresolved":true,"context_lines":[{"line_number":42,"context_line":"the API response only when permitted by the microversion, and when the API"},{"line_number":43,"context_line":"request is coming from another OpenStack service (e.g. cinder). The field will"},{"line_number":44,"context_line":"not be included in the response if the API request is coming directly from a"},{"line_number":45,"context_line":"user (including admins). The value would be obtained via an RPC call that"},{"line_number":46,"context_line":"fetches the data from libVirt on the compute host."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"The n-cpu manager code that handles the RPC request will call into the"},{"line_number":49,"context_line":"virt driver. When the driver is libVirt, the code will return:"}],"source_content_type":"text/x-rst","patch_set":3,"id":"708b210d_b55c2841","line":46,"range":{"start_line":45,"start_character":25,"end_line":46,"end_character":50},"in_reply_to":"bfe168d8_c5deca28","updated":"2025-02-26 18:04:08.000000000","message":"While I know this spec is still pending, I proposed a patch showing that\u0027s how I implemented the interface. See I3970e3ebd79d729bf17456fe6cafb8f9abaef5ab.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c23a87c8952d6e7ecefc8daf458051ff761ce09c","unresolved":true,"context_lines":[{"line_number":59,"context_line":"and end location of the block device. Those values are used to calculate a"},{"line_number":60,"context_line":"percent completion. See the References section for links to the relevant"},{"line_number":61,"context_line":"liVirt code."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Drivers other than libVirt will raise ``NotImplementedError``, which the"},{"line_number":64,"context_line":"n-cpu manager will handle by returning ``None`` in the RPC call."},{"line_number":65,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"366dbb1d_22327572","line":62,"updated":"2025-01-14 08:57:25.000000000","message":"`get_job_info` returns similar to this object if copy is in-progress.\n{\u0027type\u0027: 2, \u0027bandwidth\u0027: 0, \u0027cur\u0027: 0, \u0027end\u0027: 1}\n\nor as - `current cursor: 1073741824 final cursor: 1073741824`\nso to  retrieve %age of completion, you will use  cur and end value difference ?","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"76737a85dc2f54eb81fad601d63ea0f53f3a739d","unresolved":true,"context_lines":[{"line_number":59,"context_line":"and end location of the block device. Those values are used to calculate a"},{"line_number":60,"context_line":"percent completion. See the References section for links to the relevant"},{"line_number":61,"context_line":"liVirt code."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Drivers other than libVirt will raise ``NotImplementedError``, which the"},{"line_number":64,"context_line":"n-cpu manager will handle by returning ``None`` in the RPC call."},{"line_number":65,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"91f42397_b9bcccea","line":62,"in_reply_to":"366dbb1d_22327572","updated":"2025-01-14 20:01:22.000000000","message":"Essentially yes, though the percentage calculation is a ratio rather than a difference. But it uses the cur and end values,","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"3effd76d53d7786d255130ace7955626b24f578b","unresolved":false,"context_lines":[{"line_number":59,"context_line":"and end location of the block device. Those values are used to calculate a"},{"line_number":60,"context_line":"percent completion. See the References section for links to the relevant"},{"line_number":61,"context_line":"liVirt code."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Drivers other than libVirt will raise ``NotImplementedError``, which the"},{"line_number":64,"context_line":"n-cpu manager will handle by returning ``None`` in the RPC call."},{"line_number":65,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"f87237af_52b6f5ca","line":62,"in_reply_to":"91f42397_b9bcccea","updated":"2025-01-15 04:00:14.000000000","message":"ack\nyeah, use of \"difference\" was incorrect, I didn\u0027t meant end-cur by that.\njust that these 2 values will be used.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c23a87c8952d6e7ecefc8daf458051ff761ce09c","unresolved":true,"context_lines":[{"line_number":87,"context_line":"performance. This could be partially addressed by adding a policy to limit"},{"line_number":88,"context_line":"the feature to admins, but additional changes would be required in order"},{"line_number":89,"context_line":"to invoke the RPC only when a swap operation is actually happening."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Data model impact"},{"line_number":92,"context_line":"-----------------"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"a23298e2_4e7741f7","line":90,"updated":"2025-01-14 08:57:25.000000000","message":"instead of RPC notification-queue can be used too; \nsomething like `compute_utils.notify_volume_swap_status:42%` this can be done from libvirt itself, and on same polling for status of every 0.5 sec.\nthis should not be as expensive\n\nOther services should be able to retrieve data from there.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"3effd76d53d7786d255130ace7955626b24f578b","unresolved":true,"context_lines":[{"line_number":87,"context_line":"performance. This could be partially addressed by adding a policy to limit"},{"line_number":88,"context_line":"the feature to admins, but additional changes would be required in order"},{"line_number":89,"context_line":"to invoke the RPC only when a swap operation is actually happening."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Data model impact"},{"line_number":92,"context_line":"-----------------"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"4aec8e58_29b87fc7","line":90,"in_reply_to":"3827c840_fe86e429","updated":"2025-01-15 04:00:14.000000000","message":"Acknowledged","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8d24dc81df81854735cbe151ecba9b4e1e752ed3","unresolved":true,"context_lines":[{"line_number":87,"context_line":"performance. This could be partially addressed by adding a policy to limit"},{"line_number":88,"context_line":"the feature to admins, but additional changes would be required in order"},{"line_number":89,"context_line":"to invoke the RPC only when a swap operation is actually happening."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Data model impact"},{"line_number":92,"context_line":"-----------------"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3b23afa6_74b499a8","line":90,"in_reply_to":"4aec8e58_29b87fc7","updated":"2025-02-26 00:01:22.000000000","message":"notificatinos are fire and forget and nova does not consume its own nnotifictions internally out side of fucntioanl tests.\n\nnotificaiton are entirly optionsal so we should not rely on them for any data we want to presnet in the api.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"76737a85dc2f54eb81fad601d63ea0f53f3a739d","unresolved":true,"context_lines":[{"line_number":87,"context_line":"performance. This could be partially addressed by adding a policy to limit"},{"line_number":88,"context_line":"the feature to admins, but additional changes would be required in order"},{"line_number":89,"context_line":"to invoke the RPC only when a swap operation is actually happening."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Data model impact"},{"line_number":92,"context_line":"-----------------"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3827c840_fe86e429","line":90,"in_reply_to":"a23298e2_4e7741f7","updated":"2025-01-14 20:01:22.000000000","message":"The problem with using notifications is swapping a volume can take a looong time (hours). The swap progress is generally only of interest when an admin needs to check the status of a cinder operation, so a vast majority of these notifications will not be useful (only the latest value really matters), and flooding notifications every 0.5s would be horribly noisy over that length of time.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8d24dc81df81854735cbe151ecba9b4e1e752ed3","unresolved":true,"context_lines":[{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3d2dbad5_f0fb910e","line":127,"updated":"2025-02-26 00:01:22.000000000","message":"if we were to require the service role by the way it would be on the users token not a normal token with (admin or member or reader) + a service_user token with the  service role.\n\ni think this is likely overkill and the policy should just be project_reader_or_admim  although projoct_reader_or_admin_or_service could also make sense it depend on if cinder would ever call this iself not in responce to a user action.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"89ee06b16f738d9a4455b5bd4a798de337017cf7","unresolved":true,"context_lines":[{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"7acfde43_923c2ee1","line":127,"in_reply_to":"3d2dbad5_f0fb910e","updated":"2025-02-26 18:04:08.000000000","message":"See my reply to a previous comment on why I went with the service role, what it took to implement, and why I didn\u0027t intend for members or readers to see the data.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ddf5cb14914e4d22d45b7be82163ffa5b7e7cd6f","unresolved":true,"context_lines":[{"line_number":188,"context_line":"----------"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"* Add a new field to the attachment details response in a new microversion"},{"line_number":191,"context_line":"* Add RPC code down to the libvirt driver to generate the swap_progress value"},{"line_number":192,"context_line":"* Extend existing unit and functional tests"},{"line_number":193,"context_line":"* Update API documentation"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"be8b3c74_eb7c0db9","line":191,"updated":"2025-01-10 00:06:24.000000000","message":"Note: to be clear, this would be adding a new compute RPC API version and new virt API method.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"bb792f4f09bf6af33620a804f7f5cff5383a9339","unresolved":true,"context_lines":[{"line_number":188,"context_line":"----------"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"* Add a new field to the attachment details response in a new microversion"},{"line_number":191,"context_line":"* Add RPC code down to the libvirt driver to generate the swap_progress value"},{"line_number":192,"context_line":"* Extend existing unit and functional tests"},{"line_number":193,"context_line":"* Update API documentation"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"dc6562db_a7cf9c8b","line":191,"in_reply_to":"be8b3c74_eb7c0db9","updated":"2025-01-10 06:04:29.000000000","message":"Ack, that\u0027s what I did in the PoC.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8d24dc81df81854735cbe151ecba9b4e1e752ed3","unresolved":true,"context_lines":[{"line_number":188,"context_line":"----------"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"* Add a new field to the attachment details response in a new microversion"},{"line_number":191,"context_line":"* Add RPC code down to the libvirt driver to generate the swap_progress value"},{"line_number":192,"context_line":"* Extend existing unit and functional tests"},{"line_number":193,"context_line":"* Update API documentation"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"cca8c79b_f24537f3","line":191,"in_reply_to":"dc6562db_a7cf9c8b","updated":"2025-02-26 00:01:22.000000000","message":"right we support cinder volume with other virt drivers so its a new compute service api which will delegate to a new virt driver method internally but there is no reason this could not be impmented for vmware or ironic.\n\nboth support cinder volumes and presumable volume migration.","commit_id":"434d7b6f47d3c3aeda7ad1b3a77828d4633a9f95"}],"specs/2026.1/approved/swap-progress-in-attachment-details.rst":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a8b93efe1a557ec483e725015e4a630e57feeae7","unresolved":true,"context_lines":[{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"- As a cloud admin who is migrating a cinder volume, I need to know it"},{"line_number":31,"context_line":"  isn\u0027t taking so much time to complete that I fear the operation is stuck."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"- As a cloud admin who is migrating a cinder volume, I would like to track"},{"line_number":34,"context_line":"  the migration progress in order to gauge how much longer it might take for"}],"source_content_type":"text/x-rst","patch_set":5,"id":"3b54d148_9f66a2ac","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":75},"updated":"2025-11-25 17:57:40.000000000","message":"This can still be achieved by listing volume attachments and checking if the old volume id is not in attachments and the new volume is in attachments .","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"af32cb896fcc5561d9c7f7adf79f721858da48e0","unresolved":true,"context_lines":[{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"- As a cloud admin who is migrating a cinder volume, I need to know it"},{"line_number":31,"context_line":"  isn\u0027t taking so much time to complete that I fear the operation is stuck."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"- As a cloud admin who is migrating a cinder volume, I would like to track"},{"line_number":34,"context_line":"  the migration progress in order to gauge how much longer it might take for"}],"source_content_type":"text/x-rst","patch_set":5,"id":"49a5caf1_a58e31e8","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":75},"in_reply_to":"3b54d148_9f66a2ac","updated":"2025-11-26 14:45:35.000000000","message":"That will only tell the admin whether the migration completed. In situations where migration takes hours to complete, admins tend to fear something has gone wrong.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"293599d7cd4fe581307d6fea51996ae54eeac192","unresolved":true,"context_lines":[{"line_number":27,"context_line":"Use Cases"},{"line_number":28,"context_line":"---------"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"- As a cloud admin who is migrating a cinder volume, I need to know it"},{"line_number":31,"context_line":"  isn\u0027t taking so much time to complete that I fear the operation is stuck."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"- As a cloud admin who is migrating a cinder volume, I would like to track"},{"line_number":34,"context_line":"  the migration progress in order to gauge how much longer it might take for"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7d7be349_1fb9901b","line":31,"range":{"start_line":30,"start_character":0,"end_line":31,"end_character":75},"in_reply_to":"49a5caf1_a58e31e8","updated":"2025-12-03 18:31:35.000000000","message":"so that actully wont work.\n\nif you do a cinder imitated volume migration which as of \nhttps://wiki.openstack.org/wiki/OSSN/OSSN-0094\nis the only thing you can do the voluem id will not change.\n\nin the cinder volume migrate flow cinder allcoates a new volume on the other backend with a new volume id\nqemu does the data copy to it then when we finish the migration cidner atopicly renames the voluem to the old uuid\n\nso form novas perspecitve and the external world the voluem uuid does not change\n\nthe attchmens woudl but the voluem id wont so instance show is not going to show a change in volume ids assocated with the instnace.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"293599d7cd4fe581307d6fea51996ae54eeac192","unresolved":true,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"- As a cloud admin who is migrating a cinder volume, I would like to track"},{"line_number":34,"context_line":"  the migration progress in order to gauge how much longer it might take for"},{"line_number":35,"context_line":"  the operation to complete."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Proposed change"},{"line_number":38,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":5,"id":"704f9132_9f6065d6","line":35,"updated":"2025-12-03 18:31:35.000000000","message":"this to me is also important in the context of things like graceful shutdown or restarting nova-compute\n\nof all the operations that could be in flight that are problematic to interup \nswap volume is perhaps the highst on that list.\n\nso knowing how close it is to completing is valuable IMO","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"293599d7cd4fe581307d6fea51996ae54eeac192","unresolved":true,"context_lines":[{"line_number":43,"context_line":"request is coming from another OpenStack service (e.g. cinder). The field will"},{"line_number":44,"context_line":"not be included in the response if the API request is coming directly from a"},{"line_number":45,"context_line":"user (including admins). The value would be obtained via an RPC call that"},{"line_number":46,"context_line":"fetches the data from libVirt on the compute host."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"The n-cpu manager code that handles the RPC request will call into the"},{"line_number":49,"context_line":"virt driver. When the driver is libVirt, the code will return:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9a78dc2c_fdf079ff","line":46,"updated":"2025-12-03 18:31:35.000000000","message":"so this is sayign it will only do the rpc if the request is made by a toke with the service role on the user token\n\nhonesty i would be OK with letting admins see this also.\n\nit will be gated behind a new microverison anyway to the can opt in/out.\nbut im fine with only exposing this to cidner if that is the concencous.\n\ni expect admin will call cinder API instead to view the progress if the want to see this info.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":true,"context_lines":[{"line_number":43,"context_line":"request is coming from another OpenStack service (e.g. cinder). The field will"},{"line_number":44,"context_line":"not be included in the response if the API request is coming directly from a"},{"line_number":45,"context_line":"user (including admins). The value would be obtained via an RPC call that"},{"line_number":46,"context_line":"fetches the data from libVirt on the compute host."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"The n-cpu manager code that handles the RPC request will call into the"},{"line_number":49,"context_line":"virt driver. When the driver is libVirt, the code will return:"}],"source_content_type":"text/x-rst","patch_set":5,"id":"fa357bb0_aa9b4134","line":46,"in_reply_to":"9a78dc2c_fdf079ff","updated":"2025-12-04 19:26:05.000000000","message":"As, we only allow cinder to swap volume and even admin cannot swap volume so I am not sure about use case of admin user to know swap progress. Yes cinder can know and cinder can decide where to use and whom to show this information.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":false,"context_lines":[{"line_number":51,"context_line":"* A string representing a percent completion (e.g. \"42\" for 42% complete)"},{"line_number":52,"context_line":"  while a copy operation is running."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"* ``None`` to indicate no copy operation is running (i.e. there\u0027s no migration"},{"line_number":55,"context_line":"  happening at this time)."},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"The libVirt code relies on the exising BlockDevice ``get_job_info`` function,"},{"line_number":58,"context_line":"which reports whether a copy job is running, and pointers to the current"}],"source_content_type":"text/x-rst","patch_set":5,"id":"766352f7_5b3a5e49","line":55,"range":{"start_line":54,"start_character":0,"end_line":55,"end_character":26},"updated":"2025-12-04 19:26:05.000000000","message":"++","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":true,"context_lines":[{"line_number":60,"context_line":"percent completion. See the References section for links to the relevant"},{"line_number":61,"context_line":"liVirt code."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Drivers other than libVirt will raise ``NotImplementedError``, which the"},{"line_number":64,"context_line":"n-cpu manager will handle by returning ``None`` in the RPC call."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"At the API layer, the ``swap_progress`` field will be included in the"},{"line_number":67,"context_line":"attachment details response only when the value is a non-empty string."}],"source_content_type":"text/x-rst","patch_set":5,"id":"5f452511_6e391263","line":64,"range":{"start_line":63,"start_character":0,"end_line":64,"end_character":64},"updated":"2025-12-04 19:26:05.000000000","message":"In case of NotImplementedError, i will suggest not to include the ``swap_progress`` so that both case 1. no swap in progress and 2. swap progress is not implemented by nova driver can be differentiated.\n\nI am ok to do it in implementation and amend spec later once it is finalized in implementation.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"293599d7cd4fe581307d6fea51996ae54eeac192","unresolved":true,"context_lines":[{"line_number":64,"context_line":"n-cpu manager will handle by returning ``None`` in the RPC call."},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"At the API layer, the ``swap_progress`` field will be included in the"},{"line_number":67,"context_line":"attachment details response only when the value is a non-empty string."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"Alternatives"},{"line_number":70,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"97baa87f_b7d3153a","line":67,"updated":"2025-12-03 18:31:35.000000000","message":"ack.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f90abceeb0bc02a74a11c2b23bfa77cfa63c2de7","unresolved":true,"context_lines":[{"line_number":116,"context_line":"            \"swap_progress\": \"42\""},{"line_number":117,"context_line":"        }"},{"line_number":118,"context_line":"    }"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"Security impact"},{"line_number":121,"context_line":"---------------"},{"line_number":122,"context_line":""}],"source_content_type":"text/x-rst","patch_set":5,"id":"3ae6f836_f4cd38aa","line":119,"updated":"2025-12-04 17:57:49.000000000","message":"this change will also require the openapi schema to be updated for this change\nto capture the deleta in the respoce for the new microversion","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f68c49f586252cfa4874beb1528af95017e444f7","unresolved":true,"context_lines":[{"line_number":123,"context_line":"The ``swap_progress`` data isn\u0027t inherently sensitive, and limiting the"},{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"03657e94_b9b8e9ec","line":126,"updated":"2025-11-25 16:56:44.000000000","message":"I\u0027m a bit concerned with the DoS potential here, even if it\u0027s only requested by cinder. When and how frequently will cinder make this request? We\u0027ve already got another API call that makes a synchronous RPC to the compute node which can block and end up DoSing the API (starving it of request threads) for a similar volume-related thing. I\u0027m not sure it\u0027s best to add another way to do this, especially for a case where it\u0027s just polling. Can we not monitor this in the compute node periodically, update something in the database, and let the API just fetch it out when asked?","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a8b93efe1a557ec483e725015e4a630e57feeae7","unresolved":true,"context_lines":[{"line_number":123,"context_line":"The ``swap_progress`` data isn\u0027t inherently sensitive, and limiting the"},{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"48da6177_0e641da4","line":126,"in_reply_to":"03657e94_b9b8e9ec","updated":"2025-11-25 17:57:40.000000000","message":"I agree, even if we can make an API, policy internal for the Cinder service only, but there is always a hack to call those by users.\n\nPeriodically/Live updating the progress from compute node is good idea. We do wait for .5 sec while driver is copying the disk[1] and that is good place to either:\n\n1. Update the progress in DB and cinder fetch that\n\n2. Not sure if it will make things slow (but DB update also involve the RPC call from compute to conductor), but another idea can be that while the driver is copying the disk[1], it can call Cinder to send the progress update (with an interval, say call Cinder to send progress every 20% increment in progress). Even intervals can be configurable (0 means do not send progress, x means after x% in progress). \n\n[1] https://github.com/openstack/nova/blob/23b462d77df1a1d09c43d0918bca853ef3af1e3f/nova/virt/libvirt/driver.py#L2389","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"af32cb896fcc5561d9c7f7adf79f721858da48e0","unresolved":true,"context_lines":[{"line_number":123,"context_line":"The ``swap_progress`` data isn\u0027t inherently sensitive, and limiting the"},{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"701ba501_0b8af713","line":126,"in_reply_to":"48da6177_0e641da4","updated":"2025-11-26 14:45:35.000000000","message":"Cinder will call the API only when a user with admin credentials:\n- Views the volume\u0027s details\n- The volume is migrating\n- The volume is attached\nThere is no polling.\n\nRemember the use case is targeted at long running migrations, when an admin may occasionally check the progress. I want to avoid periodic polling or periodic (i.e. regular) DB updates because a vast majority will be wasted efforts. The goal is to provide a reasonably up-to-date progress, and only when an admin asks for the data.\n\nWhat I could do is persist the swap_progress and a refresh timestamp in the BDM. The BDM has already been fetched, so there\u0027s no additional DB read. The API layer would use the timestamp to limit calls to the n-cpu layer to something like 60-120 seconds (I don\u0027t think it would need to be configurable). This means the swap_progress would be accurate to within the last minute or so, which is perfectly fine for monitoring the progress of long running operations. The net result would be:\n- No additional DB reads\n- Calls down to n-cpu would be limited to a max of once every minute or so\n- A DB write would occur (to persist the latest swap_progress and timestamp) only when an admin asks for the progress, and only at a max rate of every minute or so.\n\nIf this is heading in a better direction then I\u0027ll update the spec.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"293599d7cd4fe581307d6fea51996ae54eeac192","unresolved":true,"context_lines":[{"line_number":123,"context_line":"The ``swap_progress`` data isn\u0027t inherently sensitive, and limiting the"},{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a00b3af6_1108237e","line":126,"in_reply_to":"701ba501_0b8af713","updated":"2025-12-03 18:31:35.000000000","message":"we have several other calls that directly result in a rpc to the compute so im perhaps less woried by ddos side.\n\none relitivly simpliel mitigation woudl be to cache the result using memcache the same way to we do that for the instance metadata.\n\nwe expire that on a 15 second interval\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#api.metadata_cache_expiration\n\nwe coudl do the same for this althoug we may want a separte tunable for that.\n\nthat would just involve checking if we have a value in the cache if not doing an rpc and storing the result.\n\nif the nova side will only do the rpc call if the reqst was made by a token with the service role that also help mitigate this.\n\nwhat do folks think about caching it in the api?\n\nto me the treat level her is no diffent then someone polling the server diagnostics \nhttps://docs.openstack.org/api-ref/compute/#show-server-diagnostics\nor console url show\nhttps://docs.openstack.org/api-ref/compute/#create-console\n\nboth involve an rpc to the compute either requiring a direct call to libvirt in the case of the diagnostics or reading config data in teh case fo the conse url endpoint.\n\nhaving the compute monitor this perodicly and updating the database woudl be an option but then you intocudeing an rpc rount trip form the compute to the conducotr on a semi regual basis even if no one asks for the data so i think taht woudl be worse performance wise the nonly doing it on demand.\n\n\nwe could make the rpc cheaper as well. \n\nif we update the progress in memory  based on the libvirt job stats and jsut have the rpc read the data in memory rather then pooling libvirt for the job info that would reduce the cost of the rpc as its just an in memory read allow us to amertise the update cost as part of our exing job tracking.\n\nwe woudl just need to modify \n\nhttps://github.com/openstack/nova/blob/23b462d77df1a1d09c43d0918bca853ef3af1e3f/nova/virt/libvirt/driver.py#L2385-L2391\nto update the progress.\n\n\nso to summarise my proposal is to modify\n\nhave https://github.com/openstack/nova/blob/23b462d77df1a1d09c43d0918bca853ef3af1e3f/nova/virt/libvirt/driver.py#L2385-L2391 update the progress fo the volume swap in a in memory dict\n\nhave the rpc read the in memofy value\nand cache the value in teh api (vai oslo.cache i.e. memcache) for a configurbale time defaulting to 15 seconds just like the instance metadata caching.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":true,"context_lines":[{"line_number":123,"context_line":"The ``swap_progress`` data isn\u0027t inherently sensitive, and limiting the"},{"line_number":124,"context_line":"the RPC to API requests coming from an OpenStack service ensures the RPC"},{"line_number":125,"context_line":"cannot be directly abused by bad actor. The cinder service will request the"},{"line_number":126,"context_line":"``swap_progress``, but cinder\u0027s own policy restricts this to only admins."},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"Notifications impact"},{"line_number":129,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"4ad1cb0e_81edcebe","line":126,"in_reply_to":"a00b3af6_1108237e","updated":"2025-12-04 19:26:05.000000000","message":"I am ok if we refresh it based on BDM timestamp and make RPC call less frequent (1 min timeframe sounds good to me) or Sean idea.\n\nI am ok to figure out it in implementation and amend the spec on whatever implementation we go for. But limiting the request solve the dos concern from my side.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e20d330a0a8c8d7c66abab870c56932a738c3bbb","unresolved":true,"context_lines":[{"line_number":151,"context_line":"The RPC to fetch the libVirt job info is triggered only when another OpenStack"},{"line_number":152,"context_line":"service (namely cinder) requests a volume\u0027s attachment details. Cinder already"},{"line_number":153,"context_line":"limits volume migration status to admins, and will request the"},{"line_number":154,"context_line":"``swap_progress`` only when nova is actually swapping a volume. In other"},{"line_number":155,"context_line":"words, the RPC will only be invoked when an admin requests a volume\u0027s"},{"line_number":156,"context_line":"migration status during the time when nova is swapping the volume."},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"Other deployer impact"},{"line_number":159,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":5,"id":"4c97deca_b9448a78","line":156,"range":{"start_line":154,"start_character":64,"end_line":156,"end_character":66},"updated":"2025-12-04 19:26:05.000000000","message":"this is something I would like to make sure to check in nova side also. If cinder or other service (it might be) calling the GET voluem attachment then Nova should check if migration is going on. I know as per current plan, cinder will call this when migration is in progress but this API is not limited to that use case only and cinder or other service can call this in other use case also in future.\n\nI think this is something we should fix in this spec.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"53a9d4c81a87191f55c575b7870d733fa1692719","unresolved":true,"context_lines":[{"line_number":168,"context_line":"Upgrade impact"},{"line_number":169,"context_line":"--------------"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"None."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"Implementation"},{"line_number":174,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":5,"id":"5182b711_cdc0afa4","line":171,"updated":"2025-12-04 18:01:46.000000000","message":"one thing that i would have liked to see covered is the upgrade scenario where\nthe API is upgrade but the compute node is not.\n\nin this case i think we can just use the rpc API version ot downgrade the request.\nbut you have not defined how this will be represented in the API response\n\ni would expect use to just return None in this case for the swap progress.","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f90abceeb0bc02a74a11c2b23bfa77cfa63c2de7","unresolved":true,"context_lines":[{"line_number":205,"context_line":"TBD. The tricky part for incorporating something into a tempest test"},{"line_number":206,"context_line":"catching a migration operation during the time when libVirt is copying"},{"line_number":207,"context_line":"the volume data, which is the only time when the ``swap_progress`` field"},{"line_number":208,"context_line":"will be present in the attachments response."},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"Documentation Impact"},{"line_number":211,"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":5,"id":"5cbcfe93_1225ce40","line":208,"updated":"2025-12-04 17:57:49.000000000","message":"we can simulate this in the libvirt funcitonal tests.\ni agree that tempest testing woudl likely be flaky and not ideal\n\nthis feature can be tested pirmary with api sample and functional tests","commit_id":"bbb55f797bc200f956b1379482ad7735359f43cd"}]}