)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6bebded18bc49f9f919b367fd3bc5133ca0252a0","unresolved":false,"context_lines":[{"line_number":24,"context_line":"value during migration, but the above referred commit (14c38ac) seemed"},{"line_number":25,"context_line":"to introduce a regression in the following ways:"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  - Wrongly set the cache mode for Ceph storage backend to be \u0027none\u0027,"},{"line_number":28,"context_line":"    while it should be set to \u0027writeback\u0027 to extract optimal"},{"line_number":29,"context_line":"    performance."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"  - It disregards the \u0027disk_cachemodes\u0027 Nova config parameter altogether"},{"line_number":32,"context_line":"    for boot disks -- i.e. even though if a user set the cache mode to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f4b6375_a0ccc334","line":29,"range":{"start_line":27,"start_character":4,"end_line":29,"end_character":16},"updated":"2017-10-26 22:15:48.000000000","message":"Where does this (not?) happen? Looks like _set_cache_mode() will get it out of the volume conf if not configured in nova.conf, but I don\u0027t actually see the LibvirtNetVolumeDriver setting the driver_cache field in the conf object, and LibvirtNetVolumeDriver is what is used for rbd volumes.","commit_id":"df464c6c845692cd024ebc7910dbb12c7698b019"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0bd231583f2be49fdc3497e89e89920a11ab790d","unresolved":false,"context_lines":[{"line_number":24,"context_line":"value during migration, but the above referred commit (14c38ac) seemed"},{"line_number":25,"context_line":"to introduce a regression in the following ways:"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  - Wrongly set the cache mode for Ceph storage backend to be \u0027none\u0027,"},{"line_number":28,"context_line":"    while it should be set to \u0027writeback\u0027 to extract optimal"},{"line_number":29,"context_line":"    performance."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"  - It disregards the \u0027disk_cachemodes\u0027 Nova config parameter altogether"},{"line_number":32,"context_line":"    for boot disks -- i.e. even though if a user set the cache mode to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f4b6375_bba074f5","line":29,"range":{"start_line":27,"start_character":4,"end_line":29,"end_character":16},"in_reply_to":"3f4b6375_3b3ec4b7","updated":"2017-10-26 22:30:35.000000000","message":"Oh but that would set writethrough if it doesn\u0027t support direct i/o, which isn\u0027t what you want for rbd - you want writeback. Maybe you\u0027re just saying, you configure nova.conf to use writeback for network source types, and with the regression, your configuration is no longer taken into account.","commit_id":"df464c6c845692cd024ebc7910dbb12c7698b019"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a1022f56673d684da76627a70853f356e0194176","unresolved":false,"context_lines":[{"line_number":24,"context_line":"value during migration, but the above referred commit (14c38ac) seemed"},{"line_number":25,"context_line":"to introduce a regression in the following ways:"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  - Wrongly set the cache mode for Ceph storage backend to be \u0027none\u0027,"},{"line_number":28,"context_line":"    while it should be set to \u0027writeback\u0027 to extract optimal"},{"line_number":29,"context_line":"    performance."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"  - It disregards the \u0027disk_cachemodes\u0027 Nova config parameter altogether"},{"line_number":32,"context_line":"    for boot disks -- i.e. even though if a user set the cache mode to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f4b6375_3b3ec4b7","line":29,"range":{"start_line":27,"start_character":4,"end_line":29,"end_character":16},"in_reply_to":"3f4b6375_6047eb78","updated":"2017-10-26 22:26:53.000000000","message":"Oh maybe it\u0027s this?\n\nhttps://github.com/openstack/nova/blob/bd57dc63096c30e205e7bb9473f4455377a1e3dc/nova/virt/libvirt/driver.py#L413\n\nThat\u0027s then passed to the imagebackend which sets the driver_cache value on the config:\n\nhttps://github.com/openstack/nova/blob/bd57dc63096c30e205e7bb9473f4455377a1e3dc/nova/virt/libvirt/driver.py#L3649\n\nhttps://github.com/openstack/nova/blob/bd57dc63096c30e205e7bb9473f4455377a1e3dc/nova/virt/libvirt/imagebackend.py#L845","commit_id":"df464c6c845692cd024ebc7910dbb12c7698b019"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"132e15695821c6768774d63916d3770c887c32d3","unresolved":false,"context_lines":[{"line_number":24,"context_line":"value during migration, but the above referred commit (14c38ac) seemed"},{"line_number":25,"context_line":"to introduce a regression in the following ways:"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"  - Wrongly set the cache mode for Ceph storage backend to be \u0027none\u0027,"},{"line_number":28,"context_line":"    while it should be set to \u0027writeback\u0027 to extract optimal"},{"line_number":29,"context_line":"    performance."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"  - It disregards the \u0027disk_cachemodes\u0027 Nova config parameter altogether"},{"line_number":32,"context_line":"    for boot disks -- i.e. even though if a user set the cache mode to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f4b6375_6047eb78","line":29,"range":{"start_line":27,"start_character":4,"end_line":29,"end_character":16},"in_reply_to":"3f4b6375_a0ccc334","updated":"2017-10-26 22:19:47.000000000","message":"There is nothing in nova that defaults the rbd disk conf cachemode to writeback, so this should be removed as it\u0027s confusing to what you\u0027re actually changing / fixing here.","commit_id":"df464c6c845692cd024ebc7910dbb12c7698b019"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6bebded18bc49f9f919b367fd3bc5133ca0252a0","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    while it should be set to \u0027writeback\u0027 to extract optimal"},{"line_number":29,"context_line":"    performance."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"  - It disregards the \u0027disk_cachemodes\u0027 Nova config parameter altogether"},{"line_number":32,"context_line":"    for boot disks -- i.e. even though if a user set the cache mode to"},{"line_number":33,"context_line":"    be \u0027writethrough\u0027, it\u0027s ignored and instead \u0027none\u0027 is set"},{"line_number":34,"context_line":"    unconditionally."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Add the _set_cache_mode() calls back in _get_guest_storage_config()."},{"line_number":37,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f4b6375_60eb6b77","line":34,"range":{"start_line":31,"start_character":4,"end_line":34,"end_character":20},"updated":"2017-10-26 22:15:48.000000000","message":"This is because _get_guest_storage_config() no longer calls _set_cache_mode() and _set_cache_mode() uses self.disk_cachemodes right? Trying to understand the comments here.","commit_id":"df464c6c845692cd024ebc7910dbb12c7698b019"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b85f96557b7671c615af475d0ca498c7f31a9635","unresolved":false,"context_lines":[{"line_number":1242,"context_line":"            disk_info[\u0027unit\u0027] \u003d self._get_scsi_controller_max_unit(guest) + 1"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"        conf \u003d self._get_volume_config(connection_info, disk_info)"},{"line_number":1245,"context_line":"        self._set_cache_mode(conf)"},{"line_number":1246,"context_line":""},{"line_number":1247,"context_line":"        self._check_discard_for_attach_volume(conf, instance)"},{"line_number":1248,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_0d7555cb","line":1245,"updated":"2017-10-24 23:11:11.000000000","message":"I wouldn\u0027t think this one needs to be added back, as _get_volume_config is calling _set_cache_mode internally.","commit_id":"64e0bf33459a5c49cc31a97c457610dc81642481"}]}
