On 10.11.2020 10:58, Nikolay Shirokovskiy wrote: > > > On 09.11.2020 16:51, Daniel Henrique Barboza wrote: >> >> >> On 11/3/20 8:59 AM, Nikolay Shirokovskiy wrote: >>> This is basically just rebase of [1] as it was not get any attention at that >>> time. >>> >>> [1] [PATCH 0/8] qemu: support renaming domains with snapshots/checkpoints >>> https://www.redhat.com/archives/libvir-list/2020-March/msg00018.html >> >> Code LGTM: >> >> Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx> >> >> >> Shouldn't you add some test cases for this new behavior though? I'm a bit >> nervous with pushing this upstream without any coverage. >> >> > > So basically we need to test how qemuDomainRenameCallback works. Currently > there is no test for this function or virDomainRename. I spent some time > looking at existing tests that need to mock driver/vm objects and it looks like > it requires a good deal of effort in order to prepare the test in this case. > At the same time those tests has many inputs so it looks like worth heavy > preparation. In case of rename the code we test does not depend greatly on > inputs so may be it does not worth adding such test given heavy preparation we > have to do. > > Of course I give the patch series some manual testing. > Thanx for the review! Pushed with next hunk squashed into the last patch: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c831ae6..fef0be6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5137,7 +5137,7 @@ qemuMigrationDstPersist(virQEMUDriverPtr driver, priv->qemuCaps))) goto error; - if (!oldDef && qemuDomainNamePathsCleanup(cfg, vmdef->name, false) < 0) + if (!oldPersist && qemuDomainNamePathsCleanup(cfg, vmdef->name, false) < 0) goto error; if (virDomainDefSave(vmdef, driver->xmlopt, cfg->configDir) < 0 && Nikolay
