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. Nikolay
