)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"82800a3de0e5261b3bf71f20ebdd024326dddf3f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"obj: Allow multiple volume attachments"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Extend bdm object."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Partially-implements: blueprint multi-attach-volume"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"5a710552_65a15c94","line":9,"updated":"2015-12-15 13:02:44.000000000","message":"This is not a very useful commit message. It would more helpful to see a description of what this change does when I get commit logs from history.","commit_id":"9d96092d607a178bbd8ece130189c15167f7b95f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8b195c4d98990c4ef4db7f5736f42e7868251dfb","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This patch also adds the get_by_instance_and_volume_id method"},{"line_number":13,"context_line":"to the BDM object. The method is needed as by enabling the multiple"},{"line_number":14,"context_line":"volume attachment support a volume can be attached to multiple"},{"line_number":15,"context_line":"Nova instances, therefore the volume_id is not enough anymore"},{"line_number":16,"context_line":"to identify the attachments."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"The information is needed at the time of detaching the volume"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"fa69d971_60c9f625","line":15,"updated":"2016-01-04 22:41:57.000000000","message":"Hmm, there are possibly a couple of issues with this, but primarily that the volume_id is sometimes None. Currently the volume_id and device_name are used as a unique constraint for a BDM, and volume_id can initially be None when the BDM is created.\n\nWe had issues with this in a cells race bug, and one part of that possible solution was to add a uuid column to BDMs, starting here:\n\nhttps://review.openstack.org/#/c/242602/\n\nThere are also the cases that the BDM represents a snapshot or image, so snapshot_id/image_id are initially set rather than volume_id and we don\u0027t set volume_id until after actually creating the volume from the snapshot or image (in compute), which was also racy until recently (see https://review.openstack.org/#/c/258118/ ).\n\nSo I guess I\u0027m wondering if we\u0027d be better off with just reviving Dan\u0027s BDM uuid changes and relying on those rather than a volume_id/device_name pair that is not always unique.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"cc457a82d5d823cab6502ceddf828efe11e0be93","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This patch also adds the get_by_instance_and_volume_id method"},{"line_number":13,"context_line":"to the BDM object. The method is needed as by enabling the multiple"},{"line_number":14,"context_line":"volume attachment support a volume can be attached to multiple"},{"line_number":15,"context_line":"Nova instances, therefore the volume_id is not enough anymore"},{"line_number":16,"context_line":"to identify the attachments."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"The information is needed at the time of detaching the volume"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"fa69d971_8665171f","line":15,"in_reply_to":"fa69d971_60c9f625","updated":"2016-01-05 07:27:04.000000000","message":"It\u0027s volume_id and instance_id pair which is supposed to be unique. It will mostly be needed at the time of detach, which should ensure we have a volume_id and instance_id also in place.\n\nThe change here is mainly the modification of an existing method to use the instance_id in the db query, when it\u0027s available. I don\u0027t see at the moment how the BDM UUID would help in this case.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6a8d3d2d1dad9ca3598ef6def19523758dc40f89","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This patch also adds the get_by_instance_and_volume_id method"},{"line_number":13,"context_line":"to the BDM object. The method is needed as by enabling the multiple"},{"line_number":14,"context_line":"volume attachment support a volume can be attached to multiple"},{"line_number":15,"context_line":"Nova instances, therefore the volume_id is not enough anymore"},{"line_number":16,"context_line":"to identify the attachments."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"The information is needed at the time of detaching the volume"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":30,"id":"fa69d971_1c516936","line":15,"in_reply_to":"fa69d971_8665171f","updated":"2016-01-05 15:02:45.000000000","message":"The volume_id may be None though, at least on initial create of the BDM:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1216\n\nAt least in that case the instance_uuid is set.\n\nThe other place a BDM is created is in the compute manager:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4646\n\nThe instance_uuid is also set there, and if the volume_id was None, it\u0027s set to \u0027reserved\u0027, so you could race to have two BDMs with the same instance_uuid and volume_id of \u0027reserved\u0027 if you\u0027re, for example, booting from volume with a snapshot or image rather than a pre-created volume.\n\nSo this is definitely not a solid fix, but it\u0027s arguably not any worse than what we already have I guess.\n\nIf what we really care about is the volume_id at the time of detach, we should have that set by the time detach is called, at least in the boot from volume case where we lock on the instance uuid. The only time we wouldn\u0027t have the volume_id set with boot from volume is if the volume create (like booting with a snapshot or image) timed out, but then the build request should also go to ERROR and you wouldn\u0027t be able to try a detach anyway.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"df299964fb40a557af6fc599cc68929d92026c75","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch adds a new function to sqlalchemy/api to retrieve"},{"line_number":10,"context_line":"BDM from database by volume id and instance id."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch also adds the get_by_instance_and_volume_id method"},{"line_number":13,"context_line":"to the BDM object. The method is needed as by enabling the multiple"},{"line_number":14,"context_line":"volume attachment support a volume can be attached to multiple"},{"line_number":15,"context_line":"Nova instances, therefore the volume_id is not enough anymore"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"da6ed579_959595e5","line":12,"range":{"start_line":12,"start_character":25,"end_line":12,"end_character":54},"updated":"2016-01-12 19:55:59.000000000","message":"This change is no longer adding a new method to the BDM object, it is, however, using it in the BDM object if a volume id and instance uuid are provided in the get query method. You should maybe clarify that in the commit message.\n\nAnd if there are race/uniqueness issues with this, as discussed in ps37, you should probably comment on those here also.","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4e7d0ab61bdfe6613e1c9324c6e885fd29326b6b","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch adds a new function to sqlalchemy/api to retrieve"},{"line_number":10,"context_line":"BDM from database by volume id and instance id."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This patch also adds the get_by_instance_and_volume_id method"},{"line_number":13,"context_line":"to the BDM object. The method is needed as by enabling the multiple"},{"line_number":14,"context_line":"volume attachment support a volume can be attached to multiple"},{"line_number":15,"context_line":"Nova instances, therefore the volume_id is not enough anymore"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"da6ed579_f0465fbe","line":12,"range":{"start_line":12,"start_character":25,"end_line":12,"end_character":54},"in_reply_to":"da6ed579_959595e5","updated":"2016-01-12 20:21:54.000000000","message":"Done","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"df299964fb40a557af6fc599cc68929d92026c75","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from an instance. The attachment_id needs to be passed to Cinder"},{"line_number":20,"context_line":"in order to terminate the connection properly."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"For more information on how Cinder handles the attachments see"},{"line_number":23,"context_line":"the reference to the Cinder and the Cinder client patches below."},{"line_number":24,"context_line":"Also for further details on volume attachment on Nova side please"},{"line_number":25,"context_line":"check the blueprint."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The API patch is part of a follow up spec that captures only the"},{"line_number":28,"context_line":"API changes in Nova."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Partially-implements: blueprint multi-attach-volume"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"da6ed579_75bb4173","line":28,"range":{"start_line":22,"start_character":0,"end_line":28,"end_character":20},"updated":"2016-01-12 19:55:59.000000000","message":"This is all boilerplate, I think you can remove it from the commit message along with the related patch links below.","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4e7d0ab61bdfe6613e1c9324c6e885fd29326b6b","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from an instance. The attachment_id needs to be passed to Cinder"},{"line_number":20,"context_line":"in order to terminate the connection properly."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"For more information on how Cinder handles the attachments see"},{"line_number":23,"context_line":"the reference to the Cinder and the Cinder client patches below."},{"line_number":24,"context_line":"Also for further details on volume attachment on Nova side please"},{"line_number":25,"context_line":"check the blueprint."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The API patch is part of a follow up spec that captures only the"},{"line_number":28,"context_line":"API changes in Nova."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Partially-implements: blueprint multi-attach-volume"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":42,"id":"da6ed579_901283b3","line":28,"range":{"start_line":22,"start_character":0,"end_line":28,"end_character":20},"in_reply_to":"da6ed579_75bb4173","updated":"2016-01-12 20:21:54.000000000","message":"Done","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"503977dc89842874357623405bf58274e932724d","unresolved":false,"context_lines":[{"line_number":14,"context_line":"is needed as by enabling the multiple volume attachment support a"},{"line_number":15,"context_line":"volume can be attached to multiple Nova instances, therefore the"},{"line_number":16,"context_line":"volume_id is not enough anymore to identify the BDM."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Partially-implements: blueprint multi-attach-volume"},{"line_number":19,"context_line":"Partial-Bug: #1528548"},{"line_number":20,"context_line":"Change-Id: I183f56fab73f815869f47e31d6d0f9461f72b9dc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"da6ed579_a6650d46","line":17,"updated":"2016-01-12 21:13:49.000000000","message":"You should probably add:\n\nCo-Authored-By: Ildiko Vancsa \u003cildiko.vancsa@ericsson.com\u003e\n\nTo this series.","commit_id":"4b9a88494344281fc72d4db04281f388109d8dde"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d20e54cf1cb8855f4996fc5b74ead985502bd0e9","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 one_or_none()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_9f09fddf","line":4046,"updated":"2016-01-04 16:12:12.000000000","message":"I don\u0027t see DB API tests for this in here:\nhttps://review.openstack.org/#/c/193133/28/nova/tests/unit/db/test_db_api.py\n\nReally you should raise the Nova exception here, so sqlalchemy details don\u0027t leak outside this db driver layer.","commit_id":"7d6d9dc9ca8854ad9def40f0ab7074accd737c56"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"84904c47bd5fc5b87f68e4acbcf1269c4ad2d7e1","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 one_or_none()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_c24a38c5","line":4046,"in_reply_to":"fa69d971_9f09fddf","updated":"2016-01-04 18:28:06.000000000","message":"Sorry, right, makes sense. Done.","commit_id":"7d6d9dc9ca8854ad9def40f0ab7074accd737c56"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d20e54cf1cb8855f4996fc5b74ead985502bd0e9","unresolved":false,"context_lines":[{"line_number":4225,"context_line":"        project_id \u003d security_group_ref[\u0027project_id\u0027]"},{"line_number":4226,"context_line":"        try:"},{"line_number":4227,"context_line":"            security_group_ref.save(session\u003dsession)"},{"line_number":4228,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4229,"context_line":"            raise exception.SecurityGroupExists("},{"line_number":4230,"context_line":"                    project_id\u003dproject_id,"},{"line_number":4231,"context_line":"                    security_group_name\u003dname)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_ff97d98d","line":4228,"updated":"2016-01-04 16:12:12.000000000","message":"See how the exceptions are usually caught in this layer, so we don\u0027t violate the principle of only this layer knowing about sqlalchemy.","commit_id":"7d6d9dc9ca8854ad9def40f0ab7074accd737c56"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d3bd40c4696eb216aaffb9cbc7c78aa870dbd87b","unresolved":false,"context_lines":[{"line_number":3967,"context_line":"    values \u003d _from_legacy_values(values, legacy, allow_updates\u003dTrue)"},{"line_number":3968,"context_line":"    convert_objects_related_datetimes(values)"},{"line_number":3969,"context_line":""},{"line_number":3970,"context_line":"    query \u003d _block_device_mapping_get_query(context).filter_by(id\u003dbdm_id)"},{"line_number":3971,"context_line":"    query.update(values)"},{"line_number":3972,"context_line":"    return query.first()"},{"line_number":3973,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_3c255bc0","line":3970,"updated":"2016-01-05 12:42:49.000000000","message":"Given this uses the id, it feels safe, despite the first thing later on.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d3bd40c4696eb216aaffb9cbc7c78aa870dbd87b","unresolved":false,"context_lines":[{"line_number":3985,"context_line":"        if values[\u0027device_name\u0027]:"},{"line_number":3986,"context_line":"            query \u003d _block_device_mapping_get_query(context, session\u003dsession)"},{"line_number":3987,"context_line":"            result \u003d query.filter_by(instance_uuid\u003dvalues[\u0027instance_uuid\u0027],"},{"line_number":3988,"context_line":"                                     device_name\u003dvalues[\u0027device_name\u0027]).first()"},{"line_number":3989,"context_line":""},{"line_number":3990,"context_line":"        if result:"},{"line_number":3991,"context_line":"            result.update(values)"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_f7183c77","line":3988,"updated":"2016-01-05 12:42:49.000000000","message":"Oh dear, this one is a bit nasty too. We should do the one_or_none() here I think. Although I guess its technically unrelated to the change here.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"743b5b8ea7a3ffde74f098161c4135d0e0c80b9b","unresolved":false,"context_lines":[{"line_number":3985,"context_line":"        if values[\u0027device_name\u0027]:"},{"line_number":3986,"context_line":"            query \u003d _block_device_mapping_get_query(context, session\u003dsession)"},{"line_number":3987,"context_line":"            result \u003d query.filter_by(instance_uuid\u003dvalues[\u0027instance_uuid\u0027],"},{"line_number":3988,"context_line":"                                     device_name\u003dvalues[\u0027device_name\u0027]).first()"},{"line_number":3989,"context_line":""},{"line_number":3990,"context_line":"        if result:"},{"line_number":3991,"context_line":"            result.update(values)"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_5fc35357","line":3988,"in_reply_to":"fa69d971_7e0ae07a","updated":"2016-01-05 15:12:18.000000000","message":"I think this method is really only used in the cells case, at least in a racy way, although we do hit it when creating volumes in the boot from volume case in the API here:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L1216\n\nAnd in this case it\u0027s possible to boot from volume without a device specified, but I think we default the device name, I\u0027m just not sure if that happens before this is called. It doesn\u0027t help that nearly all Tempest tests that do boot from volume or attach are specifying a device name and not hitting the automatic device name path in the code.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"863d4c736e3bc2f209cce851a24e82955fded481","unresolved":false,"context_lines":[{"line_number":3985,"context_line":"        if values[\u0027device_name\u0027]:"},{"line_number":3986,"context_line":"            query \u003d _block_device_mapping_get_query(context, session\u003dsession)"},{"line_number":3987,"context_line":"            result \u003d query.filter_by(instance_uuid\u003dvalues[\u0027instance_uuid\u0027],"},{"line_number":3988,"context_line":"                                     device_name\u003dvalues[\u0027device_name\u0027]).first()"},{"line_number":3989,"context_line":""},{"line_number":3990,"context_line":"        if result:"},{"line_number":3991,"context_line":"            result.update(values)"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_7e0ae07a","line":3988,"in_reply_to":"fa69d971_f7183c77","updated":"2016-01-05 14:30:57.000000000","message":"I will not touch this one in this change as it is not related to multi attach at all as far as I can see. I can do it in a follow up patch though if you think it\u0027s needed.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d3bd40c4696eb216aaffb9cbc7c78aa870dbd87b","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4036,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4037,"context_line":"                 filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":4038,"context_line":"                 first()"},{"line_number":4039,"context_line":""},{"line_number":4040,"context_line":""},{"line_number":4041,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_7cd433bc","line":4038,"updated":"2016-01-05 12:42:49.000000000","message":"Rubbish, I just spotted this one. I think we should make this one_or_none() as well, just to be sure. Mostly because the DB has no constraint on here.\n\nNow this might mean if we hit a race, we keep hitting this exception. But I much prefer that to data corruption.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"863d4c736e3bc2f209cce851a24e82955fded481","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4036,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4037,"context_line":"                 filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":4038,"context_line":"                 first()"},{"line_number":4039,"context_line":""},{"line_number":4040,"context_line":""},{"line_number":4041,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_9e0f9487","line":4038,"in_reply_to":"fa69d971_7cd433bc","updated":"2016-01-05 14:30:57.000000000","message":"Done","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"743b5b8ea7a3ffde74f098161c4135d0e0c80b9b","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4036,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4037,"context_line":"                 filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":4038,"context_line":"                 first()"},{"line_number":4039,"context_line":""},{"line_number":4040,"context_line":""},{"line_number":4041,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_ff89e726","line":4038,"in_reply_to":"fa69d971_7cd433bc","updated":"2016-01-05 15:12:18.000000000","message":"If we are worried about this, it seems we should be considering reviving Dan Smith\u0027s bdm.uuid patches.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"4b6b5e88850ad930c033af69aa78778ee78eeaab","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4036,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4037,"context_line":"                 filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":4038,"context_line":"                 first()"},{"line_number":4039,"context_line":""},{"line_number":4040,"context_line":""},{"line_number":4041,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_e0961fef","line":4038,"in_reply_to":"fa69d971_ff89e726","updated":"2016-01-05 17:09:16.000000000","message":"+1 to reviving those patches.  We\u0027re in a spot where there\u0027s no reliable way to look up a bdm for an instance in all situations.  Adding more ways to lookup a bdm without having even basic uniqueness constraints adds more potential for encountering the races we\u0027ve been working around.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"4e4f15dc536ee40c829d34c64954cade9e59ebfc","unresolved":false,"context_lines":[{"line_number":4049,"context_line":"    except MultipleResultsFound:"},{"line_number":4050,"context_line":"        raise exception.VolumeHasMultipleBDMs(id\u003dvolume_id)"},{"line_number":4051,"context_line":""},{"line_number":4052,"context_line":"    return block_device_mapping"},{"line_number":4053,"context_line":""},{"line_number":4054,"context_line":""},{"line_number":4055,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":30,"id":"fa69d971_11741ccd","line":4052,"updated":"2016-01-04 18:34:26.000000000","message":"Nit, I would just call return inside the try block, but this is cool.","commit_id":"f5f002f881c5b8083b5e9a9b382dcf5ebc698440"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b39afab79f674c20768e0c079fa82a1507c04194","unresolved":false,"context_lines":[{"line_number":4038,"context_line":"                     filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":4039,"context_line":"                     one_or_none()"},{"line_number":4040,"context_line":"    except MultipleResultsFound:"},{"line_number":4041,"context_line":"        raise exception.VolumeHasMultipleBDMs(id\u003dvolume_id)"},{"line_number":4042,"context_line":""},{"line_number":4043,"context_line":""},{"line_number":4044,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_ecea980e","line":4041,"updated":"2016-01-05 18:09:23.000000000","message":"I\u0027m wondering if this should be InvalidVolume since that\u0027s what the get_by_volume_id method in the BDM object was previously raising if you got a bdm that didn\u0027t match the instance uuid you had provided.\n\nAlso, how you you get MultipleResultsFound from sqlalchemy if you\u0027re using one_or_none()?","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3ab0c85a25a3ad594b385b6bcd6db14327b0b9b3","unresolved":false,"context_lines":[{"line_number":4038,"context_line":"                     filter_by(instance_uuid\u003dinstance_uuid).\\"},{"line_number":4039,"context_line":"                     one_or_none()"},{"line_number":4040,"context_line":"    except MultipleResultsFound:"},{"line_number":4041,"context_line":"        raise exception.VolumeHasMultipleBDMs(id\u003dvolume_id)"},{"line_number":4042,"context_line":""},{"line_number":4043,"context_line":""},{"line_number":4044,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_cc11d405","line":4041,"in_reply_to":"fa69d971_ecea980e","updated":"2016-01-05 18:12:00.000000000","message":"Nevermind the 2nd question, I see that one_or_none raises MultipleResultsFound if there are multiple:\n\nhttp://docs.sqlalchemy.org/en/rel_1_0/orm/query.html?highlight\u003dmultipleresultsfound","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c7e2d1b70aa5c7d3504f34cf9f61669e01e21dd0","unresolved":false,"context_lines":[{"line_number":4042,"context_line":"        columns_to_join\u003dNone):"},{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"            filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"            first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"da6ed579_9531150e","line":4045,"updated":"2016-01-12 19:58:56.000000000","message":"This is an unrelated change, please undo.","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4e7d0ab61bdfe6613e1c9324c6e885fd29326b6b","unresolved":false,"context_lines":[{"line_number":4042,"context_line":"        columns_to_join\u003dNone):"},{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"            filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"            first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"da6ed579_d076bb3d","line":4045,"in_reply_to":"da6ed579_9531150e","updated":"2016-01-12 20:21:54.000000000","message":"Done","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10cc483aa4d46d10f2ebdf9ee9c9bc3ab8a01a61","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_6a3c8ae2","line":4046,"updated":"2016-01-13 15:37:33.000000000","message":"I still think we should use one_or_none() instead of first here.\n\nWe could make the new exception a subclass of VolumeBDMNotFound to see if that helps the backwards compatibility.","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"96c8383687228e128273e5c8f1837731024f934c","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_2aac499f","line":4046,"in_reply_to":"da6ed579_054b81b1","updated":"2016-01-13 17:37:23.000000000","message":"We just should avoid to call this. We cannot change this here, i was pointed out earlier, this is why I changed back to the original code. In the follow up patches the get_by_volume_id calls are either updated to pass instance_uuid or blocked (only in one case for multiattach volumes). Until we cannot refactor volume_snapshot create that should be acceptable to block for the multiattach case. So this patch does everything that can be done here, the problem is not this layer IMHO.","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a667ba3d4c228f9b4498838fed0353e6c1f11795","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_054b81b1","line":4046,"in_reply_to":"da6ed579_45cbb972","updated":"2016-01-13 15:44:38.000000000","message":"actually dan is more accurate, as there is also the case where you return something attached to another instance, and your DBM has gone, but multi-attach has broken everything.","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"39e375d1ec3512200ef51b818d4aac05ec89ece7","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_a0006302","line":4046,"in_reply_to":"da6ed579_6a3c8ae2","updated":"2016-01-13 15:49:19.000000000","message":"I\u0027m trying to figure out the failure scenario here, at least wrt mixed computes (assume mitaka API, one liberty compute and one mitaka compute).\n\nThis DB API is only called (I think) in the volume_snapshot_create in the compute API, which in the following change is blocked from multiattach volumes:\n\nhttps://review.openstack.org/#/c/193134/69/nova/compute/api.py\n\nSo in what scenario do we not have an instance uuid in the BDM.get_by_volume method and have to resort to calling this method and risk getting back multiple BDMs for the same volume (but on different instances)?","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0f1c843b497a832ef6aecc66c56bdc150e0f0314","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_45cbb972","line":4046,"in_reply_to":"da6ed579_6a3c8ae2","updated":"2016-01-13 15:39:39.000000000","message":"This is basically the same as dan\u0027s comment about this breaking the object contract.","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"233e811d6dd9d8cbaa7c837a68403eb82b56adec","unresolved":false,"context_lines":[{"line_number":4043,"context_line":"    return _block_device_mapping_get_query(context,"},{"line_number":4044,"context_line":"            columns_to_join\u003dcolumns_to_join).\\"},{"line_number":4045,"context_line":"                 filter_by(volume_id\u003dvolume_id).\\"},{"line_number":4046,"context_line":"                 first()"},{"line_number":4047,"context_line":""},{"line_number":4048,"context_line":""},{"line_number":4049,"context_line":"@require_context"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_20cc7369","line":4046,"in_reply_to":"da6ed579_a0006302","updated":"2016-01-13 15:55:03.000000000","message":"OK, alaski pointed out where this falls down in liberty compute, there are three cases where in liberty compute we\u0027re calling objects.BlockDeviceMapping.get_by_volume_id without the instance:\n\n1. remove_volume_connection: https://github.com/openstack/nova/blob/stable/liberty/nova/compute/manager.py#L4879\n\n2. detach: https://github.com/openstack/nova/blob/stable/liberty/nova/compute/manager.py#L4700\n\n3. swap: https://github.com/openstack/nova/blob/stable/liberty/nova/compute/manager.py#L4829\n\n--\n\nSo if your mitaka API is allowing multiattach on a volume and you have a liberty compute instance that is attached to that volume, then when we want to detach or swap it, we might get the BDM for the wrong instance and things are broken.\n\nAnd we can\u0027t just patch those on stable/liberty and release and assume people will be at that version of liberty compute code before they upgrade their API to mitaka and start using it.\n\nSo unfortunately it sounds like, unless we\u0027re all missing something, or can pin which computes we can send the requests to (as Dan pointed out at some point), we\u0027d have to bake multiattach into the computes in mitaka and then enable the API in newton so we don\u0027t break rolling upgrades from liberty to mitaka.","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"}],"nova/objects/block_device.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e9b693269864fe28a32b219b85e216b8ab8f6ec3","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    @base.remotable_classmethod"},{"line_number":208,"context_line":"    def get_by_instance_and_volume_id(cls, context, volume_id, instance_uuid,"},{"line_number":209,"context_line":"                                      expected_attrs\u003dNone):"},{"line_number":210,"context_line":"        db_bdm \u003d db.block_device_mapping_get_by_instance_and_volume_id("},{"line_number":211,"context_line":"                context, volume_id, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_25779fa0","line":208,"updated":"2015-07-07 11:18:13.000000000","message":"please add in unit tests for this new method","commit_id":"4cf6abdc344370e2aa0457d06a72479773ae4284"},{"author":{"_account_id":13909,"name":"Chris Gacsal","email":"chris.gacsal@gmail.com","username":"chrisg"},"change_message_id":"9e7bfb8d074ded34d138a0ebe566dde79bd7ebd6","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    @base.remotable_classmethod"},{"line_number":208,"context_line":"    def get_by_instance_and_volume_id(cls, context, volume_id, instance_uuid,"},{"line_number":209,"context_line":"                                      expected_attrs\u003dNone):"},{"line_number":210,"context_line":"        db_bdm \u003d db.block_device_mapping_get_by_instance_and_volume_id("},{"line_number":211,"context_line":"                context, volume_id, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_a3eba077","line":208,"in_reply_to":"ba3cc151_25779fa0","updated":"2015-07-07 14:13:01.000000000","message":"Done","commit_id":"4cf6abdc344370e2aa0457d06a72479773ae4284"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e91c87ce919e5665981358c055308d0b3ecbec27","unresolved":false,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    @base.remotable_classmethod"},{"line_number":190,"context_line":"    def get_by_volume_id(cls, context, volume_id,"},{"line_number":191,"context_line":"                         instance_uuid\u003dNone, expected_attrs\u003dNone):"},{"line_number":192,"context_line":"        if expected_attrs is None:"},{"line_number":193,"context_line":"            expected_attrs \u003d []"},{"line_number":194,"context_line":"        db_bdm \u003d db.block_device_mapping_get_by_volume_id("}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_606e7501","line":191,"updated":"2015-08-04 11:07:59.000000000","message":"So thinking through things, I could see us deprecating this method, and adding your new method, to be removed at the next major object version bump.\n\nBut I feel like we need to look at removing all uses of get_by_volume_id in the code base, if we want to do that.\n\nIf we don\u0027t want to do that, why can\u0027t we just use the instance_uuid optional arg, and make that more aware of multiple attachments?","commit_id":"3151eb5c2f08df1869da9a7be5da1f9548914d1d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b19f3ea82a699c5cc883411121e18112fb1b9686","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        if not db_bdm:"},{"line_number":197,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":198,"context_line":"        # NOTE (ndipanov): Move this to the db layer into a"},{"line_number":199,"context_line":"        # get_by_instance_and_volume_id method"},{"line_number":200,"context_line":"        if instance_uuid and instance_uuid !\u003d db_bdm[\u0027instance_uuid\u0027]:"},{"line_number":201,"context_line":"            raise exception.InvalidVolume("},{"line_number":202,"context_line":"                    reason\u003d_(\"Volume does not belong to the \""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_3c275bb7","line":199,"updated":"2015-07-28 17:21:15.000000000","message":"How does this related to this method? it seems your new method duplicates the existing method?\n\nWhat happens to the existing callers of this method, when we now return multiple attachments?\n\nMaybe its clear if I read your next change, and maybe this is for upgrade reasons, but its not clear in isolation here.","commit_id":"3151eb5c2f08df1869da9a7be5da1f9548914d1d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ef27bcc8eb2c83eb34277958a500d558bd661fb4","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                         instance_uuid\u003dNone, expected_attrs\u003dNone):"},{"line_number":202,"context_line":"        if expected_attrs is None:"},{"line_number":203,"context_line":"            expected_attrs \u003d []"},{"line_number":204,"context_line":"        db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":205,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":206,"context_line":"        if not db_bdm:"},{"line_number":207,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_257596f8","line":204,"updated":"2015-12-17 11:04:39.000000000","message":"this might now return multiples, in bad cases, we should really rase an exception. Although this is currently not possible because the DB uses first(), so I have commented on the previous patch about adding that sort of protection.","commit_id":"6769d057d52d0cd01a6f264c230e7256c09478f1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"617ce3fbe465076da89f484db5275d9227329bc2","unresolved":false,"context_lines":[{"line_number":201,"context_line":"                         instance_uuid\u003dNone, expected_attrs\u003dNone):"},{"line_number":202,"context_line":"        if expected_attrs is None:"},{"line_number":203,"context_line":"            expected_attrs \u003d []"},{"line_number":204,"context_line":"        db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":205,"context_line":"                context, volume_id, _expected_cols(expected_attrs))"},{"line_number":206,"context_line":"        if not db_bdm:"},{"line_number":207,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_2be327a5","line":204,"in_reply_to":"3a7e1126_257596f8","updated":"2015-12-17 11:33:15.000000000","message":"I added a comment to yours in the previous patch, I will proceed according to the agreements there.","commit_id":"6769d057d52d0cd01a6f264c230e7256c09478f1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"4f7e4473e41d04fa1741817c3faa2a2dd88cfead","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        if not db_bdm:"},{"line_number":207,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":208,"context_line":"        # NOTE (ndipanov): Move this to the db layer into a"},{"line_number":209,"context_line":"        # get_by_instance_and_volume_id method"},{"line_number":210,"context_line":"        if instance_uuid and instance_uuid !\u003d db_bdm[\u0027instance_uuid\u0027]:"},{"line_number":211,"context_line":"            raise exception.InvalidVolume("},{"line_number":212,"context_line":"                    reason\u003d_(\"Volume does not belong to the \""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_e52b0e81","line":209,"updated":"2015-12-17 11:03:16.000000000","message":"why not just fix this method here?\n\nWe could just say, if instance_uuid not None, then call db.block_device_mapping_get_by_instance_and_volume_id?","commit_id":"6769d057d52d0cd01a6f264c230e7256c09478f1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"617ce3fbe465076da89f484db5275d9227329bc2","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        if not db_bdm:"},{"line_number":207,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":208,"context_line":"        # NOTE (ndipanov): Move this to the db layer into a"},{"line_number":209,"context_line":"        # get_by_instance_and_volume_id method"},{"line_number":210,"context_line":"        if instance_uuid and instance_uuid !\u003d db_bdm[\u0027instance_uuid\u0027]:"},{"line_number":211,"context_line":"            raise exception.InvalidVolume("},{"line_number":212,"context_line":"                    reason\u003d_(\"Volume does not belong to the \""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_b6269ab8","line":209,"in_reply_to":"3a7e1126_e52b0e81","updated":"2015-12-17 11:33:15.000000000","message":"Done","commit_id":"6769d057d52d0cd01a6f264c230e7256c09478f1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5fe7c681a129cf4facad2ba572c29bb113043a32","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    @base.remotable_classmethod"},{"line_number":199,"context_line":"    def get_by_volume_id(cls, context, volume_id,"},{"line_number":200,"context_line":"                         instance_uuid\u003dNone, expected_attrs\u003dNone):"},{"line_number":201,"context_line":"        if expected_attrs is None:"},{"line_number":202,"context_line":"            expected_attrs \u003d []"},{"line_number":203,"context_line":"        if instance_uuid:"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa69d971_424df63c","line":200,"updated":"2016-01-04 11:28:50.000000000","message":"Nit: We should have a comment that says to stop instance_uuid being optional when in the next major version of this object.","commit_id":"b12bd48c2dab343c1b030d7c89fe30f3be208a40"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"99981b294ce3e23274d70d953b404c03c1a0e5dc","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    @base.remotable_classmethod"},{"line_number":199,"context_line":"    def get_by_volume_id(cls, context, volume_id,"},{"line_number":200,"context_line":"                         instance_uuid\u003dNone, expected_attrs\u003dNone):"},{"line_number":201,"context_line":"        if expected_attrs is None:"},{"line_number":202,"context_line":"            expected_attrs \u003d []"},{"line_number":203,"context_line":"        if instance_uuid:"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa69d971_4898b7c0","line":200,"in_reply_to":"fa69d971_424df63c","updated":"2016-01-04 16:06:41.000000000","message":"Done","commit_id":"b12bd48c2dab343c1b030d7c89fe30f3be208a40"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5fe7c681a129cf4facad2ba572c29bb113043a32","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                         context, volume_id,"},{"line_number":206,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":209,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        if not db_bdm:"},{"line_number":211,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa69d971_a2807247","line":208,"updated":"2016-01-04 11:28:50.000000000","message":"So we have a problem here, this call will silently just pick the first bdm, if there happen to be two of them.\n\nRather than check the instance_uuid, why not make this DB call return the full list of bdms, and check here there is only one of them?","commit_id":"b12bd48c2dab343c1b030d7c89fe30f3be208a40"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"49067934b221266d3707116f1e4849e9f214c960","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                         context, volume_id,"},{"line_number":206,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":209,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        if not db_bdm:"},{"line_number":211,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa69d971_c2bb26e4","line":208,"in_reply_to":"fa69d971_a2807247","updated":"2016-01-04 11:31:22.000000000","message":"I have added a comment on the previous patch to make it clearer what I mean.","commit_id":"b12bd48c2dab343c1b030d7c89fe30f3be208a40"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"99981b294ce3e23274d70d953b404c03c1a0e5dc","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                         context, volume_id,"},{"line_number":206,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":209,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        if not db_bdm:"},{"line_number":211,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa69d971_d563dfda","line":208,"in_reply_to":"fa69d971_c2bb26e4","updated":"2016-01-04 16:06:41.000000000","message":"Done","commit_id":"b12bd48c2dab343c1b030d7c89fe30f3be208a40"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d20e54cf1cb8855f4996fc5b74ead985502bd0e9","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from sqlalchemy.orm.exc import MultipleResultsFound"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log as logging"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_dfba150a","line":15,"updated":"2016-01-04 16:12:12.000000000","message":"we should not import this stuff here, it should happen in the sqlalchemy db driver only. (also we should import modules, not classes)","commit_id":"7d6d9dc9ca8854ad9def40f0ab7074accd737c56"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"84904c47bd5fc5b87f68e4acbcf1269c4ad2d7e1","unresolved":false,"context_lines":[{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from sqlalchemy.orm.exc import MultipleResultsFound"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log as logging"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_11f5fc81","line":15,"in_reply_to":"fa69d971_dfba150a","updated":"2016-01-04 18:28:06.000000000","message":"Done","commit_id":"7d6d9dc9ca8854ad9def40f0ab7074accd737c56"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"0fcdbb796715180802cfa6dc549c49bdb8040a13","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    # Version 1.13: Instance version 1.21"},{"line_number":58,"context_line":"    # Version 1.14: Instance version 1.22"},{"line_number":59,"context_line":"    # Version 1.15: Instance version 1.23"},{"line_number":60,"context_line":"    # Version 1.16: Change get_by_volume_id method"},{"line_number":61,"context_line":"    VERSION \u003d \u00271.16\u0027"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_13f06f90","line":60,"range":{"start_line":60,"start_character":20,"end_line":60,"end_character":50},"updated":"2016-01-05 17:24:04.000000000","message":"We can\u0027t really do this with objects. There is nothing in the logic allowing us to do \"the other thing\" for versions prior to this really.\n\nThe only real option is to add a new method while we keep on handling the old one as well","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"0fcdbb796715180802cfa6dc549c49bdb8040a13","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        if expected_attrs is None:"},{"line_number":202,"context_line":"            expected_attrs \u003d []"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        # NOTE(ildikov): instance_uuid should be mandatory from the next"},{"line_number":205,"context_line":"        # major version of this object."},{"line_number":206,"context_line":"        if instance_uuid:"},{"line_number":207,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_instance_and_volume_id("},{"line_number":208,"context_line":"                         context, volume_id,"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_8504184a","line":205,"range":{"start_line":204,"start_character":7,"end_line":205,"end_character":39},"updated":"2016-01-05 17:24:04.000000000","message":"So If this is the case - we should actually add a new method and mark this one as deprecated, instead of just doing this as stated above.\n\nIt is not really possible to reliably use the old method anymore, for example here: \n\nhttps://github.com/openstack/nova/blob/300a2edf9f0c65aef0f15db032ad9c8cab80c517/nova/compute/api.py#L3267\n\nonce this series lands (although it\u0027s not really a massively used feature).","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c901adf8c39ff10f45ad3560fe85f1ac9de2d749","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        if expected_attrs is None:"},{"line_number":202,"context_line":"            expected_attrs \u003d []"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        # NOTE(ildikov): instance_uuid should be mandatory from the next"},{"line_number":205,"context_line":"        # major version of this object."},{"line_number":206,"context_line":"        if instance_uuid:"},{"line_number":207,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_instance_and_volume_id("},{"line_number":208,"context_line":"                         context, volume_id,"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_76af592a","line":205,"range":{"start_line":204,"start_character":7,"end_line":205,"end_character":39},"in_reply_to":"fa69d971_8504184a","updated":"2016-01-05 17:45:27.000000000","message":"Good point, we also need to update the spec to point out that the volume_snapshot_create API is going to have to be updated to return a 400 if a multiattach volume is passed in since we don\u0027t know which instance to use if there are multiple - or the API would have to be updated to provide an instance if it\u0027s a multiattach volume, but that could come later if we really wanted. volume_snapshot_create is an edge case that only the libvirt driver implements.","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ba2dd4afb8f2f66d0c3bbe1190e391ed9b87e9cc","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_ac5580c2","line":212,"updated":"2016-01-05 18:11:07.000000000","message":"hmm, I totally missed that the new exception does break the versioning here really, I guess it will be clearer to go back to making the new world, and keeping the old method the same, for compatibility. You could always raise VolumeBDMNotFound for finding multiple in the compatibility case.","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"40f768386bbac0d68868c9c5e467f50d9a04c107","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_6cd30805","line":212,"in_reply_to":"fa69d971_ac5580c2","updated":"2016-01-05 18:19:14.000000000","message":"In my opinion the DB functions should be changed back to the original behaviour and the race issues should be handled separately. The multi attach use case will not introduce more issues with BDMs, than how many we had earlier.\n\nIf the DB functions behave as earlier this function will also behave as earlier, even the InvalidVolume exception can be added back if needed, like how it was in the original one.","commit_id":"462808e82c917befe38944c8a9e3669ab64cc6a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"96faeaa1c9f273d1a6c9156f649f4fb4f682febe","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_8c9e6005","line":212,"updated":"2016-01-06 09:52:16.000000000","message":"But if this is done on multi-attach, and we have two bdms, now it will just use first() and pick a BDM at random.\n\nThis is likely to lead to data corruption, without any warning or error being logged or reported.","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"9cd0bfe6af03b5e7bfcbe9922333c976d3dfe80a","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_3ff9bf4e","line":212,"in_reply_to":"fa69d971_1a8db4d5","updated":"2016-01-11 11:44:28.000000000","message":"As for the problematic scenarios they are blocked in the compute_api, like boot from multiattach volume and create volume snapshot for multiattach volumes. In this sense the supported upgrade path is safe and working for these.","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"b2002fb02854e0391eea67fd1c29c8db1e439c10","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_75a73da7","line":212,"in_reply_to":"fa69d971_3ff9bf4e","updated":"2016-01-11 19:11:12.000000000","message":"As for the BDM issues, I wonder whether we could introduce multiattach support as experimental and have a config option to enable it. We can have it disabled by default in Mitaka and enable it in Newton. This way we can have a code in and add a comment that it should be enabled if the compute services are on the same version.\n\nThe two operations I mentioned in the previous comments would work anyway as they are blocked in compute_api.","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bed21cf2e826ad208aa7f1ba4222576a212d407f","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_bf15889c","line":212,"in_reply_to":"fa69d971_8c9e6005","updated":"2016-01-06 10:11:42.000000000","message":"Yeah this is a tricky one for sure. If we introduce the possiblity of a single volume_id belonging to 2 different BDMs of 2 different insances, then there is no way for the new code to know which one to give to the old code which just asks for the volume without providing any info about the instance.\n\nNot sure how to do this - maybe we\u0027d have to land first and then backport a patch to liberty stable that adds logic to all the call sites (or decorate the method) before we can do this change.\n\nNasty...","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22dc4ca4b6c0176389164aaecdd4e98aceb56c4a","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_c52e5e8b","line":212,"in_reply_to":"fa69d971_9de76cab","updated":"2016-01-06 15:04:28.000000000","message":"I feel like the .first() was added to make sure that if we ever had two (maybe due to a race, because we don\u0027t have any uniqueness constraint to prevent it), that we pick one and go with it.\n\nThe problem is that by you introducing the new concept of intentionally having more than one, the problem extends even outside the object rules for compatibility. I think there\u0027s probably code that sits above this that will expect only a single BDM/instance for a given volume.\n\nEven though I\u0027d fight hard against backporting a hack to liberty to enable us to do this without breaking it, we definitely can\u0027t backport more invasive changes to the stuff that sits above this, IMHO.","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"9521188561a131ada99f6c838dc9b1d204b05079","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_9de76cab","line":212,"in_reply_to":"fa69d971_bf15889c","updated":"2016-01-06 10:59:35.000000000","message":"Was first() added because of duplicated BDMs? Or it could happen earlier too to have BDMs for multiple instances for the same volume because of race conditions? If that could\u0027ve happened earlier this case is just slightly different. I mean now it can happen more often, although we will block several actions with multi-attach, like snapshot_create. I think in almost all other cases we have instance_uuid in place. So it should not happen more often by the end of the day.\n\nThat check in the earlier version shows that it could\u0027ve happened earlier too to return a random BDM.\n\nAlthough later on I agree that it would be better to find a way to do this more reliably.","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"159f3903fd442ab9a4d50c452cac56115b3c1ec7","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                         instance_uuid, _expected_cols(expected_attrs))"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_volume_id("},{"line_number":212,"context_line":"                         context, volume_id, _expected_cols(expected_attrs))"},{"line_number":213,"context_line":"        if not db_bdm:"},{"line_number":214,"context_line":"            raise exception.VolumeBDMNotFound(volume_id\u003dvolume_id)"},{"line_number":215,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"}],"source_content_type":"text/x-python","patch_set":37,"id":"fa69d971_1a8db4d5","line":212,"in_reply_to":"fa69d971_c52e5e8b","updated":"2016-01-06 16:05:42.000000000","message":"In case of the current code the only place the instance_uuid cannot be specified is the volume_snapshot_create, which will be blocked for volumes that support multi-attach. So in that case we still only have the race condition in place.\n\nSo basically the change I made here has nothing to do with the situation. Unfortunately the problem that the next patch introduces cannot easily be fixed here because of the compatibility issues.\n\nLiberty does not support multi-attach. Can we keep things in one place that much to not have upgrade path issues with this? I mean the compute manager layer as the API is practically involved only by the translation functions, nothing else at the moment. Besides the behaviour change of course if the the request gets through compute.manager. Does this make sense?","commit_id":"8b0899bd3e18685fe387ce626eb591f25b8c84b1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1515f9993e4f09d827d09ab5bb2b9be390b05fdf","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        if instance_uuid and instance_uuid !\u003d db_bdm[\u0027instance_uuid\u0027]:"},{"line_number":210,"context_line":"            raise exception.InvalidVolume("},{"line_number":211,"context_line":"                    reason\u003d_(\"Volume does not belong to the \""},{"line_number":212,"context_line":"                             \"requested instance.\"))"},{"line_number":213,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"},{"line_number":214,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"da6ed579_cb812a91","side":"PARENT","line":212,"updated":"2016-01-12 20:53:16.000000000","message":"Anything that does this query without instance_uuid set is expecting to get back a BDM that belongs to them, unless something really bad is going on. By suddenly making it completely legit to return other instances\u0027 BDMs here, I feel like you\u0027re breaking the contract.\n\nShouldn\u0027t we have a flag on the BDM or something and make sure we\u0027re filtering out multi-attach volumes from this response?","commit_id":"fb3a062cecfad6ff74510bcb411db37de51ae059"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"eda832107802c8033f3ac29f6fde48da68bf2984","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        if instance_uuid and instance_uuid !\u003d db_bdm[\u0027instance_uuid\u0027]:"},{"line_number":210,"context_line":"            raise exception.InvalidVolume("},{"line_number":211,"context_line":"                    reason\u003d_(\"Volume does not belong to the \""},{"line_number":212,"context_line":"                             \"requested instance.\"))"},{"line_number":213,"context_line":"        return cls._from_db_object(context, cls(), db_bdm,"},{"line_number":214,"context_line":"                                   expected_attrs\u003dexpected_attrs)"},{"line_number":215,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"da6ed579_c676013e","side":"PARENT","line":212,"in_reply_to":"da6ed579_cb812a91","updated":"2016-01-12 21:07:27.000000000","message":"There\u0027s already a blueprint to set mutliattach after the volume creation, so I would not store it in the BDM. My code change does not ruin anything compared to this version here. As if the instance_uuid is specified in the new version they will get back the right BDM or nothing.\n\nIf it\u0027s not, then they can get back a random BDM if we enable multiattach (next two patches). But if multiattach is enabled only for Mitaka or later version of computes, then we don\u0027t get into trouble as this function is always called with an instance_uuid, or not called at all for multiattach volume cases. The changes are in the follow-up patch.","commit_id":"fb3a062cecfad6ff74510bcb411db37de51ae059"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"770dd6d4619648b80525ee5e663e5fb85b239806","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        # NOTE(ildikov): \u0027instance_uuid\u0027 cannot be mandatory as we don\u0027t"},{"line_number":204,"context_line":"        # know it in case of volume_snapshot_create:"},{"line_number":205,"context_line":"        # https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3267"},{"line_number":206,"context_line":"        if instance_uuid:"},{"line_number":207,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_instance_and_volume_id("},{"line_number":208,"context_line":"                         context, volume_id,"}],"source_content_type":"text/x-python","patch_set":42,"id":"da6ed579_356b8920","line":205,"range":{"start_line":205,"start_character":10,"end_line":205,"end_character":81},"updated":"2016-01-12 19:52:47.000000000","message":"This is going to change, just remove it.","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"4e7d0ab61bdfe6613e1c9324c6e885fd29326b6b","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        # NOTE(ildikov): \u0027instance_uuid\u0027 cannot be mandatory as we don\u0027t"},{"line_number":204,"context_line":"        # know it in case of volume_snapshot_create:"},{"line_number":205,"context_line":"        # https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3267"},{"line_number":206,"context_line":"        if instance_uuid:"},{"line_number":207,"context_line":"            db_bdm \u003d db.block_device_mapping_get_by_instance_and_volume_id("},{"line_number":208,"context_line":"                         context, volume_id,"}],"source_content_type":"text/x-python","patch_set":42,"id":"da6ed579_107d331c","line":205,"range":{"start_line":205,"start_character":10,"end_line":205,"end_character":81},"in_reply_to":"da6ed579_356b8920","updated":"2016-01-12 20:21:54.000000000","message":"Done","commit_id":"69b35478da928b2e62ee1ad2519582dc39968ce8"}],"nova/tests/unit/objects/test_block_device.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10cc483aa4d46d10f2ebdf9ee9c9bc3ab8a01a61","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    @mock.patch.object(db,"},{"line_number":131,"context_line":"                       \u0027block_device_mapping_get_by_instance_and_volume_id\u0027)"},{"line_number":132,"context_line":"    def test_get_by_volume_instance_uuid_missmatch(self, get_by_inst_vol_id):"},{"line_number":133,"context_line":"        get_by_inst_vol_id.return_value \u003d None"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        self.assertRaises(exception.VolumeBDMNotFound,"}],"source_content_type":"text/x-python","patch_set":46,"id":"da6ed579_c5780992","line":132,"updated":"2016-01-13 15:37:33.000000000","message":"I am not sure this is really the same test any more?\n\nIs this not a duplicate of the test on line 180. I prefer the name of the test on line 180.","commit_id":"d118cf8c2e045583efbf11372a542bb87e324675"}],"nova/tests/unit/objects/test_objects.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"4f7e4473e41d04fa1741817c3faa2a2dd88cfead","unresolved":false,"context_lines":[{"line_number":1192,"context_line":"    \u0027VirtCPUTopology\u0027: \u00271.0-fc694de72e20298f7c6bab1083fd4563\u0027,"},{"line_number":1193,"context_line":"    \u0027VirtualInterface\u0027: \u00271.0-19921e38cba320f355d56ecbf8f29587\u0027,"},{"line_number":1194,"context_line":"    \u0027VirtualInterfaceList\u0027: \u00271.0-9750e2074437b3077e46359102779fc6\u0027,"},{"line_number":1195,"context_line":"    \u0027VolumeUsage\u0027: \u00271.0-6c8190c46ce1469bb3286a1f21c2e475\u0027"},{"line_number":1196,"context_line":"}"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_e5e22ed5","line":1195,"updated":"2015-12-17 11:03:16.000000000","message":"Please don\u0027t make this change here.","commit_id":"6769d057d52d0cd01a6f264c230e7256c09478f1"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"617ce3fbe465076da89f484db5275d9227329bc2","unresolved":false,"context_lines":[{"line_number":1192,"context_line":"    \u0027VirtCPUTopology\u0027: \u00271.0-fc694de72e20298f7c6bab1083fd4563\u0027,"},{"line_number":1193,"context_line":"    \u0027VirtualInterface\u0027: \u00271.0-19921e38cba320f355d56ecbf8f29587\u0027,"},{"line_number":1194,"context_line":"    \u0027VirtualInterfaceList\u0027: \u00271.0-9750e2074437b3077e46359102779fc6\u0027,"},{"line_number":1195,"context_line":"    \u0027VolumeUsage\u0027: \u00271.0-6c8190c46ce1469bb3286a1f21c2e475\u0027"},{"line_number":1196,"context_line":"}"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_76b4b2f1","line":1195,"in_reply_to":"3a7e1126_e5e22ed5","updated":"2015-12-17 11:33:15.000000000","message":"Done by reflex, I will remove this in the next patch set.","commit_id":"6769d057d52d0cd01a6f264c230e7256c09478f1"}]}