)]}'
{"specs/mitaka/approved/cinder-backend-report-discard.rst":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":32,"context_line":"there should be a way to do this. This will improve long term performance"},{"line_number":33,"context_line":"of such back ends."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Project Priority"},{"line_number":36,"context_line":"----------------"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"None"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_20709adb","line":35,"updated":"2015-10-16 14:19:44.000000000","message":"We can drop this section now since we dropped it from the mitaka template.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":32,"context_line":"there should be a way to do this. This will improve long term performance"},{"line_number":33,"context_line":"of such back ends."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Project Priority"},{"line_number":36,"context_line":"----------------"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"None"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_e7bdc602","line":35,"in_reply_to":"5a2ca52d_20709adb","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"\"discard\": True,"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"and the user has set hw_disk_discard in nova.conf then the driver_discard"},{"line_number":49,"context_line":"setting will be set that value."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_2b015b05","line":48,"updated":"2015-10-16 14:19:44.000000000","message":"Your code change doesn\u0027t do this:\n\nhttps://review.openstack.org/#/c/205726/8/nova/virt/libvirt/volume/volume.py\n\nDo you plan on changing that based on this spec?  Because it\u0027s one of the main issues I have with this, which is that discard for volumes is handled via image properties but for disk images it\u0027s via this config option in the libvirt driver, which is not fun from a usability perspective.  And the code that\u0027s up doesn\u0027t check for the image properties at all even though the volume driver connection says it supports discard.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"\"discard\": True,"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"and the user has set hw_disk_discard in nova.conf then the driver_discard"},{"line_number":49,"context_line":"setting will be set that value."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_62b5247c","line":48,"in_reply_to":"5a2ca52d_110cd239","updated":"2015-10-16 22:01:37.000000000","message":"Ok so I\u0027m on board for having this not check the image properties or nova.conf option. I like the idea that if Cinder reported discard\u003dTrue in the attachment properties than it is safe to assume that is the expected behavior.\n\nI do think being able to hot-plug the virtio-scsi controller for the cinder volume (regardless of whether the image had the properties on it or not) would be pretty sweet... but iirc there was some pushback on that in the earlier versions of the code changes for this.\n\nMaybe its something to look into as a next iteration on top of this.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"0bc9529cf56bca8c7700051d319e56f1065ce5d6","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"\"discard\": True,"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"and the user has set hw_disk_discard in nova.conf then the driver_discard"},{"line_number":49,"context_line":"setting will be set that value."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_110cd239","line":48,"in_reply_to":"5a2ca52d_2b015b05","updated":"2015-10-16 14:42:03.000000000","message":"Nothing should be using image properties at all. The only config should be via nova.conf for local storage, or by cinder admin for volume storage.\n\nNow, only certain types of disk support discard, so depending on what disk bus the user has for their VM, discard may or may not actually work. The disk bus can be altered via an image property, but that\u0027s really orthogonal to the actual impl of discard support, which should not concern itself with image properties.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"\"discard\": True,"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"and the user has set hw_disk_discard in nova.conf then the driver_discard"},{"line_number":49,"context_line":"setting will be set that value."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_6b32f3f3","line":49,"updated":"2015-10-16 14:19:44.000000000","message":"s/set that/set to that/","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":46,"context_line":"\"discard\": True,"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"and the user has set hw_disk_discard in nova.conf then the driver_discard"},{"line_number":49,"context_line":"setting will be set that value."},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_678216ca","line":49,"in_reply_to":"5a2ca52d_6b32f3f3","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"926998a7f6a631e7e67a87e9c4702d7fe1dfb853","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"[libvirt]"},{"line_number":99,"context_line":"hw_disk_discard \u003d unmap"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"Developer impact"},{"line_number":102,"context_line":"----------------"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_a1ac59ee","line":100,"updated":"2015-10-13 12:17:00.000000000","message":"The hw_disk_discard nova.conf setting applies to local image storage ie non-cinder storage.\n\nThe use of discard for block devices should be *exclusively* determined by the information received in the cinder connection info.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"[libvirt]"},{"line_number":99,"context_line":"hw_disk_discard \u003d unmap"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"Developer impact"},{"line_number":102,"context_line":"----------------"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_cb954787","line":100,"in_reply_to":"5a2ca52d_2df35875","updated":"2015-10-16 14:19:44.000000000","message":"Agreed, if they are separate, it needs to be clearly explained in the spec *and* I\u0027d suggest that the help text on the existing hw_disk_discard option in tree is updated to also point that out.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d17236c45f53f4935d5ad0fa064699ff54c416a2","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"[libvirt]"},{"line_number":99,"context_line":"hw_disk_discard \u003d unmap"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"Developer impact"},{"line_number":102,"context_line":"----------------"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_2df35875","line":100,"in_reply_to":"5a2ca52d_a1ac59ee","updated":"2015-10-13 13:37:49.000000000","message":"Ah, very good point, +1\n\nWe should make that clear in this spec, that its not required.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"[libvirt]"},{"line_number":99,"context_line":"hw_disk_discard \u003d unmap"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"Developer impact"},{"line_number":102,"context_line":"----------------"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_d63ddd8f","line":100,"in_reply_to":"5a2ca52d_cb954787","updated":"2015-10-16 22:01:37.000000000","message":"Totally agree, we don\u0027t even need to check this. I\u0027ll add some documentation around that.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2b3201560e828df5bc7e0f61cb07f563afb9030d","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"Work Items"},{"line_number":117,"context_line":"----------"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"Check connection_info[\u0027data\u0027] for the discard option then add code to enable"},{"line_number":120,"context_line":"discard on the device.  Add unit tests for new functionality."},{"line_number":121,"context_line":"https://review.openstack.org/#/c/205726"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_3ee70842","line":118,"updated":"2015-10-13 12:12:59.000000000","message":"This should not link to reviews, it should really just list out what needs to be done.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":115,"context_line":""},{"line_number":116,"context_line":"Work Items"},{"line_number":117,"context_line":"----------"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"Check connection_info[\u0027data\u0027] for the discard option then add code to enable"},{"line_number":120,"context_line":"discard on the device.  Add unit tests for new functionality."},{"line_number":121,"context_line":"https://review.openstack.org/#/c/205726"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_d6507d34","line":118,"in_reply_to":"5a2ca52d_3ee70842","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":120,"context_line":"discard on the device.  Add unit tests for new functionality."},{"line_number":121,"context_line":"https://review.openstack.org/#/c/205726"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Implement the functionality in Cinder as well."},{"line_number":124,"context_line":"https://review.openstack.org/#/c/205717/"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_abc6cb97","line":123,"updated":"2015-10-16 14:19:44.000000000","message":"This was released in Liberty, it might be worth pointing that out.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":120,"context_line":"discard on the device.  Add unit tests for new functionality."},{"line_number":121,"context_line":"https://review.openstack.org/#/c/205726"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Implement the functionality in Cinder as well."},{"line_number":124,"context_line":"https://review.openstack.org/#/c/205717/"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_96468579","line":123,"in_reply_to":"5a2ca52d_abc6cb97","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2b3201560e828df5bc7e0f61cb07f563afb9030d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"Cinder Blueprint:"},{"line_number":132,"context_line":"https://blueprints.launchpad.net/cinder/+spec/cinder-backend-report-discard"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"Testing"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_7ec4a091","line":132,"updated":"2015-10-13 12:12:59.000000000","message":"I see this is now implemented.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"Cinder Blueprint:"},{"line_number":132,"context_line":"https://blueprints.launchpad.net/cinder/+spec/cinder-backend-report-discard"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"Testing"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_f65499ca","line":132,"in_reply_to":"5a2ca52d_2b395ba4","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":129,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"Cinder Blueprint:"},{"line_number":132,"context_line":"https://blueprints.launchpad.net/cinder/+spec/cinder-backend-report-discard"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"Testing"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_2b395ba4","line":132,"in_reply_to":"5a2ca52d_7ec4a091","updated":"2015-10-16 14:19:44.000000000","message":"Yeah, should probably point this out, it was released in Liberty.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2b3201560e828df5bc7e0f61cb07f563afb9030d","unresolved":false,"context_lines":[{"line_number":143,"context_line":"hw_disk_discard \u003d unmap and discard \u003d False"},{"line_number":144,"context_line":"hw_disk_discard \u003d ignore and discard \u003d False"},{"line_number":145,"context_line":"hw_disk_discard \u003d unmap and discard not present"},{"line_number":146,"context_line":"hw_disk_discard \u003d ignore and discard not present"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"Unit tests will be sufficient in this case."},{"line_number":149,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_7e4b402c","line":146,"updated":"2015-10-13 12:12:59.000000000","message":"This doesn\u0027t render correctly in the html, could we just remove this detail, its best to leave this detail for the code.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"926998a7f6a631e7e67a87e9c4702d7fe1dfb853","unresolved":false,"context_lines":[{"line_number":143,"context_line":"hw_disk_discard \u003d unmap and discard \u003d False"},{"line_number":144,"context_line":"hw_disk_discard \u003d ignore and discard \u003d False"},{"line_number":145,"context_line":"hw_disk_discard \u003d unmap and discard not present"},{"line_number":146,"context_line":"hw_disk_discard \u003d ignore and discard not present"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"Unit tests will be sufficient in this case."},{"line_number":149,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_4190e595","line":146,"in_reply_to":"5a2ca52d_7e4b402c","updated":"2015-10-13 12:17:00.000000000","message":"No, this is incorrect - the hw_disk_discard setting must not be used in conjunction with cinder block devices - its only for local image storage.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2b3201560e828df5bc7e0f61cb07f563afb9030d","unresolved":false,"context_lines":[{"line_number":145,"context_line":"hw_disk_discard \u003d unmap and discard not present"},{"line_number":146,"context_line":"hw_disk_discard \u003d ignore and discard not present"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"Unit tests will be sufficient in this case."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_1e0a4cd3","line":148,"updated":"2015-10-13 12:12:59.000000000","message":"It would be nice to have a third party CI to test that this works, end to end.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":145,"context_line":"hw_disk_discard \u003d unmap and discard not present"},{"line_number":146,"context_line":"hw_disk_discard \u003d ignore and discard not present"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"Unit tests will be sufficient in this case."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_4bda974d","line":148,"in_reply_to":"5a2ca52d_1e0a4cd3","updated":"2015-10-16 14:19:44.000000000","message":"The cinder spec said that the pure storage 3rd party CI in cinder would be doing this, but I\u0027m not sure how we\u0027d be able to tell, unless we just dig through the logs to verify discard\u003dTrue is set in the volume connection, qemu meets the min version, and the attach works.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":145,"context_line":"hw_disk_discard \u003d unmap and discard not present"},{"line_number":146,"context_line":"hw_disk_discard \u003d ignore and discard not present"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"Unit tests will be sufficient in this case."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"Documentation Impact"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_f69ed961","line":148,"in_reply_to":"5a2ca52d_4bda974d","updated":"2015-10-16 22:01:37.000000000","message":"I\u0027ll update the notes on this, afaik we could actually turn this on for the Ceph jobs too (I think Ceph does Trim..will have to check). That would probably be the easiest way to get some testing on this that is easier to validate and reproduce for those who don\u0027t have Pure arrays ;)\n\nI also intend to configure the Pure Storage CI to use it, but we only test in Cinder right now... maybe in the future we should test on Nova too?","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":151,"context_line":"Documentation Impact"},{"line_number":152,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"None"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"References"},{"line_number":157,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_0bb7bff5","line":154,"updated":"2015-10-16 14:19:44.000000000","message":"It seems there should be some doc impact for how you configure this stuff, unless we are just considering that the cinder and nova specs together document this feature.","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":151,"context_line":"Documentation Impact"},{"line_number":152,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"None"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"References"},{"line_number":157,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_b68b811e","line":154,"in_reply_to":"5a2ca52d_0bb7bff5","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe4d854c68a121f198e4242cc0982352b5b77e3e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"https://blueprints.launchpad.net/cinder/+spec/cinder-backend-report-discard"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Cinder Spec:"},{"line_number":163,"context_line":"https://review.openstack.org/#/c/205714/"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"History"},{"line_number":166,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_6babd375","line":163,"updated":"2015-10-16 14:19:44.000000000","message":"Point at this instead since it\u0027s easier to read than a spec in gerrit:\n\nhttp://specs.openstack.org/openstack/cinder-specs/specs/liberty/cinder-backend-report-discard.html","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"e5ad85319a108f87a70d818dc95ffd719350569a","unresolved":false,"context_lines":[{"line_number":160,"context_line":"https://blueprints.launchpad.net/cinder/+spec/cinder-backend-report-discard"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"Cinder Spec:"},{"line_number":163,"context_line":"https://review.openstack.org/#/c/205714/"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"History"},{"line_number":166,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5a2ca52d_565d4d45","line":163,"in_reply_to":"5a2ca52d_6babd375","updated":"2015-10-16 22:01:37.000000000","message":"Done","commit_id":"9ffe5b00a016e1338c7dd1733c9e37cc6ea8f7b2"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"043204de3580af07f48d1d67c3731957bfda20b1","unresolved":false,"context_lines":[{"line_number":45,"context_line":"an image configured with ``hw_scsi_model\u003dvirio-scsi`` and ``hw_disk_bus\u003dscsi``."},{"line_number":46,"context_line":"These options are required to have the instance configured with a"},{"line_number":47,"context_line":"SCSI controller that supports the discard feature."},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"It is important to note that the nova.conf option hw_disk_discard is NOT read"},{"line_number":50,"context_line":"for this feature. We rely entirely on Cinder to specify whether or not discard"},{"line_number":51,"context_line":"should be used for the volume."}],"source_content_type":"text/x-rst","patch_set":2,"id":"3a29b11f_2187d710","line":48,"updated":"2015-10-19 08:38:26.000000000","message":"This is not actually correct. Talking to QEMU maintainers it is supported by all SCSI models that QEMU emulates, IDE and AHCI, and Xen disks.\n\nIOW, virtio-blk is the only important backend missing this support.","commit_id":"9d6b735d443fa39977fbd3a683eb6de23dc1e2c1"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"672241e61e84f6f02d4a776eef57d7656c6fd116","unresolved":false,"context_lines":[{"line_number":45,"context_line":"an image configured with ``hw_scsi_model\u003dvirio-scsi`` and ``hw_disk_bus\u003dscsi``."},{"line_number":46,"context_line":"These options are required to have the instance configured with a"},{"line_number":47,"context_line":"SCSI controller that supports the discard feature."},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"It is important to note that the nova.conf option hw_disk_discard is NOT read"},{"line_number":50,"context_line":"for this feature. We rely entirely on Cinder to specify whether or not discard"},{"line_number":51,"context_line":"should be used for the volume."}],"source_content_type":"text/x-rst","patch_set":2,"id":"3a29b11f_45243f0c","line":48,"in_reply_to":"3a29b11f_2187d710","updated":"2015-10-19 18:14:03.000000000","message":"Ah, ok, got it. I\u0027ll update this.\n\nGuess my understanding of this was biased since I\u0027ve only used the virtio-scsi w/ scsi bus way ;)","commit_id":"9d6b735d443fa39977fbd3a683eb6de23dc1e2c1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ed2a4f72db4ef6686ea5bd964aed85724d01a42c","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"  driver_discard \u003d \"unmap\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"This will only give the desired support if the instance is configured with a"},{"line_number":45,"context_line":"interface and bus type that will support Trim/Unmap commands."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Currently the virtio-blk backend does not support discard."}],"source_content_type":"text/x-rst","patch_set":3,"id":"1a26ad4f_040c9beb","line":44,"updated":"2015-11-02 18:58:09.000000000","message":"What if we\u0027re not configured properly?  Do we detect and log a warning or does the thing just silently not work?","commit_id":"5ab46332c02e286fa49c99d25a97ecaa826aa345"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"f26425150bfb11b86b8feec86e1155aa0571c330","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":"  driver_discard \u003d \"unmap\""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"This will only give the desired support if the instance is configured with a"},{"line_number":45,"context_line":"interface and bus type that will support Trim/Unmap commands."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Currently the virtio-blk backend does not support discard."}],"source_content_type":"text/x-rst","patch_set":3,"id":"1a26ad4f_fa957432","line":44,"in_reply_to":"1a26ad4f_040c9beb","updated":"2015-11-02 20:05:11.000000000","message":"Done","commit_id":"5ab46332c02e286fa49c99d25a97ecaa826aa345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ed2a4f72db4ef6686ea5bd964aed85724d01a42c","unresolved":false,"context_lines":[{"line_number":133,"context_line":"* Modify volume attach code in libvirt driver to check for the new Cinder"},{"line_number":134,"context_line":"  connection property."},{"line_number":135,"context_line":"* Add unit tests for new functionality, modify any existing as needed."},{"line_number":136,"context_line":"* Configure 3rd party CI system to enable the feature and validate it as a"},{"line_number":137,"context_line":"  Cinder CI."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"Dependencies"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1a26ad4f_048fbb2a","line":136,"updated":"2015-11-02 18:58:09.000000000","message":"This would be Pure Storage?  Can we be more specific about which cinder 3rd party CI would do this?","commit_id":"5ab46332c02e286fa49c99d25a97ecaa826aa345"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"f26425150bfb11b86b8feec86e1155aa0571c330","unresolved":false,"context_lines":[{"line_number":133,"context_line":"* Modify volume attach code in libvirt driver to check for the new Cinder"},{"line_number":134,"context_line":"  connection property."},{"line_number":135,"context_line":"* Add unit tests for new functionality, modify any existing as needed."},{"line_number":136,"context_line":"* Configure 3rd party CI system to enable the feature and validate it as a"},{"line_number":137,"context_line":"  Cinder CI."},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"Dependencies"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1a26ad4f_9aa8008e","line":136,"in_reply_to":"1a26ad4f_048fbb2a","updated":"2015-11-02 20:05:11.000000000","message":"Done","commit_id":"5ab46332c02e286fa49c99d25a97ecaa826aa345"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"465cd900d5c120b64f834f993e9c9bf10a123109","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"Other deployer impact"},{"line_number":108,"context_line":"---------------------"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"Deployers wanting to use this feature with their Cinder backend will need to"},{"line_number":111,"context_line":"ensure their Glance images contain the following properties::"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1a26ad4f_5afa6876","line":109,"updated":"2015-11-02 20:09:17.000000000","message":"The \u0027Proposed change\u0027 section says:\n\n\"There will be several ways to get an instance that will support discard, one example is to use...\"\n\nSo I\u0027m assuming there are other options that could be listed here, but we don\u0027t want to or don\u0027t know of all of the other options, except that virtio-blk won\u0027t work, is that correct?\n\nI\u0027m just wondering how much of a hard and fast rule this section should make about configurations to make this all work, or if we should just note that this is a basic way to do it and is not an exhaustive list.\n\nIn general this all feels very rigid, i.e. I have to use a specific set of image properties and a specific cinder backend to get this to work, and if the planets aren\u0027t all aligned properly it doesn\u0027t work but there isn\u0027t any good way of telling me that it doesn\u0027t work, I just don\u0027t notice performance gains (which are maybe hard to measure here anyway, I don\u0027t really know).\n\nAlso, is libvirt\u003e\u003d1.0.6 required for this?","commit_id":"c5aa542ac1739325e948b27fc4bfb096f5e78391"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"68748b1ca7a9ae1111dc732dbf4e81f8378ce312","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"Other deployer impact"},{"line_number":108,"context_line":"---------------------"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"Deployers wanting to use this feature with their Cinder backend will need to"},{"line_number":111,"context_line":"ensure their Glance images contain the following properties::"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1a26ad4f_facd7431","line":109,"in_reply_to":"1a26ad4f_5afa6876","updated":"2015-11-02 20:15:32.000000000","message":"Ah yea, good catch. Your assumption is correct, this is just one basic way to do it and virtio-blk for sure won\u0027t work.\n\nI\u0027ll update this to match the proposed section and add some info about ways to enable it.\n\nI\u0027ll also add in the 1.0.6 libvirt and 1.6.0 qemu requirements","commit_id":"c5aa542ac1739325e948b27fc4bfb096f5e78391"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4e5214b16b0e6b83b91611262b5625eb1c5165d","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"Other deployer impact"},{"line_number":108,"context_line":"---------------------"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"Deployers wanting to use this feature with their Cinder backend will need to"},{"line_number":111,"context_line":"ensure their Glance images contain the following properties::"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1a26ad4f_9a57a059","line":109,"in_reply_to":"1a26ad4f_5afa6876","updated":"2015-11-02 20:13:09.000000000","message":"Maybe you want to say something like what danpb had in a previous comment about this:\n\n\"This is not actually correct. Talking to QEMU maintainers it is supported by all SCSI models that QEMU emulates, IDE and AHCI, and Xen disks.\n\nIn other words, virtio-blk is the only important backend missing this support.\"","commit_id":"c5aa542ac1739325e948b27fc4bfb096f5e78391"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c91fe376e729f1f32bcc52cda9efc50ba69d3678","unresolved":false,"context_lines":[{"line_number":106,"context_line":""},{"line_number":107,"context_line":"Other deployer impact"},{"line_number":108,"context_line":"---------------------"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"Deployers wanting to use this feature with their Cinder backend will need to"},{"line_number":111,"context_line":"ensure their Glance images contain the following properties::"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1a26ad4f_9a6a40ec","line":109,"in_reply_to":"1a26ad4f_facd7431","updated":"2015-11-02 20:17:50.000000000","message":"OK, after those changes I think I\u0027m +2 on this.","commit_id":"c5aa542ac1739325e948b27fc4bfb096f5e78391"}]}