)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"4b362e581b1783d5e84a197d622339617aa413a1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Vu Tran \u003cvu.tran@windriver.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-09-14 19:31:08 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"libvirt: incorrectly create nova/instances/\u003cuuid\u003e/\u003cuuid\u003e_disk with lvm backend"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Configure Nova compute to use lvm backend, spawn an instance based"},{"line_number":10,"context_line":"on glance image, under folder nova/instances/\u003cuuid\u003e we don\u0027t see any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a8ec9b2_ac486bb4","line":7,"updated":"2016-09-21 19:04:08.000000000","message":"nit: commit messages should generally be wrapped at 72 (or 71) characters, except for the 50 (49) character summary line","commit_id":"1f922cfa4c210ae4e8c750defbc3f4519dd9d601"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0ece926fc2a9511589187f9b89d958c5d78e0576","unresolved":false,"context_lines":[{"line_number":6577,"context_line":"            # create backing file in case of qcow2."},{"line_number":6578,"context_line":"            instance_disk \u003d os.path.join(instance_dir, base)"},{"line_number":6579,"context_line":""},{"line_number":6580,"context_line":"            image_type \u003d libvirt_utils.get_disk_type_from_path(info[\u0027path\u0027])"},{"line_number":6581,"context_line":"            if (image_type and not self.image_backend.backend(image_type)."},{"line_number":6582,"context_line":"                    is_file_in_instance_path()):"},{"line_number":6583,"context_line":"                instance_disk \u003d info[\u0027path\u0027]"},{"line_number":6584,"context_line":""},{"line_number":6585,"context_line":"            if not info[\u0027backing_file\u0027] and not os.path.exists(instance_disk):"},{"line_number":6586,"context_line":"                libvirt_utils.create_image(info[\u0027type\u0027], instance_disk,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_2182e194","line":6583,"range":{"start_line":6580,"start_character":12,"end_line":6583,"end_character":44},"updated":"2016-09-21 01:38:28.000000000","message":"In a year from now I\u0027ll have 0 idea what this is here for. Please add a comment above this explaining what this is doing and why it\u0027s here.","commit_id":"1f922cfa4c210ae4e8c750defbc3f4519dd9d601"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5c1993f3a5ba0e058444623c9804e3533b860b13","unresolved":false,"context_lines":[{"line_number":6577,"context_line":"            # create backing file in case of qcow2."},{"line_number":6578,"context_line":"            instance_disk \u003d os.path.join(instance_dir, base)"},{"line_number":6579,"context_line":""},{"line_number":6580,"context_line":"            image_type \u003d libvirt_utils.get_disk_type_from_path(info[\u0027path\u0027])"},{"line_number":6581,"context_line":"            if (image_type and not self.image_backend.backend(image_type)."},{"line_number":6582,"context_line":"                    is_file_in_instance_path()):"},{"line_number":6583,"context_line":"                instance_disk \u003d info[\u0027path\u0027]"},{"line_number":6584,"context_line":""},{"line_number":6585,"context_line":"            if not info[\u0027backing_file\u0027] and not os.path.exists(instance_disk):"},{"line_number":6586,"context_line":"                libvirt_utils.create_image(info[\u0027type\u0027], instance_disk,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_63821b02","line":6583,"range":{"start_line":6580,"start_character":12,"end_line":6583,"end_character":44},"in_reply_to":"7a8ec9b2_2182e194","updated":"2016-09-21 09:53:41.000000000","message":"Gotta agree with Matt here, this is pretty obtuse to be uncommented!\n\nI almost like where you\u0027re going with this, though: this method takes no account of what the disk type is, which is the root of the problem. For (non-backportable) reference, change Idfbca32a resolves this completely by iterating over Image objects in the first instance. Incidentally, I *think* (although I haven\u0027t tested) that it also fixes live-migration for LVM-backed instances.\n\nHowever, I think this is too complicated unless we\u0027re actually going to use the resulting Image object, and even then this isn\u0027t a robust way to initialise an Image object. I think we can address this more simply whilst also addressing the context of this call.\n\n_create_images_and_backing is called in 2 different contexts with different requirements in each case. These are:\n\n1. Hard reboot\n\nHere we\u0027re calling it because we don\u0027t trust that image cache manager hasn\u0027t deleted backing files that we\u0027re still using. The real fix here is to fix image cache manager, but until we get there we\u0027re stuck with this.\n\nThe requirement in this case is check if the backing file is missing, and replace it if it is. We don\u0027t want to do anything if there\u0027s no backing file, e.g. Lvm.\n\n2. Live migration\n\nHere we\u0027re calling it on the destination prior to starting a block migration. Qemu requires that the target disks exist already before it starts copying the data over. Additionally, for qcow2 images libvirt will (deliberately and explicitly) only send bits from the overlay. This means that qcow2 images need to be \u0027blank\u0027, but with a populated backing file.\n\nSide note: this implementation only works for raw and qcow2 files. Rbd works coincidentally because Rbd disks have no path element in the libvirt domain xml and therefore happen to be excluded from the return of _get_instance_disk_info(), which means they\u0027re not in the data structure we\u0027re iterating over here, which is fine because they\u0027re shared anyway and have no backing files. Lvm as you\u0027ve noticed doesn\u0027t work, and Ploop will also not work.\n\nSo (1) only wants to replace a backing file and never touch the disk, (2) assumes the disk doesn\u0027t exist and always wants to create it. The current implementation achieves this for 3 backends only, through a combination of side-effects.\n\nMy problem with your change above is that it works because it implicitly interacts with os.path.exists() on line 6585, which is obtuse. The calling context is very clear here, though: if there\u0027s no backing file, we don\u0027t want to do anything in any case.\n\nTL;DR I think you should add a backing_only\u003dFalse arg to this method, and set it to True when calling from hard reboot. You can then do something like:\n\nif backing_only and not info[\u0027backing_file\u0027]:\n  get_out_of_dodge()\n\nor play with line 6585 or whatever. However, the intention would be clear.\n\nIncidentally, the hard reboot case also doesn\u0027t need to touch kernel and ramdisk, as these aren\u0027t in the image cache and are always flat.","commit_id":"1f922cfa4c210ae4e8c750defbc3f4519dd9d601"}]}