)]}'
{"nova/tests/unit/virt/libvirt/fake_libvirt_data.py":[{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"5872b1cdc1d8b27e4e229b653d314cc4857eae6e","unresolved":false,"context_lines":[{"line_number":21,"context_line":"  \u003carch\u003esparc\u003c/arch\u003e"},{"line_number":22,"context_line":"  \u003cvcpu max\u003d\u00271\u0027/\u003e"},{"line_number":23,"context_line":"  \u003cos supported\u003d\u0027yes\u0027\u003e"},{"line_number":24,"context_line":"    \u003cenum name\u003d\u0027firmware\u0027\u003e"},{"line_number":25,"context_line":"      \u003cvalue\u003eefi\u003c/value\u003e"},{"line_number":26,"context_line":"    \u003c/enum\u003e"},{"line_number":27,"context_line":"    \u003cloader supported\u003d\u0027yes\u0027\u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_eb3c5030","line":24,"updated":"2019-07-31 12:03:24.000000000","message":"Why adding this to SPARC rather than x86?","commit_id":"9dcbe716f559cfe8126333c4ff2ae4ca2bb675ec"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"4cbdfc05afe896acf2468a644199b1b3f2dd2a66","unresolved":false,"context_lines":[{"line_number":21,"context_line":"  \u003carch\u003esparc\u003c/arch\u003e"},{"line_number":22,"context_line":"  \u003cvcpu max\u003d\u00271\u0027/\u003e"},{"line_number":23,"context_line":"  \u003cos supported\u003d\u0027yes\u0027\u003e"},{"line_number":24,"context_line":"    \u003cenum name\u003d\u0027firmware\u0027\u003e"},{"line_number":25,"context_line":"      \u003cvalue\u003eefi\u003c/value\u003e"},{"line_number":26,"context_line":"    \u003c/enum\u003e"},{"line_number":27,"context_line":"    \u003cloader supported\u003d\u0027yes\u0027\u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_f32ab38e","line":24,"in_reply_to":"7faddb67_84734916","updated":"2019-08-21 15:38:32.000000000","message":"This was done in PS2.","commit_id":"9dcbe716f559cfe8126333c4ff2ae4ca2bb675ec"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e6010eabee056095bdbbeba5e4bd9c2b605532b0","unresolved":false,"context_lines":[{"line_number":21,"context_line":"  \u003carch\u003esparc\u003c/arch\u003e"},{"line_number":22,"context_line":"  \u003cvcpu max\u003d\u00271\u0027/\u003e"},{"line_number":23,"context_line":"  \u003cos supported\u003d\u0027yes\u0027\u003e"},{"line_number":24,"context_line":"    \u003cenum name\u003d\u0027firmware\u0027\u003e"},{"line_number":25,"context_line":"      \u003cvalue\u003eefi\u003c/value\u003e"},{"line_number":26,"context_line":"    \u003c/enum\u003e"},{"line_number":27,"context_line":"    \u003cloader supported\u003d\u0027yes\u0027\u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_84734916","line":24,"in_reply_to":"7faddb67_eb3c5030","updated":"2019-07-31 14:56:14.000000000","message":"Because I was jaded and blind :-)  Will fix.","commit_id":"9dcbe716f559cfe8126333c4ff2ae4ca2bb675ec"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f575105aac6ad8438bfda15b96ae4648aa2b8742","unresolved":false,"context_lines":[{"line_number":21,"context_line":"  \u003carch\u003esparc\u003c/arch\u003e"},{"line_number":22,"context_line":"  \u003cvcpu max\u003d\u00271\u0027/\u003e"},{"line_number":23,"context_line":"  \u003cos supported\u003d\u0027yes\u0027\u003e"},{"line_number":24,"context_line":"    \u003cenum name\u003d\u0027firmware\u0027\u003e"},{"line_number":25,"context_line":"      \u003cvalue\u003eefi\u003c/value\u003e"},{"line_number":26,"context_line":"    \u003c/enum\u003e"},{"line_number":27,"context_line":"    \u003cloader supported\u003d\u0027yes\u0027\u003e"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_e80a4e25","line":24,"in_reply_to":"7faddb67_f32ab38e","updated":"2019-08-28 09:30:47.000000000","message":"Yes.","commit_id":"9dcbe716f559cfe8126333c4ff2ae4ca2bb675ec"}],"nova/tests/unit/virt/libvirt/test_config.py":[{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"d9659dfdb1efa2a46ed85627ce56ae1583ba3b8c","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_307fc5e0","line":216,"updated":"2019-08-22 16:05:32.000000000","message":"This seems to be the result Kashyap intended based on his original code, but it looks weird to me.  Probably needs a rethink.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"396792b0382cfe33b034aa362f599792d298f51d","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_a1fb0cbc","line":216,"in_reply_to":"7faddb67_06db164f","updated":"2019-08-28 14:57:10.000000000","message":"Follow up to the above. We had a further discussion on #virt (not logged), OFTC network with the libvirt developer, Michal.  Summarizing here.  Adam, feel free to correct / append.\n\n* The fact that there are two or more possibilities listed in the output \n  of domainCapabilities doesn\u0027t mean that an attribute can be set to\n  both values at the same time.\n\n* The enum \"values\" from domainCapabilities show what the host \n  libvirt/QEMU is capable of.  E.g. if \u003cenum type\u003d\u0027secure\u0027\u003e only \n  contains \u003cvalue\u003eno\u003c/value\u003e, then Secure Boot cannot be supported with\n  the given libvirt/QEMU.\n\n* The values of the OS \"loader\" enums—\u0027type\u0027, \u0027secure\u0027, and \n  \u0027readonly\u0027—are mutually exclusive.  I.e. \u0027type\u0027 can either be \"rom\"\n  or \"pflash\", \u0027secure\u0027 can either be \"yes\", or \"no\", etc.\n\n* The confusion was caused by the sentence in the documentation of\n  domainCapabilities  \n  (https://libvirt.org/formatdomaincaps.html#elementsOSBIOS):  \"This \n  refers to type attribute of the \u003cloader/\u003e element\".  Where the\n  \u003cloader/\u003e element is referring to the libvirt *domain* defintion\n  (_not_ the domainCapabilities), here,\n  https://libvirt.org/formatdomain.html#elementsOSBIOS.\n\n* Adam suggested a rewrite of the confusing sentence in\n  https://libvirt.org/formatdomaincaps.html#elementsOSBIOS:\n\n    - \"This refers to type attribute of the \u003cloader/\u003e element\".\n    + \"Each \u003cvalue\u003e element under \u003cenum name\u003d\u0027type\u0027\u003e represents an \n    + allowed value for the \u0027type\u0027 attribute of the \u003cloader\u003e element in\n    + the domain definition. For example, the presence of\n    + \u003cvalue\u003erom\u003c/value\u003e means that a domain definition can contain\n    + \u003cloader type\u003d\"rom\" ...\u003e...\u003c/loader\u003e\"\n\n(Which I agree with.)","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"16f475a610170f14b2dc51f81add6fff4484948a","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_afb3e709","line":216,"in_reply_to":"7faddb67_0f9c7bbd","updated":"2019-08-29 11:07:37.000000000","message":"[...]\n\n \u003e All correct, although again here there is an ambiguity in that you\n \u003e are referring to \u003cloader\u003e in the domain, *not* the domain\n \u003e capabilities:\n \u003e \n \u003e \u003e * The values of the OS \"loader\" enums—\u0027type\u0027, \u0027secure\u0027, and\n \u003e \u0027readonly\u0027—are mutually exclusive.  I.e. \u0027type\u0027 can either be \"rom\"\n \u003e or \"pflash\", \u0027secure\u0027 can either be \"yes\", or \"no\", etc.\n \u003e \n \u003e The values are mutually exclusive in the domain, but *not* in the\n \u003e domain capabilities.  This reinforces the importance of carefully\n \u003e distinguishing which is being referring to, given how they are\n \u003e somewhat parallel in nature, on the surface at least.\n\nAgreed.  I indeed meant the \"mutually exclusive\" remark in the context of a guest config.  But as we know, in context of \"domain capabilities\", we need to know all the possible/supported values.\n\nHope we\u0027re on the same page now.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f575105aac6ad8438bfda15b96ae4648aa2b8742","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_c82672cc","line":216,"in_reply_to":"7faddb67_307fc5e0","updated":"2019-08-28 09:30:47.000000000","message":"Hmm, you mean the assertEqual is difficult to follow with these expressions?\n\n(Also, for clarity\u0027s sake, I\u0027ll put the both expressions from the assertEqual into variables.)","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"eb68b378b66f7722c6b7cddda4f2572195592de4","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_06db164f","line":216,"in_reply_to":"7faddb67_324bf45f","updated":"2019-08-28 13:56:54.000000000","message":"As discussed on IRC just now, the libvirt schema and documentation is very confusing here:\n\nhttps://libvirt.org/formatdomaincaps.html#elementsOSBIOS\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-08-28.log.html#t2019-08-28T13:39:24\n\nIn particular it is not clear whether each \u003cenum\u003e is supposed to be allowed more than one \u003cvalue\u003e.  The example XML given in the docs show that it is, but in that case the semantics make no sense, e.g. you can\u0027t have something which both is and isn\u0027t read-only at the same time.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"d9084d7976fd475dc7e85597e9e107cd3f185340","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_0f9c7bbd","line":216,"in_reply_to":"7faddb67_a1fb0cbc","updated":"2019-08-29 10:54:29.000000000","message":"\u003e Follow up to the above. We had a further discussion on #virt (not logged), OFTC network with the libvirt developer, Michal. Summarizing here.  Adam, feel free to correct / append.\n\n[snip]\n\nAll correct, although again here there is an ambiguity in that you are referring to \u003cloader\u003e in the domain, *not* the domain capabilities:\n\n \u003e * The values of the OS \"loader\" enums—\u0027type\u0027, \u0027secure\u0027, and \u0027readonly\u0027—are mutually exclusive.  I.e. \u0027type\u0027 can either be \"rom\" or \"pflash\", \u0027secure\u0027 can either be \"yes\", or \"no\", etc.\n\nThe values are mutually exclusive in the domain, but *not* in the domain capabilities.  This reinforces the importance of carefully distinguishing which is being referring to, given how they are somewhat parallel in nature, on the surface at least.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"3b9f5867c8922bec82e759a5d0b9318062e436f3","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                                  config.LibvirtConfigDomainCapsOSLoader)"},{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            self.assertEqual("},{"line_number":216,"context_line":"                {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027], \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]},"},{"line_number":217,"context_line":"                os.loader.features,"},{"line_number":218,"context_line":"                \"for arch %s\" % arch)"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_324bf45f","line":216,"in_reply_to":"7faddb67_c82672cc","updated":"2019-08-28 09:37:49.000000000","message":"No, I mean os.loader.features having keys called \u0027type\u0027 and \u0027readonly\u0027 makes no sense to me.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"4a720a6b37d6d6b37109a0aa6bd8d3e9c064b4df","unresolved":false,"context_lines":[{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            loader_enums \u003d {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027],"},{"line_number":216,"context_line":"                               \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]}"},{"line_number":217,"context_line":"            os_loader_enums \u003d os.loader.enums"},{"line_number":218,"context_line":"            self.assertEqual(loader_enums, os_loader_enums,"},{"line_number":219,"context_line":"                             \"for arch %s\" % arch)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_ef28ffee","line":217,"range":{"start_line":217,"start_character":12,"end_line":217,"end_character":27},"updated":"2019-08-29 11:19:19.000000000","message":"This temp variable is totally redundant AFAICS.","commit_id":"517619c344e590f99934dae7ec27b65cce1d67e2"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"22672eacd0eacc396b32ebcc721bd8c885e0a604","unresolved":false,"context_lines":[{"line_number":214,"context_line":"            self.assertTrue(os.loader.supported)"},{"line_number":215,"context_line":"            loader_enums \u003d {\u0027type\u0027: [\u0027rom\u0027, \u0027pflash\u0027],"},{"line_number":216,"context_line":"                               \u0027readonly\u0027: [\u0027yes\u0027, \u0027no\u0027]}"},{"line_number":217,"context_line":"            os_loader_enums \u003d os.loader.enums"},{"line_number":218,"context_line":"            self.assertEqual(loader_enums, os_loader_enums,"},{"line_number":219,"context_line":"                             \"for arch %s\" % arch)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_2f9057ed","line":217,"range":{"start_line":217,"start_character":12,"end_line":217,"end_character":27},"in_reply_to":"7faddb67_ef28ffee","updated":"2019-08-29 11:30:26.000000000","message":"Yes, it\u0027s useless.  Removed in next iteration.","commit_id":"517619c344e590f99934dae7ec27b65cce1d67e2"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"5872b1cdc1d8b27e4e229b653d314cc4857eae6e","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    @property"},{"line_number":163,"context_line":"    def os(self):"},{"line_number":164,"context_line":"        if self._os is None:"},{"line_number":165,"context_line":"            return []"},{"line_number":166,"context_line":"        return self._os"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_8b459cb6","line":165,"updated":"2019-07-31 12:03:24.000000000","message":"[] looks wrong here, isn\u0027t there only one \u003cos\u003e element max?","commit_id":"9dcbe716f559cfe8126333c4ff2ae4ca2bb675ec"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e6010eabee056095bdbbeba5e4bd9c2b605532b0","unresolved":false,"context_lines":[{"line_number":162,"context_line":"    @property"},{"line_number":163,"context_line":"    def os(self):"},{"line_number":164,"context_line":"        if self._os is None:"},{"line_number":165,"context_line":"            return []"},{"line_number":166,"context_line":"        return self._os"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_44631143","line":165,"in_reply_to":"7faddb67_8b459cb6","updated":"2019-07-31 14:56:14.000000000","message":"Indeed, there is just one \u003cos\u003e.  Will fix.","commit_id":"9dcbe716f559cfe8126333c4ff2ae4ca2bb675ec"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"14f17d45e7fdcc854ac63a224aaf6d36c3f5f1f0","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                loader.parse_dom(c)"},{"line_number":229,"context_line":"                self.loader \u003d loader"},{"line_number":230,"context_line":"            elif c.tag \u003d\u003d \"enum\":"},{"line_number":231,"context_line":"                self.firmware \u003d c.get(\u0027value\u0027)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"class LibvirtConfigDomainCapsOSLoader(LibvirtConfigObject):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_38eef8ec","line":231,"range":{"start_line":231,"start_character":16,"end_line":231,"end_character":46},"updated":"2019-08-05 17:03:13.000000000","message":"This needs fixing ... as we need to _also_ check that the enum value is \u0027firmware\u0027.  Otherwise, _any_ enum under \u003cos\u003e will be considered as \u0027firmware\u0027, which is currently true, but might not be in the future -- as libvirt could add further enums.  The same logic applies for the \u0027loader\u0027 section in the next class.","commit_id":"65153b2b818c597157055a456a82a5c98e88a9bb"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2b1be786c68810fb0879c973abfffafadc6b4118","unresolved":false,"context_lines":[{"line_number":222,"context_line":"    def parse_dom(self, xmldoc):"},{"line_number":223,"context_line":"        super(LibvirtConfigDomainCapsOS, self).parse_dom(xmldoc)"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        for c in xmldoc.getchildren():"},{"line_number":226,"context_line":"            if c.tag \u003d\u003d \"loader\":"},{"line_number":227,"context_line":"                loader \u003d LibvirtConfigDomainCapsOSLoader()"},{"line_number":228,"context_line":"                loader.parse_dom(c)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_e58e742c","line":225,"range":{"start_line":225,"start_character":24,"end_line":225,"end_character":35},"updated":"2019-08-05 19:32:05.000000000","message":"This will also be removed in the next version; as Element.getchildren() is deprecated.","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2b1be786c68810fb0879c973abfffafadc6b4118","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            # there\u0027s only one \u0027enum\u0027 under the \u0027os\u0027 attribute, in the"},{"line_number":233,"context_line":"            # future libvirt can add more of them."},{"line_number":234,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"firmware\":"},{"line_number":235,"context_line":"                self.firmwares \u003d [child.text for child in c.get(\"name\")"},{"line_number":236,"context_line":"                                    if child.tag \u003d\u003d \"value\"]"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_4542e83f","line":235,"range":{"start_line":235,"start_character":58,"end_line":235,"end_character":71},"updated":"2019-08-05 19:32:05.000000000","message":"This should be replaced with just \u0027c\u0027.  We want to loop over \u0027c\u0027, and not c.get()\u0027","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"aa2088d01e9300ab7b1a03bf4095a9802ccd4945","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        for c in xmldoc.getchildren():"},{"line_number":254,"context_line":"            if c.tag \u003d\u003d \"value\":"},{"line_number":255,"context_line":"                self.loader_value.append(c.text)"},{"line_number":256,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"type\":"},{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_a20e3289","line":256,"range":{"start_line":256,"start_character":37,"end_line":256,"end_character":53},"updated":"2019-08-05 18:18:14.000000000","message":"Small risk of KeyError from this; why not use c.get(\"name\") like you proposed earlier?","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2b1be786c68810fb0879c973abfffafadc6b4118","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        for c in xmldoc.getchildren():"},{"line_number":254,"context_line":"            if c.tag \u003d\u003d \"value\":"},{"line_number":255,"context_line":"                self.loader_value.append(c.text)"},{"line_number":256,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"type\":"},{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_c5b9f8cb","line":256,"range":{"start_line":256,"start_character":37,"end_line":256,"end_character":53},"in_reply_to":"7faddb67_a20e3289","updated":"2019-08-05 19:32:05.000000000","message":"Yep; bad self review; will replace.","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"aa2088d01e9300ab7b1a03bf4095a9802ccd4945","unresolved":false,"context_lines":[{"line_number":254,"context_line":"            if c.tag \u003d\u003d \"value\":"},{"line_number":255,"context_line":"                self.loader_value.append(c.text)"},{"line_number":256,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"type\":"},{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"},{"line_number":260,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.atrib[\"name\"] \u003d\u003d \"secure\":"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_42573e59","line":257,"range":{"start_line":257,"start_character":33,"end_line":257,"end_character":47},"updated":"2019-08-05 18:18:14.000000000","message":"Huh?  No, according to the commit message, \u0027type\u0027 is an attribute *value*:\n\n    \u003cenum name\u003d\u0027type\u0027\u003e\n\nso it can\u0027t be used to look anything up.  Additionally, this is only appending to self.enum once per occurrence of \u003cenum\u003e, but aren\u0027t you trying to get the contents of the value elements, e.g. \u0027rom\u0027 and \u0027pflash\u0027 in this snippet?\n\n          \u003cvalue\u003erom\u003c/value\u003e\n          \u003cvalue\u003epflash\u003c/value\u003e","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2b1be786c68810fb0879c973abfffafadc6b4118","unresolved":false,"context_lines":[{"line_number":254,"context_line":"            if c.tag \u003d\u003d \"value\":"},{"line_number":255,"context_line":"                self.loader_value.append(c.text)"},{"line_number":256,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"type\":"},{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"},{"line_number":260,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.atrib[\"name\"] \u003d\u003d \"secure\":"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_c59c98c1","line":257,"range":{"start_line":257,"start_character":33,"end_line":257,"end_character":47},"in_reply_to":"7faddb67_42573e59","updated":"2019-08-05 19:32:05.000000000","message":"Yes you\u0027re of course right, what I hastily added there is gibberish.  Attributes vs. child elements.\n\nI should\u0027ve put Workflow-1, thank you for jumping on this premature patch.\n\nFixing it in a different way...","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"aa2088d01e9300ab7b1a03bf4095a9802ccd4945","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"type\":"},{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"},{"line_number":260,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.atrib[\"name\"] \u003d\u003d \"secure\":"},{"line_number":261,"context_line":"                self.enum.append(c.get(\u0027secure\u0027))"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_024dc6c2","line":259,"range":{"start_line":259,"start_character":16,"end_line":259,"end_character":51},"updated":"2019-08-05 18:18:14.000000000","message":"Ditto here.","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"aa2088d01e9300ab7b1a03bf4095a9802ccd4945","unresolved":false,"context_lines":[{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"},{"line_number":260,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.atrib[\"name\"] \u003d\u003d \"secure\":"},{"line_number":261,"context_line":"                self.enum.append(c.get(\u0027secure\u0027))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_820bf67a","line":260,"range":{"start_line":260,"start_character":39,"end_line":260,"end_character":44},"updated":"2019-08-05 18:18:14.000000000","message":"Typo which strongly suggests this patchset is untested.","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2b1be786c68810fb0879c973abfffafadc6b4118","unresolved":false,"context_lines":[{"line_number":257,"context_line":"                self.enum.append(c.get(\u0027type\u0027))"},{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"},{"line_number":260,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.atrib[\"name\"] \u003d\u003d \"secure\":"},{"line_number":261,"context_line":"                self.enum.append(c.get(\u0027secure\u0027))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_e59f54b1","line":260,"range":{"start_line":260,"start_character":39,"end_line":260,"end_character":44},"in_reply_to":"7faddb67_820bf67a","updated":"2019-08-05 19:32:05.000000000","message":"Guilty (I just wanted to post what I have).  I _saw_ the typo locally; but forgot to fix it.","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"aa2088d01e9300ab7b1a03bf4095a9802ccd4945","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.attrib[\"name\"] \u003d\u003d \"readonly\":"},{"line_number":259,"context_line":"                self.enum.append(c.get(\u0027readonly\u0027))"},{"line_number":260,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.atrib[\"name\"] \u003d\u003d \"secure\":"},{"line_number":261,"context_line":"                self.enum.append(c.get(\u0027secure\u0027))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"class LibvirtConfigCapsNUMATopology(LibvirtConfigObject):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_c2c80e23","line":261,"range":{"start_line":261,"start_character":16,"end_line":261,"end_character":49},"updated":"2019-08-05 18:18:14.000000000","message":"And here.","commit_id":"30cf12856757a37208eb2f0fd29891e516424fa9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"b530c5530b823f762472ff0503ed6bc34f8f2c78","unresolved":false,"context_lines":[{"line_number":253,"context_line":"            self.supported \u003d True"},{"line_number":254,"context_line":"        for c in xmldoc:"},{"line_number":255,"context_line":"            if c.tag \u003d\u003d \"enum\":"},{"line_number":256,"context_line":"                self.features[c[\"name\"]] \u003d [child.text for child in c"},{"line_number":257,"context_line":"                                            if c.tag \u003d\u003d \"value\"]"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_c8deef0f","line":256,"range":{"start_line":256,"start_character":30,"end_line":256,"end_character":39},"updated":"2019-08-05 19:59:51.000000000","message":"Reviewer note: There\u0027s a \"theoretical risk\" for a KeyError with c[\"name\"], *if* (it\u0027s a _big_ \"if\") some future \u0027enum\u0027 gets added under the \"loader\" element here ...\n\n    \u003cloader supported\u003d\u0027yes\u0027\u003e\n      \u003cvalue\u003e/usr/share/edk2/ovmf/OVMF_CODE.fd\u003c/value\u003e\n      \u003cenum name\u003d\u0027type\u0027\u003e\n        \u003cvalue\u003erom\u003c/value\u003e\n        \u003cvalue\u003epflash\u003c/value\u003e\n      \u003c/enum\u003e\n      \u003cenum name\u003d\u0027readonly\u0027\u003e\n        \u003cvalue\u003eyes\u003c/value\u003e\n        \u003cvalue\u003eno\u003c/value\u003e\n      \u003c/enum\u003e\n      \u003cenum name\u003d\u0027secure\u0027\u003e\n        \u003cvalue\u003eno\u003c/value\u003e\n      \u003c/enum\u003e\n    \u003c/loader\u003e\n\n... *without* the \"name\u003d...\" attribute — which is _very_ unlikely and of course should not happen (if so, that\u0027s a libvirt bug).","commit_id":"06f5afc1d6057d9228b84ff6744f16369be86873"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"0837a1bf8a3e247f58f8e7b04dc90edf0a7c8ec8","unresolved":false,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        self.supported \u003d None"},{"line_number":245,"context_line":"        self.values \u003d []"},{"line_number":246,"context_line":"        self.features \u003d {}"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def parse_dom(self, xmldoc):"},{"line_number":249,"context_line":"        super(LibvirtConfigDomainCapsOSLoader, self).parse_dom(xmldoc)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_6f95ef87","line":246,"range":{"start_line":246,"start_character":13,"end_line":246,"end_character":21},"updated":"2019-08-29 10:57:05.000000000","message":"Based on our improved understanding, I think this should be called \u0027enums\u0027.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"16f475a610170f14b2dc51f81add6fff4484948a","unresolved":false,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        self.supported \u003d None"},{"line_number":245,"context_line":"        self.values \u003d []"},{"line_number":246,"context_line":"        self.features \u003d {}"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def parse_dom(self, xmldoc):"},{"line_number":249,"context_line":"        super(LibvirtConfigDomainCapsOSLoader, self).parse_dom(xmldoc)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_cf5ae3f0","line":246,"range":{"start_line":246,"start_character":13,"end_line":246,"end_character":21},"in_reply_to":"7faddb67_6f95ef87","updated":"2019-08-29 11:07:37.000000000","message":"Yes, agreed.  (Just for context, I used the term \"features\", as in \"loader\u0027s features\", given the context.)\n\nBut of course, happy to rename it to enums, as it\u0027s much less confusing.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":2394,"name":"Adam Spiers","email":"aspiers@suse.com","username":"adam.spiers"},"change_message_id":"0837a1bf8a3e247f58f8e7b04dc90edf0a7c8ec8","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        for child in xmldoc:"},{"line_number":254,"context_line":"            if child.tag \u003d\u003d \"enum\":"},{"line_number":255,"context_line":"                enum \u003d child"},{"line_number":256,"context_line":"                self.features[enum.get(\"name\")] \u003d ["},{"line_number":257,"context_line":"                    val.text for val in enum if val.tag \u003d\u003d \"value\"]"},{"line_number":258,"context_line":"            elif child.tag \u003d\u003d \"value\":"},{"line_number":259,"context_line":"                self.values.append(child.text)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_4f8a7364","line":256,"range":{"start_line":256,"start_character":21,"end_line":256,"end_character":29},"updated":"2019-08-29 10:57:05.000000000","message":"enums","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"16f475a610170f14b2dc51f81add6fff4484948a","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        for child in xmldoc:"},{"line_number":254,"context_line":"            if child.tag \u003d\u003d \"enum\":"},{"line_number":255,"context_line":"                enum \u003d child"},{"line_number":256,"context_line":"                self.features[enum.get(\"name\")] \u003d ["},{"line_number":257,"context_line":"                    val.text for val in enum if val.tag \u003d\u003d \"value\"]"},{"line_number":258,"context_line":"            elif child.tag \u003d\u003d \"value\":"},{"line_number":259,"context_line":"                self.values.append(child.text)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_6fe40f0f","line":256,"range":{"start_line":256,"start_character":21,"end_line":256,"end_character":29},"in_reply_to":"7faddb67_4f8a7364","updated":"2019-08-29 11:07:37.000000000","message":"ACK.","commit_id":"c96bfb09fe11fec1768dbcebbb988407cf31d077"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6ecfb8ecb9a75f1b67d09f09f5ab577bf11cc4c9","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            # NOTE(kchamart): Here we\u0027re making sure to look for the"},{"line_number":229,"context_line":"            # \"firmware\" enum -- although it\u0027s the only enum under the"},{"line_number":230,"context_line":"            # \u0027os\u0027 attribute, in the future, libvirt might add more of"},{"line_number":231,"context_line":"            # them."},{"line_number":232,"context_line":"            elif c.tag \u003d\u003d \"enum\" and c.get(\"name\") \u003d\u003d \"firmware\":"},{"line_number":233,"context_line":"                self.firmwares \u003d [child.text for child in c"},{"line_number":234,"context_line":"                                  if child.tag \u003d\u003d \"value\"]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_f388d722","line":231,"updated":"2019-08-30 10:29:40.000000000","message":"Yup. If/when they do, we\u0027ll want another parser class to handle this but it\u0027d be overkill rn","commit_id":"f6d48121b91c212f1b1023846c093bfbbf5ac217"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f1b068fdd1c66d6003024a0d8cbf7e3654b3afbe","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                                val.text for val in c2 if val.tag \u003d\u003d \u0027value\u0027"},{"line_number":340,"context_line":"                            ]"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"class LibvirtConfigCapsNUMATopology(LibvirtConfigObject):"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    def __init__(self, **kwargs):"}],"source_content_type":"text/x-python","patch_set":14,"id":"61cdd988_4b89e291","line":342,"updated":"2021-02-23 10:14:52.000000000","message":"Looks legit https://libvirt.org/formatdomaincaps.html#elementsOSBIOS","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"}],"nova/virt/libvirt/host.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f1b068fdd1c66d6003024a0d8cbf7e3654b3afbe","unresolved":true,"context_lines":[{"line_number":1231,"context_line":"        if self._supports_uefi is not None:"},{"line_number":1232,"context_line":"            return self._supports_uefi"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"}],"source_content_type":"text/x-python","patch_set":14,"id":"6b2dc44d_8ebbf4d0","line":1234,"range":{"start_line":1234,"start_character":52,"end_line":1234,"end_character":72},"updated":"2021-02-23 10:14:52.000000000","message":"nit: well, the nova libvirt driver, right? Think about other virt drivers.\nThat being said, just a nit again, no need for a FUP or whatever else.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31638e96f48d4e761df22b2db38ea92bb6dd2e9c","unresolved":false,"context_lines":[{"line_number":1231,"context_line":"        if self._supports_uefi is not None:"},{"line_number":1232,"context_line":"            return self._supports_uefi"},{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"}],"source_content_type":"text/x-python","patch_set":14,"id":"f88446f2_ea1fb6a9","line":1234,"range":{"start_line":1234,"start_character":52,"end_line":1234,"end_character":72},"in_reply_to":"6b2dc44d_8ebbf4d0","updated":"2021-02-23 11:44:39.000000000","message":"Fair point","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"00157b0a5e4d5ecf912d549b7d417aec105e753e","unresolved":true,"context_lines":[{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1238,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1239,"context_line":"            LOG.debug(\"Checking UEFI support for host arch (%s)\", arch)"},{"line_number":1240,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1241,"context_line":"            if _domain_caps.os.uefi_supported:"},{"line_number":1242,"context_line":"                LOG.info(\u0027UEFI support detected\u0027)"},{"line_number":1243,"context_line":"                self._supports_uefi \u003d True"},{"line_number":1244,"context_line":"                return True"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        LOG.debug(\u0027No UEFI support detected\u0027)"},{"line_number":1247,"context_line":"        self._supports_uefi \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"b5558cf4_2ea0c311","line":1244,"range":{"start_line":1236,"start_character":0,"end_line":1244,"end_character":27},"updated":"2021-03-01 17:50:20.000000000","message":"I\u0027m not sure about this, you\u0027re indicating support on a given arch by there being support by *any* of the machine types it provides. While I think this is a valid assumption on x86_64 I\u0027m not sure if it also applies to the other host arches we also support.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"052da0f1568dadd8d5a3800d087e042cc3f436a9","unresolved":true,"context_lines":[{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1238,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1239,"context_line":"            LOG.debug(\"Checking UEFI support for host arch (%s)\", arch)"},{"line_number":1240,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1241,"context_line":"            if _domain_caps.os.uefi_supported:"},{"line_number":1242,"context_line":"                LOG.info(\u0027UEFI support detected\u0027)"},{"line_number":1243,"context_line":"                self._supports_uefi \u003d True"},{"line_number":1244,"context_line":"                return True"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        LOG.debug(\u0027No UEFI support detected\u0027)"},{"line_number":1247,"context_line":"        self._supports_uefi \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"98887129_57db7726","line":1244,"range":{"start_line":1236,"start_character":0,"end_line":1244,"end_character":27},"in_reply_to":"2a7491c7_30cfa3dd","updated":"2021-03-01 22:40:30.000000000","message":"Yeah I was wondering about that and looked ahead in the series to see if that was something added in later. AFAICT it isn\u0027t.\n\nThe assumption is likely valid but either way a comment outlining the thinking here would help!","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"407f472e19e7b6d40e4a449c95602218d3ca420a","unresolved":true,"context_lines":[{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1238,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1239,"context_line":"            LOG.debug(\"Checking UEFI support for host arch (%s)\", arch)"},{"line_number":1240,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1241,"context_line":"            if _domain_caps.os.uefi_supported:"},{"line_number":1242,"context_line":"                LOG.info(\u0027UEFI support detected\u0027)"},{"line_number":1243,"context_line":"                self._supports_uefi \u003d True"},{"line_number":1244,"context_line":"                return True"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        LOG.debug(\u0027No UEFI support detected\u0027)"},{"line_number":1247,"context_line":"        self._supports_uefi \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"4c4294d0_a5a91b18","line":1244,"range":{"start_line":1236,"start_character":0,"end_line":1244,"end_character":27},"in_reply_to":"3105e1ff_92a16e3e","updated":"2021-03-02 13:26:13.000000000","message":"I\u0027m specifically concerned with the assumption that one machine type providing support is being used to indicate that _all_ machine types for a given arch provide support.\n\nThis might be a valid assumption across all the arches we support but either way needs to be documented here IMHO.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"adc957ba0afbd91672a8156e81b3d8f42c8a9ea7","unresolved":false,"context_lines":[{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1238,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1239,"context_line":"            LOG.debug(\"Checking UEFI support for host arch (%s)\", arch)"},{"line_number":1240,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1241,"context_line":"            if _domain_caps.os.uefi_supported:"},{"line_number":1242,"context_line":"                LOG.info(\u0027UEFI support detected\u0027)"},{"line_number":1243,"context_line":"                self._supports_uefi \u003d True"},{"line_number":1244,"context_line":"                return True"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        LOG.debug(\u0027No UEFI support detected\u0027)"},{"line_number":1247,"context_line":"        self._supports_uefi \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"695b5a22_c1e95b3a","line":1244,"range":{"start_line":1236,"start_character":0,"end_line":1244,"end_character":27},"in_reply_to":"4c4294d0_a5a91b18","updated":"2021-03-04 16:10:12.000000000","message":"Clarified by If527593ced072681130f97810fd839dfcda6a45d, this is just a generic test for *any* support by *any* machine_type for trait reporting. More specific checks will be carried out later once we know what the specific machine_type to be used is ahead of spawning the instance.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bc22a43e1858df002288adbf79fa1029553bb15b","unresolved":true,"context_lines":[{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1238,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1239,"context_line":"            LOG.debug(\"Checking UEFI support for host arch (%s)\", arch)"},{"line_number":1240,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1241,"context_line":"            if _domain_caps.os.uefi_supported:"},{"line_number":1242,"context_line":"                LOG.info(\u0027UEFI support detected\u0027)"},{"line_number":1243,"context_line":"                self._supports_uefi \u003d True"},{"line_number":1244,"context_line":"                return True"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        LOG.debug(\u0027No UEFI support detected\u0027)"},{"line_number":1247,"context_line":"        self._supports_uefi \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"3105e1ff_92a16e3e","line":1244,"range":{"start_line":1236,"start_character":0,"end_line":1244,"end_character":27},"in_reply_to":"98887129_57db7726","updated":"2021-03-02 11:56:42.000000000","message":"This is being used for the trait generation. An x86 host might have support for both x86 guests (via KVM) and AArch64 guests (via QEMU), but we might only have UEFI firmware for the former. As a result, we can\u0027t really have a generic check that doesn\u0027t take what you\u0027ve said into account  We have to be purposefully generic here since we don\u0027t have separate traits for different host architectures. What we\u0027re saying is \"can support UEFI _in some form_\", not \"we can definitely support UEFI\".\n\nIf you look at the SEV code below, you\u0027ll see this is doing almost the exact same thing as that and that it is generally working on the same assumptions. The only different is that the SEV checks also check non-host architectures which seems wasteful - SEV will only ever be supported on (AMD) x86 hosts, so I\u0027m not sure where the value in checking for support via e.g. the AArch64 emulator is.\n\nAll this being the case, what exactly are we missing here? A note stating that this only indicates general support and not support for a specific machine type?","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2541e917e2cdab260a714cd472cc6f957bfffdee","unresolved":true,"context_lines":[{"line_number":1233,"context_line":""},{"line_number":1234,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1235,"context_line":"        # non-host architectures currently"},{"line_number":1236,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1237,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1238,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1239,"context_line":"            LOG.debug(\"Checking UEFI support for host arch (%s)\", arch)"},{"line_number":1240,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1241,"context_line":"            if _domain_caps.os.uefi_supported:"},{"line_number":1242,"context_line":"                LOG.info(\u0027UEFI support detected\u0027)"},{"line_number":1243,"context_line":"                self._supports_uefi \u003d True"},{"line_number":1244,"context_line":"                return True"},{"line_number":1245,"context_line":""},{"line_number":1246,"context_line":"        LOG.debug(\u0027No UEFI support detected\u0027)"},{"line_number":1247,"context_line":"        self._supports_uefi \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"2a7491c7_30cfa3dd","line":1244,"range":{"start_line":1236,"start_character":0,"end_line":1244,"end_character":27},"in_reply_to":"b5558cf4_2ea0c311","updated":"2021-03-01 18:31:38.000000000","message":"if this is being used for capablity traits in placment then we would have to do it for any arch\n\nif this is being used for a specific instance boot then we should use that arch and machine type for that instance explicitly.\n\ni think lee is correct that this my vary based on machine type in some cases althoguh perhaps not.\nid does not curret to cehck explicitly since we can determin the machine type that will be used.\nwe might even want to supprot both based on if you pass the machine type depening on how this will be sued\n\ne.g. machine_type\u003dNone then any for this arch else check only the machine type provided","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"00157b0a5e4d5ecf912d549b7d417aec105e753e","unresolved":true,"context_lines":[{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1258,"context_line":"        # non-host architectures currently"},{"line_number":1259,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1260,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1261,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1262,"context_line":"            LOG.debug("},{"line_number":1263,"context_line":"                \"Checking secure boot support for host arch (%s)\","},{"line_number":1264,"context_line":"                arch,"},{"line_number":1265,"context_line":"            )"},{"line_number":1266,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1267,"context_line":"            if _domain_caps.os.secure_boot_supported:"},{"line_number":1268,"context_line":"                LOG.info(\u0027Secure Boot support detected\u0027)"},{"line_number":1269,"context_line":"                self._supports_secure_boot \u003d True"},{"line_number":1270,"context_line":"                return True"},{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        LOG.debug(\u0027No Secure Boot support detected\u0027)"},{"line_number":1273,"context_line":"        self._supports_secure_boot \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"6f7d198e_2a8e1f44","line":1270,"range":{"start_line":1259,"start_character":0,"end_line":1270,"end_character":27},"updated":"2021-03-01 17:50:20.000000000","message":"As above.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"adc957ba0afbd91672a8156e81b3d8f42c8a9ea7","unresolved":false,"context_lines":[{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1258,"context_line":"        # non-host architectures currently"},{"line_number":1259,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1260,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1261,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1262,"context_line":"            LOG.debug("},{"line_number":1263,"context_line":"                \"Checking secure boot support for host arch (%s)\","},{"line_number":1264,"context_line":"                arch,"},{"line_number":1265,"context_line":"            )"},{"line_number":1266,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1267,"context_line":"            if _domain_caps.os.secure_boot_supported:"},{"line_number":1268,"context_line":"                LOG.info(\u0027Secure Boot support detected\u0027)"},{"line_number":1269,"context_line":"                self._supports_secure_boot \u003d True"},{"line_number":1270,"context_line":"                return True"},{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        LOG.debug(\u0027No Secure Boot support detected\u0027)"},{"line_number":1273,"context_line":"        self._supports_secure_boot \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"a71e8ce9_dcd25073","line":1270,"range":{"start_line":1259,"start_character":0,"end_line":1270,"end_character":27},"in_reply_to":"3ff96936_42ba3623","updated":"2021-03-04 16:10:12.000000000","message":"As above, I\u0027m okay with this now.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2541e917e2cdab260a714cd472cc6f957bfffdee","unresolved":true,"context_lines":[{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"        # we only check the host architecture since nova doesn\u0027t support"},{"line_number":1258,"context_line":"        # non-host architectures currently"},{"line_number":1259,"context_line":"        arch \u003d self.get_capabilities().host.cpu.arch"},{"line_number":1260,"context_line":"        domain_caps \u003d self.get_domain_capabilities()"},{"line_number":1261,"context_line":"        for machine_type in domain_caps[arch]:"},{"line_number":1262,"context_line":"            LOG.debug("},{"line_number":1263,"context_line":"                \"Checking secure boot support for host arch (%s)\","},{"line_number":1264,"context_line":"                arch,"},{"line_number":1265,"context_line":"            )"},{"line_number":1266,"context_line":"            _domain_caps \u003d domain_caps[arch][machine_type]"},{"line_number":1267,"context_line":"            if _domain_caps.os.secure_boot_supported:"},{"line_number":1268,"context_line":"                LOG.info(\u0027Secure Boot support detected\u0027)"},{"line_number":1269,"context_line":"                self._supports_secure_boot \u003d True"},{"line_number":1270,"context_line":"                return True"},{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        LOG.debug(\u0027No Secure Boot support detected\u0027)"},{"line_number":1273,"context_line":"        self._supports_secure_boot \u003d False"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ff96936_42ba3623","line":1270,"range":{"start_line":1259,"start_character":0,"end_line":1270,"end_character":27},"in_reply_to":"6f7d198e_2a8e1f44","updated":"2021-03-01 18:31:38.000000000","message":"proably yes.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f1b068fdd1c66d6003024a0d8cbf7e3654b3afbe","unresolved":true,"context_lines":[{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        LOG.debug(\u0027No Secure Boot support detected\u0027)"},{"line_number":1273,"context_line":"        self._supports_secure_boot \u003d False"},{"line_number":1274,"context_line":"        return False"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"    def _kernel_supports_amd_sev(self):"},{"line_number":1277,"context_line":"        if not os.path.exists(SEV_KERNEL_PARAM_FILE):"}],"source_content_type":"text/x-python","patch_set":14,"id":"c1e50cdc_a57ad481","line":1274,"updated":"2021-02-23 10:14:52.000000000","message":"nit : you share the same logic here between secure boot and UEFI. This could have been some generic method for testing whether a domain architecture capability can be supported or not, but meh.","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31638e96f48d4e761df22b2db38ea92bb6dd2e9c","unresolved":false,"context_lines":[{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"        LOG.debug(\u0027No Secure Boot support detected\u0027)"},{"line_number":1273,"context_line":"        self._supports_secure_boot \u003d False"},{"line_number":1274,"context_line":"        return False"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"    def _kernel_supports_amd_sev(self):"},{"line_number":1277,"context_line":"        if not os.path.exists(SEV_KERNEL_PARAM_FILE):"}],"source_content_type":"text/x-python","patch_set":14,"id":"80b3dd26_8d7d9c32","line":1274,"in_reply_to":"c1e50cdc_a57ad481","updated":"2021-02-23 11:44:39.000000000","message":"I considered it but it basically saved me a line. If we hit upon the magic number of three users, I\u0027ll try again","commit_id":"4e23d93410be2f75e3cbeddc3269a7e205eb1935"}]}