[pve-devel] [PATCH v2 00/11] Refactor QemuServer to avoid dependency cycles

[Fabian Grünbichler]

On October 28, 2019 12:59 pm, Stefan Reiter wrote:
> First 3 patches are independant refactorings around get_host_arch.
> 
> Rest of the series refactors QemuServer and creates three new packages:
> * 'PVE::QemuSchema' for schema related code and common directory creation
> * 'PVE::QMP' for higher-level QMP functions
> * 'PVE::QemuServer::Machine' for QEMU machine-type related helpers
> 
> This refactoring came along because qemu_machine_feature_enabled needs to be
> used in 'PVE::QemuServer::CPUConfig', a new package that will be introduced with
> my custom CPU series [0]. This would currently require dependency cycles, but by
> extracting the code in this series and splitting it up into multiple helper
> modules, this can be avoided.
> 
> Care was taken not to introduce new dependecy cycles, though this required to
> move the 'check_running' function to QemuConfig.pm, where it doesn't *quite* fit
> IMO, but I also didn't want to create a new module just for this one function.
> Open for ideas ofc.

thanks for this big chunk of work! some comments on individual patches, 
and one high-level remark that we already discussed off-list:

I'd use this opportunity to rename PVE/QemuServer to PVE/Qemu, and move 
all/most of the split out files into PVE/Qemu as well.

we could think about moving QemuConfig and QemuMigrate there as well 
(like we have in pve-container), but that would mean touching even more 
files, including pve-firewall (which uses PVE::QemuConfig).

> 
> v2:
> * Actually test changes correctly - sorry
> * Fix a few package 'use's I missed to move to new packages
> * Fix tests for pve-manager
> * Fix missing '=' in pve-container
> 
> [0] https://pve.proxmox.com/pipermail/pve-devel/2019-October/039608.html
> 
> (@Thomas: I rebased the series just before sending to work with your cleanups)
> 
> 
> common: Stefan Reiter (1):
>   Make get_host_arch return raw uname entry
> 
>  src/PVE/Tools.pm | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> container: Stefan Reiter (1):
>   Move LXC-specific architecture translation here
> 
>  src/PVE/LXC/Setup.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> qemu-server: Stefan Reiter (5):
>   Use get_host_arch from PVE::Tools
>   refactor: create QemuSchema and move file/dir code
>   refactor: Move check_running to QemuConfig
>   refactor: create PVE::QMP for high-level QMP access
>   refactor: extract QEMU machine related helpers to package
> 
>  PVE/API2/Qemu.pm             |  45 +++---
>  PVE/API2/Qemu/Agent.pm       |   7 +-
>  PVE/CLI/qm.pm                |  27 ++--
>  PVE/Makefile                 |   4 +-
>  PVE/QMP.pm                   |  72 +++++++++
>  PVE/QMPClient.pm             |   5 +-
>  PVE/QemuConfig.pm            |  93 +++++++++--
>  PVE/QemuMigrate.pm           |  27 ++--
>  PVE/QemuSchema.pm            |  35 +++++
>  PVE/QemuServer.pm            | 295 ++++-------------------------------
>  PVE/QemuServer/Agent.pm      |   6 +-
>  PVE/QemuServer/ImportDisk.pm |   3 +-
>  PVE/QemuServer/Machine.pm    | 100 ++++++++++++
>  PVE/QemuServer/Makefile      |   1 +
>  PVE/QemuServer/Memory.pm     |  12 +-
>  PVE/VZDump/QemuServer.pm     |  23 +--
>  test/snapshot-test.pm        |  21 ++-
>  17 files changed, 421 insertions(+), 355 deletions(-)
>  create mode 100644 PVE/QMP.pm
>  create mode 100644 PVE/QemuSchema.pm
>  create mode 100644 PVE/QemuServer/Machine.pm
> 
> ha-manager: Stefan Reiter (2):
>   refactor: check_running was moved to PVE::QemuConfig
>   refactor: vm_qmp_command was moved to PVE::QMP
> 
>  src/PVE/HA/Resources/PVEVM.pm | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> manager: Stefan Reiter (2):
>   refactor: check_running was moved to QemuConfig
>   refactor: vm_mon_cmd was moved to PVE::QMP
> 
>  PVE/API2/Nodes.pm          | 6 +++---
>  PVE/Service/pvestatd.pm    | 3 ++-
>  test/ReplicationTestEnv.pm | 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> -- 
> 2.20.1
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>