> > Hi Fiona, thanks for the review
> > > The general ideas looks fine to me and my basic testing seemed fine > too, > but most patches could use a few improvements. The biggest issue is > that > getting static information in HA manager is not adapted to the > change. > yes, I known, I wanted a review first. > The virtio feature should be marked as technology preview and > mentioned > that it only works for guests running Linux >= 5.8 [0]. > yes. (and from my test, it's more 5.13~5.15 to have something working correctly) > IMHO, it'd be nicer to add the property string handling/registering > in > the Memory.pm module directly, so that all is contained in one place. ok, I'll move it,no problem. > > I also think that using a validator for the format might be worth it. > > Also, the tests should be made more host-independent, but for the > NUMA > node issue 01/10 that might require a bit of rework, to be able to > mock > the relevant parts. > I'll rework the tests, I didn't known about the host numa detection, as I'm always working on a numa server ;) .I'll mock host max memory detection too. > Should the virtio feature be QEMU-version-guarded? It's explicitly > opt-in and technology preview, so maybe not really needed? > I think it's ok, it's an opt-in option. > See the individual replies on patches for details. > I'll reply in the patches _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
