)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"009e1ee9a0696055d0a7ebeba2e81dcf3ef65842","unresolved":false,"context_lines":[{"line_number":4956,"context_line":"            if (isinstance(device, objects.DiskMetadata)"},{"line_number":4957,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4958,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4959,"context_line":"                instance.save()"},{"line_number":4960,"context_line":""},{"line_number":4961,"context_line":"    @wrap_exception()"},{"line_number":4962,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_402b9533","line":4959,"updated":"2017-02-17 14:15:10.000000000","message":"Does this mean we are saving the instance multiple times when there are multiple devices. Is there a reason we can\u0027t or shouldn\u0027t save it just the once after all the removals (if any)?","commit_id":"8f76ad0aa5ac6aec9eb580825ea51ab16868ef36"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fd4298bd6f0a4ca4496e509e132ca39c886c8ff9","unresolved":false,"context_lines":[{"line_number":4956,"context_line":"            if (isinstance(device, objects.DiskMetadata)"},{"line_number":4957,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4958,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4959,"context_line":"                instance.save()"},{"line_number":4960,"context_line":""},{"line_number":4961,"context_line":"    @wrap_exception()"},{"line_number":4962,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a1ff146_8513f95b","line":4959,"in_reply_to":"9a30ddce_8d57d724","updated":"2017-03-15 10:49:13.000000000","message":"Done","commit_id":"8f76ad0aa5ac6aec9eb580825ea51ab16868ef36"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"a92f672285fde5d469e498257ad2e561bf5813b6","unresolved":false,"context_lines":[{"line_number":4956,"context_line":"            if (isinstance(device, objects.DiskMetadata)"},{"line_number":4957,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4958,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4959,"context_line":"                instance.save()"},{"line_number":4960,"context_line":""},{"line_number":4961,"context_line":"    @wrap_exception()"},{"line_number":4962,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_61f978c8","line":4959,"in_reply_to":"fa31d9ce_402b9533","updated":"2017-02-17 16:24:04.000000000","message":"The method is called every time a tagged volume is detached, with only the single bdm being detached passed, so save() should only be called once as only a single device in the for loop will match all the conditions.","commit_id":"8f76ad0aa5ac6aec9eb580825ea51ab16868ef36"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b40dc363f5f9d3d26d95959356055dd4f5fa6607","unresolved":false,"context_lines":[{"line_number":4956,"context_line":"            if (isinstance(device, objects.DiskMetadata)"},{"line_number":4957,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4958,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4959,"context_line":"                instance.save()"},{"line_number":4960,"context_line":""},{"line_number":4961,"context_line":"    @wrap_exception()"},{"line_number":4962,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_8d57d724","line":4959,"in_reply_to":"fa31d9ce_61f978c8","updated":"2017-03-13 13:00:50.000000000","message":"I wonder if we should explicitly break here? Might yield a (very slight) performance improvement and make the above clearer","commit_id":"8f76ad0aa5ac6aec9eb580825ea51ab16868ef36"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1f98974e08e5df9bbf44f32ecf272fbf987b3eaf","unresolved":false,"context_lines":[{"line_number":4956,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4957,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4958,"context_line":"                instance.save()"},{"line_number":4959,"context_line":""},{"line_number":4960,"context_line":"    @wrap_exception()"},{"line_number":4961,"context_line":"    @wrap_instance_fault"},{"line_number":4962,"context_line":"    def detach_volume(self, context, volume_id, instance, attachment_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_0d3a07e7","line":4959,"updated":"2017-03-13 13:01:32.000000000","message":"Per comment on PS2, a break (and optional note) here could be helpful","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fa85c51fd52c1d05e2bd133445e6615eb1d6bc8c","unresolved":false,"context_lines":[{"line_number":4956,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4957,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4958,"context_line":"                instance.save()"},{"line_number":4959,"context_line":""},{"line_number":4960,"context_line":"    @wrap_exception()"},{"line_number":4961,"context_line":"    @wrap_instance_fault"},{"line_number":4962,"context_line":"    def detach_volume(self, context, volume_id, instance, attachment_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_4520219c","line":4959,"in_reply_to":"9a30ddce_0d3a07e7","updated":"2017-03-15 10:48:57.000000000","message":"Done","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e1987268d1f651232bc569ff1d571a464a7fa7a","unresolved":false,"context_lines":[{"line_number":4967,"context_line":"            phase\u003dfields.NotificationPhase.END,"},{"line_number":4968,"context_line":"            volume_id\u003dvolume_id)"},{"line_number":4969,"context_line":""},{"line_number":4970,"context_line":"        if bdm.obj_attr_is_set(\u0027tag\u0027) and bdm.tag:"},{"line_number":4971,"context_line":"            self._delete_disk_metadata(instance, bdm)"},{"line_number":4972,"context_line":"        if destroy_bdm:"},{"line_number":4973,"context_line":"            bdm.destroy()"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_2b545b52","line":4970,"range":{"start_line":4970,"start_character":11,"end_line":4970,"end_character":38},"updated":"2017-06-27 06:09:53.000000000","message":"You can do \"if \u0027tag\u0027 in bdm\" which is a bit prettier.","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e1fe92586b052270053ba9a2b0d9376dd572fac6","unresolved":false,"context_lines":[{"line_number":4967,"context_line":"            phase\u003dfields.NotificationPhase.END,"},{"line_number":4968,"context_line":"            volume_id\u003dvolume_id)"},{"line_number":4969,"context_line":""},{"line_number":4970,"context_line":"        if bdm.obj_attr_is_set(\u0027tag\u0027) and bdm.tag:"},{"line_number":4971,"context_line":"            self._delete_disk_metadata(instance, bdm)"},{"line_number":4972,"context_line":"        if destroy_bdm:"},{"line_number":4973,"context_line":"            bdm.destroy()"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_46754e63","line":4970,"range":{"start_line":4970,"start_character":11,"end_line":4970,"end_character":38},"in_reply_to":"5f201791_2b545b52","updated":"2017-06-27 07:45:03.000000000","message":"Done","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e1987268d1f651232bc569ff1d571a464a7fa7a","unresolved":false,"context_lines":[{"line_number":4975,"context_line":"    def _delete_disk_metadata(self, instance, bdm):"},{"line_number":4976,"context_line":"        for device in instance.device_metadata.devices:"},{"line_number":4977,"context_line":"            if (isinstance(device, objects.DiskMetadata)"},{"line_number":4978,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4979,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4980,"context_line":"                instance.save()"},{"line_number":4981,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_6bec938e","line":4978,"range":{"start_line":4978,"start_character":24,"end_line":4978,"end_character":47},"updated":"2017-06-27 06:09:53.000000000","message":"Older DiskMetadata objects are not going to have bdm_id set on them so you\u0027d get an AttributeError here, right? Don\u0027t you need to check if \u0027bdm_id\u0027 is in device? And if it\u0027s not found, we\u0027d have to log something probably that we couldn\u0027t determine it for detach.","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e1fe92586b052270053ba9a2b0d9376dd572fac6","unresolved":false,"context_lines":[{"line_number":4975,"context_line":"    def _delete_disk_metadata(self, instance, bdm):"},{"line_number":4976,"context_line":"        for device in instance.device_metadata.devices:"},{"line_number":4977,"context_line":"            if (isinstance(device, objects.DiskMetadata)"},{"line_number":4978,"context_line":"                    and device.bdm_id \u003d\u003d bdm.id):"},{"line_number":4979,"context_line":"                instance.device_metadata.devices.remove(device)"},{"line_number":4980,"context_line":"                instance.save()"},{"line_number":4981,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_46fb0e78","line":4978,"range":{"start_line":4978,"start_character":24,"end_line":4978,"end_character":47},"in_reply_to":"5f201791_6bec938e","updated":"2017-06-27 07:45:03.000000000","message":"Done","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c8c6a257b7880badf90b5d64c90638e76417b01d","unresolved":false,"context_lines":[{"line_number":4975,"context_line":"    def _delete_disk_metadata(self, instance, bdm):"},{"line_number":4976,"context_line":"        for device in instance.device_metadata.devices:"},{"line_number":4977,"context_line":"            if isinstance(device, objects.DiskMetadata):"},{"line_number":4978,"context_line":"                if \u0027serial\u0027 in device:"},{"line_number":4979,"context_line":"                    if device.serial \u003d\u003d bdm.volume_id:"},{"line_number":4980,"context_line":"                        instance.device_metadata.devices.remove(device)"},{"line_number":4981,"context_line":"                        instance.save()"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_f647674b","line":4978,"range":{"start_line":4978,"start_character":16,"end_line":4978,"end_character":38},"updated":"2017-06-27 09:02:13.000000000","message":"So we\u0027re now using the \u0027serial\u0027 field to reference a volume instead of adding a new \u0027bdm_id\u0027 field to reference a BDM, which would in-turn reference the volume? I assume the \u0027serial\u0027 field was unused until now? Comments in PS47 suggest this is the case alright","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0710752cb42d48a38d0e1b471689e9b3286ed81f","unresolved":false,"context_lines":[{"line_number":4975,"context_line":"    def _delete_disk_metadata(self, instance, bdm):"},{"line_number":4976,"context_line":"        for device in instance.device_metadata.devices:"},{"line_number":4977,"context_line":"            if isinstance(device, objects.DiskMetadata):"},{"line_number":4978,"context_line":"                if \u0027serial\u0027 in device:"},{"line_number":4979,"context_line":"                    if device.serial \u003d\u003d bdm.volume_id:"},{"line_number":4980,"context_line":"                        instance.device_metadata.devices.remove(device)"},{"line_number":4981,"context_line":"                        instance.save()"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_0a963094","line":4978,"range":{"start_line":4978,"start_character":16,"end_line":4978,"end_character":38},"in_reply_to":"5f201791_f647674b","updated":"2017-06-27 10:44:51.000000000","message":"\u003e So we\u0027re now using the \u0027serial\u0027 field to reference a volume\n\u003e instead of adding a new \u0027bdm_id\u0027 field to reference a BDM\n\nSo far so good.\n\n\u003e which would in-turn reference the volume?\n\nWhat do you mean by referencing the volume? We just need to find the bdm object of the disk whose DiskMetadata we have.\n\nFor the nic equivalent (find a nic\u0027s VirtualInterface object if we have its NetworkInterfaceMetadata object) we can correlate on mac address since it\u0027s unique and present in both.\n\nFor virtual disks there appeared to be no such field. I added bdm_id to DiskMetadata to create essentially an FK from DiskMetadata to bdm, but then Matt pointed out that we can use the serial-\u003evolume_id FK, we just need to set serial in DiskMetadata when we create it, which we weren\u0027t previously doing.","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c8c6a257b7880badf90b5d64c90638e76417b01d","unresolved":false,"context_lines":[{"line_number":4976,"context_line":"        for device in instance.device_metadata.devices:"},{"line_number":4977,"context_line":"            if isinstance(device, objects.DiskMetadata):"},{"line_number":4978,"context_line":"                if \u0027serial\u0027 in device:"},{"line_number":4979,"context_line":"                    if device.serial \u003d\u003d bdm.volume_id:"},{"line_number":4980,"context_line":"                        instance.device_metadata.devices.remove(device)"},{"line_number":4981,"context_line":"                        instance.save()"},{"line_number":4982,"context_line":"                        break"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_968a7367","line":4979,"range":{"start_line":4979,"start_character":40,"end_line":4979,"end_character":54},"updated":"2017-06-27 09:02:13.000000000","message":"Is it possible for there to be a bdm where this is unset? If so, we might want to change this line to the below to avoid None \u003d\u003d None issues:\n\n    if device.serial and device.serial \u003d\u003d bdm.volume_id:","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"55fc74e2e74a2d2b8bc4a54babb298e894ab3673","unresolved":false,"context_lines":[{"line_number":4976,"context_line":"        for device in instance.device_metadata.devices:"},{"line_number":4977,"context_line":"            if isinstance(device, objects.DiskMetadata):"},{"line_number":4978,"context_line":"                if \u0027serial\u0027 in device:"},{"line_number":4979,"context_line":"                    if device.serial \u003d\u003d bdm.volume_id:"},{"line_number":4980,"context_line":"                        instance.device_metadata.devices.remove(device)"},{"line_number":4981,"context_line":"                        instance.save()"},{"line_number":4982,"context_line":"                        break"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_36295fe5","line":4979,"range":{"start_line":4979,"start_character":40,"end_line":4979,"end_character":54},"in_reply_to":"5f201791_968a7367","updated":"2017-06-27 09:08:27.000000000","message":"bdm.volume_id will be set for any bdms attached to the instance for volumes, which is what we\u0027re calling this for - detaching volumes.","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c8c6a257b7880badf90b5d64c90638e76417b01d","unresolved":false,"context_lines":[{"line_number":4983,"context_line":"                else:"},{"line_number":4984,"context_line":"                    # NOTE(artom) We log the entire device object because all"},{"line_number":4985,"context_line":"                    # fields are nullable and may not be set"},{"line_number":4986,"context_line":"                    LOG.info(\u0027Unable to determine whether to clean up device \u0027"},{"line_number":4987,"context_line":"                             \u0027metadata for disk %s\u0027, device)"},{"line_number":4988,"context_line":""},{"line_number":4989,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_636f17fc","line":4986,"range":{"start_line":4986,"start_character":24,"end_line":4986,"end_character":28},"updated":"2017-06-27 09:02:13.000000000","message":"Would warning be more appropriate here?","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0710752cb42d48a38d0e1b471689e9b3286ed81f","unresolved":false,"context_lines":[{"line_number":4983,"context_line":"                else:"},{"line_number":4984,"context_line":"                    # NOTE(artom) We log the entire device object because all"},{"line_number":4985,"context_line":"                    # fields are nullable and may not be set"},{"line_number":4986,"context_line":"                    LOG.info(\u0027Unable to determine whether to clean up device \u0027"},{"line_number":4987,"context_line":"                             \u0027metadata for disk %s\u0027, device)"},{"line_number":4988,"context_line":""},{"line_number":4989,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_2fb33e9a","line":4986,"range":{"start_line":4986,"start_character":24,"end_line":4986,"end_character":28},"in_reply_to":"5f201791_636f17fc","updated":"2017-06-27 10:44:51.000000000","message":"Looking at [1] it\u0027s definitely not info, you\u0027re right. It\u0027s not strictly speaking warning either, but it\u0027s closer to warning than info, so let\u0027s change it.\n\n[1] https://specs.openstack.org/openstack/openstack-specs/specs/log-guidelines.html","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63b67330b0185ae549ba9bd9630dd72908eef20b","unresolved":false,"context_lines":[{"line_number":4985,"context_line":"                else:"},{"line_number":4986,"context_line":"                    # NOTE(artom) We log the entire device object because all"},{"line_number":4987,"context_line":"                    # fields are nullable and may not be set"},{"line_number":4988,"context_line":"                    LOG.warning(\u0027Unable to determine whether to clean up \u0027"},{"line_number":4989,"context_line":"                                \u0027device metadata for disk %s\u0027, device)"},{"line_number":4990,"context_line":""},{"line_number":4991,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_479e5ab9","line":4988,"updated":"2017-06-27 13:45:12.000000000","message":"Specify the instance\u003dinstance kwarg in the warning log message for context.","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"baa2f73dc1b3af72539d333787511cbfff4be1e1","unresolved":false,"context_lines":[{"line_number":4985,"context_line":"                else:"},{"line_number":4986,"context_line":"                    # NOTE(artom) We log the entire device object because all"},{"line_number":4987,"context_line":"                    # fields are nullable and may not be set"},{"line_number":4988,"context_line":"                    LOG.warning(\u0027Unable to determine whether to clean up \u0027"},{"line_number":4989,"context_line":"                                \u0027device metadata for disk %s\u0027, device)"},{"line_number":4990,"context_line":""},{"line_number":4991,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_879112f4","line":4988,"in_reply_to":"5f201791_479e5ab9","updated":"2017-06-27 14:04:09.000000000","message":"Done","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"}],"nova/objects/virt_device_metadata.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1f98974e08e5df9bbf44f32ecf272fbf987b3eaf","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        \u0027path\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":100,"context_line":"        # The bdm_id exists to unabiguously link the DiskMetadata object to the"},{"line_number":101,"context_line":"        # BlockDeviceMapping object whose metadata it\u0027s storing. Unlike with"},{"line_number":102,"context_line":"        # network interfaces, where we have the same MAC addressed (assumed to"},{"line_number":103,"context_line":"        # be unique) in both the VirtualInterface object and the"},{"line_number":104,"context_line":"        # NetworkInterfaceMetadata objects, a DiskMetadata object might only"},{"line_number":105,"context_line":"        # contain the tag and nothing else, giving us no way to link it to the"},{"line_number":106,"context_line":"        # corresponding BlockDeviceMapping object. Such a link needs to exist"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_30280033","line":103,"range":{"start_line":102,"start_character":67,"end_line":103,"end_character":20},"updated":"2017-03-13 13:01:32.000000000","message":"This shares a common superclass. Do devices also share a common superclass? If so, I wonder if there\u0027s any use moving \u0027bdm_id\u0027 to DeviceMetadata to allow for its (future) use with other metadata-types? For example, could we ever envision a future where MAC addresses aren\u0027t always unique? Just a thought","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0aed1570387ac590ec671dfd7c37e61dc7f85732","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        \u0027path\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":100,"context_line":"        # The bdm_id exists to unabiguously link the DiskMetadata object to the"},{"line_number":101,"context_line":"        # BlockDeviceMapping object whose metadata it\u0027s storing. Unlike with"},{"line_number":102,"context_line":"        # network interfaces, where we have the same MAC addressed (assumed to"},{"line_number":103,"context_line":"        # be unique) in both the VirtualInterface object and the"},{"line_number":104,"context_line":"        # NetworkInterfaceMetadata objects, a DiskMetadata object might only"},{"line_number":105,"context_line":"        # contain the tag and nothing else, giving us no way to link it to the"},{"line_number":106,"context_line":"        # corresponding BlockDeviceMapping object. Such a link needs to exist"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_8e36b101","line":103,"range":{"start_line":102,"start_character":67,"end_line":103,"end_character":20},"in_reply_to":"3a1ff146_65b99de9","updated":"2017-03-15 14:25:47.000000000","message":"\u003e It\u0027s a nice bit of future proofing, for sure... I\u0027m not convinced though. It\u0027s future proofing for its own sake, we have no plans to use device_id any time soon.\n\nSounds fair to me. Thanks for following up on this","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fa85c51fd52c1d05e2bd133445e6615eb1d6bc8c","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        \u0027path\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":100,"context_line":"        # The bdm_id exists to unabiguously link the DiskMetadata object to the"},{"line_number":101,"context_line":"        # BlockDeviceMapping object whose metadata it\u0027s storing. Unlike with"},{"line_number":102,"context_line":"        # network interfaces, where we have the same MAC addressed (assumed to"},{"line_number":103,"context_line":"        # be unique) in both the VirtualInterface object and the"},{"line_number":104,"context_line":"        # NetworkInterfaceMetadata objects, a DiskMetadata object might only"},{"line_number":105,"context_line":"        # contain the tag and nothing else, giving us no way to link it to the"},{"line_number":106,"context_line":"        # corresponding BlockDeviceMapping object. Such a link needs to exist"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_65b99de9","line":103,"range":{"start_line":102,"start_character":67,"end_line":103,"end_character":20},"in_reply_to":"9a30ddce_30280033","updated":"2017-03-15 10:48:57.000000000","message":"Ah, you mean turn bdm_id into device_id in the parent DeviceMetadata class, and have it point to the bdm_id in the case of DiskMetadata, to nic_id (or whatever it would end up being called) in the case of NetworkInterfaceMetadata, or to gpu_id (pure faniful speculation now) if we ever decide to expose GPU metadata.\n\nIt\u0027s a nice bit of future proofing, for sure... I\u0027m not convinced though. It\u0027s future proofing for its own sake, we have no plans to use device_id any time soon. Other device ids might not even be integers. Plus with versioned objects if we do end up needing such a change it can easily be done, so we\u0027re not locking ourselves into anything.","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e1987268d1f651232bc569ff1d571a464a7fa7a","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        # corresponding BlockDeviceMapping object. Such a link needs to exist"},{"line_number":107,"context_line":"        # when we detach a tagged volume and need to remove it from"},{"line_number":108,"context_line":"        # instance.device_metadata."},{"line_number":109,"context_line":"        \u0027bdm_id\u0027: fields.IntegerField(),"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_6bba536e","line":109,"range":{"start_line":109,"start_character":9,"end_line":109,"end_character":15},"updated":"2017-06-27 06:09:53.000000000","message":"Using the bdm id primary key is kind of gross, but it should be unique per cell, and the instance is per cell. Would we ever plan on exposing this out of the REST API? If so, we should use a uuid - which would mean finally adding a uuid field to the BDM object/table which we\u0027ve talked about forever.\n\nWould the serial be set? The serial is really the volume ID (uuid of the cinder volume), couldn\u0027t we compare that against the BDM.volume_id? It looks like we don\u0027t set serial or path anywhere though - why do we have those fields then?","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e1fe92586b052270053ba9a2b0d9376dd572fac6","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        # corresponding BlockDeviceMapping object. Such a link needs to exist"},{"line_number":107,"context_line":"        # when we detach a tagged volume and need to remove it from"},{"line_number":108,"context_line":"        # instance.device_metadata."},{"line_number":109,"context_line":"        \u0027bdm_id\u0027: fields.IntegerField(),"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_d19c7ac1","line":109,"range":{"start_line":109,"start_character":9,"end_line":109,"end_character":15},"in_reply_to":"5f201791_6bba536e","updated":"2017-06-27 07:45:03.000000000","message":"\u003e It looks like we don\u0027t set serial or path anywhere though -\n\u003e why do we have those fields then?\n\nYeah, I did a double take as well [1]. Looks like we just decided neither to set nor retrieve those for some reason [2].\nI think I can change this patch to use volume_uuid instead of adding a new device ID.\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7852-L7860\n[2] https://review.openstack.org/#/c/264016/59/nova/virt/libvirt/driver.py@7531","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63b67330b0185ae549ba9bd9630dd72908eef20b","unresolved":false,"context_lines":[{"line_number":2497,"context_line":"        bdm \u003d objects.BlockDeviceMapping(volume_id\u003duuids.volume_id, tag\u003d\u0027foo\u0027)"},{"line_number":2498,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":2499,"context_line":"        instance.device_metadata \u003d objects.InstanceDeviceMetadata("},{"line_number":2500,"context_line":"                devices\u003d[objects.DiskMetadata(serial\u003duuids.volume_id,"},{"line_number":2501,"context_line":"                                              tag\u003d\u0027foo\u0027)])"},{"line_number":2502,"context_line":"        instance.save \u003d mock.Mock()"},{"line_number":2503,"context_line":"        self.compute._delete_disk_metadata(instance, bdm)"}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_e7de863c","line":2500,"range":{"start_line":2500,"start_character":46,"end_line":2500,"end_character":52},"updated":"2017-06-27 13:45:12.000000000","message":"We should also have a unit test for the case that the serial isn\u0027t set in the DiskMetadata object (old tagged block device from Newton).","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"baa2f73dc1b3af72539d333787511cbfff4be1e1","unresolved":false,"context_lines":[{"line_number":2497,"context_line":"        bdm \u003d objects.BlockDeviceMapping(volume_id\u003duuids.volume_id, tag\u003d\u0027foo\u0027)"},{"line_number":2498,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":2499,"context_line":"        instance.device_metadata \u003d objects.InstanceDeviceMetadata("},{"line_number":2500,"context_line":"                devices\u003d[objects.DiskMetadata(serial\u003duuids.volume_id,"},{"line_number":2501,"context_line":"                                              tag\u003d\u0027foo\u0027)])"},{"line_number":2502,"context_line":"        instance.save \u003d mock.Mock()"},{"line_number":2503,"context_line":"        self.compute._delete_disk_metadata(instance, bdm)"}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_222eac91","line":2500,"range":{"start_line":2500,"start_character":46,"end_line":2500,"end_character":52},"in_reply_to":"5f201791_e7de863c","updated":"2017-06-27 14:04:09.000000000","message":"Done","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"63b67330b0185ae549ba9bd9630dd72908eef20b","unresolved":false,"context_lines":[{"line_number":2534,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("},{"line_number":2535,"context_line":"                {\u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":2536,"context_line":"                 \u0027volume_id\u0027: volume_id, \u0027device_name\u0027: \u0027/dev/vdb\u0027,"},{"line_number":2537,"context_line":"                 \u0027connection_info\u0027: \u0027{\"test\": \"test\"}\u0027, \u0027tag\u0027: \u0027foo\u0027})"},{"line_number":2538,"context_line":"        bdm \u003d objects.BlockDeviceMapping(context\u003dself.context, **fake_bdm)"},{"line_number":2539,"context_line":"        bdm_get.return_value \u003d bdm"},{"line_number":2540,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_871a3214","line":2537,"range":{"start_line":2537,"start_character":56,"end_line":2537,"end_character":68},"updated":"2017-06-27 13:45:12.000000000","message":"This isn\u0027t really needed. You\u0027ve mocked out _delete_disk_metadata so this code being run is never going to check the bdm.tag anyway.","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"baa2f73dc1b3af72539d333787511cbfff4be1e1","unresolved":false,"context_lines":[{"line_number":2534,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("},{"line_number":2535,"context_line":"                {\u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":2536,"context_line":"                 \u0027volume_id\u0027: volume_id, \u0027device_name\u0027: \u0027/dev/vdb\u0027,"},{"line_number":2537,"context_line":"                 \u0027connection_info\u0027: \u0027{\"test\": \"test\"}\u0027, \u0027tag\u0027: \u0027foo\u0027})"},{"line_number":2538,"context_line":"        bdm \u003d objects.BlockDeviceMapping(context\u003dself.context, **fake_bdm)"},{"line_number":2539,"context_line":"        bdm_get.return_value \u003d bdm"},{"line_number":2540,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_c26c98cf","line":2537,"range":{"start_line":2537,"start_character":56,"end_line":2537,"end_character":68},"in_reply_to":"5f201791_871a3214","updated":"2017-06-27 14:04:09.000000000","message":"We assert that we\u0027re calling the delete_metadata mock on L2556, so unless we\u0027re going to remove that part of the test entirely, we need to keep the tag here.","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"139a759d1ca6e3c39796b7d320f713bd68d82f4c","unresolved":false,"context_lines":[{"line_number":2534,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("},{"line_number":2535,"context_line":"                {\u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":2536,"context_line":"                 \u0027volume_id\u0027: volume_id, \u0027device_name\u0027: \u0027/dev/vdb\u0027,"},{"line_number":2537,"context_line":"                 \u0027connection_info\u0027: \u0027{\"test\": \"test\"}\u0027, \u0027tag\u0027: \u0027foo\u0027})"},{"line_number":2538,"context_line":"        bdm \u003d objects.BlockDeviceMapping(context\u003dself.context, **fake_bdm)"},{"line_number":2539,"context_line":"        bdm_get.return_value \u003d bdm"},{"line_number":2540,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_b63c412f","line":2537,"range":{"start_line":2537,"start_character":56,"end_line":2537,"end_character":68},"in_reply_to":"5f201791_a2251c0b","updated":"2017-06-27 14:35:10.000000000","message":"Done","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e0f574c98bff707bc2129a96703c5b33ee6fb251","unresolved":false,"context_lines":[{"line_number":2534,"context_line":"        fake_bdm \u003d fake_block_device.FakeDbBlockDeviceDict("},{"line_number":2535,"context_line":"                {\u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":2536,"context_line":"                 \u0027volume_id\u0027: volume_id, \u0027device_name\u0027: \u0027/dev/vdb\u0027,"},{"line_number":2537,"context_line":"                 \u0027connection_info\u0027: \u0027{\"test\": \"test\"}\u0027, \u0027tag\u0027: \u0027foo\u0027})"},{"line_number":2538,"context_line":"        bdm \u003d objects.BlockDeviceMapping(context\u003dself.context, **fake_bdm)"},{"line_number":2539,"context_line":"        bdm_get.return_value \u003d bdm"},{"line_number":2540,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_a2251c0b","line":2537,"range":{"start_line":2537,"start_character":56,"end_line":2537,"end_character":68},"in_reply_to":"5f201791_c26c98cf","updated":"2017-06-27 14:15:21.000000000","message":"Oh right because we only call _delete_disk_metadata if the bdm.tag is set. OK then. Then it also stands to reason we could/should have a simple test (or change to an existing test) to assert that _delete_disk_metadata is not called when a bdm does not have a tag set during detach.","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"636e2510c8d999de1e6bfe4e1567236f74e19ff4","unresolved":false,"context_lines":[{"line_number":2551,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":2552,"context_line":"                \u0027_notify_about_instance_usage\u0027)"},{"line_number":2553,"context_line":"    @mock.patch(\u0027nova.compute.utils.notify_about_volume_attach_detach\u0027)"},{"line_number":2554,"context_line":"    def _test_detach_volume(self, mock_notify_attach_detach, notify_inst_usage,"},{"line_number":2555,"context_line":"                            detach, bdm_get, destroy_bdm\u003dTrue):"},{"line_number":2556,"context_line":"        # TODO(lyarwood): Test DriverVolumeBlockDevice.detach in"},{"line_number":2557,"context_line":"        # ../virt/test_block_device.py"}],"source_content_type":"text/x-python","patch_set":51,"id":"5f201791_84d8ddee","line":2554,"updated":"2017-06-27 23:05:21.000000000","message":"Did you want to mock _delete_disk_metadata and assert it wasn\u0027t called since the bdm isn\u0027t tagged?","commit_id":"d4402baeffbc2abf79cae3b0269d9b922a445878"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c1184154090c75c5cd00823ce7dab79a41937d95","unresolved":false,"context_lines":[{"line_number":2551,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":2552,"context_line":"                \u0027_notify_about_instance_usage\u0027)"},{"line_number":2553,"context_line":"    @mock.patch(\u0027nova.compute.utils.notify_about_volume_attach_detach\u0027)"},{"line_number":2554,"context_line":"    def _test_detach_volume(self, mock_notify_attach_detach, notify_inst_usage,"},{"line_number":2555,"context_line":"                            detach, bdm_get, destroy_bdm\u003dTrue):"},{"line_number":2556,"context_line":"        # TODO(lyarwood): Test DriverVolumeBlockDevice.detach in"},{"line_number":2557,"context_line":"        # ../virt/test_block_device.py"}],"source_content_type":"text/x-python","patch_set":51,"id":"5f201791_5a43b542","line":2554,"in_reply_to":"5f201791_84d8ddee","updated":"2017-06-28 06:38:29.000000000","message":"Sure :)","commit_id":"d4402baeffbc2abf79cae3b0269d9b922a445878"}],"nova/tests/unit/objects/test_instance_device_metadata.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"9830d2050a10f4947556c605439dd1392f80d7aa","unresolved":false,"context_lines":[{"line_number":22,"context_line":"                            bus\u003dobjects.PCIDeviceBus(address\u003d\u00270000:00:03.0\u0027),"},{"line_number":23,"context_line":"                            vlan\u003d1000)"},{"line_number":24,"context_line":"fake_pci_disk_meta \u003d objects.DiskMetadata("},{"line_number":25,"context_line":"                            bus\u003dobjects.PCIDeviceBus(address\u003d\u00270000:00:09.0\u0027),"},{"line_number":26,"context_line":"                            tags\u003d[\u0027nfvfunc3\u0027],"},{"line_number":27,"context_line":"                            bdm_id\u003d1)"},{"line_number":28,"context_line":"fake_obj_devices_metadata \u003d objects.InstanceDeviceMetadata("}],"source_content_type":"text/x-python","patch_set":24,"id":"1f013ff3_19d603c5","line":25,"range":{"start_line":25,"start_character":40,"end_line":25,"end_character":52},"updated":"2017-05-16 13:59:58.000000000","message":"nit - Shouldn\u0027t this be something like USBDeviceBus, SCSIDeviceBus or IDEDeviceBus?\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7674-L7692","commit_id":"b252a90cc9f1f5f844f0b5e4e9e67d1a1c73981f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3dbbbad3d17e0b458ce872a00709c9391bdc7183","unresolved":false,"context_lines":[{"line_number":22,"context_line":"                            bus\u003dobjects.PCIDeviceBus(address\u003d\u00270000:00:03.0\u0027),"},{"line_number":23,"context_line":"                            vlan\u003d1000)"},{"line_number":24,"context_line":"fake_pci_disk_meta \u003d objects.DiskMetadata("},{"line_number":25,"context_line":"                            bus\u003dobjects.PCIDeviceBus(address\u003d\u00270000:00:09.0\u0027),"},{"line_number":26,"context_line":"                            tags\u003d[\u0027nfvfunc3\u0027],"},{"line_number":27,"context_line":"                            bdm_id\u003d1)"},{"line_number":28,"context_line":"fake_obj_devices_metadata \u003d objects.InstanceDeviceMetadata("}],"source_content_type":"text/x-python","patch_set":24,"id":"1f013ff3_62d430be","line":25,"range":{"start_line":25,"start_character":40,"end_line":25,"end_character":52},"in_reply_to":"1f013ff3_19d603c5","updated":"2017-05-16 15:09:45.000000000","message":"I guess... The libvirt driver, as you rightly linked, can\u0027t currently create PCI disk metadata, but other drivers might. This is just testing the object.","commit_id":"b252a90cc9f1f5f844f0b5e4e9e67d1a1c73981f"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1f98974e08e5df9bbf44f32ecf272fbf987b3eaf","unresolved":false,"context_lines":[{"line_number":1592,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1593,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1594,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1595,"context_line":"            self.assertEqual(5, metadata[3].bdm_id)"},{"line_number":1596,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1597,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1598,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a30ddce_50c73495","line":1595,"range":{"start_line":1595,"start_character":29,"end_line":1595,"end_character":30},"updated":"2017-03-13 13:01:32.000000000","message":"what is this black magic? I\u0027d expected 4 here","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0aed1570387ac590ec671dfd7c37e61dc7f85732","unresolved":false,"context_lines":[{"line_number":1592,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1593,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1594,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1595,"context_line":"            self.assertEqual(5, metadata[3].bdm_id)"},{"line_number":1596,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1597,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1598,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_4e04b916","line":1595,"range":{"start_line":1595,"start_character":29,"end_line":1595,"end_character":30},"in_reply_to":"3a1ff146_25940536","updated":"2017-03-15 14:25:47.000000000","message":"Any chance of a little comment to this effect?","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fa85c51fd52c1d05e2bd133445e6615eb1d6bc8c","unresolved":false,"context_lines":[{"line_number":1592,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1593,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1594,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1595,"context_line":"            self.assertEqual(5, metadata[3].bdm_id)"},{"line_number":1596,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1597,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1598,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a1ff146_25940536","line":1595,"range":{"start_line":1595,"start_character":29,"end_line":1595,"end_character":30},"in_reply_to":"9a30ddce_50c73495","updated":"2017-03-15 10:48:57.000000000","message":"The 4th one isn\u0027t tagged - see L1508 - and we don\u0027t expose untagged devices.","commit_id":"4d3681f2baf27b8b7bf2d7e47fd56bd2d869a454"},{"author":{"_account_id":24711,"name":"Jackie Truong","email":"jacklyn.truong@jhuapl.edu","username":"jackie-truong"},"change_message_id":"09703423aafe2126f3748a4b347dd6b0456a7554","unresolved":false,"context_lines":[{"line_number":1613,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1614,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1615,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1616,"context_line":"            self.assertEqual(5, metadata[3].bdm_id)"},{"line_number":1617,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1618,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1619,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":24,"id":"1f013ff3_4b46e823","line":1616,"updated":"2017-05-16 19:07:23.000000000","message":"NIT: Agreed that a small comment noting that the fourth fake_block_device is not tagged, which is why the fourth element of the metadata array has a bdm_id of 5, rather than 4 would clarify this assertion.","commit_id":"b252a90cc9f1f5f844f0b5e4e9e67d1a1c73981f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c8c6a257b7880badf90b5d64c90638e76417b01d","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1667,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1668,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1669,"context_line":"            self.assertEqual(uuids.volume_5, metadata[3].serial)"},{"line_number":1670,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1671,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1672,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_36e83fe0","line":1669,"range":{"start_line":1669,"start_character":29,"end_line":1669,"end_character":43},"updated":"2017-06-27 09:02:13.000000000","message":"I\u0027m still missing my inline comment from way back to explain why we skip a volume :(","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"0710752cb42d48a38d0e1b471689e9b3286ed81f","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1667,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1668,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1669,"context_line":"            self.assertEqual(uuids.volume_5, metadata[3].serial)"},{"line_number":1670,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1671,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1672,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_4f10baa8","line":1669,"range":{"start_line":1669,"start_character":29,"end_line":1669,"end_character":43},"in_reply_to":"5f201791_36e83fe0","updated":"2017-06-27 10:44:51.000000000","message":"4 isn\u0027t tagged so it won\u0027t show up in the metadata.","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"41b374a2b5794b11d26ba6fd6c28e06357662258","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1667,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1668,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1669,"context_line":"            self.assertEqual(uuids.volume_5, metadata[3].serial)"},{"line_number":1670,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1671,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1672,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_5c16314b","line":1669,"range":{"start_line":1669,"start_character":29,"end_line":1669,"end_character":43},"in_reply_to":"5f201791_4f10baa8","updated":"2017-06-27 13:17:56.000000000","message":"Right, and I\u0027d hoping for a comment here to explain that. It caught me and I\u0027m sure it\u0027d catch others","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e5ec9c513ba7a9f33a3dd2f4c8511933d9ab32bb","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1667,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1668,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1669,"context_line":"            self.assertEqual(uuids.volume_5, metadata[3].serial)"},{"line_number":1670,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1671,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1672,"context_line":"                                  objects.DiskMetadata)"}],"source_content_type":"text/x-python","patch_set":48,"id":"5f201791_5c8bd166","line":1669,"range":{"start_line":1669,"start_character":29,"end_line":1669,"end_character":43},"in_reply_to":"5f201791_5c16314b","updated":"2017-06-27 14:02:57.000000000","message":"Bleah, missed the \u0027inline comment\u0027 part, sorry :( Added.","commit_id":"7dbb0fe3e174590e26083364f1ce490aeb10b3b7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"50f12b608f0a365fa097ffbb2bf50c62573cc80c","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1667,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1668,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1669,"context_line":"            # NOTE(artom) We\u0027re not checking volume 4 because it\u0027s not tagged"},{"line_number":1670,"context_line":"            # and only tagged devices appear in the metadata"},{"line_number":1671,"context_line":"            self.assertEqual(uuids.volume_5, metadata[3].serial)"},{"line_number":1672,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1673,"context_line":"            self.assertIsInstance(metadata[4],"}],"source_content_type":"text/x-python","patch_set":52,"id":"3f1d235d_80509334","line":1670,"range":{"start_line":1669,"start_character":0,"end_line":1670,"end_character":60},"updated":"2017-06-29 13:10:59.000000000","message":"ta","commit_id":"822c609d6b0281d14ea184f2e3da3f70388aabe6"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"009e1ee9a0696055d0a7ebeba2e81dcf3ef65842","unresolved":false,"context_lines":[{"line_number":7723,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestDisk):"},{"line_number":7724,"context_line":"                bdm \u003d tagged_bdms.get(dev.target_dev)"},{"line_number":7725,"context_line":"                if not bdm:"},{"line_number":7726,"context_line":"                    continue"},{"line_number":7727,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7728,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7729,"context_line":"                if bus:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_40b555ce","side":"PARENT","line":7726,"updated":"2017-02-17 14:15:10.000000000","message":"not relevant to this change, but this continue being so far away from its containing for loop really threw me","commit_id":"e3100fb3acc754258786edc20a0ae4a9126f694d"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"9830d2050a10f4947556c605439dd1392f80d7aa","unresolved":false,"context_lines":[{"line_number":7769,"context_line":"                    continue"},{"line_number":7770,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7771,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7772,"context_line":"                device.bdm_id \u003d bdm.id"},{"line_number":7773,"context_line":"                if bus:"},{"line_number":7774,"context_line":"                    device.bus \u003d bus"},{"line_number":7775,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":24,"id":"1f013ff3_bc4dbd73","line":7772,"range":{"start_line":7772,"start_character":16,"end_line":7772,"end_character":38},"updated":"2017-05-16 13:59:58.000000000","message":"nit - likely missing something here but don\u0027t we want to also assign serial and path if set in the LibvirtConfigGuestDisk object?\n\n    if dev.serial:\n         device.serial \u003d dev.serial\n    if dev.source_path:\n         device.path \u003d dev.device.path","commit_id":"b252a90cc9f1f5f844f0b5e4e9e67d1a1c73981f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3dbbbad3d17e0b458ce872a00709c9391bdc7183","unresolved":false,"context_lines":[{"line_number":7769,"context_line":"                    continue"},{"line_number":7770,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7771,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7772,"context_line":"                device.bdm_id \u003d bdm.id"},{"line_number":7773,"context_line":"                if bus:"},{"line_number":7774,"context_line":"                    device.bus \u003d bus"},{"line_number":7775,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":24,"id":"1f013ff3_22ad0831","line":7772,"range":{"start_line":7772,"start_character":16,"end_line":7772,"end_character":38},"in_reply_to":"1f013ff3_bc4dbd73","updated":"2017-05-16 15:09:45.000000000","message":"Good point - I can\u0027t remember if there was a reason why it wasn\u0027t done originally. I\u0027m not sure the best place to add it is this patch, but we need to revisit this for sure.","commit_id":"b252a90cc9f1f5f844f0b5e4e9e67d1a1c73981f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9e1987268d1f651232bc569ff1d571a464a7fa7a","unresolved":false,"context_lines":[{"line_number":7855,"context_line":"                    continue"},{"line_number":7856,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7857,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7858,"context_line":"                device.bdm_id \u003d bdm.id"},{"line_number":7859,"context_line":"                if bus:"},{"line_number":7860,"context_line":"                    device.bus \u003d bus"},{"line_number":7861,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_6b3db3db","line":7858,"range":{"start_line":7858,"start_character":16,"end_line":7858,"end_character":38},"updated":"2017-06-27 06:09:53.000000000","message":"Why not set the serial since we already have that field, it would be the BDM.volume_id. Then we don\u0027t need to mess with bdm id primary keys. We have the same upgrade problem either way, i.e. that we might not be able to identify the device on detach since older computes aren\u0027t setting these values.","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e1fe92586b052270053ba9a2b0d9376dd572fac6","unresolved":false,"context_lines":[{"line_number":7855,"context_line":"                    continue"},{"line_number":7856,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7857,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7858,"context_line":"                device.bdm_id \u003d bdm.id"},{"line_number":7859,"context_line":"                if bus:"},{"line_number":7860,"context_line":"                    device.bus \u003d bus"},{"line_number":7861,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_91fde26f","line":7858,"range":{"start_line":7858,"start_character":16,"end_line":7858,"end_character":38},"in_reply_to":"5f201791_6b3db3db","updated":"2017-06-27 07:45:03.000000000","message":"\u003e Why not set the serial since we already have that field, it\n\u003e would be the BDM.volume_id. Then we don\u0027t need to mess with\n\u003e bdm id primary keys.\n\nDone.\n\n\u003e We have the same upgrade problem either way, i.e. that we\n\u003e might not be able to identify the device on detach since\n\u003e older computes aren\u0027t setting these values.\n\nAt first I thought you meant volume_id might not be set in the bdm (which is impossible since it\u0027s there since version 1.0 [1]) but then I understood you meant it might not be set in the device metadata.\n\n[1] https://github.com/openstack/nova/commit/5a7fe002","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"32e05d6035b3cdeccec05cda2d7d6e6894d9b1cf","unresolved":false,"context_lines":[{"line_number":7855,"context_line":"                    continue"},{"line_number":7856,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7857,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7858,"context_line":"                device.bdm_id \u003d bdm.id"},{"line_number":7859,"context_line":"                if bus:"},{"line_number":7860,"context_line":"                    device.bus \u003d bus"},{"line_number":7861,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_d1611aab","line":7858,"range":{"start_line":7858,"start_character":16,"end_line":7858,"end_character":38},"in_reply_to":"5f201791_6b3db3db","updated":"2017-06-27 07:07:54.000000000","message":"By the way, we also need to handle this for the hyperv driver which supports tagged devices.","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"e1fe92586b052270053ba9a2b0d9376dd572fac6","unresolved":false,"context_lines":[{"line_number":7855,"context_line":"                    continue"},{"line_number":7856,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7857,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7858,"context_line":"                device.bdm_id \u003d bdm.id"},{"line_number":7859,"context_line":"                if bus:"},{"line_number":7860,"context_line":"                    device.bus \u003d bus"},{"line_number":7861,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":47,"id":"5f201791_b168de1c","line":7858,"range":{"start_line":7858,"start_character":16,"end_line":7858,"end_character":38},"in_reply_to":"5f201791_d1611aab","updated":"2017-06-27 07:45:03.000000000","message":"Done. We should get Claudio to check it out to make sure I didn\u0027t do anything stupid.","commit_id":"24fb8df23c940f419c42bece8b2e7ed4a18f75d7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3f2dfba0ec1fef061ec9ad17bda2b5b7ffac1bdc","unresolved":false,"context_lines":[{"line_number":7855,"context_line":"                    continue"},{"line_number":7856,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7857,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7858,"context_line":"                device.serial \u003d bdm.volume_id"},{"line_number":7859,"context_line":"                if bus:"},{"line_number":7860,"context_line":"                    device.bus \u003d bus"},{"line_number":7861,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_5c2f5187","line":7858,"updated":"2017-06-27 13:20:59.000000000","message":"Might be good to put a comment here that the serial is set for uniquely identifying the device later and actually, now that I saw this, could we not identify which device to remove based on the bdm.tag? Can an instance have more than one attached volume with the same tag? It looks like you could because from the data model, there is no unique constraint for bdm + instance uuid + tag, so I can have an instance with two attached volumes with the same device tag. :( That seems like an oversight in our data modeling.","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"baa2f73dc1b3af72539d333787511cbfff4be1e1","unresolved":false,"context_lines":[{"line_number":7855,"context_line":"                    continue"},{"line_number":7856,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7857,"context_line":"                device \u003d objects.DiskMetadata(tags\u003d[bdm.tag])"},{"line_number":7858,"context_line":"                device.serial \u003d bdm.volume_id"},{"line_number":7859,"context_line":"                if bus:"},{"line_number":7860,"context_line":"                    device.bus \u003d bus"},{"line_number":7861,"context_line":"                devices.append(device)"}],"source_content_type":"text/x-python","patch_set":50,"id":"5f201791_477c1aeb","line":7858,"in_reply_to":"5f201791_5c2f5187","updated":"2017-06-27 14:04:09.000000000","message":"\u003e Might be good to put a comment here that the serial is set\n\u003e for uniquely identifying the device later\n\nDone.\n\n\u003e Can an instance have more than one attached volume with the\n\u003e same tag? It looks like you could because from the data\n\u003e model, there is no unique constraint for bdm + instance uuid\n\u003e + tag, so I can have an instance with two attached volumes\n\u003e with the same device tag. :( That seems like an oversight in\n\u003e our data modeling.\n\nAs discussed on IRC there was no real argument for or against unique tags. While I don\u0027t see a use case for non-unique tags, I can conceive of a couple of situations where someone might want them, for example two volumes with the same purpose and therefore the same tag, or two nics that are intended for bonding and have the same tag.","commit_id":"09613d2271133c7f8fb2bfe42f88353d46b61420"}]}