On November 4, 2019 2:57 pm, Stefan Reiter wrote: > This series refactors QemuServer and creates three new packages: > * 'PVE::QemuServer::Helpers' for general purpose helpers > * 'PVE::QemuServer::Monitor' 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. > > New functions are created as PVE::QemuServer, not PVE::Qemu, to be consistent > for now.
some comments on indivual patches, looking mostly good or even great! it's always good to note inter-package breakage/dependencies, such as in this case: qemu-server versioned breaks on old pve-ha-manager & pve-manager pve-manager & pve-ha-manager versioned dependency on new qemu-server either in the cover-letter, or on the individual patches (IIRC Thomas prefers the latter). in this case breakage would be obvious, but that is not always the case. > v3: > * QMP -> QemuServer::Monitor > * QemuSchema -> QemuServer::Helpers (more or less) > * split check_running (but keep old version to not change all callers) > * split qemu_machine_feature_enabled > * include "check_cmdline -> parse_cmdline" patch in series (needed for later > live migration with custom CPU types anyway) > * redo patches to pve-manager/pve-ha-manager to accomodate changes in v3 > > 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 > > > qemu-server: Stefan Reiter (6): > refactor: create QemuServer::Helpers and move file/dir code > Change check_cmdline to parse_cmdline > refactor: split check_running into _exists_ and _running_ > refactor: create QemuServer::Monitor for high-level QMP access > refactor: extract QEMU machine related helpers to package > refactor: split qemu_machine_feature_enabled > > PVE/API2/Qemu.pm | 15 +- > PVE/API2/Qemu/Agent.pm | 7 +- > PVE/CLI/qm.pm | 15 +- > PVE/QMPClient.pm | 5 +- > PVE/QemuConfig.pm | 42 ++- > PVE/QemuMigrate.pm | 24 +- > PVE/QemuServer.pm | 421 +++++++-------------------- > PVE/QemuServer/Agent.pm | 3 +- > PVE/QemuServer/Helpers.pm | 138 +++++++++ > PVE/QemuServer/Machine.pm | 74 +++++ > PVE/QemuServer/Makefile | 3 + > PVE/QemuServer/Memory.pm | 9 +- > PVE/VZDump/QemuServer.pm | 16 +- > test/cfg2cmd/pinned-version.conf | 15 + > test/cfg2cmd/pinned-version.conf.cmd | 30 ++ > test/snapshot-test.pm | 31 +- > 16 files changed, 465 insertions(+), 383 deletions(-) > create mode 100644 PVE/QemuServer/Helpers.pm > create mode 100644 PVE/QemuServer/Machine.pm > create mode 100644 test/cfg2cmd/pinned-version.conf > create mode 100644 test/cfg2cmd/pinned-version.conf.cmd > > ha-manager: Stefan Reiter (1): > refactor: vm_qmp_command was moved to PVE::QemuServer::Monitor > > src/PVE/HA/Resources/PVEVM.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > > qemu-server: Stefan Reiter (6): > refactor: create QemuServer::Helpers and move file/dir code > Change check_cmdline to parse_cmdline > refactor: split check_running into _exists_ and _running_ > refactor: create QemuServer::Monitor for high-level QMP access > refactor: extract QEMU machine related helpers to package > refactor: split qemu_machine_feature_enabled > > PVE/API2/Qemu.pm | 15 +- > PVE/API2/Qemu/Agent.pm | 7 +- > PVE/CLI/qm.pm | 16 +- > PVE/QMPClient.pm | 5 +- > PVE/QemuConfig.pm | 42 ++- > PVE/QemuMigrate.pm | 24 +- > PVE/QemuServer.pm | 421 +++++++-------------------- > PVE/QemuServer/Agent.pm | 3 +- > PVE/QemuServer/Helpers.pm | 138 +++++++++ > PVE/QemuServer/Machine.pm | 74 +++++ > PVE/QemuServer/Makefile | 3 + > PVE/QemuServer/Memory.pm | 9 +- > PVE/VZDump/QemuServer.pm | 16 +- > test/cfg2cmd/pinned-version.conf | 15 + > test/cfg2cmd/pinned-version.conf.cmd | 30 ++ > test/snapshot-test.pm | 31 +- > 16 files changed, 466 insertions(+), 383 deletions(-) > create mode 100644 PVE/QemuServer/Helpers.pm > create mode 100644 PVE/QemuServer/Machine.pm > create mode 100644 test/cfg2cmd/pinned-version.conf > create mode 100644 test/cfg2cmd/pinned-version.conf.cmd > > ha-manager: Stefan Reiter (1): > refactor: vm_qmp_command was moved to PVE::QemuServer::Monitor > > src/PVE/HA/Resources/PVEVM.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > > qemu-server: Stefan Reiter (6): > refactor: create QemuServer::Helpers and move file/dir code > Change check_cmdline to parse_cmdline > refactor: split check_running into _exists_ and _running_ > refactor: create QemuServer::Monitor for high-level QMP access > refactor: extract QEMU machine related helpers to package > refactor: split qemu_machine_feature_enabled > > PVE/API2/Qemu.pm | 15 +- > PVE/API2/Qemu/Agent.pm | 7 +- > PVE/CLI/qm.pm | 16 +- > PVE/QMPClient.pm | 5 +- > PVE/QemuConfig.pm | 42 ++- > PVE/QemuMigrate.pm | 24 +- > PVE/QemuServer.pm | 421 +++++++-------------------- > PVE/QemuServer/Agent.pm | 3 +- > PVE/QemuServer/Helpers.pm | 138 +++++++++ > PVE/QemuServer/Machine.pm | 73 +++++ > PVE/QemuServer/Makefile | 3 + > PVE/QemuServer/Memory.pm | 9 +- > PVE/VZDump/QemuServer.pm | 16 +- > test/cfg2cmd/pinned-version.conf | 15 + > test/cfg2cmd/pinned-version.conf.cmd | 30 ++ > test/snapshot-test.pm | 31 +- > 16 files changed, 465 insertions(+), 383 deletions(-) > create mode 100644 PVE/QemuServer/Helpers.pm > create mode 100644 PVE/QemuServer/Machine.pm > create mode 100644 test/cfg2cmd/pinned-version.conf > create mode 100644 test/cfg2cmd/pinned-version.conf.cmd > > ha-manager: Stefan Reiter (1): > refactor: vm_qmp_command was moved to PVE::QemuServer::Monitor > > src/PVE/HA/Resources/PVEVM.pm | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > manager: Stefan Reiter (1): > refactor: vm_mon_cmd is now Monitor::mon_cmd > > PVE/Service/pvestatd.pm | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > -- > 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
