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@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel