)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"00ceba0401439b336476bae4f585d1ed6022a6aa","unresolved":false,"context_lines":[{"line_number":10,"context_line":"to allow for both privileged and unprivileged calls to be made."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"A privileged_qemu_img_info function is introduced allowing QEMU to"},{"line_number":13,"context_line":"access devices requiring root privileges, such as host block devices."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I5ac03f923d9d181d22d44d8ec8fbc31eb0c3999e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"1fa4df85_2c39b2f1","line":13,"range":{"start_line":13,"start_character":68,"end_line":13,"end_character":69},"updated":"2020-03-11 15:01:44.000000000","message":"Maybe add:\n\n  This isn\u0027t used yet. That will come in a future change \u003cid here?\u003e","commit_id":"865a10c590ba68ff85b4471af9f0b9ad7f0c063b"}],"nova/privsep/qemu.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98724fde45c7ab267a9b9fc525c9b255f0ddf1dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3fa7e38b_d9251860","updated":"2020-02-19 13:05:40.000000000","message":"I realised after leaving all the comments below that this is a copy-paste of an existing module, so feel free to ignore all of these. Fixing them up in a pre-/postcursor patch would be nice though :)","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"cf006687abf395440da596f8b9a0d6a2e8421562","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3fa7e38b_e01614f4","in_reply_to":"3fa7e38b_d9251860","updated":"2020-02-19 15:22:33.000000000","message":"ACK, I\u0027ve already got a few FUPs lined up on the following topic so I\u0027ll just address these there:\n\nhttps://review.opendev.org/#/q/topic:bug/1861071_followups","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98724fde45c7ab267a9b9fc525c9b255f0ddf1dc","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        path, format\u003dformat, qemu_version\u003dqemu_version)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def unprivileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":94,"context_line":"    \"\"\"Return an object containing the parsed output from qemu-img info.\"\"\""},{"line_number":95,"context_line":"    try:"},{"line_number":96,"context_line":"        # The following check is about ploop images that reside within"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_3986ac61","line":93,"range":{"start_line":93,"start_character":37,"end_line":93,"end_character":43},"updated":"2020-02-19 13:05:40.000000000","message":"could we use fmt instead since \u0027format\u0027 is a built-in function","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98724fde45c7ab267a9b9fc525c9b255f0ddf1dc","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # The following check is about ploop images that reside within"},{"line_number":97,"context_line":"        # directories and always have DiskDescriptor.xml file beside them"},{"line_number":98,"context_line":"        if (os.path.isdir(path) and"},{"line_number":99,"context_line":"            os.path.exists(os.path.join(path, \"DiskDescriptor.xml\"))):"},{"line_number":100,"context_line":"            path \u003d os.path.join(path, \"root.hds\")"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        cmd \u003d (\u0027env\u0027, \u0027LC_ALL\u003dC\u0027, \u0027LANG\u003dC\u0027, \u0027qemu-img\u0027, \u0027info\u0027, path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_d9c998ae","line":99,"updated":"2020-02-19 13:05:40.000000000","message":"nit: weird indentation. I wonder if the \u0027isdir\u0027 is necessary since you can\u0027t have a file called \u0027foo/DiskDescriptor.xml\u0027","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98724fde45c7ab267a9b9fc525c9b255f0ddf1dc","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            cmd \u003d cmd + (\u0027-f\u0027, format)"},{"line_number":105,"context_line":"        # Check to see if the qemu version is \u003e\u003d 2.10 because if so, we need"},{"line_number":106,"context_line":"        # to add the --force-share flag."},{"line_number":107,"context_line":"        if qemu_version and operator.ge(qemu_version, QEMU_VERSION_REQ_SHARED):"},{"line_number":108,"context_line":"            cmd \u003d cmd + (\u0027--force-share\u0027,)"},{"line_number":109,"context_line":"        out, err \u003d processutils.execute(*cmd, prlimit\u003dQEMU_IMG_LIMITS)"},{"line_number":110,"context_line":"    except processutils.ProcessExecutionError as exp:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_d92e785a","line":107,"range":{"start_line":107,"start_character":28,"end_line":107,"end_character":79},"updated":"2020-02-19 13:05:40.000000000","message":"Why not:\n\n  qemu_version \u003e\u003d QEMU_VERSION_REQ_SHARED:\n\n?","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98724fde45c7ab267a9b9fc525c9b255f0ddf1dc","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        # Check to see if the qemu version is \u003e\u003d 2.10 because if so, we need"},{"line_number":106,"context_line":"        # to add the --force-share flag."},{"line_number":107,"context_line":"        if qemu_version and operator.ge(qemu_version, QEMU_VERSION_REQ_SHARED):"},{"line_number":108,"context_line":"            cmd \u003d cmd + (\u0027--force-share\u0027,)"},{"line_number":109,"context_line":"        out, err \u003d processutils.execute(*cmd, prlimit\u003dQEMU_IMG_LIMITS)"},{"line_number":110,"context_line":"    except processutils.ProcessExecutionError as exp:"},{"line_number":111,"context_line":"        if exp.exit_code \u003d\u003d -9:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_f93334c2","line":108,"updated":"2020-02-19 13:05:40.000000000","message":"everything until now could have been done outside the try-except","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"68211d4461592780a72298c8c8306e51867c224e","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"QEMU_IMG_LIMITS \u003d processutils.ProcessLimits("},{"line_number":33,"context_line":"    cpu_time\u003d30,"},{"line_number":34,"context_line":"    address_space\u003d1 * units.Gi)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"QEMU_VERSION_REQ_SHARED \u003d 2010000"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_a63cf292","line":36,"range":{"start_line":32,"start_character":0,"end_line":36,"end_character":33},"updated":"2020-03-10 18:09:52.000000000","message":"this also feels weired to me\n\neither i would expect the limtes to apply to all image operation like convert image or for this to be in a different module. i also would not expect the verion requiremnt to be in this file.","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6c3b2ebaf47de9248a70cf90887b2bb053ef722a","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"QEMU_IMG_LIMITS \u003d processutils.ProcessLimits("},{"line_number":33,"context_line":"    cpu_time\u003d30,"},{"line_number":34,"context_line":"    address_space\u003d1 * units.Gi)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"QEMU_VERSION_REQ_SHARED \u003d 2010000"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_a34ffa32","line":36,"range":{"start_line":32,"start_character":0,"end_line":36,"end_character":33},"in_reply_to":"1fa4df85_a63cf292","updated":"2020-03-11 08:49:05.000000000","message":"As called out in the commit this is just code motion, I\u0027ll follow up and wire QEMU_IMG_LIMITS into the convert calls.","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"68211d4461592780a72298c8c8306e51867c224e","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        path, format\u003dformat, qemu_version\u003dqemu_version)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def unprivileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":94,"context_line":"    \"\"\"Return an object containing the parsed output from qemu-img info.\"\"\""},{"line_number":95,"context_line":"    try:"},{"line_number":96,"context_line":"        # The following check is about ploop images that reside within"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_e6df4a63","line":93,"range":{"start_line":93,"start_character":3,"end_line":93,"end_character":16},"updated":"2020-03-10 18:09:52.000000000","message":"as i noted in the commit message you have inverted the piroity of this.\n\ni get that one might assume that you would assume in this module that function might be privileged by default but\n\nwe should be signalling when a function is privileged not unprivileged so that when its used in normal code its very clear when it is priviledged.","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6c3b2ebaf47de9248a70cf90887b2bb053ef722a","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        path, format\u003dformat, qemu_version\u003dqemu_version)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def unprivileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":94,"context_line":"    \"\"\"Return an object containing the parsed output from qemu-img info.\"\"\""},{"line_number":95,"context_line":"    try:"},{"line_number":96,"context_line":"        # The following check is about ploop images that reside within"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_630822d9","line":93,"range":{"start_line":93,"start_character":3,"end_line":93,"end_character":16},"in_reply_to":"1fa4df85_e6df4a63","updated":"2020-03-11 08:49:05.000000000","message":"Yup that\u0027s fair, I\u0027ll ensure the above is called out as privileged.","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"68211d4461592780a72298c8c8306e51867c224e","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            cmd \u003d cmd + (\u0027-f\u0027, format)"},{"line_number":105,"context_line":"        # Check to see if the qemu version is \u003e\u003d 2.10 because if so, we need"},{"line_number":106,"context_line":"        # to add the --force-share flag."},{"line_number":107,"context_line":"        if qemu_version and operator.ge(qemu_version, QEMU_VERSION_REQ_SHARED):"},{"line_number":108,"context_line":"            cmd \u003d cmd + (\u0027--force-share\u0027,)"},{"line_number":109,"context_line":"        out, err \u003d processutils.execute(*cmd, prlimit\u003dQEMU_IMG_LIMITS)"},{"line_number":110,"context_line":"    except processutils.ProcessExecutionError as exp:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_66583a30","line":107,"range":{"start_line":107,"start_character":4,"end_line":107,"end_character":79},"updated":"2020-03-10 18:09:52.000000000","message":"by the way whil i like the operator module if this is all you are using it for i would just do \n\nif qemu_version and (qemu_version \u003e\u003d QEMU_VERSION_REQ_SHARED):","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6c3b2ebaf47de9248a70cf90887b2bb053ef722a","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            cmd \u003d cmd + (\u0027-f\u0027, format)"},{"line_number":105,"context_line":"        # Check to see if the qemu version is \u003e\u003d 2.10 because if so, we need"},{"line_number":106,"context_line":"        # to add the --force-share flag."},{"line_number":107,"context_line":"        if qemu_version and operator.ge(qemu_version, QEMU_VERSION_REQ_SHARED):"},{"line_number":108,"context_line":"            cmd \u003d cmd + (\u0027--force-share\u0027,)"},{"line_number":109,"context_line":"        out, err \u003d processutils.execute(*cmd, prlimit\u003dQEMU_IMG_LIMITS)"},{"line_number":110,"context_line":"    except processutils.ProcessExecutionError as exp:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_e36e5295","line":107,"range":{"start_line":107,"start_character":4,"end_line":107,"end_character":79},"in_reply_to":"1fa4df85_66583a30","updated":"2020-03-11 08:49:05.000000000","message":"Again, just code motion, this is actually dropped in the follow up series by If878a023c69f25a9ea45b7de2ff9eb1976aaeb8c.","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e141cb29c7c11f617b944af1f6afa217c801e279","unresolved":false,"context_lines":[{"line_number":83,"context_line":"    cmd \u003d cmd + (source, dest)"},{"line_number":84,"context_line":"    processutils.execute(*cmd)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":88,"context_line":"def privileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":89,"context_line":"    \"\"\"Return an oject containing the parsed output from qemu-img info"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    This is a privileged call to qemu-img info using the sys_admin_pctxt"},{"line_number":92,"context_line":"    entrypoint allowing host block devices etc to be accessed."},{"line_number":93,"context_line":"    \"\"\""},{"line_number":94,"context_line":"    return unprivileged_qemu_img_info("},{"line_number":95,"context_line":"        path, format\u003dformat, qemu_version\u003dqemu_version)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"def unprivileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_891f7db6","line":95,"range":{"start_line":86,"start_character":0,"end_line":95,"end_character":55},"updated":"2020-03-11 10:59:02.000000000","message":"this is now not called anywhere.\nis that intentional?","commit_id":"865a10c590ba68ff85b4471af9f0b9ad7f0c063b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"65b7bfa9fe4b23559529e6eb7db24c7aeedc8a10","unresolved":false,"context_lines":[{"line_number":83,"context_line":"    cmd \u003d cmd + (source, dest)"},{"line_number":84,"context_line":"    processutils.execute(*cmd)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":88,"context_line":"def privileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":89,"context_line":"    \"\"\"Return an oject containing the parsed output from qemu-img info"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    This is a privileged call to qemu-img info using the sys_admin_pctxt"},{"line_number":92,"context_line":"    entrypoint allowing host block devices etc to be accessed."},{"line_number":93,"context_line":"    \"\"\""},{"line_number":94,"context_line":"    return unprivileged_qemu_img_info("},{"line_number":95,"context_line":"        path, format\u003dformat, qemu_version\u003dqemu_version)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"def unprivileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_8c754b35","line":95,"range":{"start_line":86,"start_character":0,"end_line":95,"end_character":55},"in_reply_to":"1fa4df85_49bc256c","updated":"2020-03-11 11:47:31.000000000","message":"thanks you had the run_as_root change in the last reviesion so when i saw this was un used i was not sure if this was a mistake but that makes sense.","commit_id":"865a10c590ba68ff85b4471af9f0b9ad7f0c063b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5da0dc1de34e8c61d8dde82ebc91d7e2b167796a","unresolved":false,"context_lines":[{"line_number":83,"context_line":"    cmd \u003d cmd + (source, dest)"},{"line_number":84,"context_line":"    processutils.execute(*cmd)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":88,"context_line":"def privileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":89,"context_line":"    \"\"\"Return an oject containing the parsed output from qemu-img info"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    This is a privileged call to qemu-img info using the sys_admin_pctxt"},{"line_number":92,"context_line":"    entrypoint allowing host block devices etc to be accessed."},{"line_number":93,"context_line":"    \"\"\""},{"line_number":94,"context_line":"    return unprivileged_qemu_img_info("},{"line_number":95,"context_line":"        path, format\u003dformat, qemu_version\u003dqemu_version)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"def unprivileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_49bc256c","line":95,"range":{"start_line":86,"start_character":0,"end_line":95,"end_character":55},"in_reply_to":"1fa4df85_891f7db6","updated":"2020-03-11 11:08:18.000000000","message":"Yes, until I0c3f14100a18107f7e416293f3d4fcc641ce5e55.\n\nThe only reason to move unprivileged_qemu_img_info here is to introduce a privileged version but I didn\u0027t want to also introduce the bugfix that actually calls privileged_qemu_img_info in the same change.","commit_id":"865a10c590ba68ff85b4471af9f0b9ad7f0c063b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cb04bdce31a24190bdaf1f7058941ec40aadd5a5","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"@nova.privsep.sys_admin_pctxt.entrypoint"},{"line_number":88,"context_line":"def privileged_qemu_img_info(path, format\u003dNone, qemu_version\u003dNone):"},{"line_number":89,"context_line":"    \"\"\"Return an oject containing the parsed output from qemu-img info"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    This is a privileged call to qemu-img info using the sys_admin_pctxt"},{"line_number":92,"context_line":"    entrypoint allowing host block devices etc to be accessed."}],"source_content_type":"text/x-python","patch_set":9,"id":"1fa4df85_89fe4603","line":89,"range":{"start_line":89,"start_character":17,"end_line":89,"end_character":22},"updated":"2020-03-17 11:28:33.000000000","message":"nit: object","commit_id":"03d6eb500f85a417f84a7bf4a9f4ceb275c9a67e"}],"nova/virt/images.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98724fde45c7ab267a9b9fc525c9b255f0ddf1dc","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    if not os.path.exists(path) and CONF.libvirt.images_type !\u003d \u0027rbd\u0027:"},{"line_number":51,"context_line":"        raise exception.DiskNotFound(location\u003dpath)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    with compute_utils.disk_ops_semaphore:"},{"line_number":54,"context_line":"        if not run_as_root:"},{"line_number":55,"context_line":"            info \u003d nova.privsep.qemu.unprivileged_qemu_img_info("},{"line_number":56,"context_line":"                path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_3910ecf4","line":53,"updated":"2020-02-19 13:05:40.000000000","message":"The semaphore is new, yeah? Could you note it in the commit message if so?","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"cf006687abf395440da596f8b9a0d6a2e8421562","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    if not os.path.exists(path) and CONF.libvirt.images_type !\u003d \u0027rbd\u0027:"},{"line_number":51,"context_line":"        raise exception.DiskNotFound(location\u003dpath)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    with compute_utils.disk_ops_semaphore:"},{"line_number":54,"context_line":"        if not run_as_root:"},{"line_number":55,"context_line":"            info \u003d nova.privsep.qemu.unprivileged_qemu_img_info("},{"line_number":56,"context_line":"                path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_c0de783c","line":53,"in_reply_to":"3fa7e38b_3910ecf4","updated":"2020-02-19 15:22:33.000000000","message":"ACK, wasn\u0027t sure if that would also need a unit test, couldn\u0027t find any other examples iirc but happy to add if I\u0027ve missed some.","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5ce263565956c084501aca57c01032365caa6beb","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    if not os.path.exists(path) and CONF.libvirt.images_type !\u003d \u0027rbd\u0027:"},{"line_number":51,"context_line":"        raise exception.DiskNotFound(location\u003dpath)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    with compute_utils.disk_ops_semaphore:"},{"line_number":54,"context_line":"        if not run_as_root:"},{"line_number":55,"context_line":"            info \u003d nova.privsep.qemu.unprivileged_qemu_img_info("},{"line_number":56,"context_line":"                path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_a0233cbb","line":53,"in_reply_to":"3fa7e38b_c0de783c","updated":"2020-02-19 15:47:38.000000000","message":"Actually lets drop this and look at introducing it in a FUP, I think it\u0027s the correct thing to do but it isn\u0027t required in this bugfix series.","commit_id":"862d8cffe229af15423aa538018e71b3fad7019d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"68211d4461592780a72298c8c8306e51867c224e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"IMAGE_API \u003d glance.API()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def qemu_img_info(path, format\u003dNone, run_as_root\u003dFalse):"},{"line_number":47,"context_line":"    \"\"\"Return an object containing the parsed output from qemu-img info.\"\"\""},{"line_number":48,"context_line":"    # TODO(mikal): this code should not be referring to a libvirt specific"},{"line_number":49,"context_line":"    # flag."},{"line_number":50,"context_line":"    if not os.path.exists(path) and CONF.libvirt.images_type !\u003d \u0027rbd\u0027:"},{"line_number":51,"context_line":"        raise exception.DiskNotFound(location\u003dpath)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    if not run_as_root:"},{"line_number":54,"context_line":"        info \u003d nova.privsep.qemu.unprivileged_qemu_img_info("},{"line_number":55,"context_line":"            path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"},{"line_number":56,"context_line":"    else:"},{"line_number":57,"context_line":"        info \u003d nova.privsep.qemu.qemu_img_info("},{"line_number":58,"context_line":"            path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"},{"line_number":59,"context_line":"    return imageutils.QemuImgInfo(info)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def convert_image(source, dest, in_format, out_format, run_as_root\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_c6b6ce3e","line":59,"range":{"start_line":46,"start_character":0,"end_line":59,"end_character":39},"updated":"2020-03-10 18:09:52.000000000","message":"this also is not really a good patteren.\n\nwhy do we some time run this as root and other not.\n\nthis is creating a sometimes privaledged function based on the flag","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6c3b2ebaf47de9248a70cf90887b2bb053ef722a","unresolved":false,"context_lines":[{"line_number":43,"context_line":"IMAGE_API \u003d glance.API()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def qemu_img_info(path, format\u003dNone, run_as_root\u003dFalse):"},{"line_number":47,"context_line":"    \"\"\"Return an object containing the parsed output from qemu-img info.\"\"\""},{"line_number":48,"context_line":"    # TODO(mikal): this code should not be referring to a libvirt specific"},{"line_number":49,"context_line":"    # flag."},{"line_number":50,"context_line":"    if not os.path.exists(path) and CONF.libvirt.images_type !\u003d \u0027rbd\u0027:"},{"line_number":51,"context_line":"        raise exception.DiskNotFound(location\u003dpath)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    if not run_as_root:"},{"line_number":54,"context_line":"        info \u003d nova.privsep.qemu.unprivileged_qemu_img_info("},{"line_number":55,"context_line":"            path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"},{"line_number":56,"context_line":"    else:"},{"line_number":57,"context_line":"        info \u003d nova.privsep.qemu.qemu_img_info("},{"line_number":58,"context_line":"            path, format\u003dformat, qemu_version\u003dQEMU_VERSION)"},{"line_number":59,"context_line":"    return imageutils.QemuImgInfo(info)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def convert_image(source, dest, in_format, out_format, run_as_root\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_43888621","line":59,"range":{"start_line":46,"start_character":0,"end_line":59,"end_character":39},"in_reply_to":"1fa4df85_c6b6ce3e","updated":"2020-03-11 08:49:05.000000000","message":"All existing calls are against local files, the new call [1][2] could also be against host block devices and RBD volumes, both of which require root privileges to either read directly or access secrets in /etc/ceph/.\n\nI really don\u0027t like this API as well FWIW but I\u0027m again limiting churn as much as possible to allow for a backportable fix here.\n\n[1] https://review.opendev.org/#/c/706900/10/nova/virt/libvirt/driver.py@1965\n[2] https://review.opendev.org/#/c/710785/2/nova/virt/libvirt/driver.py@1959","commit_id":"0f739e6e471e357eaa8f2da9782563dd772906ec"}]}