Re: [PATCH 02/11] hyperv: implement domainUndefine and domainUndefineFlags

> On Nov 26, 2020, at 9:26 AM, Daniel P. Berrangé <berrange(a)redhat.com&gt; wrote: > > On Tue, Nov 24, 2020 at 02:48:31PM -0500, Matt Coleman wrote: >> + /* try to shut down the VM if it's not disabled, just to be safe */ >> + if (computerSystem->data->EnabledState != MSVM_COMPUTERSYSTEM_ENABLEDSTATE_DISABLED && >> + hypervDomainShutdown(domain) < 0) { >> + goto cleanup; >> + } > > Many, but not all, drivers in libvirt allow undefining the cofig for a running > VM. ie we can delete the config on disk, without affecting the running VM. > This results in a so called "transient" VM - basically a VM which only exists > as long as it is running - virDomainCreateXML creates such a beast, while > virDomainDefineXML+virDomainCreate creates a "persistent" VM and starts it. > > If hyperv doesn't allow that, then shutting down the VM is likely to be > surprising. > > I'd suggest we report an error indicating undefine is not permitted for a > running VM in such a case. Hyper-V does not allow a VM's definition to be deleted if it's active, paused, or in state transition. If I remove the check entirely, it displays an error including Hyper-V's method name, the error code from Hyper-V, and a string that's generated by hypervReturnCodeToString() from hyperv_wmi.c. For example, `virsh undefine runningvm` would throw the error: error: Failed to undefine domain runningvm error: internal error: Invocation of DestroySystem returned an error: Invalid state for this operation (32775) That simplifies the code significantly and provides details (the method name and error code) that users could look up in Microsoft's documentation. However, I don't think it's very clearly phrased. "Domain must not be active or in state transition" with the error code set to VIR_ERR_OPERATION_INVALID produces the following error output: error: Failed to undefine domain testvm error: Requested operation is not valid: Domain must not be active or in state transition It's a difference of 18 lines (51 vs 33). The shorter version eliminates the result and computerSystem variables, does not use goto, and only sends a single WMI request to Hyper-V since it does not have to query the VM state before attempting to undefine it.

Would you prefer to have simpler code or a custom error message?