)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"dae95484e7b63e9e775f880144b795b0bf65bb9b","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Microversion 2.20 allows attaching a volume to a shelved-offloaed"},{"line_number":10,"context_line":"instance. Since a shelved-offloaded instance does not have a host"},{"line_number":11,"context_line":"associated it, a block device name is not reserved on the compute host"},{"line_number":12,"context_line":"for the instance, since there is no compute host for the instance,"},{"line_number":13,"context_line":"so BlockDeviceMapping.device_name is just set to None."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When showing details on this attached volume for a shelved offloaded"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7ffa3b31_eda39c4e","line":12,"range":{"start_line":12,"start_character":8,"end_line":12,"end_character":23},"updated":"2017-04-17 17:01:13.000000000","message":"s/instance, since/instance. Since/","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"}],"nova/api/openstack/compute/legacy_v2/contrib/volumes.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"5cd98fe335da37674d2082ab02131ebcd61f4744","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                assigned_mountpoint \u003d bdm.device_name"},{"line_number":276,"context_line":"                break"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"        if assigned_mountpoint is None:"},{"line_number":279,"context_line":"            msg \u003d _(\"volume_id not found: %s\") % volume_id"},{"line_number":280,"context_line":"            raise exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        return {\u0027volumeAttachment\u0027: _translate_attachment_detail_view("},{"line_number":283,"context_line":"            volume_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5aef4532_c7220801","side":"PARENT","line":280,"range":{"start_line":278,"start_character":2,"end_line":280,"end_character":51},"updated":"2016-03-09 01:04:30.000000000","message":"v2 legacy code is deprecated, so it is not necessary to change API behavior at least.","commit_id":"6b2e8ed9bd7bd068b23879a60f634ac94cf563ec"}],"nova/api/openstack/compute/volumes.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"5cd98fe335da37674d2082ab02131ebcd61f4744","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                assigned_mountpoint \u003d bdm.device_name"},{"line_number":285,"context_line":"                break"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"        if assigned_mountpoint is None:"},{"line_number":288,"context_line":"            msg \u003d _(\"volume_id not found: %s\") % volume_id"},{"line_number":289,"context_line":"            raise exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5aef4532_671334b9","side":"PARENT","line":287,"range":{"start_line":287,"start_character":8,"end_line":287,"end_character":39},"updated":"2016-03-09 01:04:30.000000000","message":"This patch changes the existing api behavior, and this case is not defined as microversion exception on our rule.\n\nAccording to the bug report, this problem has appeared since v2.20. So I am fine if this behavior change happens on v2.20+.","commit_id":"6b2e8ed9bd7bd068b23879a60f634ac94cf563ec"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d0716905a82cd7473e3361d332788a6a95addf14","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                assigned_mountpoint \u003d bdm.device_name"},{"line_number":285,"context_line":"                break"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"        if assigned_mountpoint is None:"},{"line_number":288,"context_line":"            msg \u003d _(\"volume_id not found: %s\") % volume_id"},{"line_number":289,"context_line":"            raise exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9ff52f3f_e1e626e9","side":"PARENT","line":287,"range":{"start_line":287,"start_character":8,"end_line":287,"end_character":39},"in_reply_to":"5aef4532_3f192889","updated":"2017-04-13 18:26:14.000000000","message":"I don\u0027t agree that we need a microversion for this change. This code was clearly written before the 2.20 use case. I don\u0027t think one should have to use microversion 2.20+ to view the volumes attached to a server. We should just check if the instance is in shelved_offloaded state and if so, ignore this 404 case.","commit_id":"6b2e8ed9bd7bd068b23879a60f634ac94cf563ec"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"52584e8656516cd6bec77e1d306543e160084821","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                assigned_mountpoint \u003d bdm.device_name"},{"line_number":285,"context_line":"                break"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"        if assigned_mountpoint is None:"},{"line_number":288,"context_line":"            msg \u003d _(\"volume_id not found: %s\") % volume_id"},{"line_number":289,"context_line":"            raise exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5aef4532_3f192889","side":"PARENT","line":287,"range":{"start_line":287,"start_character":8,"end_line":287,"end_character":39},"in_reply_to":"5aef4532_671334b9","updated":"2016-03-09 08:46:43.000000000","message":"I agree, we can\u0027t change the behaviour here for all the versions.","commit_id":"6b2e8ed9bd7bd068b23879a60f634ac94cf563ec"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4caa4b3f1878bc1c8cec554d948edb1045b46840","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                assigned_mountpoint \u003d bdm.device_name"},{"line_number":285,"context_line":"                break"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"        if assigned_mountpoint is None:"},{"line_number":288,"context_line":"            msg \u003d _(\"volume_id not found: %s\") % volume_id"},{"line_number":289,"context_line":"            raise exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":290,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9ff52f3f_41e1529a","side":"PARENT","line":287,"range":{"start_line":287,"start_character":8,"end_line":287,"end_character":39},"in_reply_to":"9ff52f3f_e1e626e9","updated":"2017-04-13 18:33:31.000000000","message":"The one issue might be that for show it looks like our api-ref docs don\u0027t list device as optional. However, I think we could still return the field, but just set to None since that is totally legitimate for the 2.20 case.","commit_id":"6b2e8ed9bd7bd068b23879a60f634ac94cf563ec"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"dae95484e7b63e9e775f880144b795b0bf65bb9b","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_300afbd1","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"updated":"2017-04-17 17:01:13.000000000","message":"I am not sure why we need this code.\nIf assigned_mountpoint is None, the \u0027device\u0027 should be None by calling _translate_attachment_detail_view() without this code.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1248ecc42933c9046c243bf1e406fb1cd7f4e426","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_cdd93523","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"updated":"2017-04-14 00:02:39.000000000","message":"yea this seems consistent. \n\nother way i was thinking not to append \u0027device\u0027 is no mountpoint like LIST does. But that makes SHOW API inconsistent between instance in SHELVED_OFFLOADED and other state.\n\neven it is inconsistent with LIST API, looks fine to me.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a76c6e7ea2d07d730fa424b1b3afb56b73c8b038","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_07a7b151","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_0b57d004","updated":"2017-04-18 08:59:24.000000000","message":"non-None is being checked after server is unshelved. It used to assert when server was in shelved_offload state where no device attached to server.\n\ntempest tests did not add that check before server unshelve because this API used to 400 in that case.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"a91566f2b7c7f0d62ad207a9b8944e483c0731e4","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_0b57d004","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_300afbd1","updated":"2017-04-17 17:26:05.000000000","message":"Now I am a little confused..\nThe corresponding Tempest test requires \"non None\" on this cases(shelve and shelved-offload) for microversion 2.20+ like https://github.com/openstack/tempest/blob/3b46d27c90aa6289724f4c137a0838c89d116b62/tempest/api/compute/volumes/test_attach_volume.py#L241\n\nbut this patch allows None on the response. Maybe now the Tempest test doesn\u0027t run on shelved-offload case on the gate actually, and this patch can pass.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a76c6e7ea2d07d730fa424b1b3afb56b73c8b038","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_67db15ed","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_300afbd1","updated":"2017-04-18 08:59:24.000000000","message":"_translate_attachment_detail_view()  does not include \u0027device\u0027 is mountpoint is None. here we should always return \u0027device\u0027(with none in case of shelved_offload) for consistency.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"ba9bd20385d5bee10a9e435fcce58da7ad2eea3e","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_78bc21bb","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_385b392f","updated":"2017-04-14 00:57:48.000000000","message":"yea +1 on returning it always and with None if no mountpoint as done in this change","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"f800aafdde8324ed3a5305707cc59f442486dba8","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_2f9f4304","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_67db15ed","updated":"2017-04-25 00:30:15.000000000","message":"I see your saying, but I don\u0027t agree with adding None as \u0027device\u0027 for consistency only. According to the code, \u0027device\u0027 doesn\u0027t appear intentionally on a response.\n\n 230\n 231     d[\u0027serverId\u0027] \u003d instance_uuid\n 232     if mountpoint:\n 233         d[\u0027device\u0027] \u003d mountpoint  \u003cHERE\u003e\n 234\n 235     return d\n\nAnd there is a lot of inconsistency on Nova API and we cannot fix it without microversion bumps in general.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a78574a906181b911a69e73a3ef05f9aef16b893","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_385b392f","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_8dbd6d6a","updated":"2017-04-14 00:22:04.000000000","message":"This check goes way back 5 years:\n\nhttps://review.openstack.org/#/c/7406/\n\nAnd makes really no sense why this is here. It actually looks like an uncaught mistake as a result of a refactor.\n\nBefore 2.20 you can\u0027t get a bdm without a device name if it\u0027s attached to the server, because we set it in the API when we create the bdm record. So this check made no sense, and I don\u0027t think it\u0027s something you could hit, or as a user need to code around. I really don\u0027t think we need to consider this codified into the API. It was just never a problem until it became a real use case with 2.20, but we can fix that with a simple vm_state check.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9f6e169dc7d16e521ff79d4897859d5b6db4f60e","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            instance.uuid,"},{"line_number":322,"context_line":"            assigned_mountpoint)}"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if assigned_mountpoint is None and is_shelved_offloaded:"},{"line_number":325,"context_line":"            body[\u0027volumeAttachment\u0027][\u0027device\u0027] \u003d None"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        return body"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_8dbd6d6a","line":325,"range":{"start_line":324,"start_character":0,"end_line":325,"end_character":53},"in_reply_to":"7ffa3b31_cdd93523","updated":"2017-04-14 00:09:16.000000000","message":"We need to return the device key to be backward compatible with the response, since Tempest probably validates that. There is no contract on the value.","commit_id":"45147f0a555ea2a4ed4f03dcb29ccad357dc328c"}]}