)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1c63461b2e533e691532e998b1cd95474d50462f","unresolved":true,"context_lines":[{"line_number":18,"context_line":"_check_mem_encryption_uses_uefi_image. Therefore we can skip whole cycle"},{"line_number":19,"context_line":"if we dont even have image specs present."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Bug: https://bugs.launchpad.net/nova/+bug/2007697"},{"line_number":22,"context_line":"Change-Id: I5b0db2d524b7a97ddf64c669a7ce1e60c80078eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"407a8f24_5d5191df","line":21,"updated":"2023-02-21 17:33:06.000000000","message":"note: please modify your commit msg to rather use Closes-Bug: #\u003cbug_number\u003e in order to have Launchpad correctly tracking it.","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":35540,"name":"Samuel Kunkel","email":"samuel.kunkel@mail.schwarz","username":"samuel.kunkel"},"change_message_id":"7bc8a0d42e5a5df46556f54d65e87400981f2ac3","unresolved":false,"context_lines":[{"line_number":18,"context_line":"_check_mem_encryption_uses_uefi_image. Therefore we can skip whole cycle"},{"line_number":19,"context_line":"if we dont even have image specs present."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Bug: https://bugs.launchpad.net/nova/+bug/2007697"},{"line_number":22,"context_line":"Change-Id: I5b0db2d524b7a97ddf64c669a7ce1e60c80078eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"c2c2c57d_7d33bc52","line":21,"in_reply_to":"407a8f24_5d5191df","updated":"2023-02-22 15:47:34.000000000","message":"adjusted","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2ac027c60d5bb32ba72c1b265ec5498e65af53e","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This leads to a run through the validation cycle without image_metadata"},{"line_number":21,"context_line":"and the subsequent fail during the validation if uefi is set within the"},{"line_number":22,"context_line":"image_metadata."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Closes-Bug: #2007697"},{"line_number":25,"context_line":"Change-Id: I5b0db2d524b7a97ddf64c669a7ce1e60c80078eb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"edbb17b0_773656d6","line":23,"updated":"2023-11-13 12:46:53.000000000","message":"please split up this commit into two commits\n\nthe first shoudl just be the regression fucntionla assertign the incorerct behaivor\nthe scond shoudl adress teh failure and update the regression test to assert the new workign behavior.\n\nboth commit should be submited for review as a chain fo commits\n\nnot like you did in the the previous revision fo this chagne https://review.opendev.org/c/openstack/nova/+/874264/2","commit_id":"5242cb976fa16b485f8d64e1a2c13c42c366262d"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"43764f40daa42c454a830b6563ac896758d2c66c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"09aed915_0f98f8d0","updated":"2023-02-17 16:49:49.000000000","message":"ill try an review this next week but im not entirly sure we can skip this.\n\nas written this patch should not merge as it has not test coverage to demonstrate teh bug and show that this fixes it so at a minium i would like to see a fucntional repoducer for this issue before we merge this.\n","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"}],"nova/tests/functional/regressions/test_bug_2007697.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7200b1b3b79961d5808dd3c1077e835e47c67f69","unresolved":true,"context_lines":[{"line_number":16,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":17,"context_line":"from nova.tests.unit.virt.libvirt.test_utils import LibvirtUtilsTestCase"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class AMDSEVnotcheckUEFIwithoutIMGproperties(LibvirtUtilsTestCase):"},{"line_number":20,"context_line":"    def test_uefi_image_check_not_called_without_valid_image(self):"},{"line_number":21,"context_line":"        flavor \u003d objects.Flavor("},{"line_number":22,"context_line":"            id\u003d1, flavorid\u003d\u0027fakeid-1\u0027, name\u003d\u0027fake1.small\u0027, memory_mb\u003d128,"}],"source_content_type":"text/x-python","patch_set":3,"id":"8fec47f9_66281ca4","line":19,"updated":"2023-02-28 13:53:53.000000000","message":"this is not a functionl regression test.\n\n\nfirstly it should creating instance of the nova compute, scduler, conductor service and creating a vm. this is a unit test not a funcitonal test as it is testing a single function call.\nit is not testing an end to end api call that an end user could make.\n\nsecondly the way we create regression test is to first propose a test only patch that demonstrates the problem you are solving. then in the seocnd patch we fix the issue and update the test to assert the expect behavior.","commit_id":"bbc38074129e79cef0d26be572f71840971a21c5"},{"author":{"_account_id":35540,"name":"Samuel Kunkel","email":"samuel.kunkel@mail.schwarz","username":"samuel.kunkel"},"change_message_id":"3a26e3522be90388822248e59820aeffbcdb06c8","unresolved":true,"context_lines":[{"line_number":16,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":17,"context_line":"from nova.tests.unit.virt.libvirt.test_utils import LibvirtUtilsTestCase"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class AMDSEVnotcheckUEFIwithoutIMGproperties(LibvirtUtilsTestCase):"},{"line_number":20,"context_line":"    def test_uefi_image_check_not_called_without_valid_image(self):"},{"line_number":21,"context_line":"        flavor \u003d objects.Flavor("},{"line_number":22,"context_line":"            id\u003d1, flavorid\u003d\u0027fakeid-1\u0027, name\u003d\u0027fake1.small\u0027, memory_mb\u003d128,"}],"source_content_type":"text/x-python","patch_set":3,"id":"82673de4_96214a0e","line":19,"in_reply_to":"36ebc0b8_5f312dac","updated":"2023-03-06 13:59:10.000000000","message":"Regressiontest added for reproduction (the fix is commented out therefore)","commit_id":"bbc38074129e79cef0d26be572f71840971a21c5"},{"author":{"_account_id":35540,"name":"Samuel Kunkel","email":"samuel.kunkel@mail.schwarz","username":"samuel.kunkel"},"change_message_id":"3350bf0ac174abc09fc138e75809729670ef5493","unresolved":true,"context_lines":[{"line_number":16,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":17,"context_line":"from nova.tests.unit.virt.libvirt.test_utils import LibvirtUtilsTestCase"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class AMDSEVnotcheckUEFIwithoutIMGproperties(LibvirtUtilsTestCase):"},{"line_number":20,"context_line":"    def test_uefi_image_check_not_called_without_valid_image(self):"},{"line_number":21,"context_line":"        flavor \u003d objects.Flavor("},{"line_number":22,"context_line":"            id\u003d1, flavorid\u003d\u0027fakeid-1\u0027, name\u003d\u0027fake1.small\u0027, memory_mb\u003d128,"}],"source_content_type":"text/x-python","patch_set":3,"id":"36ebc0b8_5f312dac","line":19,"in_reply_to":"8fec47f9_66281ca4","updated":"2023-02-28 14:39:41.000000000","message":"Understood. Will have a look and adjust for the required way of testing.\n\nWill move the current test to a unittest then as it fits better here. I was just not sure what is part of a regressiontest as I had a look into the other regressiontests to find a mixed result here.\n\nThanks for clarification. Will work in the next weeks on this.","commit_id":"bbc38074129e79cef0d26be572f71840971a21c5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7200b1b3b79961d5808dd3c1077e835e47c67f69","unresolved":true,"context_lines":[{"line_number":24,"context_line":"            deleted\u003dFalse, extra_specs\u003d{"},{"line_number":25,"context_line":"                \u0027hw:mem_encryption\u0027: \u0027true\u0027"},{"line_number":26,"context_line":"            })"},{"line_number":27,"context_line":"        try:"},{"line_number":28,"context_line":"            libvirt_utils.get_flags_by_flavor_specs(flavor)"},{"line_number":29,"context_line":"        except NotImplementedError:"},{"line_number":30,"context_line":"            self.fail(\"uefi check triggered but no img metadata are present\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"faa3a0f0_28bdcdb5","line":29,"range":{"start_line":27,"start_character":1,"end_line":29,"end_character":35},"updated":"2023-02-28 13:53:53.000000000","message":"in a unit test we would use assertRaises to assert this.","commit_id":"bbc38074129e79cef0d26be572f71840971a21c5"}],"nova/virt/hardware.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1c63461b2e533e691532e998b1cd95474d50462f","unresolved":true,"context_lines":[{"line_number":1194,"context_line":"        requesters.append(\"hw_mem_encryption property of image %s\" %"},{"line_number":1195,"context_line":"                          image_meta.id)"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    if image_meta.__contains__(\"id\"):"},{"line_number":1198,"context_line":"        # during the iterations triggered within scheduler utils we dont"},{"line_number":1199,"context_line":"        # have any image_meta present. Therefore we would always fail here."},{"line_number":1200,"context_line":"        # Assuming a valid image_meta atleast needs to contain an id we skip"}],"source_content_type":"text/x-python","patch_set":1,"id":"e7f2017c_ff5433c7","line":1197,"updated":"2023-02-21 17:33:06.000000000","message":"no, you can\u0027t introspect the field like this.\n\nThe ImageMeta object has the id field which is not nullable \nhttps://github.com/openstack/nova/blob/439c67254859485011e7fd2859051464e570d78b/nova/objects/image_meta.py#L58\nSo, by default, you should be able to directly call image_meta.id\n\n\nIf something isn\u0027t correctly setting the object correctly, then we should rather fix that.","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7200b1b3b79961d5808dd3c1077e835e47c67f69","unresolved":true,"context_lines":[{"line_number":1194,"context_line":"        requesters.append(\"hw_mem_encryption property of image %s\" %"},{"line_number":1195,"context_line":"                          image_meta.id)"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    if image_meta.__contains__(\"id\"):"},{"line_number":1198,"context_line":"        # during the iterations triggered within scheduler utils we dont"},{"line_number":1199,"context_line":"        # have any image_meta present. Therefore we would always fail here."},{"line_number":1200,"context_line":"        # Assuming a valid image_meta atleast needs to contain an id we skip"}],"source_content_type":"text/x-python","patch_set":1,"id":"68c40d49_5a9ba4b0","line":1197,"in_reply_to":"b3c169cd_dd2e3510","updated":"2023-02-28 13:53:53.000000000","message":"this is likely an issue with lazy loading.\n\n\nfrom a python style point of view you should not call __contains__ directly\n\nthe pythonic way of doing image_meta.__contains__(\"id\")\n\nis \"\"id\" in image_meta\"\n\nbe we need to actully see why we do not have the image ref here and fix that.","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":35540,"name":"Samuel Kunkel","email":"samuel.kunkel@mail.schwarz","username":"samuel.kunkel"},"change_message_id":"5f51b5d8da6142c6533cd7675a7db8da080636d9","unresolved":true,"context_lines":[{"line_number":1194,"context_line":"        requesters.append(\"hw_mem_encryption property of image %s\" %"},{"line_number":1195,"context_line":"                          image_meta.id)"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"    if image_meta.__contains__(\"id\"):"},{"line_number":1198,"context_line":"        # during the iterations triggered within scheduler utils we dont"},{"line_number":1199,"context_line":"        # have any image_meta present. Therefore we would always fail here."},{"line_number":1200,"context_line":"        # Assuming a valid image_meta atleast needs to contain an id we skip"}],"source_content_type":"text/x-python","patch_set":1,"id":"b3c169cd_dd2e3510","line":1197,"in_reply_to":"e7f2017c_ff5433c7","updated":"2023-02-22 08:17:32.000000000","message":"Appreciate the input - but then something is very amiss here.\n\nLike I mentioned before, we run through this function three times. The second time we run through it from the scheduler_utils.from_request_spec.\n\nWorking runs 1 and 3:\n\u003e /usr/local/lib/python3.10/dist-packages/nova/virt/hardware.py(1198)get_mem_encryption_constraint()\n\u003e (Pdb) image_meta.id\n\u003e \u00276bda8bf8-86ff-4020-be42-b0322d8a59f4\u0027\n\nBut the problematic second run looks like this. \n\u003e /usr/local/lib/python3.10/dist-packages/nova/virt/hardware.py(1198)get_mem_encryption_constraint()\n\u003e (Pdb)  image_meta.id\n\u003e *** NotImplementedError: Cannot load \u0027id\u0027 in the base class\n\nand therefore\n\u003e /usr/local/lib/python3.10/dist-packages/nova/virt/hardware.py(1198)get_mem_encryption_constraint()\n\u003e (Pdb)  image_meta.__contains__(\"id\")\n\u003e False\n\nworks properly.\n\nThe image_meta at this stage is just am empty object of ImageMeta.\n\u003e /usr/local/lib/python3.10/dist-packages/nova/virt/hardware.py(1198)get_mem_encryption_constraint()\n\u003e (Pdb) image_meta\n\u003e ImageMeta(checksum\u003d\u003c?\u003e,container_format\u003d\u003c?\u003e,created_at\u003d\u003c?\u003e,direct_url\u003d         \u003c?\u003e,disk_format\u003d\u003c?\u003e,id\u003d\u003c?\u003e,min_disk\u003d\u003c?\u003e,min_ram\u003d\u003c?\u003e,name\u003d\u003c?\u003e,owner\u003d\u003c?\u003e,properties\u003dImageMetaProps,protected\u003d\u003c?\u003e,size\u003d\u003c?\u003e,status\u003d\u003c?\u003e,tags\u003d\u003c?\u003e,updated_at\u003d\u003c?\u003e,virtual_size\u003d\u003c?\u003e,visibility\u003d\u003c?\u003e)\n\nAlso this is expected as we only pass flavor as requests spec to from_request_spec and therefore we run into \n\n        if \u0027image\u0027 in request_spec and request_spec.image:\n            image \u003d request_spec.image\n        else:\n            image \u003d objects.ImageMeta(properties\u003dobjects.ImageMetaProps())\n\n\nTo adjust here in the ImageMeta Object this \"could\" be more of an effort. So if we want to go that route I need to dive way deeper into it. If you have some ideas - let me know and I have a look.","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1c63461b2e533e691532e998b1cd95474d50462f","unresolved":true,"context_lines":[{"line_number":1200,"context_line":"        # Assuming a valid image_meta atleast needs to contain an id we skip"},{"line_number":1201,"context_line":"        # here if no id is present in the provided image_meta"},{"line_number":1202,"context_line":"        _check_mem_encryption_uses_uefi_image(requesters, image_meta)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"    _check_mem_encryption_machine_type(image_meta, machine_type)"},{"line_number":1205,"context_line":""},{"line_number":1206,"context_line":"    LOG.debug(\"Memory encryption requested by %s\", \" and \".join(requesters))"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf2bea42_8367b7d1","line":1203,"updated":"2023-02-21 17:33:06.000000000","message":"also, like Sean, you need to demonstrate the regression with a functional test (and you also need a unittest)","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":35540,"name":"Samuel Kunkel","email":"samuel.kunkel@mail.schwarz","username":"samuel.kunkel"},"change_message_id":"7bc8a0d42e5a5df46556f54d65e87400981f2ac3","unresolved":true,"context_lines":[{"line_number":1200,"context_line":"        # Assuming a valid image_meta atleast needs to contain an id we skip"},{"line_number":1201,"context_line":"        # here if no id is present in the provided image_meta"},{"line_number":1202,"context_line":"        _check_mem_encryption_uses_uefi_image(requesters, image_meta)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"    _check_mem_encryption_machine_type(image_meta, machine_type)"},{"line_number":1205,"context_line":""},{"line_number":1206,"context_line":"    LOG.debug(\"Memory encryption requested by %s\", \" and \".join(requesters))"}],"source_content_type":"text/x-python","patch_set":1,"id":"411901a0_e5965294","line":1203,"in_reply_to":"9c90eee1_4b224be8","updated":"2023-02-22 15:47:34.000000000","message":"regression test added.","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"},{"author":{"_account_id":35540,"name":"Samuel Kunkel","email":"samuel.kunkel@mail.schwarz","username":"samuel.kunkel"},"change_message_id":"5f51b5d8da6142c6533cd7675a7db8da080636d9","unresolved":true,"context_lines":[{"line_number":1200,"context_line":"        # Assuming a valid image_meta atleast needs to contain an id we skip"},{"line_number":1201,"context_line":"        # here if no id is present in the provided image_meta"},{"line_number":1202,"context_line":"        _check_mem_encryption_uses_uefi_image(requesters, image_meta)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"    _check_mem_encryption_machine_type(image_meta, machine_type)"},{"line_number":1205,"context_line":""},{"line_number":1206,"context_line":"    LOG.debug(\"Memory encryption requested by %s\", \" and \".join(requesters))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9c90eee1_4b224be8","line":1203,"in_reply_to":"bf2bea42_8367b7d1","updated":"2023-02-22 08:17:32.000000000","message":"Understood. We are currently working on a regression test for that.","commit_id":"6931f26df52149c4a3470d1209d4ca967cc80efc"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2ac027c60d5bb32ba72c1b265ec5498e65af53e","unresolved":true,"context_lines":[{"line_number":5348,"context_line":""},{"line_number":5349,"context_line":"        # compare flavor trait and cpu models, select the first mathched model"},{"line_number":5350,"context_line":"        if flavor and mode \u003d\u003d \"custom\":"},{"line_number":5351,"context_line":"            flags \u003d libvirt_utils.get_flags_by_flavor_specs(flavor, image_meta)"},{"line_number":5352,"context_line":"            if flags:"},{"line_number":5353,"context_line":"                cpu.model \u003d self._match_cpu_model_by_flags(models, flags)"},{"line_number":5354,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"a0cd3434_c9324cd1","line":5351,"range":{"start_line":5351,"start_character":34,"end_line":5351,"end_character":59},"updated":"2023-11-13 12:46:53.000000000","message":"we would need to rename this if we took this approch","commit_id":"5242cb976fa16b485f8d64e1a2c13c42c366262d"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2ac027c60d5bb32ba72c1b265ec5498e65af53e","unresolved":true,"context_lines":[{"line_number":653,"context_line":"    return name"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"def get_flags_by_flavor_specs(flavor: \u0027objects.Flavor\u0027,"},{"line_number":657,"context_line":"                              image_meta: \u0027objects.ImageMeta\u0027) -\u003e ty.Set[str]:"},{"line_number":658,"context_line":"    req_spec \u003d objects.RequestSpec(flavor\u003dflavor, image\u003dimage_meta)"},{"line_number":659,"context_line":"    resource_request \u003d scheduler_utils.ResourceRequest.from_request_spec("}],"source_content_type":"text/x-python","patch_set":6,"id":"aee7b7fc_b759e822","line":656,"range":{"start_line":656,"start_character":4,"end_line":656,"end_character":29},"updated":"2023-11-13 12:46:53.000000000","message":"this wound need to be renamed","commit_id":"5242cb976fa16b485f8d64e1a2c13c42c366262d"}]}