Re: [PATCH 1/2] hyperv: XML parsing of storage volumes

On Wed, Nov 18, 2020 at 01:48:24PM -0500, Matt Coleman wrote: > dumpxml can now serialize: > * floppy drives > * file-backed and device-backed disk drives > * images mounted to virtual CD/DVD drives > * IDE and SCSI controllers > > Co-authored-by: Sri Ramanujam <sramanujam(a)datto.com&gt; > Signed-off-by: Matt Coleman <matt(a)datto.com&gt; > --- > src/hyperv/hyperv_driver.c | 408 +++++++++++++++++++++++++- > src/hyperv/hyperv_driver.h | 3 + > src/hyperv/hyperv_wmi.c | 45 +++ > src/hyperv/hyperv_wmi.h | 8 + > src/hyperv/hyperv_wmi_classes.h | 19 ++ > src/hyperv/hyperv_wmi_generator.input | 134 +++++++++ > 6 files changed, 616 insertions(+), 1 deletion(-) > > diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c > index 40739595ac..ce9ba2a02a 100644 > --- a/src/hyperv/hyperv_driver.c > +++ b/src/hyperv/hyperv_driver.c > @@ -293,6 +293,385 @@ hypervCapsInit(hypervPrivate *priv) > return NULL; > } > > +/* > + * Virtual device functions > + */ > +static int > +hypervGetDeviceParentRasdFromDeviceId(const char *parentDeviceId, > + Msvm_ResourceAllocationSettingData *list, > + Msvm_ResourceAllocationSettingData **out) > +{ > + Msvm_ResourceAllocationSettingData *entry = list; > + g_autofree char *escapedDeviceId = NULL; > + *out = NULL; > + > + while (entry) { > + escapedDeviceId = g_strdup_printf("%s\"", entry->data->InstanceID); > + escapedDeviceId = virStringReplace(escapedDeviceId, "\\", "\\\\"); This leaks the original escapedDeviceId pointer. The autofree only runs when variables go out of scope, not when you overwrite pointers. It'll also leak on every loop iteration. > + > + if (g_str_has_suffix(parentDeviceId, escapedDeviceId)) { > + *out = entry; > + break; > + } > + > + entry = entry->next; > + } > + > + if (*out) > + return 0; > + > + return -1; > +} > diff --git a/src/hyperv/hyperv_driver.h b/src/hyperv/hyperv_driver.h > index 8099b5714b..e49550a2c8 100644 > --- a/src/hyperv/hyperv_driver.h > +++ b/src/hyperv/hyperv_driver.h > @@ -22,4 +22,7 @@ > > #pragma once > > +#define HYPERV_MAX_SCSI_CONTROLLERS 4 > +#define HYPERV_MAX_IDE_CONTROLLERS 2 Is it really possible to have 2 IDE controllers ?

Most cases there is a single IDE controller, with two channels, and each channel has two devices, giving a total of 4 disks. I'm just wondering if the code is mistakenly creating separate libvirt controllers for the 2 IDE channels