)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d056ef25052e651514df5cbec986d2638bffb62","unresolved":true,"context_lines":[{"line_number":13,"context_line":"This option allows a Cinder volume driver to set the rotation_rate based"},{"line_number":14,"context_line":"on the actual rate of a drive, for example 7200 or 1 in case of an SSD."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Relates to blueprint rotation-rate"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I5124d9408b04e5687411479560e3ebd8e7dd11fa"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fdcfded5_f19f7497","line":16,"updated":"2024-05-30 13:15:34.000000000","message":"this should be `Implements: blueprint rotation-rate`\n\nthat also need to be approved before this can proceed.\n\nyou can ask for that to be review/approved in the next nova team meeting which happes on tusday \n\nhttps://wiki.openstack.org/wiki/Meetings/Nova#Weekly_Nova_team_meeting\n\naddtionaly you should add a release not for this feature\n\n`tox -e venv reno new rotation-rate` will generate a template file for you\nyou will only need the features section.\n\n\nyou can effectly just use the summary you have n the bluprint description.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"},{"author":{"_account_id":17440,"name":"Marlin Cremers","display_name":"Marlin Cremers","email":"marlin@cbws.nl","username":"mcremers"},"change_message_id":"81a20a8e7a40314f3210f1bbd7d15ace56b2cb50","unresolved":true,"context_lines":[{"line_number":13,"context_line":"This option allows a Cinder volume driver to set the rotation_rate based"},{"line_number":14,"context_line":"on the actual rate of a drive, for example 7200 or 1 in case of an SSD."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Relates to blueprint rotation-rate"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I5124d9408b04e5687411479560e3ebd8e7dd11fa"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7acf8861_e12547b5","line":16,"in_reply_to":"fdcfded5_f19f7497","updated":"2024-05-31 14:05:33.000000000","message":"Acknowledged, I\u0027ll try and join the meeting on Tuesday.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":17440,"name":"Marlin Cremers","display_name":"Marlin Cremers","email":"marlin@cbws.nl","username":"mcremers"},"change_message_id":"81a20a8e7a40314f3210f1bbd7d15ace56b2cb50","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6280ea2c_a2925b36","updated":"2024-05-31 14:05:33.000000000","message":"Thank you for your very detailed review Sean","commit_id":"daa5f80756a390d7e2a6aa9ba972ff90dd291232"}],"nova/tests/unit/virt/libvirt/test_config.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d056ef25052e651514df5cbec986d2638bffb62","unresolved":true,"context_lines":[{"line_number":1471,"context_line":"              \u003csource file\u003d\"/tmp/hello\"/\u003e"},{"line_number":1472,"context_line":"              \u003ctarget bus\u003d\"ide\" dev\u003d\"/dev/hda\" rotation_rate\u003d\"1\"/\u003e"},{"line_number":1473,"context_line":"              \u003creadonly/\u003e"},{"line_number":1474,"context_line":"            \u003c/disk\u003e\"\"\")"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"class LibvirtConfigGuestSnapshotDiskTest(LibvirtConfigBaseTest):"}],"source_content_type":"text/x-python","patch_set":4,"id":"41eac823_02c9589a","line":1474,"updated":"2024-05-30 13:15:34.000000000","message":"we may also need to extend the libvirt live migration functional  to ensure this is adiquetly tested.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"},{"author":{"_account_id":17440,"name":"Marlin Cremers","display_name":"Marlin Cremers","email":"marlin@cbws.nl","username":"mcremers"},"change_message_id":"81a20a8e7a40314f3210f1bbd7d15ace56b2cb50","unresolved":true,"context_lines":[{"line_number":1471,"context_line":"              \u003csource file\u003d\"/tmp/hello\"/\u003e"},{"line_number":1472,"context_line":"              \u003ctarget bus\u003d\"ide\" dev\u003d\"/dev/hda\" rotation_rate\u003d\"1\"/\u003e"},{"line_number":1473,"context_line":"              \u003creadonly/\u003e"},{"line_number":1474,"context_line":"            \u003c/disk\u003e\"\"\")"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"class LibvirtConfigGuestSnapshotDiskTest(LibvirtConfigBaseTest):"}],"source_content_type":"text/x-python","patch_set":4,"id":"99a8f281_e8b6cc4d","line":1474,"in_reply_to":"41eac823_02c9589a","updated":"2024-05-31 14:05:33.000000000","message":"Acknowledged, I\u0027ll see how I can add a functional test for this as this is using volumes which I haven\u0027t seen much of a similar live migration test for already.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"}],"nova/virt/libvirt/volume/volume.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d056ef25052e651514df5cbec986d2638bffb62","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            conf.logical_block_size \u003d data[\u0027logical_block_size\u0027]"},{"line_number":61,"context_line":"        if \u0027physical_block_size\u0027 in data:"},{"line_number":62,"context_line":"            conf.physical_block_size \u003d data[\u0027physical_block_size\u0027]"},{"line_number":63,"context_line":"        if \u0027rotation_rate\u0027 in data:"},{"line_number":64,"context_line":"            conf.target_rotation_rate \u003d data[\u0027rotation_rate\u0027]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # Extract rate_limit control parameters"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f2bfaa4_5f7119ab","line":63,"updated":"2024-05-30 13:15:34.000000000","message":"this feature was added in libvirt 7.3.0 based on https://libvirt.org/formatdomain.html\n\nour min libvirt is 7.0.0\n\nhttps://github.com/openstack/nova/blob/96268d4e7a0dd1872c77641a634a94f599d59fe0/nova/virt/libvirt/driver.py#L219\n\nso you will need a min libvirt version check and we can enable this unconditionally.\n\nnova will not allow you to live migrate to a host with an older libvirt so we dont need to report this as a trait nessisarly to have correct schduling but wither this will be added will depend on the libvirt installed.\n\none thing we need to confirm is if the existing live migration code tha tupdates the xml would correctly handle this addtional atribute.\n\n\nhttps://github.com/openstack/nova/blob/96268d4e7a0dd1872c77641a634a94f599d59fe0/nova/virt/libvirt/migration.py#L280-L334\n\nskiming it quickly (i have not review it properly)\nwhile we do modify the voluem defintion inn the xml i dont think we are modifying the target element\n\n\nthe upgrade concern here is ensuring that the unmodifed code on  2024.1 caracal\nis able to supprot migrating the vm form the older node to a upgaded node\n\ni.e. to supprot 2024.2(master/damation) -\u003e2024.1 -\u003e 2024.2\n\nwe need to support back and fort migration in generl betten supported release.\n\nthe way to mitigate this is to add a comptue service version bump and a min compute service version check to enabel this functionalty\nalthernitvly we can use a compute capablity triat  but that is more work.\n\ngiven those outstandign issue this patch need to be revies to adress them.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1312506dd47e018f077410612543fa6349f7babb","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            conf.logical_block_size \u003d data[\u0027logical_block_size\u0027]"},{"line_number":61,"context_line":"        if \u0027physical_block_size\u0027 in data:"},{"line_number":62,"context_line":"            conf.physical_block_size \u003d data[\u0027physical_block_size\u0027]"},{"line_number":63,"context_line":"        if \u0027rotation_rate\u0027 in data:"},{"line_number":64,"context_line":"            conf.target_rotation_rate \u003d data[\u0027rotation_rate\u0027]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # Extract rate_limit control parameters"}],"source_content_type":"text/x-python","patch_set":4,"id":"77524a72_9405d696","line":63,"in_reply_to":"8f231aa4_71dbe138","updated":"2024-06-04 11:51:46.000000000","message":"if the destiatnion libvirt was too old the call to libvirt on the source would return an error when we start the mightion becuase it would not be able to create the destination instance.\n\nso it would hard fail at that point.\n\nthe nova will not select a host with an older libvirt than the source so that should not be a problem.\nthe probelm is really with mixed nova version which is required for upgrades.\nthis state can persis for month at a time.\noften it will only be hours but nova like many service is expceted to supprot the mixed config for protracted periods of time provide it does not cross the n-1/n-2 bondary of a skip level upgrade release bondary.\n\nfor what its worth the grenade test passed on this which is at least promicing.\n\nthe recommend way to upgrade openstack and the operating system is to do both seperatlly\n\ni.e. upgrade openstack to a verion supproted on the old and new operating system then when openstack if fully upgraded to a single version you may start upgrading the OS.\n\nthis means that in general that only one of the libvirt version or the compute service version  should be changing at any one time. so we  would not expect to have to deal with both at once although an opteror could attemept to do both at once but it would not be recommended.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"},{"author":{"_account_id":17440,"name":"Marlin Cremers","display_name":"Marlin Cremers","email":"marlin@cbws.nl","username":"mcremers"},"change_message_id":"81a20a8e7a40314f3210f1bbd7d15ace56b2cb50","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            conf.logical_block_size \u003d data[\u0027logical_block_size\u0027]"},{"line_number":61,"context_line":"        if \u0027physical_block_size\u0027 in data:"},{"line_number":62,"context_line":"            conf.physical_block_size \u003d data[\u0027physical_block_size\u0027]"},{"line_number":63,"context_line":"        if \u0027rotation_rate\u0027 in data:"},{"line_number":64,"context_line":"            conf.target_rotation_rate \u003d data[\u0027rotation_rate\u0027]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        # Extract rate_limit control parameters"}],"source_content_type":"text/x-python","patch_set":4,"id":"8f231aa4_71dbe138","line":63,"in_reply_to":"9f2bfaa4_5f7119ab","updated":"2024-05-31 14:05:33.000000000","message":"I have added a check that should check against libvirt version 7.3.0. I still have to look into the other comments, like confirming if this will cause problems with live migration.\n\nI did not expect the added complexity in ensuring compatibility between nodes of different compute versions but this makes sense. I wonder what would happen if one would migrate from one node to another that doesn\u0027t support the flag, would the XML be coppied over from the source node, in which cause it might work if the supported libvirt version is on the destination. Will need to look into that further.","commit_id":"ec4b24d53d63e797d741ffa848eb4483dc2f8081"}]}