On 11/19/19 10:59 AM, Thomas Lamprecht wrote:
On 11/19/19 10:30 AM, Fabian Grünbichler wrote:
+
# BEGIN implemented abstract methods from PVE::AbstractConfig
IMHO, vm_exists_on_node is a prime candidate for being put into
AbstractConfig - it's not qemu-specific at all, we probably want to do a
similar split for pve-container as well, and we want to use it when
appropriate in AbstractConfig.
any objections to moving it there? maybe adapt the name?
s/vm/guest_config/ ? actual moving could also be done at a latter point
to avoid the bump in + depends on pve-guest-common
sounds OK, I'd maybe go for "config_exists_on_node", the guest is
somewhat implicit for methods coming out of that module..
+1
Else, as "AbstractConfig->config_file" already can be passed a node,
it could also be ommitted, just using, for example:
if ! -f AbstractConfig->config_file(...)
over
if ! AbstractConfig->config_exists_on_node(...)
seems not like the biggest win anyway, but no hard feelings...
I like it because it abstracts the whole 'die' and error message code, a
bit more readable on the call site IMO. Although it might be better
named "check_config_exists_on_node", since without the $noerr (where
Fabian was right, it is unnecessary) the function will always die if
there's an error?
In general I'd like to leave it in, but should I do so in AbstractConfig
or leave it here for now?
_______________________________________________
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
