On 1/22/21 11:05 AM, Laine Stump wrote:
(Thought I sent this 7 hours ago before I went to sleep, but when I sat down this morning I saw it was still sitting there as a draft :-/)
On 1/21/21 1:50 PM, Matt Coleman wrote:
This series of patches simplifies the code in several ways and makes a few changes required by the next round of patches that I'll submit.
Simplifications:
* add a macro to cut down on repetitive SettingData code * enable GLib auto-cleanup for hypervObject and several OpenWSMAN types
Changes:
* store the version in hypervPrivate, which will be used to handle breaking changes in the Hyper-V API: despite 2012R2 and 2016+ all using Hyper-V's "V2" API, backwards-incompatible changes were made in 2016 * add inheritance to the WMI generator to simplify handling of the backwards-incompatible changes introduced in Hyper-V 2016
I've gone through all of these, and just have two questions that affect multiple patches each (I've replied to the associated patches):
1) There are several cleanup functions in external libraries that in the past were only called after checking that the pointer was != NULL. g_autoptr cleanups need to handle being called with NULL as a NOP, and I'm concerned that these functions may not behave properly in that case. Can you either verify that it's safe to call them with NULL, or provide a wrapper function that checks for NULL and use that as the cleanup?
2) There are several places where you're returning a value that is retrieved from an object that is being auto-freed, and I don't know enough about the details of the teardown code that's generated by the compiler to be certain that the return value would be retrieved from the object *before* it's freed rather than *after*. If someone knows the answer to this, then that's great, otherwise someone should compile a test program and list out the assembly code using gdb to see what the order is.
I asked about item (2) on IRC just now, and danpb produced a short example program that proves it is okay to use values from auto-freed objects as the return value of a function. So there is only question (1) left. Let me know and I'll either push or wait for modified patches accordingly.
Once those two questions are resolved (possibly requiring no change to your patches), then
Reviewed-by: Laine Stump <laine@redhat.com>
Matt Coleman (55): hyperv: add a macro for retrieving setting data hyperv: store the Hyper-V version when connecting hyperv: add inheritance to the WMI generator hyperv: store hypervPrivate in hypervObject hyperv: enable use of g_autoptr for hypervObject hyperv: enable use of g_autoptr for the rest of the CIM/WMI classes hyperv: enable automatic cleanup for OpenWSMAN types hyperv: use g_autoptr for Win32_OperatingSystem in hypervConnectOpen hyperv: use g_autoptr for Win32_ComputerSystem in hypervConnectGetHostname hyperv: use g_autoptr for Msvm_ProcessorSettingData in hypervConnectGetMaxVcpus hyperv: use g_autoptr for WMI classes in hypervNodeGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByUUID hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainLookupByName hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainResume hyperv: use g_autoptr for WMI classes in hypervDomainShutdownFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainDestroyFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetMaxMemory hyperv: use g_autoptr for WMI classes in hypervDomainSetMemoryProperty hyperv: use g_autoptr for Msvm_ComputerSystem in hypervRequestStateChange hyperv: use g_autoptr for Win32_ComputerSystemProduct in hypervLookupHostSystemBiosUuid hyperv: use g_autoptr for Msvm_ResourceAllocationSettingData in hypervDomainAttachPhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainAttachStorage hyperv: use g_autoptr for Msvm_DiskDrive in hypervDomainDefParsePhysicalDisk hyperv: use g_autoptr for WMI classes in hypervDomainGetInfo hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainGetState hyperv: use g_autoptr for WMI classes in hypervDomainSetVcpusFlags hyperv: use g_autoptr for WMI classes in hypervDomainGetVcpusFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectNumOfDefinedDomains hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainCreateWithFlags hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainGetAutostart hyperv: use g_autoptr for Msvm_VirtualSystemSettingData in hypervDomainSetAutostart hyperv: use g_autoptr for WMI classes in hypervDomainGetSchedulerParametersFlags hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainIsActive hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSave hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainHasManagedSaveImage hyperv: use g_autoptr for Msvm_ComputerSystem in hypervDomainManagedSaveRemove hyperv: use g_autoptr for Msvm_ComputerSystem in hypervConnectListAllDomains hyperv: use GLib auto-cleanup in hypervDomainSendKey hyperv: use GLib auto-cleanup in hypervInvokeMethod hyperv: use GLib auto-cleanup in hypervInvokeMsvmComputerSystemRequestStateChange hyperv: use GLib auto-cleanup in hypervMsvmVSMSAddResourceSettings and hypervMsvmVSMSModifyResourceSettings hyperv: use g_autoptr for Win32_PerfRawData_HvStats_HyperVHypervisorVirtualProcessor in hypervDomainGetVcpus hyperv: use g_autoptr for Win32_OperatingSystem in hypervNodeGetFreeMemory hyperv: use GLib auto-cleanup in hypervDomainGetXMLDesc hyperv: use g_autoptr for WMI classes in hypervDomainAttachDeviceFlags hyperv: use GLib auto-cleanup in hypervSerializeEprParam hyperv: use GLib auto-cleanup in hypervEnumAndPull hyperv: use GLib auto-cleanup in hypervSerializeEmbeddedParam hyperv: use GLib auto-cleanup in hypervCreateInvokeXmlDoc hyperv: use g_auto for WsXmlDocH in hypervDomainAttachVirtualDisk hyperv: use g_auto for WsXmlDocH in hypervDomainAttachCDROM
scripts/hyperv_wmi_generator.py | 16 +- src/hyperv/hyperv_driver.c | 755 +++++++++++--------------------- src/hyperv/hyperv_private.h | 4 +- src/hyperv/hyperv_wmi.c | 408 +++++++---------- src/hyperv/hyperv_wmi.h | 4 +- src/hyperv/hyperv_wsman.h | 28 ++ 6 files changed, 457 insertions(+), 758 deletions(-) create mode 100644 src/hyperv/hyperv_wsman.h