)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b7f9a18cacc84b3d87f6a3ccf308a2ccb2764fe4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Alexandre Arents \u003calexandre.arents@corp.ovh.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-09-01 08:50:21 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Update image_base_image_ref during rebuild."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In different location we assume system_metadata.image_base_image_ref"},{"line_number":10,"context_line":"exists, because it is set during instance creation in method"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9f560f44_111a1216","line":7,"range":{"start_line":7,"start_character":42,"end_line":7,"end_character":43},"updated":"2020-09-02 14:46:27.000000000","message":"supernit - not required","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b7f9a18cacc84b3d87f6a3ccf308a2ccb2764fe4","unresolved":false,"context_lines":[{"line_number":10,"context_line":"exists, because it is set during instance creation in method"},{"line_number":11,"context_line":"_populate_instance_for_create"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"But once instance is rebuild, all system_metadata image property a dropped"},{"line_number":14,"context_line":"and replace by new image property and without setting back"},{"line_number":15,"context_line":"image_base_image_ref."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This change propose to set image_base_image_ref during rebuild."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"In specific case of shelve/unshelve in Qcow2 backend, image_base_image_ref is"},{"line_number":20,"context_line":"used to rebase disk image, so we ensure this property is set as instance may"},{"line_number":21,"context_line":"have been rebuild before the fix was apply."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Related-Bug: #1732428"},{"line_number":24,"context_line":"Closes-Bug: #1893618"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9f560f44_f114beeb","line":21,"range":{"start_line":13,"start_character":0,"end_line":21,"end_character":43},"updated":"2020-09-02 14:46:27.000000000","message":"supernit - please wrap these lines in future","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"}],"nova/compute/api.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fc330a0e99e125a18b8c369e3521f23627decc6a","unresolved":false,"context_lines":[{"line_number":3534,"context_line":"            new_sys_metadata \u003d utils.get_system_metadata_from_image("},{"line_number":3535,"context_line":"                image, flavor)"},{"line_number":3536,"context_line":""},{"line_number":3537,"context_line":"            new_sys_metadata.update({\u0027image_base_image_ref\u0027: image_id})"},{"line_number":3538,"context_line":""},{"line_number":3539,"context_line":"            instance.system_metadata.update(new_sys_metadata)"},{"line_number":3540,"context_line":"            instance.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_51fe616b","line":3537,"range":{"start_line":3537,"start_character":61,"end_line":3537,"end_character":69},"updated":"2020-09-01 12:58:44.000000000","message":"oh this is coming from the other function.\nthat was not really that obvious i would prefer if you passed it in instead.","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b65c37357f9ce7b1282e429a3d82ead51c34103b","unresolved":false,"context_lines":[{"line_number":3534,"context_line":"            new_sys_metadata \u003d utils.get_system_metadata_from_image("},{"line_number":3535,"context_line":"                image, flavor)"},{"line_number":3536,"context_line":""},{"line_number":3537,"context_line":"            new_sys_metadata.update({\u0027image_base_image_ref\u0027: image_id})"},{"line_number":3538,"context_line":""},{"line_number":3539,"context_line":"            instance.system_metadata.update(new_sys_metadata)"},{"line_number":3540,"context_line":"            instance.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_6dfc543e","line":3537,"range":{"start_line":3537,"start_character":61,"end_line":3537,"end_character":69},"in_reply_to":"9f560f44_07ee6b8a","updated":"2020-09-02 14:19:25.000000000","message":"i guess this could be fix and made consitent later but it makes this hard to test. so ill swap form a -1 to +1 but if you have time to add a follow up to take it as a paramater i thinkt that would be better","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"50f44635da589f8e4f7a9e0986b234aaf76dfb24","unresolved":false,"context_lines":[{"line_number":3534,"context_line":"            new_sys_metadata \u003d utils.get_system_metadata_from_image("},{"line_number":3535,"context_line":"                image, flavor)"},{"line_number":3536,"context_line":""},{"line_number":3537,"context_line":"            new_sys_metadata.update({\u0027image_base_image_ref\u0027: image_id})"},{"line_number":3538,"context_line":""},{"line_number":3539,"context_line":"            instance.system_metadata.update(new_sys_metadata)"},{"line_number":3540,"context_line":"            instance.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_07ee6b8a","line":3537,"range":{"start_line":3537,"start_character":61,"end_line":3537,"end_character":69},"in_reply_to":"9f560f44_51fe616b","updated":"2020-09-01 14:41:34.000000000","message":"Yeah a bit disappointing, I just mimic what have been done on the lines upper, for consistency if I pass in, I should do the same for instance, flavor, image, image_id:\ndef _reset_image_metadata(instance, flavor, image, image_id):\nSo I don\u0027t know if it is better or not?","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0681ccffbd8113ee010accaa69b9d47be8b13b08","unresolved":false,"context_lines":[{"line_number":4177,"context_line":"        # needed during unshelve and instance rebuild done before Bug/1893618"},{"line_number":4178,"context_line":"        # Fix dropped it."},{"line_number":4179,"context_line":"        instance.system_metadata.update("},{"line_number":4180,"context_line":"                {\u0027image_base_image_ref\u0027: instance.image_ref}"},{"line_number":4181,"context_line":"        )"},{"line_number":4182,"context_line":""},{"line_number":4183,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_b4f89049","line":4180,"range":{"start_line":4180,"start_character":12,"end_line":4180,"end_character":16},"updated":"2020-09-02 15:44:31.000000000","message":"nit","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2c2a9164ef991f0b6db502511acb775413563709","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        \"\"\""},{"line_number":4174,"context_line":"        instance.task_state \u003d task_states.SHELVING"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"        # NOTE(aarents): Ensure image_base_image_ref is present as it will be"},{"line_number":4177,"context_line":"        # needed during unshelve and instance rebuild done before Bug/1893618"},{"line_number":4178,"context_line":"        # Fix dropped it."},{"line_number":4179,"context_line":"        instance.system_metadata.update("},{"line_number":4180,"context_line":"                {\u0027image_base_image_ref\u0027: instance.image_ref}"},{"line_number":4181,"context_line":"        )"},{"line_number":4182,"context_line":""},{"line_number":4183,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":4184,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_990b052a","line":4181,"range":{"start_line":4176,"start_character":0,"end_line":4181,"end_character":9},"updated":"2020-09-01 08:57:34.000000000","message":"Isn\u0027t L3537 enough? De we drop image meta during other then rebuild?","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7fc5b2ebd244d2819701526539be0e4c98e258c8","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        \"\"\""},{"line_number":4174,"context_line":"        instance.task_state \u003d task_states.SHELVING"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"        # NOTE(aarents): Ensure image_base_image_ref is present as it will be"},{"line_number":4177,"context_line":"        # needed during unshelve and instance rebuild done before Bug/1893618"},{"line_number":4178,"context_line":"        # Fix dropped it."},{"line_number":4179,"context_line":"        instance.system_metadata.update("},{"line_number":4180,"context_line":"                {\u0027image_base_image_ref\u0027: instance.image_ref}"},{"line_number":4181,"context_line":"        )"},{"line_number":4182,"context_line":""},{"line_number":4183,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":4184,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_2b7c0c1f","line":4181,"range":{"start_line":4176,"start_character":0,"end_line":4181,"end_character":9},"in_reply_to":"9f560f44_67f48788","updated":"2020-09-02 08:26:53.000000000","message":"Thanks for the explanation. Good point that we need to handle already rebuilt instances.","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"ee7d78a4917375c177105394a7e9eee84f3b7228","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        \"\"\""},{"line_number":4174,"context_line":"        instance.task_state \u003d task_states.SHELVING"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"        # NOTE(aarents): Ensure image_base_image_ref is present as it will be"},{"line_number":4177,"context_line":"        # needed during unshelve and instance rebuild done before Bug/1893618"},{"line_number":4178,"context_line":"        # Fix dropped it."},{"line_number":4179,"context_line":"        instance.system_metadata.update("},{"line_number":4180,"context_line":"                {\u0027image_base_image_ref\u0027: instance.image_ref}"},{"line_number":4181,"context_line":"        )"},{"line_number":4182,"context_line":""},{"line_number":4183,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":4184,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_bf8ce97f","line":4181,"range":{"start_line":4176,"start_character":0,"end_line":4181,"end_character":9},"in_reply_to":"9f560f44_990b052a","updated":"2020-09-01 10:16:31.000000000","message":"It is enough for CI and new instance/deployment, But If we consider upgrade from a stable release, I afraid that plenty of instance miss this parameter due to rebuild and fall again in that bug.\n\nFrom my understanding I don\u0027t see other place where property are dropped.\n\nBUT\nother thing :\nwhen we spawn from snapshot,\nnew instance have their system_metada.image_base_image_ref don\u0027t point to image_id of the snapshot but on base_image_ref property of the snapshot image.\n\nI have a doubt if it should be the expected behavior.\nin other words, when we cascade \"spawn from snapshot\"\nshould all intance share the same image_base_image_ref or should it reflect image id of the image from where it have been spawn?\n\n\nDuring creation setdefault() method do not override if it exist:                      \n                                                                                                                                       \nsystem_meta.setdefault(\u0027image_base_image_ref\u0027, instance.image_ref)","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":28332,"name":"Alexandre arents","email":"alexandre.arents@corp.ovh.com","username":"aarents"},"change_message_id":"50f44635da589f8e4f7a9e0986b234aaf76dfb24","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        \"\"\""},{"line_number":4174,"context_line":"        instance.task_state \u003d task_states.SHELVING"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"        # NOTE(aarents): Ensure image_base_image_ref is present as it will be"},{"line_number":4177,"context_line":"        # needed during unshelve and instance rebuild done before Bug/1893618"},{"line_number":4178,"context_line":"        # Fix dropped it."},{"line_number":4179,"context_line":"        instance.system_metadata.update("},{"line_number":4180,"context_line":"                {\u0027image_base_image_ref\u0027: instance.image_ref}"},{"line_number":4181,"context_line":"        )"},{"line_number":4182,"context_line":""},{"line_number":4183,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":4184,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_67f48788","line":4181,"range":{"start_line":4176,"start_character":0,"end_line":4181,"end_character":9},"in_reply_to":"9f560f44_b1c41d61","updated":"2020-09-01 14:41:34.000000000","message":"Yes unfornunately, this block is required for legacy and robustness after upgrade I think.\n\nFor the update versus setdefault discuss look at the example in main body of this reply.","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fc330a0e99e125a18b8c369e3521f23627decc6a","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        \"\"\""},{"line_number":4174,"context_line":"        instance.task_state \u003d task_states.SHELVING"},{"line_number":4175,"context_line":""},{"line_number":4176,"context_line":"        # NOTE(aarents): Ensure image_base_image_ref is present as it will be"},{"line_number":4177,"context_line":"        # needed during unshelve and instance rebuild done before Bug/1893618"},{"line_number":4178,"context_line":"        # Fix dropped it."},{"line_number":4179,"context_line":"        instance.system_metadata.update("},{"line_number":4180,"context_line":"                {\u0027image_base_image_ref\u0027: instance.image_ref}"},{"line_number":4181,"context_line":"        )"},{"line_number":4182,"context_line":""},{"line_number":4183,"context_line":"        instance.save(expected_task_state\u003d[None])"},{"line_number":4184,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_b1c41d61","line":4181,"range":{"start_line":4176,"start_character":0,"end_line":4181,"end_character":9},"in_reply_to":"9f560f44_bf8ce97f","updated":"2020-09-01 12:58:44.000000000","message":"utils.get_system_metadata_from_image start from an empty dict and never set image_base_image_ref\n\nso it will never be set druing creation before we get to the set default call.\n\nits possible that at some time in the past the behavior was different but on master the set default call always set the vaule to the instance.image_ref.\n\n\n\nin practice \n\n  new_sys_metadata \u003d utils.get_system_metadata_from_image(\n                image, flavor)\n\n            new_sys_metadata.update({\u0027image_base_image_ref\u0027: image_id})\n\n\nand \n\n  new_sys_metadata \u003d utils.get_system_metadata_from_image(\n                image, flavor)\n\n            new_sys_metadata.setdefault({\u0027image_base_image_ref\u0027: image_id})\n\n\nwill do exactly the same thing\n\ni do think that yes this is required.although ideally rebuild would do the right thing going forward and this is just required for legacy instances.","commit_id":"fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d"}]}