[pve-devel] [PATCH 0/2] code refactoring for lxc pending changes feature

[Thomas Lamprecht]

On 05.09.19 11:13, Oguz Bektas wrote:
> hi,
> 
> On Thu, Sep 05, 2019 at 11:01:57AM +0200, Thomas Lamprecht wrote:
>> On 04.09.19 18:00, Oguz Bektas wrote:
>>> this patch series moves some pending changes related functions into
>>> AbstractConfig. the only thing that changes is that these are now class methods,
>>> since they need to be inherited in LXC::Config and QemuConfig.
>>> (the functionality in Qemu side should stay the same)
>>>
>>> for now, vmconfig_apply_pending and vmconfig_hotplug_pending are left to be
>>> implemented in the respective Config classes, since there are different
>>> special cases between Qemu and LXC to be handled.
>>>
>>>
>>>
>>
>> I want to see the LXC code too, to ensure the method split and use
>> is a good fit to share the implementation between both as much as
>> possible, please send that too..
> 
> still working on it, need to polish and test a bit more.
> 
> however, except vmconfig_hotplug_pending and vmconfig_apply_pending,
> they are generic config-file-related functions (mostly for the 'delete'
> option), so they are applicable on both sides.
> 
> i opted not to break down vmconfig_hotplug_pending, since there
> are too many special cases being handled in there in the qemu code. so i
> think it's better to keep it abstract and implement separately, as the
> cases differ a lot between qemu and lxc.
> 
> vmconfig_apply_pending could possibly be generalized/abstracted, but i
> didn't want to get carried away in the abstraction layer for now.
> 
> will send the lxc code soon.

As already told you a few times, I need to see the other side too
for a review. If one moves code arround to achieve exactly one thing
but only send the "code movement" parts but not the actual desired thing
no one can say review the refactoring and say that it will be OK for
the thing you want to achieve (in this case pending changes).

Send them all together, I won't review or apply it as is.

>>>
>>>
>>> pve-guest-common:
>>>
>>> Oguz Bektas (1):
>>>   move pending changes related functions into AbstractConfig
>>>
>>>  PVE/AbstractConfig.pm | 79 +++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 79 insertions(+)
>>>
>>>
>>> qemu-server:
>>>
>>> Oguz Bektas (1):
>>>   move pending changes related functions from QemuServer to QemuConfig
>>>
>>>  PVE/API2/Qemu.pm             |  20 +--
>>>  PVE/QemuConfig.pm            | 233 ++++++++++++++++++++++++++
>>>  PVE/QemuServer.pm            | 306 +----------------------------------
>>>  PVE/QemuServer/ImportDisk.pm |   4 +-
>>>  4 files changed, 250 insertions(+), 313 deletions(-)
>>>
>>