)]}'
{"nova/virt/libvirt/driver.py":[{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"d794ad930bc8bf96ce2c86a619f4f80ee4299ce0","unresolved":false,"context_lines":[{"line_number":3224,"context_line":"            create_func \u003d lambda target: self._create_ephemeral("},{"line_number":3225,"context_line":"                    target, ephemeral_gb, \u0027ephemeral0\u0027, instance.os_type,"},{"line_number":3226,"context_line":"                    is_block_dev\u003ddisk_image.is_block_dev)"},{"line_number":3227,"context_line":"            with disk_image.lock():"},{"line_number":3228,"context_line":"                cache_name \u003d \"ephemeral_%s_%s\" % (ephemeral_gb, file_extension)"},{"line_number":3229,"context_line":"                if disk_image.exists():"},{"line_number":3230,"context_line":"                    disk_image.check_backing_from_func(create_func, cache_name,"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_7612dc0a","line":3227,"updated":"2016-05-14 06:50:05.000000000","message":"Could we place this logic in a method of base Image class?\n\ndriver.py should not be responsible for backing file logic at least. Also it is huge enough to restrict its growth. Moreover, that logic is duplicated here.","commit_id":"901ea4f611d02ba813a05136f3dbddee78ecc05b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5c559636dc5a4a87533e329da3ffb2fc834cdd98","unresolved":false,"context_lines":[{"line_number":3224,"context_line":"            create_func \u003d lambda target: self._create_ephemeral("},{"line_number":3225,"context_line":"                    target, ephemeral_gb, \u0027ephemeral0\u0027, instance.os_type,"},{"line_number":3226,"context_line":"                    is_block_dev\u003ddisk_image.is_block_dev)"},{"line_number":3227,"context_line":"            with disk_image.lock():"},{"line_number":3228,"context_line":"                cache_name \u003d \"ephemeral_%s_%s\" % (ephemeral_gb, file_extension)"},{"line_number":3229,"context_line":"                if disk_image.exists():"},{"line_number":3230,"context_line":"                    disk_image.check_backing_from_func(create_func, cache_name,"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_e331ba0c","line":3227,"in_reply_to":"dab17558_7612dc0a","updated":"2016-05-16 14:27:47.000000000","message":"I\u0027m sympathetic, but I\u0027m inclined to leave it as is because I want to kill this functionality entirely anyway. check_backing_from_func is a workaround for bugs elsewhere because:\n\na) we create ephemeral disks with backing disks, which is a bug and makes no sense anyway.\nb) image cache manager is broken (it deletes things which are still in use).\n\nI\u0027d much prefer to solve the underlying issues, then this mess can go away cleanly without having created new mess elsewhere.\n\nTake, for example, this entire function, which either creates or checks an image. Except that it\u0027s not at all clear to the reader that it does that. Well it wasn\u0027t until I made it explicit here. This function should not be required to have this behaviour at all. I don\u0027t want to have to create a \u0027create_or_check_from_image\u0027 and then subsequently refactor all the backends again when we fix the caller.","commit_id":"901ea4f611d02ba813a05136f3dbddee78ecc05b"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"97fbf20e99397e9acc5251a3b5ce61f347955e3a","unresolved":false,"context_lines":[{"line_number":3224,"context_line":"            create_func \u003d lambda target: self._create_ephemeral("},{"line_number":3225,"context_line":"                    target, ephemeral_gb, \u0027ephemeral0\u0027, instance.os_type,"},{"line_number":3226,"context_line":"                    is_block_dev\u003ddisk_image.is_block_dev)"},{"line_number":3227,"context_line":"            with disk_image.lock():"},{"line_number":3228,"context_line":"                cache_name \u003d \"ephemeral_%s_%s\" % (ephemeral_gb, file_extension)"},{"line_number":3229,"context_line":"                if disk_image.exists():"},{"line_number":3230,"context_line":"                    disk_image.check_backing_from_func(create_func, cache_name,"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_927851c6","line":3227,"in_reply_to":"dab17558_e331ba0c","updated":"2016-05-19 06:58:09.000000000","message":"Besides the check for backing file this logic includes locking and testing for existence. External modules do not have to know about locking and make checks. They might, though, since this module is overloaded by tons of code, it makes sense for me to get it rid of all responsibilities that we can do.\n\nOther argument is cache-create_image pair of functions. The first func is called from outside, the second func is called from the first one getting rid it of standard actions (in theory, but now it requires refactoring as well). It would be consistent to have a similar design for another method of disks creation.\n\nAs for methods name, i hope we\u0027ll find something suitable.","commit_id":"901ea4f611d02ba813a05136f3dbddee78ecc05b"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"97a419690f866437c3b61ce84aaac994153fbe74","unresolved":false,"context_lines":[{"line_number":2901,"context_line":"                      disk_images\u003dNone, network_info\u003dNone,"},{"line_number":2902,"context_line":"                      block_device_info\u003dNone, files\u003dNone,"},{"line_number":2903,"context_line":"                      admin_pass\u003dNone, inject_files\u003dTrue,"},{"line_number":2904,"context_line":"                      fallback_from_host\u003dNone):"},{"line_number":2905,"context_line":"        booted_from_volume \u003d self._is_booted_from_volume("},{"line_number":2906,"context_line":"            instance, disk_mapping)"},{"line_number":2907,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_d7869612","line":2904,"range":{"start_line":2904,"start_character":22,"end_line":2904,"end_character":40},"updated":"2016-06-04 03:56:35.000000000","message":"Consider renaming to simply \u0027fallback\u0027 to match the others.","commit_id":"e02a02ac042b697f055a75764311d015de6372c3"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"97a419690f866437c3b61ce84aaac994153fbe74","unresolved":false,"context_lines":[{"line_number":2996,"context_line":"                    fallback\u003dfallback_from_host)"},{"line_number":2997,"context_line":"            else:"},{"line_number":2998,"context_line":"                root_disk.create_from_image(context, disk_images[\u0027image_id\u0027],"},{"line_number":2999,"context_line":"                                            size, fallback\u003dfallback_from_host)"},{"line_number":3000,"context_line":""},{"line_number":3001,"context_line":"            if need_inject:"},{"line_number":3002,"context_line":"                self._inject_data(root_disk, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_f740f209","line":2999,"range":{"start_line":2999,"start_character":44,"end_line":2999,"end_character":48},"updated":"2016-06-04 03:56:35.000000000","message":"It looks like size could be None here. The onus is currently on the caller to supply a correct size, but I can change that if None is a valid noop case (resize, preallocate, etc). Is it? For all backends? What about LVM (lvm.create_volume)?","commit_id":"e02a02ac042b697f055a75764311d015de6372c3"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"97a419690f866437c3b61ce84aaac994153fbe74","unresolved":false,"context_lines":[{"line_number":6253,"context_line":"            disk \u003d self.image_backend.image(instance, name, image_type\u003d\u0027flat\u0027)"},{"line_number":6254,"context_line":""},{"line_number":6255,"context_line":"            if not disk.exists():"},{"line_number":6256,"context_line":"                disk.create_from_image(context, image_id, 0,"},{"line_number":6257,"context_line":"                                       fallback\u003dfallback_from_host)"},{"line_number":6258,"context_line":""},{"line_number":6259,"context_line":"    def rollback_live_migration_at_destination(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_f708f22a","line":6256,"range":{"start_line":6256,"start_character":58,"end_line":6256,"end_character":59},"updated":"2016-06-04 03:56:35.000000000","message":"Passing a size of zero to \u0027create_from_image\u0027 will always result in errors like the following: \"FlavorDiskSmallerThanImage: Flavor\u0027s disk is too small for requested image. Flavor disk is 0 bytes, image is 4979712 bytes\".\n\nThe onus is currently on the caller to supply a correct size, but I can change that if zero (or perhaps None) is a valid noop case (resize, preallocate, etc).\n\nWhat happens if you create a LVM volume with a size of zero though? Or does this code path not apply to all image backends?","commit_id":"e02a02ac042b697f055a75764311d015de6372c3"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d137efbb822bb9d2edcb7a39a6d096170469262f","unresolved":false,"context_lines":[{"line_number":6253,"context_line":"            disk \u003d self.image_backend.image(instance, name, image_type\u003d\u0027flat\u0027)"},{"line_number":6254,"context_line":""},{"line_number":6255,"context_line":"            if not disk.exists():"},{"line_number":6256,"context_line":"                disk.create_from_image(context, image_id, 0,"},{"line_number":6257,"context_line":"                                       fallback\u003dfallback_from_host)"},{"line_number":6258,"context_line":""},{"line_number":6259,"context_line":"    def rollback_live_migration_at_destination(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_e7201c4c","line":6256,"range":{"start_line":6256,"start_character":58,"end_line":6256,"end_character":59},"in_reply_to":"7aa08908_f708f22a","updated":"2016-06-06 10:09:39.000000000","message":"Yup, the old code accepted size\u003dNone and ignored it, which is what we should be doing here. Will fix.","commit_id":"e02a02ac042b697f055a75764311d015de6372c3"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"fd493d9c4bb2c239d8f398f8648a4d91fccdf574","unresolved":false,"context_lines":[{"line_number":2990,"context_line":"            if instance.task_state \u003d\u003d task_states.RESIZE_FINISH:"},{"line_number":2991,"context_line":"                root_disk.create_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME)"},{"line_number":2992,"context_line":""},{"line_number":2993,"context_line":"            if root_disk.exists():"},{"line_number":2994,"context_line":"                root_disk.check_backing_from_image("},{"line_number":2995,"context_line":"                    context, disk_images[\u0027image_id\u0027],"},{"line_number":2996,"context_line":"                    fallback\u003dfallback_from_host)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7aa08908_f501ee07","line":2993,"updated":"2016-06-15 17:17:17.000000000","message":"Here are several problems:\n\n1 Implementations of create_from_image do not look to include existing disk resizing ability (image.cache method includes it). So that resize feature will not work with this code.\n\n2 Backing file is a part of qcow backend only, driver should not know anything about it. Could we rename the method at least? E.g. to map, or ensure_connection, or validate_existing, or something else which would be more abstract.\n\n3 create_snap is used for resizing, in this case disks exists, so it makes sense to move create_snap under root_disk.exists check.","commit_id":"44c2451b563d12599df6427715a4ccbb3549ac9d"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"5b7e349b5cee10334d4cb8d70b6515aa1c50fc8e","unresolved":false,"context_lines":[{"line_number":2990,"context_line":"            if instance.task_state \u003d\u003d task_states.RESIZE_FINISH:"},{"line_number":2991,"context_line":"                root_disk.create_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME)"},{"line_number":2992,"context_line":""},{"line_number":2993,"context_line":"            if root_disk.exists():"},{"line_number":2994,"context_line":"                root_disk.check_backing_from_image("},{"line_number":2995,"context_line":"                    context, disk_images[\u0027image_id\u0027],"},{"line_number":2996,"context_line":"                    fallback\u003dfallback_from_host)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7aa08908_78945836","line":2993,"in_reply_to":"7aa08908_040ec631","updated":"2016-06-17 19:21:24.000000000","message":"@feodor:\n\nMatt is off now for the weekend, but he started to put together documentation that lists the expectations of the `_create_image` callers. It looks like the resize functionality was indeed missed in the `finish_migration` case. Thanks for catching this!\n\n    https://review.openstack.org/#/c/331116/\n\nOne of the main goals of this refactor is to make the callers of the old `Image.cache` paths explicitly call the functionality they need rather than pigeonholing them all down the same code path that does \"all the things\". We\u0027ll look into addressing the missing resize functionality next week, and then call it explicitly in the cases that require it.\n\nThanks \u0026 have a great weekend!","commit_id":"44c2451b563d12599df6427715a4ccbb3549ac9d"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"fee95617b2abe9f297badd4105d702a487ff2799","unresolved":false,"context_lines":[{"line_number":2990,"context_line":"            if instance.task_state \u003d\u003d task_states.RESIZE_FINISH:"},{"line_number":2991,"context_line":"                root_disk.create_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME)"},{"line_number":2992,"context_line":""},{"line_number":2993,"context_line":"            if root_disk.exists():"},{"line_number":2994,"context_line":"                root_disk.check_backing_from_image("},{"line_number":2995,"context_line":"                    context, disk_images[\u0027image_id\u0027],"},{"line_number":2996,"context_line":"                    fallback\u003dfallback_from_host)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7aa08908_040ec631","line":2993,"in_reply_to":"7aa08908_f501ee07","updated":"2016-06-16 19:20:32.000000000","message":"Re: disk resizing during resize instance operation\n\nThe old code would have called `_try_fetch_image_cache`, which would have called `image.cache`.\n\nIt would have skipped the `create_image` call in `image.cache` because it already exists.\n\n    if not self.exists() or not os.path.exists(base):\n        self.create_image(fetch_func_sync, base, size, *args, **kwargs)\n\nInstead, it would have just resized and fallocated:\n\n   if size:\n        if size \u003e self.get_disk_size(base):\n            self.resize_image(size)\n\n        if (self.preallocate and self._can_fallocate() and\n                os.access(self.path, os.W_OK)):\n            utils.execute(\u0027fallocate\u0027, \u0027-n\u0027, \u0027-l\u0027, size, self.path)\n\nIt does appear that this case isn\u0027t handled by the new code which only calls `check_backing_from_image` if the disk already exists.\n\nmdbooth?","commit_id":"44c2451b563d12599df6427715a4ccbb3549ac9d"},{"author":{"_account_id":10224,"name":"Feodor Tersin","email":"ftersin@hotmail.com","username":"ftersin"},"change_message_id":"a1c2110d90de6381837390f4355315ad85079fc1","unresolved":false,"context_lines":[{"line_number":2991,"context_line":"                root_disk.create_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME)"},{"line_number":2992,"context_line":""},{"line_number":2993,"context_line":"            if root_disk.exists():"},{"line_number":2994,"context_line":"                root_disk.check_backing_from_image("},{"line_number":2995,"context_line":"                    context, disk_images[\u0027image_id\u0027],"},{"line_number":2996,"context_line":"                    fallback\u003dfallback_from_host)"},{"line_number":2997,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":14,"id":"7aa08908_6a1b4281","line":2994,"updated":"2016-06-16 08:49:29.000000000","message":"Questions, asked in the previous patch set at this place, are still actual.","commit_id":"ae326a3e5f573aaa92c7b0aaab1cd8ee356ebe0a"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"2a240bbf0b841b1542d150ba162991b9c80712b2","unresolved":false,"context_lines":[{"line_number":6515,"context_line":"                    disk.check_backing_from_func(create_func,"},{"line_number":6516,"context_line":"                                                 fallback\u003dfallback_from_host)"},{"line_number":6517,"context_line":"                else:"},{"line_number":6518,"context_line":"                    disk.check_backing_from_image(context, instance.image_ref,"},{"line_number":6519,"context_line":"                                                  fallback\u003dfallback_from_host)"},{"line_number":6520,"context_line":""},{"line_number":6521,"context_line":"        # if image has kernel and ramdisk, just download"}],"source_content_type":"text/x-python","patch_set":16,"id":"3aaa91ec_8fb1e049","line":6518,"range":{"start_line":6518,"start_character":20,"end_line":6518,"end_character":49},"updated":"2016-06-24 13:29:12.000000000","message":"Based on the DiskNotFound errors in the logs, if looks like this might need to be revisited. The `checking_backing_from_image` method assumes the disk\u0027s `self.path` exists, and will otherwise raise DiskNotFound.\n\nhttps://github.com/openstack/nova/blob/5730a39ed79a4b1d35d230fc9d4ba239481c9532/nova/virt/images.py#L51","commit_id":"f5bcf0b0c3affa36e88879aee62380c1af10a284"}]}