)]}'
{"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"874aff322c80b9f647898d6879cbcb266e264c6c","unresolved":true,"context_lines":[{"line_number":2273,"context_line":""},{"line_number":2274,"context_line":"        if live and live_dev is None:"},{"line_number":2275,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2276,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2277,"context_line":""},{"line_number":2278,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2279,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":1,"id":"4cc8c1bf_7a8475f9","line":2276,"updated":"2021-03-19 08:46:46.000000000","message":"This does not make sense now as the caller does not request live detach now, they request detach-from-all-form-of-the-domain.","commit_id":"1567e1eb4fbc0b795860bf0d45187738f2d8eb4e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5481d453fb3b5c0c4ea039b2b17af823a2fe020c","unresolved":false,"context_lines":[{"line_number":2273,"context_line":""},{"line_number":2274,"context_line":"        if live and live_dev is None:"},{"line_number":2275,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2276,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2277,"context_line":""},{"line_number":2278,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2279,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":1,"id":"a510987e_84e4bc97","line":2276,"in_reply_to":"4cc8c1bf_7a8475f9","updated":"2021-04-14 16:34:02.000000000","message":"Done","commit_id":"1567e1eb4fbc0b795860bf0d45187738f2d8eb4e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5481d453fb3b5c0c4ea039b2b17af823a2fe020c","unresolved":false,"context_lines":[{"line_number":2277,"context_line":""},{"line_number":2278,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2279,"context_line":"            # caller requested detach from the persistent domain but device is"},{"line_number":2280,"context_line":"            # not present"},{"line_number":2281,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2282,"context_line":""},{"line_number":2283,"context_line":"        if persistent_dev:"}],"source_content_type":"text/x-python","patch_set":1,"id":"e9d8e901_aff0ee7a","line":2280,"updated":"2021-04-14 16:34:02.000000000","message":"this is also not valid any more so I rephrased it.","commit_id":"1567e1eb4fbc0b795860bf0d45187738f2d8eb4e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"eec422f1557f2772d226ea5a1981d27f5793ee8f","unresolved":true,"context_lines":[{"line_number":2231,"context_line":"    ) -\u003e None:"},{"line_number":2232,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2233,"context_line":""},{"line_number":2234,"context_line":"        If the guest is a running state then the detach is performend both on"},{"line_number":2235,"context_line":"        the persistent and on the live domain."},{"line_number":2236,"context_line":""},{"line_number":2237,"context_line":"        In case of live detach this call will wait for the libvirt event"}],"source_content_type":"text/x-python","patch_set":5,"id":"dcbfb466_8508620a","line":2234,"range":{"start_line":2234,"start_character":59,"end_line":2234,"end_character":69},"updated":"2021-04-22 09:53:57.000000000","message":"performed","commit_id":"0b3bea77ae24f71be98ff2e72c06302a8dacbf18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cb43e88e7e28434106f99ce3b28042e75233a780","unresolved":false,"context_lines":[{"line_number":2231,"context_line":"    ) -\u003e None:"},{"line_number":2232,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2233,"context_line":""},{"line_number":2234,"context_line":"        If the guest is a running state then the detach is performend both on"},{"line_number":2235,"context_line":"        the persistent and on the live domain."},{"line_number":2236,"context_line":""},{"line_number":2237,"context_line":"        In case of live detach this call will wait for the libvirt event"}],"source_content_type":"text/x-python","patch_set":5,"id":"e85c404c_ef37c914","line":2234,"range":{"start_line":2234,"start_character":59,"end_line":2234,"end_character":69},"in_reply_to":"dcbfb466_8508620a","updated":"2021-04-22 12:58:13.000000000","message":"Done","commit_id":"0b3bea77ae24f71be98ff2e72c06302a8dacbf18"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"eec422f1557f2772d226ea5a1981d27f5793ee8f","unresolved":true,"context_lines":[{"line_number":2232,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2233,"context_line":""},{"line_number":2234,"context_line":"        If the guest is a running state then the detach is performend both on"},{"line_number":2235,"context_line":"        the persistent and on the live domain."},{"line_number":2236,"context_line":""},{"line_number":2237,"context_line":"        In case of live detach this call will wait for the libvirt event"},{"line_number":2238,"context_line":"        signalling the end of the detach process."}],"source_content_type":"text/x-python","patch_set":5,"id":"506486ac_eb0a2263","line":2235,"range":{"start_line":2235,"start_character":8,"end_line":2235,"end_character":45},"updated":"2021-04-22 09:53:57.000000000","message":"grammar nit: This reads oddly. Something like:\n\n  both on the persistent domain and on the live domain.\n\nor:\n\n  on both the persistent and live domains.\n\nwould read a little better (I\u0027d opt for the latter, personally)\n(no, I don\u0027t know what grammatical rule is in play here /o\\)","commit_id":"0b3bea77ae24f71be98ff2e72c06302a8dacbf18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cb43e88e7e28434106f99ce3b28042e75233a780","unresolved":false,"context_lines":[{"line_number":2232,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2233,"context_line":""},{"line_number":2234,"context_line":"        If the guest is a running state then the detach is performend both on"},{"line_number":2235,"context_line":"        the persistent and on the live domain."},{"line_number":2236,"context_line":""},{"line_number":2237,"context_line":"        In case of live detach this call will wait for the libvirt event"},{"line_number":2238,"context_line":"        signalling the end of the detach process."}],"source_content_type":"text/x-python","patch_set":5,"id":"7090ec53_4d151434","line":2235,"range":{"start_line":2235,"start_character":8,"end_line":2235,"end_character":45},"in_reply_to":"506486ac_eb0a2263","updated":"2021-04-22 12:58:13.000000000","message":"Done","commit_id":"0b3bea77ae24f71be98ff2e72c06302a8dacbf18"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"eec422f1557f2772d226ea5a1981d27f5793ee8f","unresolved":true,"context_lines":[{"line_number":2303,"context_line":"            if live_dev is None:"},{"line_number":2304,"context_line":"                # this is probably an inconsistency between the persistent"},{"line_number":2305,"context_line":"                # and the live domain."},{"line_number":2306,"context_line":"                raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2307,"context_line":""},{"line_number":2308,"context_line":"            self._detach_from_live_with_retry("},{"line_number":2309,"context_line":"                guest, instance_uuid, live_dev, get_device_conf_func,"}],"source_content_type":"text/x-python","patch_set":5,"id":"d2bf52d3_51abc09e","line":2306,"updated":"2021-04-22 09:53:57.000000000","message":"So it\u0027s okay for the persistent device config information to be missing when a domain has the persistent flag, but it\u0027s not okay for live device config information to be missing when a domain has the live flag? That seems...odd. Shouldn\u0027t we be consistent about this? Something like:\n\n  persistent_dev \u003d None\n  if persistent:\n      persistent_dev \u003d get_device_conf_func(from_persistent_config\u003dTrue)\n      if persistent_dev is None:\n          raise exception.DeviceNotFound(device\u003ddevice_name)\n\n  live_dev \u003d None\n  if live:\n      live_dev \u003d get_device_conf_func()\n      if live_dev is None:\n          raise exception.DeviceNotFound(device\u003ddevice_name)\n\nAlternatively, if we\u0027re doubling down on \"it\u0027s okay if the device isn\u0027t present in both live and persistent, so long as it\u0027s in one of them, could we do:\n\n  persistent_dev \u003d None\n  if persistent:\n      persistent_dev \u003d get_device_conf_func(from_persistent_config\u003dTrue)\n\n  live_dev \u003d None\n  if live:\n      live_dev \u003d get_device_conf_func()\n\n  # didn\u0027t find the device in either domain\n  if persistent_dev is None and live_dev is None:\n      raise exception.DeviceNotFound(device\u003ddevice_name)\n\nIMO either option would be that bit clearer","commit_id":"0b3bea77ae24f71be98ff2e72c06302a8dacbf18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85352dabd340d828b1258ba026e77cd987d3ab35","unresolved":true,"context_lines":[{"line_number":2303,"context_line":"            if live_dev is None:"},{"line_number":2304,"context_line":"                # this is probably an inconsistency between the persistent"},{"line_number":2305,"context_line":"                # and the live domain."},{"line_number":2306,"context_line":"                raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2307,"context_line":""},{"line_number":2308,"context_line":"            self._detach_from_live_with_retry("},{"line_number":2309,"context_line":"                guest, instance_uuid, live_dev, get_device_conf_func,"}],"source_content_type":"text/x-python","patch_set":5,"id":"e3739708_745a0943","line":2306,"in_reply_to":"d2bf52d3_51abc09e","updated":"2021-04-22 10:47:26.000000000","message":"Originally the caller requested live detach with a flag in the function\u0027s signature. But this patch moves the logic from the callers into the detach function to decide if live detach is needed or not. \n\nIn the past if the caller asked for a live detach but there was no device in the live domain the it made sense to tell the caller that the request was wrong. Now it does not make any sense any more as the caller only asks for detach-it-from-every-domain. So the device exists in at least one of the domains then we should not raise back. So I will follow your second suggestion.\n\nAlso see https://review.opendev.org/c/openstack/nova/+/770246/24/nova/virt/libvirt/driver.py#2275 that is somewhat related and led to some of the changes in this patch.","commit_id":"0b3bea77ae24f71be98ff2e72c06302a8dacbf18"}]}