)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"916e8a49d4b549a1cbe53eb5f65a17af19cf4540","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"96bcc963_6f3afd55","updated":"2022-08-31 10:42:26.000000000","message":"this obvioulsy needs testing and more work so -1 but this works for me locally.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9d1cdeb2e4a551aafee8550e12c4e1836fe2a2a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c9397774_87f5e50d","updated":"2022-09-02 09:24:29.000000000","message":"this shouuld stilll have -w by the way im also going to part this until we disucss this further.","commit_id":"d874f2323e6f6a89e5e78a89e74a8feda708a9b8"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"494f50ad719cb00b1c40efc086bc76280ba270fc","unresolved":true,"context_lines":[{"line_number":3975,"context_line":"            context, xml, instance, network_info, block_device_info,"},{"line_number":3976,"context_line":"            vifs_already_plugged\u003dvifs_already_plugged,"},{"line_number":3977,"context_line":"            external_events\u003dexternal_events,"},{"line_number":3978,"context_line":"            post_xml_callback\u003dgen_confdrive"},{"line_number":3979,"context_line":"        )"},{"line_number":3980,"context_line":""},{"line_number":3981,"context_line":"        def _wait_for_reboot():"}],"source_content_type":"text/x-python","patch_set":1,"id":"f2e9d217_b19961ea","line":3978,"updated":"2022-09-01 09:43:43.000000000","message":"is there a specific reason we do the file generation in the callback? I agree that we should do it between destroy and create_guest but I don\u0027t see the reason of using the callback","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa0925f6cf2912cd37fb170026f0247dbaaa45d3","unresolved":false,"context_lines":[{"line_number":3975,"context_line":"            context, xml, instance, network_info, block_device_info,"},{"line_number":3976,"context_line":"            vifs_already_plugged\u003dvifs_already_plugged,"},{"line_number":3977,"context_line":"            external_events\u003dexternal_events,"},{"line_number":3978,"context_line":"            post_xml_callback\u003dgen_confdrive"},{"line_number":3979,"context_line":"        )"},{"line_number":3980,"context_line":""},{"line_number":3981,"context_line":"        def _wait_for_reboot():"}],"source_content_type":"text/x-python","patch_set":1,"id":"87edd85c_b45637e7","line":3978,"in_reply_to":"dedbfa51_d0f91b48","updated":"2022-09-02 11:04:35.000000000","message":"thanks. That explains it.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e13ad52d8c3602799b6e9a51a2aef6f54a587f5","unresolved":true,"context_lines":[{"line_number":3975,"context_line":"            context, xml, instance, network_info, block_device_info,"},{"line_number":3976,"context_line":"            vifs_already_plugged\u003dvifs_already_plugged,"},{"line_number":3977,"context_line":"            external_events\u003dexternal_events,"},{"line_number":3978,"context_line":"            post_xml_callback\u003dgen_confdrive"},{"line_number":3979,"context_line":"        )"},{"line_number":3980,"context_line":""},{"line_number":3981,"context_line":"        def _wait_for_reboot():"}],"source_content_type":"text/x-python","patch_set":1,"id":"dedbfa51_d0f91b48","line":3978,"in_reply_to":"f2e9d217_b19961ea","updated":"2022-09-01 11:30:24.000000000","message":"this is what we do in spawn. the reason is we need to get infro from the xml to embed in the config drive. specificaly we are trying to extract the libvirt generated pci adress infomation for the device role tagging feature.\n\ni suspect this is why they orginaly did it before destory because they needed the xml for this to succeed.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"494f50ad719cb00b1c40efc086bc76280ba270fc","unresolved":false,"context_lines":[{"line_number":4975,"context_line":"                    # possible while we\u0027re still using cache() under the hood."},{"line_number":4976,"context_line":"                    config_disk_local_path \u003d os.path.join("},{"line_number":4977,"context_line":"                        libvirt_utils.get_instance_path(instance),"},{"line_number":4978,"context_line":"                        name + \u0027.temp\u0027"},{"line_number":4979,"context_line":"                    )"},{"line_number":4980,"context_line":"                    LOG.info(\u0027Creating config drive at %(path)s\u0027,"},{"line_number":4981,"context_line":"                             {\u0027path\u0027: config_disk_local_path},"}],"source_content_type":"text/x-python","patch_set":1,"id":"b05b7533_3b3b5f8d","line":4978,"updated":"2022-09-01 09:43:43.000000000","message":"OK, we need this now as we never generated a config drive while we already had one. Now opt to generate it at a temp place then copy it to the image backend via the image_import below.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e13ad52d8c3602799b6e9a51a2aef6f54a587f5","unresolved":false,"context_lines":[{"line_number":4975,"context_line":"                    # possible while we\u0027re still using cache() under the hood."},{"line_number":4976,"context_line":"                    config_disk_local_path \u003d os.path.join("},{"line_number":4977,"context_line":"                        libvirt_utils.get_instance_path(instance),"},{"line_number":4978,"context_line":"                        name + \u0027.temp\u0027"},{"line_number":4979,"context_line":"                    )"},{"line_number":4980,"context_line":"                    LOG.info(\u0027Creating config drive at %(path)s\u0027,"},{"line_number":4981,"context_line":"                             {\u0027path\u0027: config_disk_local_path},"}],"source_content_type":"text/x-python","patch_set":1,"id":"4da584aa_73fcfc66","line":4978,"in_reply_to":"b05b7533_3b3b5f8d","updated":"2022-09-01 11:30:24.000000000","message":"yep without appending .temp we will get the same permission error because the file will exist and it will be owned by qemu so we generate it and then copy via privsep.\n\ni could perhaps make the config dirve generation privladge instead but i think this flow is cleaner long term as we get to consolidate the workflow.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"494f50ad719cb00b1c40efc086bc76280ba270fc","unresolved":true,"context_lines":[{"line_number":4994,"context_line":"                finally:"},{"line_number":4995,"context_line":"                    # NOTE(sean-k-mooney): we now use import to copy"},{"line_number":4996,"context_line":"                    # the config drive to the final localtion for all backends."},{"line_number":4997,"context_line":"                    # as a result we should always unlink the temp copy."},{"line_number":4998,"context_line":"                    LOG.info(\u0027Deleting temporary config drive %(path)s \u0027"},{"line_number":4999,"context_line":"                             \u0027because it was imported.\u0027,"},{"line_number":5000,"context_line":"                             {\u0027path\u0027: config_disk_local_path},"}],"source_content_type":"text/x-python","patch_set":1,"id":"ee59afd9_4458a5df","line":4997,"updated":"2022-09-01 09:43:43.000000000","message":"+1","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"38e8d4be1108553266a33921d4bcf88b6f477bfd","unresolved":true,"context_lines":[{"line_number":5028,"context_line":"                f\"Config drive regeneration failed with error: {e}s\","},{"line_number":5029,"context_line":"                instance\u003dinstance"},{"line_number":5030,"context_line":"            )"},{"line_number":5031,"context_line":"            raise"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":""},{"line_number":5034,"context_line":"    def _detach_pci_devices(self, guest, pci_devs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b771bb63_a229feb1","line":5031,"updated":"2022-09-01 15:17:26.000000000","message":"how will we know if it will work synchronously by the API ? Would we rather get an ERROR ?","commit_id":"d874f2323e6f6a89e5e78a89e74a8feda708a9b8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9d1cdeb2e4a551aafee8550e12c4e1836fe2a2a9","unresolved":true,"context_lines":[{"line_number":5028,"context_line":"                f\"Config drive regeneration failed with error: {e}s\","},{"line_number":5029,"context_line":"                instance\u003dinstance"},{"line_number":5030,"context_line":"            )"},{"line_number":5031,"context_line":"            raise"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":""},{"line_number":5034,"context_line":"    def _detach_pci_devices(self, guest, pci_devs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"2ca59dce_dce042ef","line":5031,"in_reply_to":"b771bb63_a229feb1","updated":"2022-09-02 09:24:29.000000000","message":"the intent was to put the vm into an error state and then you would fix that with anohter hard reboot.\n\nso it would be reported the same way a normal hard reboot failure would be","commit_id":"d874f2323e6f6a89e5e78a89e74a8feda708a9b8"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"494f50ad719cb00b1c40efc086bc76280ba270fc","unresolved":true,"context_lines":[{"line_number":487,"context_line":"    def import_file(self, instance, local_file, remote_name):"},{"line_number":488,"context_line":"        \"\"\"Import an image from local storage into this backend."},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        Import a local file into the store used by this image type. Note that"},{"line_number":491,"context_line":"        this is a noop for stores using local disk (the local file is"},{"line_number":492,"context_line":"        considered \"in the store\")."},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"        If the image already exists it will be overridden by the new file"},{"line_number":495,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"26ed61dc_a851f8e1","line":492,"range":{"start_line":490,"start_character":68,"end_line":492,"end_character":35},"updated":"2022-09-01 09:43:43.000000000","message":"This is not true any more as you added the default copy implementation and Qcow and Flat are not overriding this function to noop. This also means that calling import_file where local_file and remove_name points to the same file was OK for Qcow and Flat but now it will fail as you cannot copy the file to itself. I\u0027m not sure we relied on import_file to do this tough .","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e13ad52d8c3602799b6e9a51a2aef6f54a587f5","unresolved":true,"context_lines":[{"line_number":487,"context_line":"    def import_file(self, instance, local_file, remote_name):"},{"line_number":488,"context_line":"        \"\"\"Import an image from local storage into this backend."},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        Import a local file into the store used by this image type. Note that"},{"line_number":491,"context_line":"        this is a noop for stores using local disk (the local file is"},{"line_number":492,"context_line":"        considered \"in the store\")."},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"        If the image already exists it will be overridden by the new file"},{"line_number":495,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8a90ac48_90933ab3","line":492,"range":{"start_line":490,"start_character":68,"end_line":492,"end_character":35},"in_reply_to":"26ed61dc_a851f8e1","updated":"2022-09-01 11:30:24.000000000","message":"ack i would hope that we did not but i can gard against that case with a simple if.\n\nif local_file !\u003d self.path:\n  nova.privsep.path.copy_file(local_file, self.path)\n  \ni guess the question is should i raise an error if they are the same or ignore it.\nits probaly a logic bug if we were doing it so \n\n\nif local_file \u003d\u003d self.path:\n  raise Something\n\nnova.privsep.path.copy_file(local_file, self.path)\n\nis proably better right?\neither way the comment need to be updated.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa0925f6cf2912cd37fb170026f0247dbaaa45d3","unresolved":true,"context_lines":[{"line_number":487,"context_line":"    def import_file(self, instance, local_file, remote_name):"},{"line_number":488,"context_line":"        \"\"\"Import an image from local storage into this backend."},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        Import a local file into the store used by this image type. Note that"},{"line_number":491,"context_line":"        this is a noop for stores using local disk (the local file is"},{"line_number":492,"context_line":"        considered \"in the store\")."},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"        If the image already exists it will be overridden by the new file"},{"line_number":495,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f6d923f_cd9272b2","line":492,"range":{"start_line":490,"start_character":68,"end_line":492,"end_character":35},"in_reply_to":"8a90ac48_90933ab3","updated":"2022-09-02 11:04:35.000000000","message":"I would be explicit and raise.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"916e8a49d4b549a1cbe53eb5f65a17af19cf4540","unresolved":true,"context_lines":[{"line_number":497,"context_line":"        :param remote_name: the name for the file in the store"},{"line_number":498,"context_line":"        \"\"\""},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        nova.privsep.path.copy_file(local_file, self.path)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def create_snap(self, name):"},{"line_number":503,"context_line":"        \"\"\"Create a snapshot on the image.  A noop on backends that don\u0027t"}],"source_content_type":"text/x-python","patch_set":1,"id":"e3c4765c_446d651a","line":500,"range":{"start_line":500,"start_character":0,"end_line":500,"end_character":58},"updated":"2022-08-31 10:42:26.000000000","message":"for lvm this will work for config drive but only because we use a file in that case.\n\nim going to add a check in the lvm class that assert the remote_name contains disk.config and raise a not implemented error if it does not.\n\nthat will protect use from acidentally using this for other cases.\n\nin the future we can add an implemation that will import images to lvm properly if we need it by delegating to create_image to create the volume and using the local_path as the base file.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa0925f6cf2912cd37fb170026f0247dbaaa45d3","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        :param remote_name: the name for the file in the store"},{"line_number":498,"context_line":"        \"\"\""},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        nova.privsep.path.copy_file(local_file, self.path)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def create_snap(self, name):"},{"line_number":503,"context_line":"        \"\"\"Create a snapshot on the image.  A noop on backends that don\u0027t"}],"source_content_type":"text/x-python","patch_set":1,"id":"711b061c_ab552244","line":500,"range":{"start_line":500,"start_character":0,"end_line":500,"end_character":58},"in_reply_to":"303b3332_efd4f242","updated":"2022-09-02 11:04:35.000000000","message":"that is OK to me","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4e13ad52d8c3602799b6e9a51a2aef6f54a587f5","unresolved":true,"context_lines":[{"line_number":497,"context_line":"        :param remote_name: the name for the file in the store"},{"line_number":498,"context_line":"        \"\"\""},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"        nova.privsep.path.copy_file(local_file, self.path)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def create_snap(self, name):"},{"line_number":503,"context_line":"        \"\"\"Create a snapshot on the image.  A noop on backends that don\u0027t"}],"source_content_type":"text/x-python","patch_set":1,"id":"303b3332_efd4f242","line":500,"range":{"start_line":500,"start_character":0,"end_line":500,"end_character":58},"in_reply_to":"e3c4765c_446d651a","updated":"2022-09-01 11:30:24.000000000","message":"@gibi \n\nare you ok with this by they way \n\ni think that should be pretty clean and as i sad if we need too or want to make this work for non config drive i can.","commit_id":"66737d80def8cabece030aa67e8ae80094652d23"}]}