On 10/30/19 11:49 AM, Fabian Grünbichler wrote: > 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 talked about this off-list, and as said there this is something I thought of too, so ACK. But, I'd rather not do it over the mailinglist (@Stefan), but that's something I, or you (@Fabian), can do directly in git and push it out. I see it unrelated to the refactoring also, so not something you should pay attention to in a v2, Stefan.
> > 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). > If we start doing this then fully, a little headache more, or less, is rather irrelevant - IMO. _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
