)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48bbf5ed309de8776086252722448fd0dab5045c","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Previously virDomainBlockRebase [1] was used by swap_volume to switch"},{"line_number":10,"context_line":"between volumes presented to the compute host as block devices or files."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"virDomainBlockCopy [2] is a superset of virDomainBlockRebase when the"},{"line_number":13,"context_line":"VIR_DOMAIN_BLOCK_REBASE_COPY flag is provided as is the case within"},{"line_number":14,"context_line":"swap_volume. As such we can switch to virDomainBlockCopy and expand"},{"line_number":15,"context_line":"support for swap_volume outside of just host block devices and files."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"In order to allow swap_volume to support RBD volumes we also need the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1fa4df85_4dcc1661","line":14,"range":{"start_line":12,"start_character":0,"end_line":14,"end_character":10},"updated":"2020-03-12 15:37:12.000000000","message":"I guess this is the crucial thing in virDomainBlockCopy:\n\n  This command is a superset of the older virDomainBlockRebase() when used\n  with the VIR_DOMAIN_BLOCK_REBASE_COPY flag, and offers better control\n  over the destination format, the ability to copy to a destination that\n  is not a local file, and the possibility of additional tuning parameters.\n\nMight be worth reproducing verbatim here. Took me a bit of reading to grasp why virDomainBlockCopy being a superset of virDomainBlockRebase was significant","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5cefabb60fc5c6dc193d8028f3bda04663904a21","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Previously virDomainBlockRebase [1] was used by swap_volume to switch"},{"line_number":10,"context_line":"between volumes presented to the compute host as block devices or files."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"virDomainBlockCopy [2] is a superset of virDomainBlockRebase when the"},{"line_number":13,"context_line":"VIR_DOMAIN_BLOCK_REBASE_COPY flag is provided as is the case within"},{"line_number":14,"context_line":"swap_volume. As such we can switch to virDomainBlockCopy and expand"},{"line_number":15,"context_line":"support for swap_volume outside of just host block devices and files."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"In order to allow swap_volume to support RBD volumes we also need the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1fa4df85_b9493933","line":14,"range":{"start_line":12,"start_character":0,"end_line":14,"end_character":10},"in_reply_to":"1fa4df85_4dcc1661","updated":"2020-03-13 14:58:03.000000000","message":"Done","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8015006e28385b9c43db2d5d95c71c0446e1b87c","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockRebase"},{"line_number":31,"context_line":"[2] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I8e8035dcf508f5215bba9b7575c5c6abfe41da31"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"df33271e_10ab4989","line":32,"updated":"2020-03-24 15:30:35.000000000","message":"Is it a bugfix or a small libvirt driver feature? Can we track this change in some existing item?","commit_id":"9d8141c9c40eb58c6bc04e6188e9ebe313da7ce7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"11d923be171ce085c26182423b2b91ec41961537","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"[1] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockRebase"},{"line_number":31,"context_line":"[2] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Change-Id: I8e8035dcf508f5215bba9b7575c5c6abfe41da31"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"df33271e_5c66792b","line":32,"in_reply_to":"df33271e_10ab4989","updated":"2020-03-25 12:39:54.000000000","message":"I wasn\u0027t going to but if anything I\u0027d classify this as a small bugfix given this isn\u0027t something we expose through the API.\n\nI\u0027ve written up a bug #1868996 and will amend it here now.","commit_id":"9d8141c9c40eb58c6bc04e6188e9ebe313da7ce7"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5e8d0d8ae2a9ec76e114d36ba522bca83670a74c","unresolved":false,"context_lines":[{"line_number":18649,"context_line":"        mock_dev.copy.assert_called_once_with("},{"line_number":18650,"context_line":"            mock.sentinel.conf_xml, reuse_ext\u003dTrue)"},{"line_number":18651,"context_line":""},{"line_number":18652,"context_line":"        # Assert the new domain XML is written to disk on success"},{"line_number":18653,"context_line":"        mock_write_instance_config.assert_called_once_with("},{"line_number":18654,"context_line":"            mock.sentinel.original_xml_desc)"},{"line_number":18655,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"1fa4df85_1c715be1","line":18652,"range":{"start_line":18652,"start_character":21,"end_line":18652,"end_character":24},"updated":"2020-03-13 15:43:27.000000000","message":"original","commit_id":"7259a78bb17884d6f13fbb6b74b6184155dc340a"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"cc10fec1a7019472317b16367db0d1b8fb13de35","unresolved":false,"context_lines":[{"line_number":283,"context_line":"MIN_QEMU_PMEM_SUPPORT \u003d (3, 1, 0)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# -blockdev support (replacing -drive)"},{"line_number":286,"context_line":"MIN_LIBVIRT_BLOCKDEV \u003d (5, 10, 0)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_f592574b","line":286,"range":{"start_line":286,"start_character":0,"end_line":286,"end_character":33},"updated":"2020-03-13 15:00:18.000000000","message":"Even this might not be sufficient due to a regression affecting versions: libvirt-5.10, libvirt-6.0 and libvirt-6.1. \n\nI wonder if we have to wait for the next libvirt, with the fix for the regressions.  See my exchange with Peter Krempa in this thread:\n\nhttps://www.redhat.com/archives/libvir-list/2020-March/msg00495.html –  \"[PATCH 5/5] qemu: blockcopy: Allow late opening of the backing chain of a shallow copy\"","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2db9fb4fa979f4f4388858a7cfe1a334fd2d7cac","unresolved":false,"context_lines":[{"line_number":283,"context_line":"MIN_QEMU_PMEM_SUPPORT \u003d (3, 1, 0)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# -blockdev support (replacing -drive)"},{"line_number":286,"context_line":"MIN_LIBVIRT_BLOCKDEV \u003d (5, 10, 0)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_d762a7ad","line":286,"range":{"start_line":286,"start_character":0,"end_line":286,"end_character":33},"in_reply_to":"1fa4df85_7c340fdf","updated":"2020-03-16 15:00:16.000000000","message":"Okay, we\u0027re not using that flag; but was just noting, in case anything else from that fixed version we might need here.\n\nThat said, 5.10.0 is still not recommended by libvirt folks.  Peter Krempa helpfully reminded that 5.10.0 has a potential image corruption bug if you don\u0027t specify the format, so we don\u0027t want that.\n\nWhat\u0027s recommended:\n\n- libvirt 6.1.0\n- QEMU 4.2\n\nSo we also need MIN_QEMU_BLOCKDEV.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7e142aaaa50de2d22eec24bd93b88c3c42090a03","unresolved":false,"context_lines":[{"line_number":283,"context_line":"MIN_QEMU_PMEM_SUPPORT \u003d (3, 1, 0)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# -blockdev support (replacing -drive)"},{"line_number":286,"context_line":"MIN_LIBVIRT_BLOCKDEV \u003d (5, 10, 0)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_ed6cb49d","line":286,"range":{"start_line":286,"start_character":0,"end_line":286,"end_character":33},"in_reply_to":"1fa4df85_d762a7ad","updated":"2020-03-16 15:47:53.000000000","message":"We\u0027ve spoken about this downstream and initial I was against doing this given the feature landed earlier.\n\nHowever on reflection given this covers an image corruption bug I\u0027m going to update to force these constraints.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dd24ff85586835f95edf93273f336fbd8813f5a5","unresolved":false,"context_lines":[{"line_number":283,"context_line":"MIN_QEMU_PMEM_SUPPORT \u003d (3, 1, 0)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# -blockdev support (replacing -drive)"},{"line_number":286,"context_line":"MIN_LIBVIRT_BLOCKDEV \u003d (5, 10, 0)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_7c340fdf","line":286,"range":{"start_line":286,"start_character":0,"end_line":286,"end_character":33},"in_reply_to":"1fa4df85_f592574b","updated":"2020-03-13 15:03:39.000000000","message":"This isn\u0027t using the VIR_DOMAIN_BLOCK_COPY_SHALLOW flag so I think we should be fine.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48bbf5ed309de8776086252722448fd0dab5045c","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                                        encryption\u003dencryption)"},{"line_number":1768,"context_line":""},{"line_number":1769,"context_line":"    def _swap_volume(self, guest, disk_dev, conf, resize_to):"},{"line_number":1770,"context_line":"        \"\"\"Swap existing disk with a new block device.\"\"\""},{"line_number":1771,"context_line":"        dev \u003d guest.get_block_device(disk_dev)"},{"line_number":1772,"context_line":""},{"line_number":1773,"context_line":"        # Save a copy of the domain\u0027s persistent XML file. We\u0027ll use this"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_08312ced","line":1770,"range":{"start_line":1770,"start_character":8,"end_line":1770,"end_character":57},"updated":"2020-03-12 15:37:12.000000000","message":"optional and kind of unrelated, but any chance of getting a docstring while you\u0027re here? Type hints would be cool too","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5cefabb60fc5c6dc193d8028f3bda04663904a21","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                                        encryption\u003dencryption)"},{"line_number":1768,"context_line":""},{"line_number":1769,"context_line":"    def _swap_volume(self, guest, disk_dev, conf, resize_to):"},{"line_number":1770,"context_line":"        \"\"\"Swap existing disk with a new block device.\"\"\""},{"line_number":1771,"context_line":"        dev \u003d guest.get_block_device(disk_dev)"},{"line_number":1772,"context_line":""},{"line_number":1773,"context_line":"        # Save a copy of the domain\u0027s persistent XML file. We\u0027ll use this"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_794e61c2","line":1770,"range":{"start_line":1770,"start_character":8,"end_line":1770,"end_character":57},"in_reply_to":"1fa4df85_08312ced","updated":"2020-03-13 14:58:03.000000000","message":"Done","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"5fbcb8a4def22d6552fd95005f2b3d38e7b5cc13","unresolved":false,"context_lines":[{"line_number":1782,"context_line":"            pass"},{"line_number":1783,"context_line":""},{"line_number":1784,"context_line":"        try:"},{"line_number":1785,"context_line":"            # NOTE (rmk): blockRebase cannot be executed on persistent"},{"line_number":1786,"context_line":"            #             domains, so we need to temporarily undefine it."},{"line_number":1787,"context_line":"            #             If any part of this block fails, the domain is"},{"line_number":1788,"context_line":"            #             re-defined regardless."},{"line_number":1789,"context_line":"            if guest.has_persistent_configuration():"},{"line_number":1790,"context_line":"                support_uefi \u003d self._has_uefi_support()"},{"line_number":1791,"context_line":"                guest.delete_configuration(support_uefi)"},{"line_number":1792,"context_line":""},{"line_number":1793,"context_line":"            try:"},{"line_number":1794,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_88b99cea","line":1791,"range":{"start_line":1785,"start_character":0,"end_line":1791,"end_character":56},"updated":"2020-03-12 15:29:51.000000000","message":"[Not the problem of this patch, but worth fixing, IMHO.]\n\nA couple of notes:\n\n- The comment by \u0027rmk\u0027 should be extended to note that blockRebase() *and* blockCopy() both have this limitation of \"you need to undefine the guest, do the block{Rebase,Copy}, then re-define it\".\n\n- Also I\u0027d add a TODO item here: in a future patch, we should explore using the new-and-fancy virDomainBackupBegin() API, which does not have the above mentioned limitation of having to temporarily undefine the and later re-define it.  This API is one of the building blocks of the \u0027incremental backup\u0027 work in libvirt; hence the name.  (https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBackupBegin)\n\nNote though, virDomainBackupBegin() will be a _replacement_ for virDomainBlockCopy().  So if we\u0027re not in a big hurry, we could even start exploring virDomainBackupBegin() to begin with.  (I have a personal TODO item for this, and also run this idea past libvirt / QEMU Block Folks.)\n\nBut since you\u0027ve already done this work, we can go with virDomainBlockCopy() and explore virDomainBackupBegin() as a future work item.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2db9fb4fa979f4f4388858a7cfe1a334fd2d7cac","unresolved":false,"context_lines":[{"line_number":1782,"context_line":"            pass"},{"line_number":1783,"context_line":""},{"line_number":1784,"context_line":"        try:"},{"line_number":1785,"context_line":"            # NOTE (rmk): blockRebase cannot be executed on persistent"},{"line_number":1786,"context_line":"            #             domains, so we need to temporarily undefine it."},{"line_number":1787,"context_line":"            #             If any part of this block fails, the domain is"},{"line_number":1788,"context_line":"            #             re-defined regardless."},{"line_number":1789,"context_line":"            if guest.has_persistent_configuration():"},{"line_number":1790,"context_line":"                support_uefi \u003d self._has_uefi_support()"},{"line_number":1791,"context_line":"                guest.delete_configuration(support_uefi)"},{"line_number":1792,"context_line":""},{"line_number":1793,"context_line":"            try:"},{"line_number":1794,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_f747a3bb","line":1791,"range":{"start_line":1785,"start_character":0,"end_line":1791,"end_character":56},"in_reply_to":"1fa4df85_192c0d46","updated":"2020-03-16 15:00:16.000000000","message":"tl;dr: let\u0027s abandon this blockBackup() route, and just go with blockCopy() as you have it now.\n\nYou\u0027re indeed right in that blockBackup() does not allow to pivot.  (Aside: My reference was just a conversation with Eric Blake from some months ago, where he mentioned backupBegin() could be a replacement.  But when I checked this morning with Peter, he suggests _not_ to use it.)\n\nI assumed this would be possible: (a) issue a call to backupBegin(); then (b) call blockJobAbort() with _JOB_ABORT_PIVOT flag.  (To avoid the \"undefine + copy + re-redefine\" dance.)\n\nBut talking to Peter Krempa this morning, the above is \"impossible\" because the image state of backupBegin() is from the point where the backup job has started, and thus switching to that backup will break the guest.\n\nIn summary, two main differences b/n blockCopy() and backupBegin():\n\n- The resulting image created by backupBegin() has a copy up to the point where the backupBegin() call was issued\n- You cannot switch to the copy at the end of the job.  (Even if you _could_, that\u0027s not what we want—because of the earlier mentioned reasons.)\n\nSo, let\u0027s park this \"make the guest temporarily transient\" thing aside for now, and proceed with blockCopy() usage.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5cefabb60fc5c6dc193d8028f3bda04663904a21","unresolved":false,"context_lines":[{"line_number":1782,"context_line":"            pass"},{"line_number":1783,"context_line":""},{"line_number":1784,"context_line":"        try:"},{"line_number":1785,"context_line":"            # NOTE (rmk): blockRebase cannot be executed on persistent"},{"line_number":1786,"context_line":"            #             domains, so we need to temporarily undefine it."},{"line_number":1787,"context_line":"            #             If any part of this block fails, the domain is"},{"line_number":1788,"context_line":"            #             re-defined regardless."},{"line_number":1789,"context_line":"            if guest.has_persistent_configuration():"},{"line_number":1790,"context_line":"                support_uefi \u003d self._has_uefi_support()"},{"line_number":1791,"context_line":"                guest.delete_configuration(support_uefi)"},{"line_number":1792,"context_line":""},{"line_number":1793,"context_line":"            try:"},{"line_number":1794,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_192c0d46","line":1791,"range":{"start_line":1785,"start_character":0,"end_line":1791,"end_character":56},"in_reply_to":"1fa4df85_88b99cea","updated":"2020-03-13 14:58:03.000000000","message":"I\u0027ll update the comment.\n\nI\u0027m not sure about using virDomainBackupBegin here however.\n\nThere\u0027s no concept of pivoting to the new disk in that API AFAICT. Can you provide any references to it replacing virDomainBlockCopy in this use case where we copy data from a src disk to dst and then switch to the dst disk.\n\nI get that it would be a replacement in the backup usecase but I can\u0027t think how it would be for ours.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48bbf5ed309de8776086252722448fd0dab5045c","unresolved":false,"context_lines":[{"line_number":1809,"context_line":"                dev.abort_job(pivot\u003dTrue)"},{"line_number":1810,"context_line":""},{"line_number":1811,"context_line":"            except Exception as exc:"},{"line_number":1812,"context_line":"                new_path \u003d conf.target_dev"},{"line_number":1813,"context_line":"                old_path \u003d disk_dev"},{"line_number":1814,"context_line":"                if conf.source_path:"},{"line_number":1815,"context_line":"                    new_path \u003d conf.source_path"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_a874989d","line":1812,"range":{"start_line":1812,"start_character":16,"end_line":1812,"end_character":42},"updated":"2020-03-12 15:37:12.000000000","message":"nit:\n\n  new_path \u003d conf.source_path or conf.target_dev\n\n? Also, a comment explaining why we switch would be helpful","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5cefabb60fc5c6dc193d8028f3bda04663904a21","unresolved":false,"context_lines":[{"line_number":1809,"context_line":"                dev.abort_job(pivot\u003dTrue)"},{"line_number":1810,"context_line":""},{"line_number":1811,"context_line":"            except Exception as exc:"},{"line_number":1812,"context_line":"                new_path \u003d conf.target_dev"},{"line_number":1813,"context_line":"                old_path \u003d disk_dev"},{"line_number":1814,"context_line":"                if conf.source_path:"},{"line_number":1815,"context_line":"                    new_path \u003d conf.source_path"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_b920594b","line":1812,"range":{"start_line":1812,"start_character":16,"end_line":1812,"end_character":42},"in_reply_to":"1fa4df85_a874989d","updated":"2020-03-13 14:58:03.000000000","message":"ACK thanks.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e19314c20e82b96a60dcbf8de705b4520b995e44","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"                guest.delete_configuration(support_uefi)"},{"line_number":1807,"context_line":""},{"line_number":1808,"context_line":"            try:"},{"line_number":1809,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"},{"line_number":1810,"context_line":"                # with -blockdev domains allowing QEMU to copy to remote disks"},{"line_number":1811,"context_line":"                if self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_BLOCKDEV,"},{"line_number":1812,"context_line":"                                              hv_ver\u003dMIN_QEMU_BLOCKDEV):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_8da18049","line":1809,"range":{"start_line":1809,"start_character":72,"end_line":1809,"end_character":78},"updated":"2020-03-16 16:37:47.000000000","message":"Forgot to change to 6.1.0 :)\n\nUltra-nit: \"libvirt\" is always lower-case, except for the obvious case (when it\u0027s at the start of a sentence).","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6957fcb47735145380cf202e849b981f82c222b8","unresolved":false,"context_lines":[{"line_number":1806,"context_line":"                guest.delete_configuration(support_uefi)"},{"line_number":1807,"context_line":""},{"line_number":1808,"context_line":"            try:"},{"line_number":1809,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"},{"line_number":1810,"context_line":"                # with -blockdev domains allowing QEMU to copy to remote disks"},{"line_number":1811,"context_line":"                if self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_BLOCKDEV,"},{"line_number":1812,"context_line":"                                              hv_ver\u003dMIN_QEMU_BLOCKDEV):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_862dd547","line":1809,"range":{"start_line":1809,"start_character":72,"end_line":1809,"end_character":78},"in_reply_to":"1fa4df85_8da18049","updated":"2020-03-17 10:42:07.000000000","message":"Done","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e19314c20e82b96a60dcbf8de705b4520b995e44","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"            try:"},{"line_number":1809,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"},{"line_number":1810,"context_line":"                # with -blockdev domains allowing QEMU to copy to remote disks"},{"line_number":1811,"context_line":"                if self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_BLOCKDEV,"},{"line_number":1812,"context_line":"                                              hv_ver\u003dMIN_QEMU_BLOCKDEV):"},{"line_number":1813,"context_line":"                    dev.copy(conf.to_xml(), reuse_ext\u003dTrue)"},{"line_number":1814,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_08025eb5","line":1811,"range":{"start_line":1811,"start_character":24,"end_line":1811,"end_character":45},"updated":"2020-03-16 16:37:47.000000000","message":"Also, I\u0027ll take it that this check will run on destination, too, to take into account of: destination being remote RBD, but doesn\u0027t support \u0027-blockdev\u0027.","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"7fbe54c8f2d999d3f0932c6466216476547096a2","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"            try:"},{"line_number":1809,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"},{"line_number":1810,"context_line":"                # with -blockdev domains allowing QEMU to copy to remote disks"},{"line_number":1811,"context_line":"                if self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_BLOCKDEV,"},{"line_number":1812,"context_line":"                                              hv_ver\u003dMIN_QEMU_BLOCKDEV):"},{"line_number":1813,"context_line":"                    dev.copy(conf.to_xml(), reuse_ext\u003dTrue)"},{"line_number":1814,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_62f3cf9f","line":1811,"range":{"start_line":1811,"start_character":24,"end_line":1811,"end_character":45},"in_reply_to":"1fa4df85_0627a514","updated":"2020-03-18 13:29:50.000000000","message":"No problem.  The wording in the \"copy to remote disks\" comment made me trip, then maybe that should be rephrased?\n\nBesides that I don\u0027t see any big issues here.","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6957fcb47735145380cf202e849b981f82c222b8","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"            try:"},{"line_number":1809,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"},{"line_number":1810,"context_line":"                # with -blockdev domains allowing QEMU to copy to remote disks"},{"line_number":1811,"context_line":"                if self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_BLOCKDEV,"},{"line_number":1812,"context_line":"                                              hv_ver\u003dMIN_QEMU_BLOCKDEV):"},{"line_number":1813,"context_line":"                    dev.copy(conf.to_xml(), reuse_ext\u003dTrue)"},{"line_number":1814,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_a63011a0","line":1811,"range":{"start_line":1811,"start_character":24,"end_line":1811,"end_character":45},"in_reply_to":"1fa4df85_08025eb5","updated":"2020-03-17 10:42:07.000000000","message":"Done","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"a8b3632a674a41cd39fc4aed416ccb1c8ac13ba9","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"            try:"},{"line_number":1809,"context_line":"                # NOTE(lyarwood): Use virDomainBlockCopy from Libvirt \u003e\u003d5.10.0"},{"line_number":1810,"context_line":"                # with -blockdev domains allowing QEMU to copy to remote disks"},{"line_number":1811,"context_line":"                if self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_BLOCKDEV,"},{"line_number":1812,"context_line":"                                              hv_ver\u003dMIN_QEMU_BLOCKDEV):"},{"line_number":1813,"context_line":"                    dev.copy(conf.to_xml(), reuse_ext\u003dTrue)"},{"line_number":1814,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_0627a514","line":1811,"range":{"start_line":1811,"start_character":24,"end_line":1811,"end_character":45},"in_reply_to":"1fa4df85_a63011a0","updated":"2020-03-17 11:06:20.000000000","message":"Apologies I didn\u0027t mean to mark this as Done.\n\nI\u0027m not entirely sure what you mean here, there\u0027s no concept of remote hosts when swapping volumes. The check here is just against a single host.","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e19314c20e82b96a60dcbf8de705b4520b995e44","unresolved":false,"context_lines":[{"line_number":1813,"context_line":"                    dev.copy(conf.to_xml(), reuse_ext\u003dTrue)"},{"line_number":1814,"context_line":"                else:"},{"line_number":1815,"context_line":"                    # TODO(lyarwood): Remove the following use of"},{"line_number":1816,"context_line":"                    # virDomainBlockRebase once MIN_LIBVIRT hits \u003e\u003d 5.10.0"},{"line_number":1817,"context_line":"                    # Start copy with VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT flag to"},{"line_number":1818,"context_line":"                    # allow writing to existing external volume file. Use"},{"line_number":1819,"context_line":"                    # VIR_DOMAIN_BLOCK_REBASE_COPY_DEV if it\u0027s a block device"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_ed9114f3","line":1816,"range":{"start_line":1816,"start_character":68,"end_line":1816,"end_character":74},"updated":"2020-03-16 16:37:47.000000000","message":"Here, too, 6.1.0.","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6957fcb47735145380cf202e849b981f82c222b8","unresolved":false,"context_lines":[{"line_number":1813,"context_line":"                    dev.copy(conf.to_xml(), reuse_ext\u003dTrue)"},{"line_number":1814,"context_line":"                else:"},{"line_number":1815,"context_line":"                    # TODO(lyarwood): Remove the following use of"},{"line_number":1816,"context_line":"                    # virDomainBlockRebase once MIN_LIBVIRT hits \u003e\u003d 5.10.0"},{"line_number":1817,"context_line":"                    # Start copy with VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT flag to"},{"line_number":1818,"context_line":"                    # allow writing to existing external volume file. Use"},{"line_number":1819,"context_line":"                    # VIR_DOMAIN_BLOCK_REBASE_COPY_DEV if it\u0027s a block device"}],"source_content_type":"text/x-python","patch_set":8,"id":"1fa4df85_4627dd65","line":1816,"range":{"start_line":1816,"start_character":68,"end_line":1816,"end_character":74},"in_reply_to":"1fa4df85_ed9114f3","updated":"2020-03-17 10:42:07.000000000","message":"Done","commit_id":"4159961a43865908702474fc635c02991b7a49b4"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"4fc23457758391e4217aebf3978eae9d72dff9b1","unresolved":false,"context_lines":[{"line_number":283,"context_line":"MIN_QEMU_PMEM_SUPPORT \u003d (3, 1, 0)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# -blockdev support (replacing -drive)"},{"line_number":286,"context_line":"MIN_LIBVIRT_BLOCKDEV \u003d (6, 1, 0)"},{"line_number":287,"context_line":"MIN_QEMU_BLOCKDEV \u003d (4, 3, 0)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_4e738416","line":287,"range":{"start_line":286,"start_character":0,"end_line":287,"end_character":29},"updated":"2020-03-23 15:12:55.000000000","message":"Good news:\n\nYes, for libvirt we can lower to 6.0.0, given that we have this[1] merged.  (I also confirmed this w/ upstream libvirt folks.)  So the new constants are:\n\n* MIN_LIBVIRT_BLOCKDEV \u003d 6.0.0\n* MIN_QEMU_BLOCKDEV \u003d 4.2.0  (this was already the case; the 4.3.0 in Lee\u0027s patch is just an inadvertent typo)\n\n[1] https://opendev.org/openstack/nova/commit/0cfe9c81e3fe4d – libvirt: Provide the backing file format when creating qcow2 disks","commit_id":"b6b8073d4386317f5ffbbb24ec0b5a5f1252b644"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fbd475e8905971f0afa1384690d0c6591cd35823","unresolved":false,"context_lines":[{"line_number":283,"context_line":"MIN_QEMU_PMEM_SUPPORT \u003d (3, 1, 0)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# -blockdev support (replacing -drive)"},{"line_number":286,"context_line":"MIN_LIBVIRT_BLOCKDEV \u003d (6, 1, 0)"},{"line_number":287,"context_line":"MIN_QEMU_BLOCKDEV \u003d (4, 3, 0)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_4ea9c479","line":287,"range":{"start_line":286,"start_character":0,"end_line":287,"end_character":29},"updated":"2020-03-23 15:03:30.000000000","message":"we might want to drop this to libvirt:6.0 and qemu:4.2\nso that we can test this with ubuntu 20.04 in the future.","commit_id":"b6b8073d4386317f5ffbbb24ec0b5a5f1252b644"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48bbf5ed309de8776086252722448fd0dab5045c","unresolved":false,"context_lines":[{"line_number":756,"context_line":"            cur\u003dstatus[\u0027cur\u0027],"},{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_0d9a9e6d","line":759,"range":{"start_line":759,"start_character":61,"end_line":759,"end_character":76},"updated":"2020-03-12 15:37:12.000000000","message":"...and neither is this. Is this purely here to fully expose this libvirt API? Looking down, I guess we do this elsewhere too but I\u0027m curious","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48bbf5ed309de8776086252722448fd0dab5045c","unresolved":false,"context_lines":[{"line_number":756,"context_line":"            cur\u003dstatus[\u0027cur\u0027],"},{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_ed9ce26b","line":759,"range":{"start_line":759,"start_character":29,"end_line":759,"end_character":42},"updated":"2020-03-12 15:37:12.000000000","message":"This isn\u0027t set to non-default values anywhere...","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5cefabb60fc5c6dc193d8028f3bda04663904a21","unresolved":false,"context_lines":[{"line_number":756,"context_line":"            cur\u003dstatus[\u0027cur\u0027],"},{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_b96b791d","line":759,"range":{"start_line":759,"start_character":29,"end_line":759,"end_character":42},"in_reply_to":"1fa4df85_ed9ce26b","updated":"2020-03-13 14:58:03.000000000","message":"Yup just following the apparent practice for the other block calls, I can remove but it\u0027s trivial to add these now tbh.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"5fbcb8a4def22d6552fd95005f2b3d38e7b5cc13","unresolved":false,"context_lines":[{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":763,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_2d3afacc","line":760,"range":{"start_line":760,"start_character":11,"end_line":760,"end_character":58},"updated":"2020-03-12 15:29:51.000000000","message":"Since right below is rebase() wrapper, it would be useful to extend the documentation to contrast with it those-who-don\u0027t-dwell-on-the-Block-Matters.\n\nIn the expanded version of the docs, I\u0027d consider the below two points:\n\n- \"Nova\u0027s copy() is a wrapper method around libvirt\u0027s blockCopy() API, just as the rebase() is a wrapper around the blockRebase() API.  blockCopy() is a superset of blockRebase() — i.e. whatever you had working with blockRebase() still should/will work with blockRebase().\"\n\n- Also I wonder how much of some of blockCopy()\u0027s useful features to be mentioned here in the docstring, as we might want to use some of them.  E.g. the blockCopy() job has two phases: in phase-1, it copies all data from the source, and while in this phase, \"the job can only be canceled to revert to the source disk, with no guarantees about the destination\"; after phase-1 completes, both the source and the destination remain mirrored until a call to blockJobAbort() with the flag _ABORT (means: the destination is a \"faithful copy of that point in time\") or _PIVOT (means: \"pivot over to the new copy on the destination\").\n\nI wonder if we should also add a TODO to deprecate and remove the use of rebase() method in Nova.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"07333f96a6613879acbc2b853fbf2961e9553600","unresolved":false,"context_lines":[{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":763,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_c3e0d7cf","line":760,"range":{"start_line":760,"start_character":11,"end_line":760,"end_character":58},"in_reply_to":"1fa4df85_2d3afacc","updated":"2020-03-12 15:56:23.000000000","message":"\u003e Since right below is rebase() wrapper, it would be useful to extend\n \u003e the documentation to contrast with it those-who-don\u0027t-dwell-on-the-Block-Matters. \n\nMissing a word: \"with it\" --\u003e \"with it for those-who...\"\n\n[...]","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"5cefabb60fc5c6dc193d8028f3bda04663904a21","unresolved":false,"context_lines":[{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":763,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_59660554","line":760,"range":{"start_line":760,"start_character":11,"end_line":760,"end_character":58},"in_reply_to":"1fa4df85_2d3afacc","updated":"2020-03-13 14:58:03.000000000","message":"We can\u0027t deprecate rebase until we also switch _live_snapshot:\n\nhttps://github.com/openstack/nova/blob/e483ca1cd9a5db5856f87fc69ca07c42d2be5def/nova/virt/libvirt/driver.py#L2478\n\nI\u0027m not going to repeat the Libvirt documentation in here as it\u0027s going to get stale fast if virDomainBlockCopy changes. I think a reference to the official docs is enough here IMHO.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7e142aaaa50de2d22eec24bd93b88c3c42090a03","unresolved":false,"context_lines":[{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":763,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_cd0778c9","line":760,"range":{"start_line":760,"start_character":11,"end_line":760,"end_character":58},"in_reply_to":"1fa4df85_37da1be9","updated":"2020-03-16 15:47:53.000000000","message":"I\u0027m still leaving that to the Libvirt docs.","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"2db9fb4fa979f4f4388858a7cfe1a334fd2d7cac","unresolved":false,"context_lines":[{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":763,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_37da1be9","line":760,"range":{"start_line":760,"start_character":11,"end_line":760,"end_character":58},"in_reply_to":"1fa4df85_59660554","updated":"2020-03-16 15:00:16.000000000","message":"Oh, right, on switching _live_snapshot(); can be separately tackled.\n\nOn docs: I\u0027m not saying to repeat everything, but having a concise doc-string right there is useful.  Also, most people dwell far away from these Block Layer intricacies :)  (On getting stale: the core semantics of the API won\u0027t change, so FWIW, I don\u0027t see staleness as an issue.)","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"e19314c20e82b96a60dcbf8de705b4520b995e44","unresolved":false,"context_lines":[{"line_number":757,"context_line":"            end\u003dstatus[\u0027end\u0027])"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":"    def copy(self, dest_xml, shallow\u003dFalse, reuse_ext\u003dFalse, transient\u003dFalse):"},{"line_number":760,"context_line":"        \"\"\"Copy the guest-visible contents into a new disk"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"        http://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCopy"},{"line_number":763,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1fa4df85_681c3213","line":760,"range":{"start_line":760,"start_character":11,"end_line":760,"end_character":58},"in_reply_to":"1fa4df85_cd0778c9","updated":"2020-03-16 16:37:47.000000000","message":"I won\u0027t block on it, but I don\u0027t consider it a good practice.  (Not everyone can parse libvirt docs that way you or other Block-capable person can.)","commit_id":"60123be880ce3602995fe86ce459ab65c181a24b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8015006e28385b9c43db2d5d95c71c0446e1b87c","unresolved":false,"context_lines":[{"line_number":770,"context_line":"        flags \u003d shallow and libvirt.VIR_DOMAIN_BLOCK_COPY_SHALLOW or 0"},{"line_number":771,"context_line":"        flags |\u003d reuse_ext and libvirt.VIR_DOMAIN_BLOCK_COPY_REUSE_EXT or 0"},{"line_number":772,"context_line":"        flags |\u003d transient and libvirt.VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB or 0"},{"line_number":773,"context_line":"        return self._guest._domain.blockCopy(self._disk, dest_xml, flags\u003dflags)"},{"line_number":774,"context_line":""},{"line_number":775,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":776,"context_line":"               copy\u003dFalse, relative\u003dFalse, copy_dev\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":12,"id":"df33271e_50d031f8","line":773,"updated":"2020-03-24 15:30:35.000000000","message":"Regarding the flags I rely on the positive review from Kashyap and Stephen.","commit_id":"9d8141c9c40eb58c6bc04e6188e9ebe313da7ce7"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"4865f06344d827dea77a77884b382c4b2c6338e0","unresolved":false,"context_lines":[{"line_number":770,"context_line":"        flags \u003d shallow and libvirt.VIR_DOMAIN_BLOCK_COPY_SHALLOW or 0"},{"line_number":771,"context_line":"        flags |\u003d reuse_ext and libvirt.VIR_DOMAIN_BLOCK_COPY_REUSE_EXT or 0"},{"line_number":772,"context_line":"        flags |\u003d transient and libvirt.VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB or 0"},{"line_number":773,"context_line":"        return self._guest._domain.blockCopy(self._disk, dest_xml, flags\u003dflags)"},{"line_number":774,"context_line":""},{"line_number":775,"context_line":"    def rebase(self, base, shallow\u003dFalse, reuse_ext\u003dFalse,"},{"line_number":776,"context_line":"               copy\u003dFalse, relative\u003dFalse, copy_dev\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":12,"id":"df33271e_bf5a17f0","line":773,"in_reply_to":"df33271e_50d031f8","updated":"2020-03-25 13:12:18.000000000","message":"Yeah, these flags, and the bitwise-OR do look sane to me, FWIW.  (And it is modelled after rebase() below.)\n\nWriting for myself.  The line: \n\n    flags |\u003d reuse_ext and libvirt.VIR_DOMAIN_BLOCK_COPY_REUSE_EXT or 0\n\nCan be translated as:\n\n    flags \u003d flags | (reuse_ext and VIR_DOMAIN_BLOCK_COPY_REUSE_EXT) or 0\n\nI.e. if \u0027reuse_ext\u0027 is true, it returns the libvirt.VIR_DOMAIN_BLOCK_COPY_REUSE_EXT constant, and so forth.","commit_id":"9d8141c9c40eb58c6bc04e6188e9ebe313da7ce7"}]}