)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3eacc07ad9a480103feeb8ed0f102f176d148030","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6e369606_1cbc2551","updated":"2024-08-07 10:17:33.000000000","message":"+1 also but what do ye think about the unused code?\nshoudl we remove the dead code? personally i see dead code as tech debt an a potentil security issues depending on the context. in this case its safe but unused fucntion tagged with the privsep decoraor for example shoudl alway be deleted in my view. some non privladged functions also fall in that catagory in my view.\n\nit just leads to more complicated tests and confustion when reviewing if we have branches that are never used.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a3d1fe9e7340a974e5ba21b4ffe3cba831893d5f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"521594e7_be58094d","updated":"2024-08-07 07:42:33.000000000","message":"Looks good to me. But I hold my +2 until you agree with Sean about the validate param.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8efc47106e7b70696e01a12fefade71ad2c05255","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cf509926_b46eccab","in_reply_to":"6e369606_1cbc2551","updated":"2024-08-07 16:10:45.000000000","message":"I think that\u0027s a good point from the perspective of dead code .. I hadn\u0027t thought of it that way initially. I\u0027ll respin.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b6862f9d47add99715c7869e3308d731d04b59cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0c6c201a_747e108b","updated":"2024-08-07 17:29:27.000000000","message":"if you respin again please add a small release note as im sure upstream users of vgpus on ubuntu 22.04 will likely be interested \nbut im happy with the code content as is assuming ci is green","commit_id":"f63029b461b81ad93e0681973ed9b5bfca405d5a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8aa05d5466ffaf0afe4a72bbbba3c362c7bf2c0c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"08d63799_f2188d26","updated":"2024-08-08 10:14:10.000000000","message":"recheck post failure in nova-live-migration-ceph,\n\nit looks like installing ceph on compute node 1 failed so all the migration tests failed as a result. we were unable to download cephadm\n\n2024-08-07 17:03:47.789 | ++ /opt/stack/devstack-plugin-ceph/devstack/lib/cephadm:get_cephadm:172 :   curl -f -O https://download.ceph.com/rpm-/el9/noarch/cephadm\n2024-08-07 17:03:47.806 |   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current\n2024-08-07 17:03:47.807 |                                  Dload  Upload   Total   Spent    Left  Speed\n2024-08-07 17:03:47.944 | \n  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0\n  0   146    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0\n2024-08-07 17:03:47.946 | curl: (22) The requested URL returned error: 404\n2024-08-07 17:03:47.952 | + /opt/stack/devstack-plugin-ceph/devstack/lib/cephadm:get_cephadm:1 :   exit_trap\n\nwe shoudl likely be caching that in the image or mirroring it to avoid that type of failure in the future","commit_id":"f63029b461b81ad93e0681973ed9b5bfca405d5a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0e5e3e4e1169ee6f09f629f2ca04b296a286efba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ead41eb1_fa63b9e3","in_reply_to":"0c6c201a_747e108b","updated":"2024-08-07 17:51:49.000000000","message":"I don\u0027t mind doing that but I did notice that persistent mdevs has not been released yet, so I\u0027m not sure how useful or clear an additional release note would be. If you think it would be useful for those deploying from trunk, then I\u0027ll add one.","commit_id":"f63029b461b81ad93e0681973ed9b5bfca405d5a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3ffc49f0cf21dbb994c24992c691963c93fa9bee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bf72ba6e_29bf12f0","in_reply_to":"ead41eb1_fa63b9e3","updated":"2024-08-08 10:09:41.000000000","message":"oh this just missed feature freeze last cycle\nsorry i forgot that. ya in that case we dont need one","commit_id":"f63029b461b81ad93e0681973ed9b5bfca405d5a"}],"nova/virt/libvirt/host.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ae07ac847c243bc225206651ff9291fc387ed77","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"        \"\"\""},{"line_number":1255,"context_line":"        return self.get_connection().nodeDeviceLookupByName(name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    def device_create(self, conf, validate\u003dFalse):"},{"line_number":1258,"context_line":"        \"\"\"Create a node device from specified device XML"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"        This creates the device as transient."}],"source_content_type":"text/x-python","patch_set":1,"id":"eb180ae6_b577b942","line":1257,"updated":"2024-08-06 23:30:16.000000000","message":"by the way i dont think validate is ever set to true.\n\nwhy is this here?\n\nI dont recall this being part of any planned feature to enable libvrt domain validation.\n\n\ndo we need to ever enablt the xml validation?\n\nwe do not use it here \n\nhttps://github.com/openstack/nova/blob/bb2d7f9cad577f3a32cb9523e2b1d9a6d6db3407/nova/virt/libvirt/driver.py#L8830","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3eacc07ad9a480103feeb8ed0f102f176d148030","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"        \"\"\""},{"line_number":1255,"context_line":"        return self.get_connection().nodeDeviceLookupByName(name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    def device_create(self, conf, validate\u003dFalse):"},{"line_number":1258,"context_line":"        \"\"\"Create a node device from specified device XML"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"        This creates the device as transient."}],"source_content_type":"text/x-python","patch_set":1,"id":"b1f92813_f89044ac","line":1257,"in_reply_to":"00c71440_b8ebb5e3","updated":"2024-08-07 10:17:33.000000000","message":"i would say we shoudl not have any dead code so without using this we shoudl remove it.\n\nif we want to use it when libvirt is new enough that ok but then we don\u0027t need the parameter and we can just enable it internally.\n\nif we wanted this to be generic i would allow flags to be passed in like the libvirt api does but again unless we are going to use flags i would not have them in the code right now and only add them when needed.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b6862f9d47add99715c7869e3308d731d04b59cd","unresolved":false,"context_lines":[{"line_number":1254,"context_line":"        \"\"\""},{"line_number":1255,"context_line":"        return self.get_connection().nodeDeviceLookupByName(name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    def device_create(self, conf, validate\u003dFalse):"},{"line_number":1258,"context_line":"        \"\"\"Create a node device from specified device XML"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"        This creates the device as transient."}],"source_content_type":"text/x-python","patch_set":1,"id":"01204757_9d20ec06","line":1257,"in_reply_to":"b1f92813_f89044ac","updated":"2024-08-07 17:29:27.000000000","message":"Done","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d562f1d0a474b29308c5eaa8d46f41154198b82","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"        \"\"\""},{"line_number":1255,"context_line":"        return self.get_connection().nodeDeviceLookupByName(name)"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    def device_create(self, conf, validate\u003dFalse):"},{"line_number":1258,"context_line":"        \"\"\"Create a node device from specified device XML"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"        This creates the device as transient."}],"source_content_type":"text/x-python","patch_set":1,"id":"00c71440_b8ebb5e3","line":1257,"in_reply_to":"eb180ae6_b577b942","updated":"2024-08-06 23:47:14.000000000","message":"No it\u0027s not currently called with True by anything. I think I just included it because validation is something that can be requested and wasn\u0027t sure what was our convention around providing access to libvirt APIs in here.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ae07ac847c243bc225206651ff9291fc387ed77","unresolved":true,"context_lines":[{"line_number":1273,"context_line":"        device_xml \u003d conf.to_xml()"},{"line_number":1274,"context_line":"        return self.get_connection().nodeDeviceCreateXML(device_xml, flags)"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"    def device_define(self, conf, validate\u003dFalse):"},{"line_number":1277,"context_line":"        \"\"\"Define a node device from specified device XML"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        This defines the device to make it persistent."}],"source_content_type":"text/x-python","patch_set":1,"id":"d6c3c177_5c5f11cb","line":1276,"updated":"2024-08-06 23:30:16.000000000","message":"or hear for define \n\nhttps://github.com/openstack/nova/blob/bb2d7f9cad577f3a32cb9523e2b1d9a6d6db3407/nova/virt/libvirt/driver.py#L8832\n\nwhich i think are the only usage of these furnction so unless we have a usecase that requries the validation option i think we should just drop it and never set this flag.\n\n\nif we were to enabel validation i would think it should be unconditional if libvirt suppoted it but is dont see an explictly reason to enabel just for these function so if we dont need it we proably shoudl remove it. which simplies the code as aw never need to to pass any flags then and 0 is fine.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7d562f1d0a474b29308c5eaa8d46f41154198b82","unresolved":true,"context_lines":[{"line_number":1273,"context_line":"        device_xml \u003d conf.to_xml()"},{"line_number":1274,"context_line":"        return self.get_connection().nodeDeviceCreateXML(device_xml, flags)"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"    def device_define(self, conf, validate\u003dFalse):"},{"line_number":1277,"context_line":"        \"\"\"Define a node device from specified device XML"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        This defines the device to make it persistent."}],"source_content_type":"text/x-python","patch_set":1,"id":"8881a71a_52fc2ec9","line":1276,"in_reply_to":"d6c3c177_5c5f11cb","updated":"2024-08-06 23:47:14.000000000","message":"That\u0027s fair enough. I can remove it.","commit_id":"b6dc67c8caae0fe69fd2a9875b5d66319b155446"}]}
