)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"fcb9a5cfbe75871b66853057bedda5365f7e19e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6638ccef_f587e60e","updated":"2024-02-22 11:49:02.000000000","message":"recheck post failure in nova-next","commit_id":"564780c1747582bac220db8e67b678da2483da31"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"0b3f44749903ecbae949f1b74d971fa701532c8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"35dcb1b1_2a41c5e3","updated":"2024-06-11 02:10:15.000000000","message":"recheck","commit_id":"ef5184dc30ec0d1bf371a1aede5e42e8294eea3e"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"08d83d974f680d6afb3c336a0a41d90fe10226bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8e14a742_ffc36f42","updated":"2024-05-24 13:21:38.000000000","message":"recheck\n\nguest kernel panic in nova-ceph-multistore","commit_id":"ef5184dc30ec0d1bf371a1aede5e42e8294eea3e"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"b4546e72a0d550a0f646560445747ce4f39b5a3c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"cf042b7f_844681c3","updated":"2024-06-03 15:52:10.000000000","message":"recheck\n\nread timeout on `POST /servers` in unrelated test in nova-lvm job (slow env?)","commit_id":"ef5184dc30ec0d1bf371a1aede5e42e8294eea3e"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"48475c8298dc85c48d6d95599c7610ecbab74ea7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f97bcc42_1ededb0d","updated":"2024-06-28 03:23:49.000000000","message":"recheck ... because I saw it seem ci can work on Jun 27 according to - https://review.opendev.org/c/openstack/nova/+/920203","commit_id":"ef5184dc30ec0d1bf371a1aede5e42e8294eea3e"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"0b3e4214f06b0980f0823b20d6cefc0405030595","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5b4e00f2_4cb932f7","updated":"2024-08-21 03:52:33.000000000","message":"hi @pshchelokovskyy@mirantis.com , my deepcopy patch has been merged, could you pls rebase this patch to let this review to continue, thanks.","commit_id":"3746a9bfa9850030015878604bab9059f7642bf5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5ab94491f45d45dd08aab70d0ebeaf962f156937","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a3f053a0_aeee65ed","updated":"2024-08-21 16:55:17.000000000","message":"+1 because this need manual testing.\n\nwe do not have enough ci coverage to verify this currently.","commit_id":"4ca2e75db9eb4d080390379c994d52ffa95c83fb"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"fc2a59cd064050d97ca1e98bd5000965b5cdab8d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"220f90d1_e19c126e","updated":"2024-10-17 09:02:02.000000000","message":"Hi sean, sorry for the late reply due to a long holiday a while ago. I did some tests using jammy-bobcat, but hit the error \u0027Image not in a supported format\u0027, pls see the paste[0] for more details.\n\nI suspected taht it might be due to jammy-bobcat not including this cve fix[1], so then I set up a devstack env with the master branch to do some more tests, but the results were the same. Even without applying the iso patch[3], both tests with disk-format\u003draw and disk-format\u003diso resulted in the error \u0027Image content does not match\u0027 disk_format, pls see the paste[2] for more details.\n\n[0] https://paste.ubuntu.com/p/dvpgpb6mGn/\n[1] https://review.opendev.org/c/openstack/nova/+/923729/1/nova/virt/images.py\n[2] https://paste.ubuntu.com/p/VKtM8Nb2qT/\n[3] https://review.opendev.org/c/openstack/nova/+/909611","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"ab139220f3274aa08c7b7ab3b82996224e221f38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8847b62c_7cff1978","updated":"2024-09-19 05:29:34.000000000","message":"Hi, I wanted to check in and see if there\u0027s any progress on the review for this change. We understand that everyone is busy, but this change is quite important for us and would greatly benefit from your feedback. Could we kindly ask for a review when possible? Thank you!","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"078095f4eaa7e8b22b9527a07ee005ae2faaaf44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"ecd8f5f9_51819d1c","updated":"2025-01-23 07:05:39.000000000","message":"I have tested this patch with iso_gpt patch (https://review.opendev.org/c/openstack/nova/+/931833), it works well. For more details, pls refer to this paste - https://paste.openstack.org/show/b8TUfJVzOhHY5eaowOYk/","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"66ded7d44e80a199cfa47eae2008a9edfd147be2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"2a0be72c_9aea1a6e","updated":"2024-10-05 17:02:21.000000000","message":"Sean,","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0dd2229712e5a8a10e93851bd607138c084227b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"38275c27_aed35530","updated":"2024-10-08 15:43:54.000000000","message":"for other reasons i downloads a windows server 2019 iso last week ill see if i can deploy a devtack and do some testing with this tomorrow.\n\nwhat i think we  can do is try and take this in stages.\n\nwhat im hoping we do not need to do is inspcect the content of the iso and determin if it supprot emulation vs https://wiki.osdev.org/El-Torito vs https://wiki.osdev.org/Bootable_El-Torito_CD_with_GRUB_Legacy\n\nto determin if the image shoudl be attached as a block device or a optical disk.\n\n\nif we cant come up with a reasonabel solution to make this work automatically we may need to expose this to the end user.\nwe may want to consier adding a image property to allow you to request that in the future. we can do that in the block device mappigs i belive today but i dont think we can do that with image metadtaa.\n\n\nthe multiple formats issue we will likely fix sepreatly.\n\ndan has started to provide the oslo functionatliy to enable that.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a954cc2aaaf0121dcb37235937eb169736457391","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e7c0be20_d17b8927","updated":"2024-09-24 11:46:49.000000000","message":"im on pto so this is not a proper review. i think we can proceed with this but we need to manually thest this with isos that current can boot and currently cant\n\ni.e. ones that can only boot when using cdrom mode and  isos that work as block devices to pervent regresions.\n\nwe also need to be careful with live migration\n\nit would be good to do some manual testing of this and report back because our current ci and the testing in this patch does not validate this and cant in the case of the diffent isos.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f665399671e0012e24eaa9cbcfb9437d4382b2b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b7763853_85da5f8c","updated":"2024-10-05 17:43:33.000000000","message":"so this has regress futher since we swap to using the image inspector form oslo\n\nin the specail caseof a image that matches both iso format and gpt we can aloow that as safe.\n\nthat check is there to prevent embedding qcow or vmkds in the system area.\n\ndan, melaine what to do you think about catching and allow iso to be both \"iso and gpt\" ?","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4ff1e37bddf52db8f85135a04f3650855ebe2502","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b83c60ad_693352b9","in_reply_to":"1d1cd207_c124e2dc","updated":"2024-10-07 14:59:24.000000000","message":"Okay, I\u0027ll have to look at an ISOLINUX .. maybe it does the same thing as VFAT and looks a lot like an MBR. But like vfat, perhaps we can exclude it and/or treat it separately. I\u0027ll look at how I might expose the two hits so nova can decide.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c035c9c1f3a7a4bc36dabda4c7a30b7e0429c0ef","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1d1cd207_c124e2dc","in_reply_to":"71ae1c9f_5033202a","updated":"2024-10-07 14:39:21.000000000","message":"from Pavlo\u0027s testing ubuntu and alpine iso images\n\nas far as i am aware the only isos that work when we were attaching iso as block devices worked because of the gpt partion table and isolinux\n\nso i think this would be a pretty common requirement. we may need to check windows iso to see what they use today to see if they woudl also be impacted.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":36909,"name":"Carlos Bravo","display_name":"Carlos Bravo","email":"cbravo@itcservicios.com","username":"cbravo"},"change_message_id":"57255b9152de82751bfc74df8e6384cba6b857c1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"7b003e46_2401f143","in_reply_to":"b27f8f78_223f1cd4","updated":"2024-10-08 15:21:22.000000000","message":"Yes, the error is a screen showing \u0027no bootable device\u0027 for any windows based ISOs. The instance does get created, but the ISO is created as \u0027Disk\u0027 in the libvirt xml. When the instance boots, it hangs at \u0027no bootable device\u0027.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d6969f9050347652d195b7dc795fc00f8d0c25d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"71ae1c9f_5033202a","in_reply_to":"b7763853_85da5f8c","updated":"2024-10-07 13:39:14.000000000","message":"What real-world image needs an MBR in the system area of an ISO?\n\nI\u0027m hesitant to encode \"gpt in iso is okay, qcow in iso is not\" into oslo for everyone. That feels like nova-specific behavior that shouldn\u0027t be applied globally. If we need to be able to detect and allow that, I think we probably need a better way to communicate \"here are the two formats I detected\" and let nova decide what to do.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"d0daf7f81d9f1791e3544adffd2773718e8db085","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b27f8f78_223f1cd4","in_reply_to":"b83c60ad_693352b9","updated":"2024-10-08 10:52:03.000000000","message":"according to a comment in the LP issue\nhttps://bugs.launchpad.net/nova/+bug/2054446/comments/8\nWindows does not work too,\nnot super clear from the comment, but I read it as the problem is \u0027no bootable device\u0027, not image introspection.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"66ded7d44e80a199cfa47eae2008a9edfd147be2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"0f52247d_20f8c520","in_reply_to":"e7c0be20_d17b8927","updated":"2024-10-05 17:02:21.000000000","message":"I\u0027ve done some manual testing and so far the situation is the following:\n\ncurrent master\n- latest ubuntu and alpine ISOs fail to create because of image format introspection:\nImage ... is unacceptable: Image content does not match disk_format\nwhich in turn is caused by\noslo_utils.imageutils.format_inspector.ImageFormatError: Multiple formats detected: gpt,iso\n\n.. which is bad on its own right, it seems we can not boot those \u0027isos that work as block devices\u0027 anyway due to multiple formats detected\n\n- tinycore 15 (latest), tinycore 14 - fail to boot because of no boot device (these are pure isos it seems)\n\nWith this patch:\n- latest ubuntu and alpine fail the same\n- tinycore 15 and 14 boot ok\n\nI\u0027ve tried both oslo.utils 7.3.0 (latest release) and master, no difference in this regard. I\u0027ve also tried uploading iso image as `--disk-format raw` instead of `iso` for those multiple-formatted images, again, no difference.","commit_id":"a4632d0b7277ce6165559eac65272a9a74a469a2"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1b340aa2f1b046b3fdd7a7cae7c259bd01e923bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"c80e9163_790611ca","updated":"2025-02-25 23:13:29.000000000","message":"Looks OK to me","commit_id":"96a5c21f24f6a89411d12677ae5fa75adf26ed61"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"332747d84d816e916c87d53b93b882b6b340d57b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"87f115fd_93b6a609","updated":"2025-02-17 08:51:37.000000000","message":"The following CI error is not caused by the patch,\n\nDetails: {\u0027code\u0027: 400, \u0027message\u0027: \u0027No valid host was found. Unable to move instance 1c2adfd9-6457-439d-94e2-41e3dc9e76df to host np0039653699. There is not enough capacity on the host for the instance.\u0027}","commit_id":"96a5c21f24f6a89411d12677ae5fa75adf26ed61"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"873287725b104f2b562589f79f3e41fe9452755d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e279a80b_2ae37402","updated":"2025-01-23 10:49:28.000000000","message":"rebasing to include the iso+gpt handling patch","commit_id":"96a5c21f24f6a89411d12677ae5fa75adf26ed61"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"da34cee20eea2548be0702cdcc2ce94b2c9cae87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"de433a85_d9716deb","updated":"2025-02-17 08:52:17.000000000","message":"recheck","commit_id":"96a5c21f24f6a89411d12677ae5fa75adf26ed61"}],"nova/virt/libvirt/blockinfo.py":[{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"ef9efa7593e4f52aa9df91b9d85179ac876e916a","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"48af164b_b44cd877","line":449,"updated":"2024-05-19 02:48:35.000000000","message":"Hi Pavlo, have you tested this patch? I tested it, but I hit this error(AttributeError: \u0027BlockDeviceMapping\u0027 object has no attribute \u0027copy\u0027):\n\n$ nova show 60082fc2-7ac5-49c1-b401-f6ff8827e5bc |grep -i AttributeError\n| AttributeError: \u0027BlockDeviceMapping\u0027 object has no attribute \u0027copy\u0027\n\nIt seems BlockDeviceMapping oject has no attribute \u0027copy\u0027, maybe copy.deepcopy can instead of it.","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"14bce31749d4d34a054114195985699ac7b5c444","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"22969c79_86b39497","line":449,"in_reply_to":"046eedac_f8f6bb34","updated":"2024-05-23 03:14:35.000000000","message":"and I added the following code in the file /opt/stack/nova/nova/virt/libvirt/blockinfo.py to print the log, the log shows root_bdm is an instance of BlockDeviceMapping, not an instance of DriverBlockDevice\n\nvim /opt/stack/nova/nova/virt/libvirt/blockinfo.py\n425 def get_root_info(instance, virt_type, image_meta, root_bdm,                                                                                                                       \n426                   disk_bus, cdrom_bus, root_device_name\u003dNone):                                                                                                                     \n427                                                                                                                                                                                    \n428     from oslo_log import log as logging                                                                                                                                            \n429     LOG \u003d logging.getLogger(__name__)                                                                                                                                              \n430     LOG.error(\"by joshua - Type of root_bdm: %s\", type(root_bdm)) \n\nMay 23 11:07:55 minipc nova-compute[514589]: #033[01;31mERROR nova.virt.libvirt.blockinfo [#033[01;36mNone req-96d492b4-4fb7-4296-b963-9febb9499e59 #033[00;36madmin admin#033[01;31m] #033[01;35m#033[01;31mby joshua - Type of root_bdm: \u003cclass \u0027nova.objects.block_device.BlockDeviceMapping\u0027\u003e#033[00m#033[00m\nMay 23 11:07:55 minipc nova-compute: #033[01;31mERROR nova.virt.libvirt.blockinfo [#033[01;36mNone req-96d492b4-4fb7-4296-b963-9febb9499e59 #033[00;36madmin admin#033[01;31m] #033[01;35m#033[01;31mby joshua - Type of root_bdm: \u003cclass \u0027nova.objects.block_device.BlockDeviceMapping\u0027\u003e#033[00m","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"60b59cd82da13715e862f60763991179a14874ba","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"f8ea2063_440eac0c","line":449,"in_reply_to":"22969c79_86b39497","updated":"2024-05-23 17:39:54.000000000","message":"Thanks Zhang Hua, that helps a lot. I just couldn\u0027t guess where/how a objects.BlockDeviceMapping was getting to the method, this makes it clear.\n\nIt looks like (for better or for worse) this method is actually intended to work with both objects.BlockDeviceMapping and DriverBlockDevice. We have some other methods like this and of course they are error prone. These are one of the places I think type hints would have helped a lot to avoid the bug you uncovered.\n\nI think you are right that copy.deepcopy() would make this work with either type because oslo.versionedobjects happens to implement ``__deepcopy__`` [1]. Not sure that\u0027s the best way to deal with this, but it\u0027s one of the ways. We could also check isinstance() and decide what to do based on that. Either way, I think we need code comment(s) about the multiple types and type hints on this method to help avoid future problems.\n\nAs for L454, I\u0027m guessing we have just been lucky that the particular conditional there just happens to never get a objects.BlockDeviceMapping passed in it. That needs to be fixed too.\n\nThe copy() and typing problem is unrelated to this patch so I\u0027m not sure if maybe we should create a separate fix patch for L454 and then rebase this patch on top of it.\n\n[1] https://github.com/openstack/oslo.versionedobjects/blob/6af8327f31fa5d2bbf8838255f1bdacf8c6104f3/oslo_versionedobjects/base.py#L415","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"59e37dc72df973b6e719a725251ffb84759bd89c","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"e98356f5_366ad1f2","line":449,"in_reply_to":"47b9f563_2de0be10","updated":"2024-05-24 11:22:14.000000000","message":"this was definitely working before. but now indeed I see that during successfull boot this method is called (at least) 4 times, only the first time root_bdm is `nova.objects.block_device.BlockDeviceMapping`, in the rest of the calls it is `nova.virt.block_device.DriverImageBlockDevice`.\n\nI will rebase on Zhang\u0027s patch and use the deepcopy too.","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"01f30d5aec556d43f6c5df7e7a48db149ae69944","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a1bfa1c0_c2e11614","line":449,"in_reply_to":"48af164b_b44cd877","updated":"2024-05-21 20:45:41.000000000","message":"interesting, must be some recent change.. and that means the code below (L454) is also broken","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9c58e93380768e71b94b5aeed80dfb74f5d80fd7","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c325f9c7_58e683db","line":449,"in_reply_to":"a1bfa1c0_c2e11614","updated":"2024-05-22 03:21:03.000000000","message":"Hm ... ``root_bdm`` should be an instance of ``DriverBlockDevice``, which is a subclass of ``dict``. So calling ``copy()`` should be valid.\n\nAnd the pasted output is saying this came from a ``nova show``? I\u0027m confused as to how any libvirt related code would be called to do a ``nova show``.\n\n@Zhang Hua: What version of Nova did you test this with? Is there a traceback in any Nova log that you could create a paste so we can see what path is reaching this code?","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"c8dcd54e0efc96f190220a6a4d079173f6f6ace3","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"2aff1ef3_828e1138","line":449,"in_reply_to":"a1bfa1c0_c2e11614","updated":"2024-05-22 03:13:33.000000000","message":"Yeah, I also noticed that L454 is indeed using BlockDeviceMapping.copy(root_bdm.copy), but I did encounter that error during my actual testing. So can you also do a pratical test to see if our test results are the same. thanks","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"f0f81b872fcaac324220047673ebd6b6772c30ed","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"046eedac_f8f6bb34","line":449,"in_reply_to":"b252dd75_76b021fa","updated":"2024-05-23 02:53:45.000000000","message":"I used my existing devstack env for testing, which is the version stable/2023.1. you can find both the steps and the traceback in this pastebin link - https://paste.openstack.org/show/bz9Nsu3a78zVvrhmWQQd/","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6243b23fcab69ab699b5eda1c2f1db35b2f367fa","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"b252dd75_76b021fa","line":449,"in_reply_to":"c325f9c7_58e683db","updated":"2024-05-22 03:22:40.000000000","message":"Or can you tell what steps you did to reproduce this so we can try it and investigate?","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"0183ab28e1851a8b3b120ce493e0e331ad110138","unresolved":false,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"6fd5f47d_09f860b7","line":449,"in_reply_to":"e98356f5_366ad1f2","updated":"2024-05-24 11:22:53.000000000","message":"Done","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":2711,"name":"Zhang Hua","display_name":"Zhang Hua","email":"joshua.zhang@canonical.com","username":"zhhuabj"},"change_message_id":"fd3b4176117041ccd4b08c95c4ab9b357bd79405","unresolved":true,"context_lines":[{"line_number":446,"context_line":""},{"line_number":447,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":448,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":449,"context_line":"        root_bdm \u003d root_bdm.copy()"},{"line_number":450,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"},{"line_number":451,"context_line":"        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"47b9f563_2de0be10","line":449,"in_reply_to":"f8ea2063_440eac0c","updated":"2024-05-24 08:06:59.000000000","message":"Hi melwitt, I created a separate fix patch for L454[1] according to your comment so that this patch can continue after fixing deepcopy staff. thanks.\n\n[1] https://review.opendev.org/c/openstack/nova/+/920374","commit_id":"8b915e058122448698a0d4747ac93dd0581dd4cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e550298dc95c0f0337a09bdb2213a166a94e496e","unresolved":true,"context_lines":[{"line_number":446,"context_line":"                \u0027dev\u0027: block_device.strip_dev(root_device_name),"},{"line_number":447,"context_line":"                \u0027boot_index\u0027: \u00271\u0027}"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":450,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":451,"context_line":"        root_bdm \u003d copy.deepcopy(root_bdm)"},{"line_number":452,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f48771d_b6add5be","line":449,"range":{"start_line":449,"start_character":8,"end_line":449,"end_character":49},"updated":"2024-08-21 12:20:32.000000000","message":"so this can be simplifed to \n\n```\n\u0027disk_format\u0027 in image_meta\n```\n\nyou do not need to change this as you are just keeping this the same as the logic above.\n\nbut i feel like we should try  to deduplicate this \n\n\n\nyour current patch has \n```\ndef get_root_info(instance, virt_type, image_meta, root_bdm,\n                  disk_bus, cdrom_bus, root_device_name\u003dNone):\n    if root_bdm is None:\n        # NOTE(mriedem): In case the image_meta object was constructed from\n        # an empty dict, like in the case of evacuate, we have to first check\n        # if disk_format is set on the ImageMeta object.\n        if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and\n                image_meta.disk_format \u003d\u003d \u0027iso\u0027):\n            root_device_bus \u003d cdrom_bus\n            root_device_type \u003d \u0027cdrom\u0027\n        else:\n            root_device_bus \u003d disk_bus\n            root_device_type \u003d \u0027disk\u0027\n        if not root_device_name:\n            root_device_name \u003d find_disk_dev_for_disk_bus({}, root_device_bus)\n        return {\u0027bus\u0027: root_device_bus,\n                \u0027type\u0027: root_device_type,\n                \u0027dev\u0027: block_device.strip_dev(root_device_name),\n                \u0027boot_index\u0027: \u00271\u0027}\n    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and\n            image_meta.disk_format \u003d\u003d \u0027iso\u0027):\n        root_bdm \u003d copy.deepcopy(root_bdm)\n        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus\n        root_bdm[\u0027device_type\u0027] \u003d \u0027cdrom\u0027\n    if not get_device_name(root_bdm) and root_device_name:\n        root_bdm \u003d copy.deepcopy(root_bdm)\n        root_bdm[\u0027device_name\u0027] \u003d root_device_name\n    return get_info_from_bdm(\n        instance, virt_type, image_meta, root_bdm, {}, disk_bus,\n    )\n  \n ```\n but i feel like this would be better.\n \n ```\n def get_root_info(instance, virt_type, image_meta, root_bdm,\n                  disk_bus, cdrom_bus, root_device_name\u003dNone):\n                  \n    root_device_bus \u003d disk_bus\n    root_device_type \u003d \u0027disk\u0027\n    # NOTE(mriedem): In case the image_meta object was constructed from\n    # an empty dict, like in the case of evacuate, we have to first check\n    # if disk_format is set on the ImageMeta object.\n    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and\n        image_meta.disk_format \u003d\u003d \u0027iso\u0027):\n        root_device_bus \u003d cdrom_bus\n        root_device_type \u003d \u0027cdrom\u0027\n    if not root_device_name:\n       root_device_name \u003d find_disk_dev_for_disk_bus({}, root_device_bus)\n                 \n    if root_bdm is None:\n        return {\u0027bus\u0027: root_device_bus,\n                \u0027type\u0027: root_device_type,\n                \u0027dev\u0027: block_device.strip_dev(root_device_name),\n                \u0027boot_index\u0027: \u00271\u0027}\n     \n    root_bdm \u003d copy.deepcopy(root_bdm)\n    root_bdm[\u0027disk_bus\u0027] \u003d root_device_bus\n    root_bdm[\u0027device_type\u0027] \u003d root_device_type\n    if not get_device_name(root_bdm) and root_device_name:\n        root_bdm[\u0027device_name\u0027] \u003d root_device_name\n    return get_info_from_bdm(\n        instance, virt_type, image_meta, root_bdm, {}, root_device_bus,\n    )\n```\n\nim not sure if that will break any exisitng usages but that feels more correct to me.","commit_id":"3746a9bfa9850030015878604bab9059f7642bf5"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"bb52cdd2b5ba0d8a0846d2183fc019e53188400c","unresolved":true,"context_lines":[{"line_number":446,"context_line":"                \u0027dev\u0027: block_device.strip_dev(root_device_name),"},{"line_number":447,"context_line":"                \u0027boot_index\u0027: \u00271\u0027}"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":450,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":451,"context_line":"        root_bdm \u003d copy.deepcopy(root_bdm)"},{"line_number":452,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"}],"source_content_type":"text/x-python","patch_set":6,"id":"64eb694c_028af5ed","line":449,"range":{"start_line":449,"start_character":8,"end_line":449,"end_character":49},"in_reply_to":"3f48771d_b6add5be","updated":"2024-08-21 16:37:09.000000000","message":"yeah, this kind of potentially changes root_bdm where it wasn\u0027t changed before, so I\u0027m hesitant a bit for sure... I\u0027ll try to extract the \u0027iso check\u0027 to deduplicate a bit, but still would like to change the root bdm only in the same cases it is changed currently","commit_id":"3746a9bfa9850030015878604bab9059f7642bf5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5ab94491f45d45dd08aab70d0ebeaf962f156937","unresolved":true,"context_lines":[{"line_number":446,"context_line":"                \u0027dev\u0027: block_device.strip_dev(root_device_name),"},{"line_number":447,"context_line":"                \u0027boot_index\u0027: \u00271\u0027}"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    if (image_meta.obj_attr_is_set(\u0027disk_format\u0027) and"},{"line_number":450,"context_line":"            image_meta.disk_format \u003d\u003d \u0027iso\u0027):"},{"line_number":451,"context_line":"        root_bdm \u003d copy.deepcopy(root_bdm)"},{"line_number":452,"context_line":"        root_bdm[\u0027disk_bus\u0027] \u003d cdrom_bus"}],"source_content_type":"text/x-python","patch_set":6,"id":"4593df9b_086813c2","line":449,"range":{"start_line":449,"start_character":8,"end_line":449,"end_character":49},"in_reply_to":"64eb694c_028af5ed","updated":"2024-08-21 16:55:17.000000000","message":"i guess we cloud proceed with this refafctoring serperatly form teh fix\n\none thing we need to ensure we do not regress is the iso supprot is only broken in 1 of the 2 modes\n\ncurrently you can boot from isos if the iso has isolinux and is formated so it can be dd\u0027d directly to a usb/blockdevice\n\n\nso we shoudl try and test this with both an old and new iso","commit_id":"3746a9bfa9850030015878604bab9059f7642bf5"}]}
