)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1849,"name":"Joe Gordon","email":"joe.gordon0@gmail.com","username":"jogo"},"change_message_id":"095795e6ee402c97adbeb179ca55dfb3065846f3","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Joel Coffman \u003cjoel.coffman@jhuapl.edu\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-08-30 10:05:37 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add encryption support for volumes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add support to encrypt Cinder volumes. Hooks within the libvirt"},{"line_number":10,"context_line":"driver encrypt volumes during the attach call. Created a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"AAAASn%2F%2FjlU%3D","line":7,"updated":"2013-09-03 15:31:42.000000000","message":"if this is only for libvirt the first line should say so. \n\nhttps://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages","commit_id":"f9fc5698b823ed7b3c7abe6fdb20071d365f7e1f"}],"etc/nova.conf.sample":[{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"88399b158b180111756a7710f30d878347241592","unresolved":false,"context_lines":[{"line_number":1,"context_line":"[DEFAULT]"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# Options defined in nova.availability_zones"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAQn%2F%2B%2FQk%3D","line":1,"updated":"2013-08-08 20:21:11.000000000","message":"This file should not have been added and will be removed in the next patch set.","commit_id":"28f6ecb06dce6a886a6aacaa34e4946df9ec64f0"}],"etc/nova/rootwrap.d/compute.filters":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":200,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":201,"context_line":"sg_scan: CommandFilter, sg_scan, root"},{"line_number":202,"context_line":"cryptsetup: CommandFilter, cryptsetup, root"},{"line_number":203,"context_line":"ln: CommandFilter, ln, root"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"# nova/virt/xenapi/vm_utils.py:"},{"line_number":206,"context_line":"xenstore-read: CommandFilter, xenstore-read, root"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAPX%2F%2FnHY%3D","line":203,"updated":"2013-06-14 12:39:50.000000000","message":"should we not restrict the use of \"ln\" a bit more than this? Maybe restrict it to particular params, so it is not quite as dangerous?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":200,"context_line":"sginfo: CommandFilter, sginfo, root"},{"line_number":201,"context_line":"sg_scan: CommandFilter, sg_scan, root"},{"line_number":202,"context_line":"cryptsetup: CommandFilter, cryptsetup, root"},{"line_number":203,"context_line":"ln: CommandFilter, ln, root"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"# nova/virt/xenapi/vm_utils.py:"},{"line_number":206,"context_line":"xenstore-read: CommandFilter, xenstore-read, root"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAPn%2F%2FdQI%3D","line":203,"in_reply_to":"AAAAPX%2F%2FnHY%3D","updated":"2013-07-02 14:52:49.000000000","message":"Done","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"}],"nova/compute/manager.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":2926,"context_line":"        if \u0027serial\u0027 not in connection_info:"},{"line_number":2927,"context_line":"            connection_info[\u0027serial\u0027] \u003d volume_id"},{"line_number":2928,"context_line":""},{"line_number":2929,"context_line":"        volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":2930,"context_line":"        try:"},{"line_number":2931,"context_line":"            self.driver.attach_volume(connection_info,"},{"line_number":2932,"context_line":"                                      instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2FnHQ%3D","line":2929,"updated":"2013-06-14 12:39:50.000000000","message":"can we not extend connection info with this data?\n\nIf we keep this extra API call for every volume operation (which seems a bit odd), could we call it something that better represents that it is to do with encryption?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":2926,"context_line":"        if \u0027serial\u0027 not in connection_info:"},{"line_number":2927,"context_line":"            connection_info[\u0027serial\u0027] \u003d volume_id"},{"line_number":2928,"context_line":""},{"line_number":2929,"context_line":"        volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":2930,"context_line":"        try:"},{"line_number":2931,"context_line":"            self.driver.attach_volume(connection_info,"},{"line_number":2932,"context_line":"                                      instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2FcSQ%3D","line":2929,"in_reply_to":"AAAAPX%2F%2FnHQ%3D","updated":"2013-07-02 14:52:49.000000000","message":"Adding the information to connection_info is simpler, but it does \"pollute\" the connection information so to speak.\n\nThis issue may resolve itself with some related Cinder changes that we\u0027re working on (https://review.openstack.org/#/c/30974/). We\u0027ll see how those changes affect this code and update accordingly.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":3148,"context_line":"            connection_info[\u0027serial\u0027] \u003d volume_id"},{"line_number":3149,"context_line":""},{"line_number":3150,"context_line":"        volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":3151,"context_line":"        volume_type \u003d self._get_volume_type(context, volume)"},{"line_number":3152,"context_line":""},{"line_number":3153,"context_line":"        try:"},{"line_number":3154,"context_line":"            self.driver.attach_volume(connection_info,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FePQ%3D","line":3151,"updated":"2013-07-30 16:54:22.000000000","message":"Is there a reason this information is not part of the connection_info?\n\nIt seems wrong to add this extra call to cinder for every volume connection, even if no information from the call may be needed by most drivers.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":3148,"context_line":"            connection_info[\u0027serial\u0027] \u003d volume_id"},{"line_number":3149,"context_line":""},{"line_number":3150,"context_line":"        volume \u003d self.volume_api.get(context, volume_id)"},{"line_number":3151,"context_line":"        volume_type \u003d self._get_volume_type(context, volume)"},{"line_number":3152,"context_line":""},{"line_number":3153,"context_line":"        try:"},{"line_number":3154,"context_line":"            self.driver.attach_volume(connection_info,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FS7s%3D","line":3151,"in_reply_to":"AAAAQn%2F%2FePQ%3D","updated":"2013-08-02 18:45:08.000000000","message":"Understood. A slightly modified implementation adds \u0027encrypted\u0027 to the connection information, and only if the volume is encrypted is another API request made.\n\nPassing all the encryption metadata (e.g., the encryption key UUID) as connection information does not seem ideal from a security perspective.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ec07da76cf9ddac6d4f80be8d24d43d56f0b4fa5","unresolved":false,"context_lines":[{"line_number":3507,"context_line":"            self.driver.attach_volume(connection_info,"},{"line_number":3508,"context_line":"                                      instance,"},{"line_number":3509,"context_line":"                                      mountpoint,"},{"line_number":3510,"context_line":"                                      context\u003dcontext,"},{"line_number":3511,"context_line":"                                      encryption\u003dencryption)"},{"line_number":3512,"context_line":"        except Exception:  # pylint: disable\u003dW0702"},{"line_number":3513,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FIgM%3D","line":3510,"updated":"2013-09-11 12:33:03.000000000","message":"Why do we need the context here?\nFrom what I have seen in the code it to enable cinder to be called. Why not do the calling here and pass the data through as a parameter.","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"0398402f8e6741ce5c8f6da9e751adf249b3bfd5","unresolved":false,"context_lines":[{"line_number":3507,"context_line":"            self.driver.attach_volume(connection_info,"},{"line_number":3508,"context_line":"                                      instance,"},{"line_number":3509,"context_line":"                                      mountpoint,"},{"line_number":3510,"context_line":"                                      context\u003dcontext,"},{"line_number":3511,"context_line":"                                      encryption\u003dencryption)"},{"line_number":3512,"context_line":"        except Exception:  # pylint: disable\u003dW0702"},{"line_number":3513,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FIWo%3D","line":3510,"in_reply_to":"AAAASn%2F%2FIgM%3D","updated":"2013-09-11 13:33:41.000000000","message":"It\u0027s actually passed to the key manager when the key is retrieved. We\u0027d like to avoid unnecessary handling of the encryption key, which is why it isn\u0027t retrieved here and passed as a parameter.","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"}],"nova/tests/virt/libvirt/encryptors/test_base.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        super(VolumeEncryptorTestCase, self).setUp()"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"        self.connection_info \u003d {"},{"line_number":28,"context_line":"            \"data\": {"},{"line_number":29,"context_line":"                \"device_path\": \"/dev/disk/by-path/\""},{"line_number":30,"context_line":"                    \"ip-192.0.2.0:3260-iscsi-iqn.2010-10.org.openstack\""},{"line_number":31,"context_line":"                    \":volume-23006472-40e2-4b68-836c-c57cf7c62147-lun-1\""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2Fefg%3D","line":28,"updated":"2013-07-30 16:54:22.000000000","message":"Rather than inherit from this test, I would just keep this value as module level property or similar.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/tests/virt/libvirt/encryptors/test_cryptsetup.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CONF \u003d cfg.CONF"},{"line_number":26,"context_line":"CONF.import_opt(\u0027cryptsetup_cipher\u0027,"},{"line_number":27,"context_line":"                \u0027nova.virt.libvirt.encryptors.cryptsetup\u0027)"},{"line_number":28,"context_line":"CONF.import_opt(\u0027cryptsetup_key_size\u0027,"},{"line_number":29,"context_line":"                \u0027nova.virt.libvirt.encryptors.cryptsetup\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FegE%3D","line":26,"updated":"2013-07-30 16:54:22.000000000","message":"Why is this here?","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"CONF \u003d cfg.CONF"},{"line_number":26,"context_line":"CONF.import_opt(\u0027cryptsetup_cipher\u0027,"},{"line_number":27,"context_line":"                \u0027nova.virt.libvirt.encryptors.cryptsetup\u0027)"},{"line_number":28,"context_line":"CONF.import_opt(\u0027cryptsetup_key_size\u0027,"},{"line_number":29,"context_line":"                \u0027nova.virt.libvirt.encryptors.cryptsetup\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FRvs%3D","line":26,"in_reply_to":"AAAAQn%2F%2FegE%3D","updated":"2013-08-02 18:45:08.000000000","message":"Deleted.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/virt/driver.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bba91e21f7265d1e24654e0a724089ff07d6c245","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        raise NotImplementedError()"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint,"},{"line_number":324,"context_line":"                      context\u003dNone, encryption\u003dNone):"},{"line_number":325,"context_line":"        \"\"\"Attach the disk to the instance at mountpoint using info.\"\"\""},{"line_number":326,"context_line":"        raise NotImplementedError()"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FOuQ%3D","line":324,"updated":"2013-09-09 18:17:30.000000000","message":"We might want to make \u0027context\u0027 the first argument after self as is a very common case in a lot of our methods and is sometimes expected by the decorators we use. \n\nWill result in a more code being affected so might be worth a separate patch. Not -1 material imho but could be fixed.","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"eafd5cce88fe2b985d711672d8864ad45a5e9f1c","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        raise NotImplementedError()"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint,"},{"line_number":324,"context_line":"                      context\u003dNone, encryption\u003dNone):"},{"line_number":325,"context_line":"        \"\"\"Attach the disk to the instance at mountpoint using info.\"\"\""},{"line_number":326,"context_line":"        raise NotImplementedError()"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FIog%3D","line":324,"in_reply_to":"AAAASn%2F%2FOuQ%3D","updated":"2013-09-11 11:32:45.000000000","message":"Done","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":952,"context_line":"        method \u003d getattr(driver, method_name)"},{"line_number":953,"context_line":"        return method(connection_info, *args, **kwargs)"},{"line_number":954,"context_line":""},{"line_number":955,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint, **kwargs):"},{"line_number":956,"context_line":"        instance_name \u003d instance[\u0027name\u0027]"},{"line_number":957,"context_line":"        virt_dom \u003d self._lookup_by_name(instance_name)"},{"line_number":958,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2FnG8%3D","line":955,"updated":"2013-06-14 12:39:50.000000000","message":"if we extend this signature, presumably the other drivers will all need updating too?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":952,"context_line":"        method \u003d getattr(driver, method_name)"},{"line_number":953,"context_line":"        return method(connection_info, *args, **kwargs)"},{"line_number":954,"context_line":""},{"line_number":955,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint, **kwargs):"},{"line_number":956,"context_line":"        instance_name \u003d instance[\u0027name\u0027]"},{"line_number":957,"context_line":"        virt_dom \u003d self._lookup_by_name(instance_name)"},{"line_number":958,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2FcR8%3D","line":955,"in_reply_to":"AAAAPX%2F%2FnG8%3D","updated":"2013-07-02 14:52:49.000000000","message":"Probably, yes. Should keyword arguments be added to all the other drivers as part of this patch?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b5bfb594087b71f69832b5f9c777c07f5f4f3e84","unresolved":false,"context_lines":[{"line_number":952,"context_line":"        method \u003d getattr(driver, method_name)"},{"line_number":953,"context_line":"        return method(connection_info, *args, **kwargs)"},{"line_number":954,"context_line":""},{"line_number":955,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint, **kwargs):"},{"line_number":956,"context_line":"        instance_name \u003d instance[\u0027name\u0027]"},{"line_number":957,"context_line":"        virt_dom \u003d self._lookup_by_name(instance_name)"},{"line_number":958,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2FZ80%3D","line":955,"in_reply_to":"AAAAPn%2F%2FcR8%3D","updated":"2013-07-02 16:24:17.000000000","message":"yes, if we really need this.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"34193e401a5237a0477c7413a7215649d5d3b979","unresolved":false,"context_lines":[{"line_number":976,"context_line":"            if state \u003d\u003d power_state.RUNNING:"},{"line_number":977,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_AFFECT_LIVE"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"            volume.get_volume_encryptor(kwargs.get(\u0027volume_type\u0027),"},{"line_number":980,"context_line":"                                        connection_info).attach_volume()"},{"line_number":981,"context_line":""},{"line_number":982,"context_line":"            virt_dom.attachDeviceFlags(conf.to_xml(), flags)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2FiYc%3D","line":979,"updated":"2013-05-31 00:02:24.000000000","message":"this mention returns VolumeEncryptor(connection_info) but you don\u0027t seem to use it?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"7f467b39a1ca15753b4cf275c0e59ec8257dabba","unresolved":false,"context_lines":[{"line_number":976,"context_line":"            if state \u003d\u003d power_state.RUNNING:"},{"line_number":977,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_AFFECT_LIVE"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"            volume.get_volume_encryptor(kwargs.get(\u0027volume_type\u0027),"},{"line_number":980,"context_line":"                                        connection_info).attach_volume()"},{"line_number":981,"context_line":""},{"line_number":982,"context_line":"            virt_dom.attachDeviceFlags(conf.to_xml(), flags)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2FgvA%3D","line":979,"in_reply_to":"AAAAPH%2F%2FiYc%3D","updated":"2013-05-31 16:27:27.000000000","message":"attach_volume() is a method of the VolumeEncryptor class that is responsible for setting up the encryption.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        method \u003d getattr(driver, method_name)"},{"line_number":1006,"context_line":"        return method(connection_info, *args, **kwargs)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint, **kwargs):"},{"line_number":1009,"context_line":"        instance_name \u003d instance[\u0027name\u0027]"},{"line_number":1010,"context_line":"        virt_dom \u003d self._lookup_by_name(instance_name)"},{"line_number":1011,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeVs%3D","line":1008,"updated":"2013-07-30 16:54:22.000000000","message":"I would rather see volume_type and encryption key explicitly here, or some sort of volume_info dictionary instead.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        method \u003d getattr(driver, method_name)"},{"line_number":1006,"context_line":"        return method(connection_info, *args, **kwargs)"},{"line_number":1007,"context_line":""},{"line_number":1008,"context_line":"    def attach_volume(self, connection_info, instance, mountpoint, **kwargs):"},{"line_number":1009,"context_line":"        instance_name \u003d instance[\u0027name\u0027]"},{"line_number":1010,"context_line":"        virt_dom \u003d self._lookup_by_name(instance_name)"},{"line_number":1011,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FS6w%3D","line":1008,"in_reply_to":"AAAAQn%2F%2FeVs%3D","updated":"2013-08-02 18:45:08.000000000","message":"Done","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":1029,"context_line":"            if state \u003d\u003d power_state.RUNNING:"},{"line_number":1030,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_AFFECT_LIVE"},{"line_number":1031,"context_line":""},{"line_number":1032,"context_line":"            encryptors.get_volume_encryptor(kwargs.get(\u0027volume_type\u0027),"},{"line_number":1033,"context_line":"                                            connection_info,"},{"line_number":1034,"context_line":"                                            encryption_key\u003dkwargs."},{"line_number":1035,"context_line":"                                                get(\u0027encryption_key\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeUQ%3D","line":1032,"updated":"2013-07-30 16:54:22.000000000","message":"maybe split the get_volume_encryptor and the attach volume call? This is quite confusing to read.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":1029,"context_line":"            if state \u003d\u003d power_state.RUNNING:"},{"line_number":1030,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_AFFECT_LIVE"},{"line_number":1031,"context_line":""},{"line_number":1032,"context_line":"            encryptors.get_volume_encryptor(kwargs.get(\u0027volume_type\u0027),"},{"line_number":1033,"context_line":"                                            connection_info,"},{"line_number":1034,"context_line":"                                            encryption_key\u003dkwargs."},{"line_number":1035,"context_line":"                                                get(\u0027encryption_key\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FS7M%3D","line":1032,"in_reply_to":"AAAAQn%2F%2FeUQ%3D","updated":"2013-08-02 18:45:08.000000000","message":"Done","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"                # it from its encryptor. Otherwise, the encryptor may report"},{"line_number":1100,"context_line":"                # that the volume is still in use."},{"line_number":1101,"context_line":"                encryptors.get_volume_encryptor(kwargs.get(\u0027volume_type\u0027),"},{"line_number":1102,"context_line":"                                                connection_info"},{"line_number":1103,"context_line":"                                               ).detach_volume()"},{"line_number":1104,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1105,"context_line":"            # NOTE(vish): This is called to cleanup volumes after live"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeQE%3D","line":1102,"updated":"2013-07-30 16:54:22.000000000","message":"is this not missing a parram?","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"                # it from its encryptor. Otherwise, the encryptor may report"},{"line_number":1100,"context_line":"                # that the volume is still in use."},{"line_number":1101,"context_line":"                encryptors.get_volume_encryptor(kwargs.get(\u0027volume_type\u0027),"},{"line_number":1102,"context_line":"                                                connection_info"},{"line_number":1103,"context_line":"                                               ).detach_volume()"},{"line_number":1104,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":1105,"context_line":"            # NOTE(vish): This is called to cleanup volumes after live"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FRvg%3D","line":1102,"in_reply_to":"AAAAQn%2F%2FeQE%3D","updated":"2013-08-02 18:45:08.000000000","message":"Don\u0027t think so, but I\u0027ve updated the interface per your other comments.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bba91e21f7265d1e24654e0a724089ff07d6c245","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":325,"context_line":"        self._caps \u003d None"},{"line_number":326,"context_line":"        self._vcpu_total \u003d 0"},{"line_number":327,"context_line":"        self._conductor \u003d conductor.API()"},{"line_number":328,"context_line":"        self.read_only \u003d read_only"},{"line_number":329,"context_line":"        self.firewall_driver \u003d firewall.load_driver("},{"line_number":330,"context_line":"            DEFAULT_FIREWALL_DRIVER,"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FOVg%3D","line":327,"updated":"2013-09-09 18:17:30.000000000","message":"Pleae use the virtapi instead - it now has the block_device_mapping_update method.","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"eafd5cce88fe2b985d711672d8864ad45a5e9f1c","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        self._wrapped_conn_lock \u003d threading.Lock()"},{"line_number":325,"context_line":"        self._caps \u003d None"},{"line_number":326,"context_line":"        self._vcpu_total \u003d 0"},{"line_number":327,"context_line":"        self._conductor \u003d conductor.API()"},{"line_number":328,"context_line":"        self.read_only \u003d read_only"},{"line_number":329,"context_line":"        self.firewall_driver \u003d firewall.load_driver("},{"line_number":330,"context_line":"            DEFAULT_FIREWALL_DRIVER,"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FIrI%3D","line":327,"in_reply_to":"AAAASn%2F%2FOVg%3D","updated":"2013-09-11 11:32:45.000000000","message":"Done","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bba91e21f7265d1e24654e0a724089ff07d6c245","unresolved":false,"context_lines":[{"line_number":3180,"context_line":"                    \u0027device_name\u0027: disk_dev,"},{"line_number":3181,"context_line":"                    \u0027volume_id\u0027: connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":3182,"context_line":"                }"},{"line_number":3183,"context_line":"                self._conductor.block_device_mapping_update_or_create(context,"},{"line_number":3184,"context_line":"                                                                      values)"},{"line_number":3185,"context_line":""},{"line_number":3186,"context_line":"                volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FOVo%3D","line":3183,"updated":"2013-09-09 18:17:30.000000000","message":"We never want to create a bdm entry here - only update it, and we only want to update the connection_info - we have the id available in volume.id - why not do \n\n  self.virtapi.block_device_mapping_update(context, vol.id, {\u0027connection_info\u0027: connection_info})","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"eafd5cce88fe2b985d711672d8864ad45a5e9f1c","unresolved":false,"context_lines":[{"line_number":3180,"context_line":"                    \u0027device_name\u0027: disk_dev,"},{"line_number":3181,"context_line":"                    \u0027volume_id\u0027: connection_info[\u0027data\u0027][\u0027volume_id\u0027]"},{"line_number":3182,"context_line":"                }"},{"line_number":3183,"context_line":"                self._conductor.block_device_mapping_update_or_create(context,"},{"line_number":3184,"context_line":"                                                                      values)"},{"line_number":3185,"context_line":""},{"line_number":3186,"context_line":"                volume_id \u003d connection_info[\u0027data\u0027][\u0027volume_id\u0027]"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FIrE%3D","line":3183,"in_reply_to":"AAAASn%2F%2FOVo%3D","updated":"2013-09-11 11:32:45.000000000","message":"Done","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"}],"nova/virt/libvirt/encryptors/__init__.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    :returns VolumeEncryptor: the VolumeEncryptor for the volume"},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    if volume_type:"},{"line_number":34,"context_line":"        # TODO(joel-coffman): use nova.openstack.common.importutils"},{"line_number":35,"context_line":"        # import_object to initiate the object specified by the volume_type"},{"line_number":36,"context_line":"        if volume_type \u003d\u003d \u0027LUKS\u0027:"},{"line_number":37,"context_line":"            return luks.LuksEncryptor(connection_info, **kwargs)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeLo%3D","line":34,"updated":"2013-07-30 16:54:22.000000000","message":"when is the plan to do these TODOs?","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    :returns VolumeEncryptor: the VolumeEncryptor for the volume"},{"line_number":32,"context_line":"    \"\"\""},{"line_number":33,"context_line":"    if volume_type:"},{"line_number":34,"context_line":"        # TODO(joel-coffman): use nova.openstack.common.importutils"},{"line_number":35,"context_line":"        # import_object to initiate the object specified by the volume_type"},{"line_number":36,"context_line":"        if volume_type \u003d\u003d \u0027LUKS\u0027:"},{"line_number":37,"context_line":"            return luks.LuksEncryptor(connection_info, **kwargs)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FS6s%3D","line":34,"in_reply_to":"AAAAQn%2F%2FeLo%3D","updated":"2013-08-02 18:45:08.000000000","message":"Done","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/virt/libvirt/encryptors/base.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        :param: the connection information used to attach the volume"},{"line_number":40,"context_line":"        \"\"\""},{"line_number":41,"context_line":"        return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def attach_volume(self):"},{"line_number":44,"context_line":"        \"\"\"Hook called immediately prior to attaching a volume to an instance."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeVM%3D","line":41,"updated":"2013-07-30 16:54:22.000000000","message":"I still don\u0027t understand why this should do anything at all?","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        :param: the connection information used to attach the volume"},{"line_number":40,"context_line":"        \"\"\""},{"line_number":41,"context_line":"        return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def attach_volume(self):"},{"line_number":44,"context_line":"        \"\"\"Hook called immediately prior to attaching a volume to an instance."}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FS6o%3D","line":41,"in_reply_to":"AAAAQn%2F%2FeVM%3D","updated":"2013-08-02 18:45:08.000000000","message":"It was just stubbed... now it\u0027s integrated with the key manager interface.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/virt/libvirt/encryptors/cryptsetup.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # a unique name for the volume -- e.g., the iSCSI participant name"},{"line_number":62,"context_line":"        self.dev_name \u003d self.symlink_path.split(\u0027/\u0027)[-1]"},{"line_number":63,"context_line":"        # the device\u0027s actual path on the compute host -- e.g., /dev/sd_"},{"line_number":64,"context_line":"        self.dev_path \u003d os.path.realpath(self.symlink_path)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeNM%3D","line":64,"updated":"2013-07-30 16:54:22.000000000","message":"I think there needs to be a more explicit way to get this information. This doesn\u0027t seem like a general solution.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # a unique name for the volume -- e.g., the iSCSI participant name"},{"line_number":62,"context_line":"        self.dev_name \u003d self.symlink_path.split(\u0027/\u0027)[-1]"},{"line_number":63,"context_line":"        # the device\u0027s actual path on the compute host -- e.g., /dev/sd_"},{"line_number":64,"context_line":"        self.dev_path \u003d os.path.realpath(self.symlink_path)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FRkU%3D","line":64,"in_reply_to":"AAAAQn%2F%2FeNM%3D","updated":"2013-08-02 18:45:08.000000000","message":"Correct me if I\u0027m wrong, but the actual device path is set by `iscsiadm`. I don\u0027t know how to retrieve the iSCSI device (e.g., /dev/sdb) without asking the OS for the symlink target.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/virt/libvirt/encryptors/luks.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        key \u003d self._get_key()"},{"line_number":96,"context_line":"        # LUKS uses a passphrase rather than a raw key -- convert to string"},{"line_number":97,"context_line":"        passphrase \u003d \u0027\u0027.join(hex(x).replace(\u00270x\u0027, \u0027\u0027) for x in key)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        try:"},{"line_number":100,"context_line":"            self._open_volume(passphrase)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeMg%3D","line":97,"updated":"2013-07-30 16:54:22.000000000","message":"is there not a python string formatter for this?","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        key \u003d self._get_key()"},{"line_number":96,"context_line":"        # LUKS uses a passphrase rather than a raw key -- convert to string"},{"line_number":97,"context_line":"        passphrase \u003d \u0027\u0027.join(hex(x).replace(\u00270x\u0027, \u0027\u0027) for x in key)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        try:"},{"line_number":100,"context_line":"            self._open_volume(passphrase)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FR%2Fw%3D","line":97,"in_reply_to":"AAAAQn%2F%2FeMg%3D","updated":"2013-08-02 18:45:08.000000000","message":"I\u0027m not aware of one... We\u0027re converting a list of bytes into a hex string representation.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            pattern \u003d re.compile(\u0027Device \\S+ is not a valid LUKS device.\u0027)"},{"line_number":103,"context_line":"            if e.exit_code \u003d\u003d 1 and pattern.search(e.stderr):"},{"line_number":104,"context_line":"                # the device has never been formatted; format it and try again"},{"line_number":105,"context_line":"                self._format_volume(passphrase)"},{"line_number":106,"context_line":"                self._open_volume(passphrase)"},{"line_number":107,"context_line":"            else:"},{"line_number":108,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeM4%3D","line":105,"updated":"2013-07-30 16:54:22.000000000","message":"Should this kind of thing not really be in cinder? Seems like you might accidentally overwrite someone else\u0027s data.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            pattern \u003d re.compile(\u0027Device \\S+ is not a valid LUKS device.\u0027)"},{"line_number":103,"context_line":"            if e.exit_code \u003d\u003d 1 and pattern.search(e.stderr):"},{"line_number":104,"context_line":"                # the device has never been formatted; format it and try again"},{"line_number":105,"context_line":"                self._format_volume(passphrase)"},{"line_number":106,"context_line":"                self._open_volume(passphrase)"},{"line_number":107,"context_line":"            else:"},{"line_number":108,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FR%2B8%3D","line":105,"in_reply_to":"AAAAQn%2F%2FeM4%3D","updated":"2013-08-02 18:45:08.000000000","message":"Perhaps, but the creation and use of encrypted volumes would be more complex, spanning both Cinder and Nova. The check for the exit code *and* check of standard error should prevent overwriting data by accident (unless of course the header has already been overwritten in which case it\u0027s impossible to recover the unencrypted data).","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/virt/libvirt/encryptors/nop.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def __init__(self, connection_info, **kwargs):"},{"line_number":35,"context_line":"        super(NoOpEncryptor, self).__init__(connection_info, **kwargs)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def attach_volume(self):"},{"line_number":38,"context_line":"        pass"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def detach_volume(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeVE%3D","line":37,"updated":"2013-07-30 16:54:22.000000000","message":"Can\u0027t this be the base class? it would make more sense I think.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def __init__(self, connection_info, **kwargs):"},{"line_number":35,"context_line":"        super(NoOpEncryptor, self).__init__(connection_info, **kwargs)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def attach_volume(self):"},{"line_number":38,"context_line":"        pass"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def detach_volume(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FRvw%3D","line":37,"in_reply_to":"AAAAQn%2F%2FeVE%3D","updated":"2013-08-02 18:45:08.000000000","message":"Please see prior comments on patch set 2: https://review.openstack.org/#/c/30976/2/nova/virt/libvirt/volume.py\n\nIt seemed like a base class that passed on these methods was less desirable than a separate \"no-op\" version.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/virt/libvirt/volume.py":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"34193e401a5237a0477c7413a7215649d5d3b979","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    :param: the connection information used to attach the volume"},{"line_number":880,"context_line":"    \"\"\""},{"line_number":881,"context_line":"    return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"def get_volume_encryptor(volume_type, connection_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2FiYs%3D","line":881,"updated":"2013-05-31 00:02:24.000000000","message":"did you mean to use connection_info here somewhere? It\u0027s not currently used.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    :param: the connection information used to attach the volume"},{"line_number":880,"context_line":"    \"\"\""},{"line_number":881,"context_line":"    return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"def get_volume_encryptor(volume_type, connection_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2FnFo%3D","line":881,"in_reply_to":"AAAAPH%2F%2Fguk%3D","updated":"2013-06-14 12:39:50.000000000","message":"is it cleaner just to throw NotImplemented, and stub it out in the tests?\n\nMaybe this should really go in the VolumeEncryptor base class?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"7f467b39a1ca15753b4cf275c0e59ec8257dabba","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    :param: the connection information used to attach the volume"},{"line_number":880,"context_line":"    \"\"\""},{"line_number":881,"context_line":"    return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"def get_volume_encryptor(volume_type, connection_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2Fguk%3D","line":881,"in_reply_to":"AAAAPH%2F%2FiTo%3D","updated":"2013-05-31 16:27:27.000000000","message":"Yes, this is merely a stub until a key manager is available (or at least a key manager interface is agreed upon).\n\nWe have prototype code that invokes the Cinder client to retrieve the key id, but this code depends upon changes to Cinder\u0027s existing API (see https://review.openstack.org/#/c/30974/), and Gerrit doesn\u0027t seem to support cross-project dependencies yet.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":4992,"name":"malini-k-bhandaru","email":"malini.k.bhandaru@intel.com","username":"malini-k-bhandaru"},"change_message_id":"d8602a601146ff7f3d3cc6957fae97a582c98cdc","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    :param: the connection information used to attach the volume"},{"line_number":880,"context_line":"    \"\"\""},{"line_number":881,"context_line":"    return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"def get_volume_encryptor(volume_type, connection_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2FiTo%3D","line":881,"in_reply_to":"AAAAPH%2F%2FiYs%3D","updated":"2013-05-31 02:16:49.000000000","message":"I think this is a stub till integration with key manager. And is great to get the encryption/decryption aspect implemented and tested. \n\nPersonally think this should be a call to cinder client to get key which in turn should call the key-manager-client to get key after retrieving from cinder the key-id from the volume meta data stored.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    :param: the connection information used to attach the volume"},{"line_number":880,"context_line":"    \"\"\""},{"line_number":881,"context_line":"    return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"def get_volume_encryptor(volume_type, connection_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2FcnE%3D","line":881,"in_reply_to":"AAAAPX%2F%2FnFo%3D","updated":"2013-07-02 14:52:49.000000000","message":"Probably for unit testing, but we wanted end-to-end integration tests to also work... Integration with a key manager will resolve this issue (see comments above).","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6783,"name":"Nathan Reller","email":"nathan.s.reller@gmail.com","username":"rellerreller"},"change_message_id":"c8f16b325f9ce9f1f97f21a3244b7cb51257bb0c","unresolved":false,"context_lines":[{"line_number":878,"context_line":""},{"line_number":879,"context_line":"    :param: the connection information used to attach the volume"},{"line_number":880,"context_line":"    \"\"\""},{"line_number":881,"context_line":"    return array.array(\u0027B\u0027, \u00270123456789ABCDEF\u0027.decode(\"hex\")).tolist()"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"def get_volume_encryptor(volume_type, connection_info):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2Fm6c%3D","line":881,"in_reply_to":"AAAAPX%2F%2FnFo%3D","updated":"2013-06-14 14:15:05.000000000","message":"We did not stub out this code because we did want to provide an encryption capability.  We know that it is not a good solution, but as Joel said it is a place holder until the key manager interface is approved.\n\nThis will allow others to use the encryption feature without having to modify the code.  It will eventually be replaced with a call to a key manager.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"34193e401a5237a0477c7413a7215649d5d3b979","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    if volume_type \u003d\u003d CONF.dmcrypt_volume_encryptor_type:"},{"line_number":891,"context_line":"        return DmCryptEncryptor(connection_info)"},{"line_number":892,"context_line":"    elif volume_type:"},{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2FiYU%3D","line":893,"updated":"2013-05-31 00:02:24.000000000","message":"Should we raise?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    if volume_type \u003d\u003d CONF.dmcrypt_volume_encryptor_type:"},{"line_number":891,"context_line":"        return DmCryptEncryptor(connection_info)"},{"line_number":892,"context_line":"    elif volume_type:"},{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2FnGA%3D","line":893,"in_reply_to":"AAAAPH%2F%2Fgt8%3D","updated":"2013-06-14 12:39:50.000000000","message":"Maybe its worth a warn rather than debug?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"7f467b39a1ca15753b4cf275c0e59ec8257dabba","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    if volume_type \u003d\u003d CONF.dmcrypt_volume_encryptor_type:"},{"line_number":891,"context_line":"        return DmCryptEncryptor(connection_info)"},{"line_number":892,"context_line":"    elif volume_type:"},{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPH%2F%2Fgt8%3D","line":893,"in_reply_to":"AAAAPH%2F%2FiYU%3D","updated":"2013-05-31 16:27:27.000000000","message":"Certainly an option. The default behavior is a volume encryptor that provides *no* encryption so the compute manager or virtualization driver don\u0027t need to know which volumes / volume types are or are not encrypted.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":890,"context_line":"    if volume_type \u003d\u003d CONF.dmcrypt_volume_encryptor_type:"},{"line_number":891,"context_line":"        return DmCryptEncryptor(connection_info)"},{"line_number":892,"context_line":"    elif volume_type:"},{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2FcnQ%3D","line":893,"in_reply_to":"AAAAPX%2F%2FnGA%3D","updated":"2013-07-02 14:52:49.000000000","message":"All volume types aren\u0027t related to encryption -- for example, some may be used for quality of service. Some changes suggested by the Cinder team should allow us to make this distinction and raise an exception when necessary.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"},{"line_number":897,"context_line":""},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"class VolumeEncryptor(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2FnF8%3D","line":896,"updated":"2013-06-14 12:39:50.000000000","message":"Maybe it would be better having a name like NoOpVolumeEncryptor?\n\nClearly it shouldn\u0027t then be the base class, but I am not sure the base class gives you much. You could make the base class a proper abstract (style) class, with a NoOp impl as another child?","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"},{"line_number":897,"context_line":""},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"class VolumeEncryptor(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2Fc8g%3D","line":896,"in_reply_to":"AAAAPX%2F%2FnF8%3D","updated":"2013-07-02 14:52:49.000000000","message":"Done","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6783,"name":"Nathan Reller","email":"nathan.s.reller@gmail.com","username":"rellerreller"},"change_message_id":"c8f16b325f9ce9f1f97f21a3244b7cb51257bb0c","unresolved":false,"context_lines":[{"line_number":893,"context_line":"        LOG.debug(_(\"volume type (%s) not recognized as encrypted\"),"},{"line_number":894,"context_line":"                  volume_type)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    return VolumeEncryptor(connection_info)"},{"line_number":897,"context_line":""},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"class VolumeEncryptor(object):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2Fm50%3D","line":896,"in_reply_to":"AAAAPX%2F%2FnF8%3D","updated":"2013-06-14 14:15:05.000000000","message":"This is certainly doable, and I like the idea.  We were not sure how to handle this because Python does not have interfaces or abstract classes.  We were unsure what best practices were.\n\nWe can create a base VolumeEncryptor class that throws exceptions, a NoOpVolumeEncryptor for no encryption, and keep DmCryptEncryptor.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"00848a3f87f9a56e4e5f8a77b90ba529ad86c744","unresolved":false,"context_lines":[{"line_number":919,"context_line":"        pass"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"class DmCryptEncryptor(VolumeEncryptor):"},{"line_number":923,"context_line":"    \"\"\"A VolumeEncryptor based on dm-crypt."},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"    This VolumeEncryptor uses dm-crypt and LUKS to encrypt the specified"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPX%2F%2FnGQ%3D","line":922,"updated":"2013-06-14 12:39:50.000000000","message":"Should we not follow the model used for cinder volume drivers? Have theses in separate modules loaded via config with some mapping between the type_name and the correct module to load.","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"858656660ba506db458f8f61cd6d996c9b1040a5","unresolved":false,"context_lines":[{"line_number":919,"context_line":"        pass"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"class DmCryptEncryptor(VolumeEncryptor):"},{"line_number":923,"context_line":"    \"\"\"A VolumeEncryptor based on dm-crypt."},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"    This VolumeEncryptor uses dm-crypt and LUKS to encrypt the specified"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2Fck8%3D","line":922,"in_reply_to":"AAAAPX%2F%2FnGQ%3D","updated":"2013-07-02 14:52:49.000000000","message":"Done","commit_id":"2b5ce748ca38f69d9136abd410369d4671b89de3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"026a865fed6d3599b7d184ddcdfe33608382de0e","unresolved":false,"context_lines":[{"line_number":265,"context_line":"            if multipath_device is not None:"},{"line_number":266,"context_line":"                host_device \u003d multipath_device"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        connection_info[\u0027data\u0027][\u0027device_path\u0027] \u003d host_device"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        conf.source_type \u003d \"block\""},{"line_number":271,"context_line":"        conf.source_path \u003d host_device"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FeUo%3D","line":268,"updated":"2013-07-30 16:54:22.000000000","message":"You are returning information via a dictionary you passed it, can we avoid this somehow? Its always confusing when this happens.\n\nEven worse, is that connection_info is currently just the structure returned from cinder, it seems bad to modify that.","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9a3d7a38f00157cdeb9c6afcca66a0853321c674","unresolved":false,"context_lines":[{"line_number":265,"context_line":"            if multipath_device is not None:"},{"line_number":266,"context_line":"                host_device \u003d multipath_device"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        connection_info[\u0027data\u0027][\u0027device_path\u0027] \u003d host_device"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        conf.source_type \u003d \"block\""},{"line_number":271,"context_line":"        conf.source_path \u003d host_device"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2FRnw%3D","line":268,"in_reply_to":"AAAAQn%2F%2FeUo%3D","updated":"2013-08-02 18:45:08.000000000","message":"Done","commit_id":"30022e5ae4177270a85246e7c5ac72a52b73f333"}],"nova/volume/encryptors/__init__.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bba91e21f7265d1e24654e0a724089ff07d6c245","unresolved":false,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def get_volume_encryptor(device_path, **kwargs):"},{"line_number":30,"context_line":"    \"\"\"Creates a VolumeEncryptor used to encrypt the specified volume."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    :param: the connection information used to attach the volume"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FOTE%3D","line":29,"updated":"2013-09-09 18:17:30.000000000","message":"Why is this called device_path - when we\u0027re passing in connetion info - my guess is it needs a rename.","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"eafd5cce88fe2b985d711672d8864ad45a5e9f1c","unresolved":false,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def get_volume_encryptor(device_path, **kwargs):"},{"line_number":30,"context_line":"    \"\"\"Creates a VolumeEncryptor used to encrypt the specified volume."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    :param: the connection information used to attach the volume"}],"source_content_type":"text/x-python","patch_set":16,"id":"AAAASn%2F%2FIr4%3D","line":29,"in_reply_to":"AAAASn%2F%2FOTE%3D","updated":"2013-09-11 11:32:45.000000000","message":"Done","commit_id":"1784f0e62cadbaa9f3197725fc1dd13d351c44e0"}]}