)]}'
{"nova/tests/virt/libvirt/test_libvirt_utils.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"90f5cfc44d3605ccfd1a3f8e16066940c4784ead","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import test"},{"line_number":20,"context_line":"from nova import utils"},{"line_number":21,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":22,"context_line":"from oslo.config import cfg"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONF \u003d cfg.CONF"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2Froc%3D","line":22,"updated":"2014-02-11 14:50:17.000000000","message":"this is a third party import so it should be in its own section","commit_id":"26d0206b846b375e8833a99b7e829508b06744ce"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8bae0c30eaff954caef626b0ac3d49823b1e7368","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import utils"},{"line_number":20,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from oslo.config import cfg"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONF \u003d cfg.CONF"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2Fq4w%3D","line":22,"updated":"2014-02-11 16:11:13.000000000","message":"sorry for nit again - this should be between \nimport os \nand \nfrom nova import test","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"d9e2d335cc317db3c2ebf8925f2174a425ea2921","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import utils"},{"line_number":20,"context_line":"from nova.virt.libvirt import utils as libvirt_utils"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from oslo.config import cfg"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONF \u003d cfg.CONF"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2Fql8%3D","line":22,"in_reply_to":"AAAAUX%2F%2Fq4w%3D","updated":"2014-02-11 17:40:46.000000000","message":"You are right and it\u0027s good to catch these things, but there is code in hacking to check for these things and we should be using it. What I\u0027m saying is that I\u0027d prefer to see code coming in to check for these then continuously depend on reviewers to catch it.","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"d9e2d335cc317db3c2ebf8925f2174a425ea2921","unresolved":false,"context_lines":[{"line_number":66,"context_line":"               default\u003d\u0027zero\u0027,"},{"line_number":67,"context_line":"               help\u003d\u0027Method used to wipe old volumes (valid options are: \u0027"},{"line_number":68,"context_line":"                    \u0027none, zero, shred)\u0027),"},{"line_number":69,"context_line":"    cfg.IntOpt(\u0027volume_clear_size\u0027,"},{"line_number":70,"context_line":"               default\u003d0,"},{"line_number":71,"context_line":"               help\u003d\u0027Size in MiB to wipe at start of old volumes. 0 \u003d\u003e all\u0027),"},{"line_number":72,"context_line":"    cfg.StrOpt(\u0027images_rbd_pool\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2FqI0%3D","line":69,"updated":"2014-02-11 17:40:46.000000000","message":"I\u0027m missing what is the actual use case for this? I guess for performance reasons we want to have an upper limit on how much we wipe? In that case it might be useful to rename it to something that communicates that intention a bit better (like for example volume_mac_clear_size or smth)","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"5d2d1aac856ea51c4e29a90a477228e54980c6b2","unresolved":false,"context_lines":[{"line_number":66,"context_line":"               default\u003d\u0027zero\u0027,"},{"line_number":67,"context_line":"               help\u003d\u0027Method used to wipe old volumes (valid options are: \u0027"},{"line_number":68,"context_line":"                    \u0027none, zero, shred)\u0027),"},{"line_number":69,"context_line":"    cfg.IntOpt(\u0027volume_clear_size\u0027,"},{"line_number":70,"context_line":"               default\u003d0,"},{"line_number":71,"context_line":"               help\u003d\u0027Size in MiB to wipe at start of old volumes. 0 \u003d\u003e all\u0027),"},{"line_number":72,"context_line":"    cfg.StrOpt(\u0027images_rbd_pool\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2Fp%2F0%3D","line":69,"in_reply_to":"AAAAUX%2F%2FqI0%3D","updated":"2014-02-11 18:02:59.000000000","message":"Well this option is already in cinder so we\u0027re kinda restricted on naming. The main use case would be to overwrite keys at the the start of an encrypted volume.","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"90f5cfc44d3605ccfd1a3f8e16066940c4784ead","unresolved":false,"context_lines":[{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    if volume_clear \u003d\u003d \u0027none\u0027:"},{"line_number":408,"context_line":"        return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    volume_clear_size \u003d int(CONF.libvirt.volume_clear_size) * units.Mi"},{"line_number":411,"context_line":"    volume_size \u003d logical_volume_size(path)"},{"line_number":412,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2Frl4%3D","line":409,"updated":"2014-02-11 14:50:17.000000000","message":"if volume_clear not in [...]:\n    raise exception","commit_id":"26d0206b846b375e8833a99b7e829508b06744ce"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"90f5cfc44d3605ccfd1a3f8e16066940c4784ead","unresolved":false,"context_lines":[{"line_number":408,"context_line":"        return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    volume_clear_size \u003d int(CONF.libvirt.volume_clear_size) * units.Mi"},{"line_number":411,"context_line":"    volume_size \u003d logical_volume_size(path)"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"    if volume_clear_size !\u003d 0 and volume_clear_size \u003c volume_size:"},{"line_number":414,"context_line":"        volume_size \u003d volume_clear_size"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2Frmo%3D","line":411,"updated":"2014-02-11 14:50:17.000000000","message":"minor nit - maybe we should check that the volume_clear is of the correct type prior to reading the logical_volume_size. see comment above","commit_id":"26d0206b846b375e8833a99b7e829508b06744ce"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"d9e2d335cc317db3c2ebf8925f2174a425ea2921","unresolved":false,"context_lines":[{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    if volume_clear \u003d\u003d \u0027zero\u0027:"},{"line_number":422,"context_line":"        #NOTE(p-draigbrady): we could use shred to do the zeroing"},{"line_number":423,"context_line":"        #with -n0 -z, however only versions \u003e\u003d 8.22 perform as well as dd"},{"line_number":424,"context_line":"        _zero_logical_volume(path, volume_size)"},{"line_number":425,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2FqOY%3D","line":423,"updated":"2014-02-11 17:40:46.000000000","message":"Awesome that you profiled this :)","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"5d2d1aac856ea51c4e29a90a477228e54980c6b2","unresolved":false,"context_lines":[{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    if volume_clear \u003d\u003d \u0027zero\u0027:"},{"line_number":422,"context_line":"        #NOTE(p-draigbrady): we could use shred to do the zeroing"},{"line_number":423,"context_line":"        #with -n0 -z, however only versions \u003e\u003d 8.22 perform as well as dd"},{"line_number":424,"context_line":"        _zero_logical_volume(path, volume_size)"},{"line_number":425,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2Fp%2Fk%3D","line":423,"in_reply_to":"AAAAUX%2F%2FqOY%3D","updated":"2014-02-11 18:02:59.000000000","message":"Well I wrote the more performant code in coreutils :)","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"d9e2d335cc317db3c2ebf8925f2174a425ea2921","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        #with -n0 -z, however only versions \u003e\u003d 8.22 perform as well as dd"},{"line_number":424,"context_line":"        _zero_logical_volume(path, volume_size)"},{"line_number":425,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":427,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":428,"context_line":"    else:"},{"line_number":429,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not supported\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2Fq1A%3D","line":426,"updated":"2014-02-11 17:40:46.000000000","message":"Do we want to check if this failed? Same for the abouve call to dd?","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"5d2d1aac856ea51c4e29a90a477228e54980c6b2","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        #with -n0 -z, however only versions \u003e\u003d 8.22 perform as well as dd"},{"line_number":424,"context_line":"        _zero_logical_volume(path, volume_size)"},{"line_number":425,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":427,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":428,"context_line":"    else:"},{"line_number":429,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not supported\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAUX%2F%2Fp%2FM%3D","line":426,"in_reply_to":"AAAAUX%2F%2Fq1A%3D","updated":"2014-02-11 18:02:59.000000000","message":"We get an exception on any failure, as before. That seems appropriate for a failure of this nature","commit_id":"69ba0e3c64b5c5cecbe36dfa047bc2ca4b7a17fe"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"543830bf9848d51fed7448ed4684dcbb501cec00","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2FqJc%3D","line":381,"updated":"2014-02-11 17:47:48.000000000","message":"If I\u0027m understanding this correctly, this is actually fixing a bug in the existing code where O_DIRECT would only be passed on the first block, and O_SYNC thereafter (which conflicted with the comment).\n\nIf that\u0027s the case, perhaps this should be broken out into a separate commit.","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"5e3367654f460aa6278e06ffe6f2534f6adc964a","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2Foro%3D","line":381,"in_reply_to":"AAAAUX%2F%2Fo7k%3D","updated":"2014-02-11 20:46:49.000000000","message":"Either oflag\u003ddirect _or_ conv\u003dfdatasync is sufficient to ensure the writes are not discarded by the operating system\n\nWe use O_DIRECT with the large 1MiB blocks where it should work without issue. For slop at the end we resort to fdatasync() calls after the writes.\n\nThis is encompassed in the tests","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"9acbb4aeb487abc76d129350c8487e3f591bbddb","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2FoDA%3D","line":381,"in_reply_to":"AAAAUX%2F%2FopQ%3D","updated":"2014-02-11 22:16:56.000000000","message":"This is a fair point. What we are guaranteeing is that the OS won\u0027t discard the writes if the blocks are allocated for something else before they\u0027re written out. However if the power fails between the clear_volume( ) returning success and the write cache actually being written to stable storage then we will not know about the uncleared data. Now acting on that information is another thing. At a high level we\u0027d have to reschedule the complete clear_volume( ) again, and also we\u0027d have to distinguish different write errors so that we wouldn\u0027t continue to retry writing to faulty disks etc. So yes this should be considered but probably separately and in a larger context.\n\nVery considered review. Thanks!","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"65410f8a9a2018b7aab5c1112b605eb79acb13a6","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2FopQ%3D","line":381,"in_reply_to":"AAAAUX%2F%2Foro%3D","updated":"2014-02-11 20:54:28.000000000","message":"Cool, thanks for clarifying here.\n\nhttps://lwn.net/Articles/457667/ seems to say something slightly different (\"so fsync() is still required for files opened with O_DIRECT in order to save the data to stable storage\") but I might be misunderstanding the problem at hand.","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"f2df86ca836fe9f94bc38f71e981664f31c64054","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2Fpo0%3D","line":381,"in_reply_to":"AAAAUX%2F%2Fp8Y%3D","updated":"2014-02-11 18:41:25.000000000","message":"Actually the old code also handled the \u003c 1MiB block size case and was cleaner as it didn\u0027t have nested loops. So reverting to that again. Test cases now confirm all cases work.","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"42c7675723aec35d4fd5eb7e8ba3f0dc6ca6b28f","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2Fo7k%3D","line":381,"in_reply_to":"AAAAUX%2F%2Fpo0%3D","updated":"2014-02-11 20:16:03.000000000","message":"Yeah so what I thought was a bug really wasn\u0027t.\n\nBut I *think* there might be a fence-post bug; if the size is evenly divisible by 1024**2, then we\u0027ll never call dd with O_SYNC. This is rare, but still could be a security issue.\n\nNot evenly divisible:\n\n102340002\n\n(\u0027dd\u0027, \u0027bs\u003d1048576\u0027, \u0027if\u003d/dev/zero\u0027, \u0027of\u003dfoo\u0027, \u0027seek\u003d0\u0027,     \u0027count\u003d97\u0027, \u0027oflag\u003ddirect\u0027)\n\n(\u0027dd\u0027, \u0027bs\u003d1024\u0027, \u0027if\u003d/dev/zero\u0027, \u0027of\u003dfoo\u0027, \u0027seek\u003d99328\u0027, \u0027count\u003d613\u0027, \u0027conv\u003dfdatasync\u0027)\n\n(\u0027dd\u0027, \u0027bs\u003d1\u0027, \u0027if\u003d/dev/zero\u0027, \u0027of\u003dfoo\u0027, \u0027seek\u003d102339584\u0027, \u0027count\u003d418\u0027, \u0027conv\u003dfdatasync\u0027)\n\n\nEvenly divisible:\n\n1048576\n\n(\u0027dd\u0027, \u0027bs\u003d1048576\u0027, \u0027if\u003d/dev/zero\u0027, \u0027of\u003dfoo\u0027, \u0027seek\u003d0\u0027, \u0027count\u003d1\u0027, \u0027oflag\u003ddirect\u0027)\n\nIs this correct?\n\nIf so, we can handle that in a separate bugfix...","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"97720f5f0c704b6a1bc7c2891d8d9323c3d5312e","unresolved":false,"context_lines":[{"line_number":378,"context_line":"            bs /\u003d units.Ki  # Limit to 3 iterations"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Use O_DIRECT with initial block size and fdatasync otherwise"},{"line_number":381,"context_line":"        if bs \u003e\u003d units.Mi:"},{"line_number":382,"context_line":"            direct_flags \u003d (\u0027oflag\u003ddirect\u0027,)"},{"line_number":383,"context_line":"            sync_flags \u003d ()"},{"line_number":384,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2Fp8Y%3D","line":381,"in_reply_to":"AAAAUX%2F%2FqJc%3D","updated":"2014-02-11 18:27:11.000000000","message":"The new code is more general in that it accepts volume_size \u003c 1MiB. But that\u0027s just cleanup and doesn\u0027t practically change anything. The outer loop should only every iterate a max of 3 times, the first with O_DIRECT (with a 1MiB block size)","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"543830bf9848d51fed7448ed4684dcbb501cec00","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        #with -n0 -z, however only versions \u003e\u003d 8.22 perform as well as dd"},{"line_number":424,"context_line":"        _zero_logical_volume(path, volume_size)"},{"line_number":425,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":427,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":428,"context_line":"    else:"},{"line_number":429,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not supported\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2FqIc%3D","line":426,"updated":"2014-02-11 17:47:48.000000000","message":"Should the shred passes be configurable?\n\nIf it feels like if we\u0027re trying to make the security/speed trade-off configurable, we should go ahead and do that as well.","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"97720f5f0c704b6a1bc7c2891d8d9323c3d5312e","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        #with -n0 -z, however only versions \u003e\u003d 8.22 perform as well as dd"},{"line_number":424,"context_line":"        _zero_logical_volume(path, volume_size)"},{"line_number":425,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":427,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":428,"context_line":"    else:"},{"line_number":429,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not supported\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2Fpxc%3D","line":426,"in_reply_to":"AAAAUX%2F%2FqIc%3D","updated":"2014-02-11 18:27:11.000000000","message":"So I was aligning with what cinder currently supports.\n\nYou would use shred to be \"more secure\" than writing zeros.\n-n3 matches the current upstream coreutils default as a\nsensible more secure default, and will write 3 random\npatterns. If you increase the iterations, shred will start using\npatterns targeted at RLL and MFM encodings which are\nfairly special case these days really.\n\nIf we did want to configure stuff further then one would\nbe looking at other DoD secure wipe standards or\nleveraging \"hardware\" erase functionality available in\nmany storage subsystems these days.","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":475,"name":"Rick Harris","email":"rick.harris@rackspace.com","username":"rconradharris"},"change_message_id":"543830bf9848d51fed7448ed4684dcbb501cec00","unresolved":false,"context_lines":[{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":427,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":428,"context_line":"    else:"},{"line_number":429,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not supported\")"},{"line_number":430,"context_line":"                                % volume_clear)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2FqJA%3D","line":429,"updated":"2014-02-11 17:47:48.000000000","message":"This can\u0027t be reached, but I\u0027m fine keeping it here just for symmetry (and in case the above guard is ever removed).","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"97720f5f0c704b6a1bc7c2891d8d9323c3d5312e","unresolved":false,"context_lines":[{"line_number":426,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":427,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":428,"context_line":"    else:"},{"line_number":429,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not supported\")"},{"line_number":430,"context_line":"                                % volume_clear)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAUX%2F%2Fpwc%3D","line":429,"in_reply_to":"AAAAUX%2F%2FqJA%3D","updated":"2014-02-11 18:27:11.000000000","message":"True. I noticed this myself and was considering removing it, but will leave and adjust the error message to indicate an internal rather than external error.","commit_id":"eddad3fb87eb1431d422072a11ecb6e28d982433"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"04f879938549452402cb2b88dac0935f5cb323ee","unresolved":false,"context_lines":[{"line_number":420,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":421,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":422,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":423,"context_line":"    else:"},{"line_number":424,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not handled\")"},{"line_number":425,"context_line":"                                % volume_clear)"},{"line_number":426,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2FgY4%3D","line":423,"updated":"2014-02-12 18:12:59.000000000","message":"This case should never be reached given that volume_clear is set to \u0027zero\u0027 if it isn\u0027t recognized.","commit_id":"dcd85bdb70eb2f6d44b30e723a05366bf7f48164"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"cc11c37b4811c2ba9b3a75ae5ac0fa48a93c2a6c","unresolved":false,"context_lines":[{"line_number":420,"context_line":"    elif volume_clear \u003d\u003d \u0027shred\u0027:"},{"line_number":421,"context_line":"        utils.execute(\u0027shred\u0027, \u0027-n3\u0027, \u0027-s%d\u0027 % volume_size, path,"},{"line_number":422,"context_line":"                      run_as_root\u003dTrue)"},{"line_number":423,"context_line":"    else:"},{"line_number":424,"context_line":"        raise exception.Invalid(_(\"volume_clear\u003d\u0027%s\u0027 is not handled\")"},{"line_number":425,"context_line":"                                % volume_clear)"},{"line_number":426,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUX%2F%2FejE%3D","line":423,"in_reply_to":"AAAAUX%2F%2FgY4%3D","updated":"2014-02-12 22:28:11.000000000","message":"True, that was noted previously, but the reasoning was to leave it  in case this function ever got out of sync with the supported wipe methods. Note the message is an internal consistency one, rather than indicating a config error to the user for example.","commit_id":"dcd85bdb70eb2f6d44b30e723a05366bf7f48164"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"fa8df216fa1adf602c908e9e71a8740cedd6571b","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"    if volume_clear not in (\u0027none\u0027, \u0027shred\u0027, \u0027zero\u0027):"},{"line_number":403,"context_line":"        LOG.error(_(\"ignoring unrecognized volume_clear\u003d\u0027%s\u0027 value\")"},{"line_number":404,"context_line":"                  % volume_clear)"},{"line_number":405,"context_line":"        volume_clear \u003d \u0027zero\u0027"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    if volume_clear \u003d\u003d \u0027none\u0027:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUX%2F%2FEiI%3D","line":404,"updated":"2014-02-17 10:17:45.000000000","message":"please use a , instead of the %","commit_id":"a20248c4aa3a2516d4de4e9e6ac8e2244b3e34f8"}]}