[PATCH v2 00/10] qemu: support renaming domains with snapshots/checkpoints

[Nikolay Shirokovskiy]

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 at gmail.com>
>>
>>
>> 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