[PATCH 00/55] Hyper-V: code cleanup & prep for future changes
Laine Stump
laine at redhat.com
Fri Jan 22 16:30:40 UTC 2021
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 at 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
>>
>
More information about the libvir-list
mailing list