[libvirt] [PATCH] hyperv: recognize array property as distinct type.
When hyperv code generator for WMI classes identifies common properties, it needs to take into account array type as a distinct type, i.e string != string[]. This is the case where v1 of the Msvm_VirtualSystemSettingData has Notes property as string whereas v2 uses Notes[], therefore they have to be treated as different fields and cannot be placed in the "common" struct. --- src/hyperv/hyperv_driver.c | 26 ++++++++++++++++++++++++-- src/hyperv/hyperv_wmi_generator.py | 2 +- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 090ea24..5a27908 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -894,8 +894,30 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0) goto cleanup; - if (VIR_STRDUP(def->description, virtualSystemSettingData->data.common->Notes) < 0) - goto cleanup; + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + if (VIR_STRDUP(def->description, + virtualSystemSettingData->data.v1->Notes) < 0) + goto cleanup; + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 && + virtualSystemSettingData->data.v2->Notes.data != NULL) { + char **notes = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i = 0; + + /* in practice Notes has 1 element */ + for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++) { + /* but if there's more than 1, separate by double new line */ + if (virBufferUse(&buf) > 0) + virBufferAddLit(&buf, "\n\n"); + + notes = (char **) virtualSystemSettingData->data.v2->Notes.data; + virBufferAdd(&buf, *notes, -1); + notes++; + } + + def->description = virBufferContentAndReset(&buf); + } + virDomainDefSetMemoryTotal(def, memorySettingData->data.common->Limit * 1024); /* megabyte to kilobyte */ def->mem.cur_balloon = memorySettingData->data.common->VirtualQuantity * 1024; /* megabyte to kilobyte */ diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py index c15d97a..9aee0b9 100755 --- a/src/hyperv/hyperv_wmi_generator.py +++ b/src/hyperv/hyperv_wmi_generator.py @@ -251,7 +251,7 @@ class WmiClass: for cls in self.versions: for prop in cls.properties: # consdered same if matches by name AND type - key = "%s_%s" % (prop.name, prop.type) + key = "%s_%s_%s" % (prop.name, prop.type, prop.is_array) if key in property_info: property_info[key][1] += 1 -- 2.9.3
2017-04-11 17:02 GMT+02:00 Dawid Zamirski <dzamirski@datto.com>:
When hyperv code generator for WMI classes identifies common properties, it needs to take into account array type as a distinct type, i.e string != string[]. This is the case where v1 of the Msvm_VirtualSystemSettingData has Notes property as string whereas v2 uses Notes[], therefore they have to be treated as different fields and cannot be placed in the "common" struct. --- src/hyperv/hyperv_driver.c | 26 ++++++++++++++++++++++++-- src/hyperv/hyperv_wmi_generator.py | 2 +- 2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 090ea24..5a27908 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -894,8 +894,30 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0) goto cleanup;
- if (VIR_STRDUP(def->description, virtualSystemSettingData->data.common->Notes) < 0) - goto cleanup; + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + if (VIR_STRDUP(def->description, + virtualSystemSettingData->data.v1->Notes) < 0) + goto cleanup; + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 && + virtualSystemSettingData->data.v2->Notes.data != NULL) { + char **notes = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i = 0; + + /* in practice Notes has 1 element */ + for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++) { + /* but if there's more than 1, separate by double new line */ + if (virBufferUse(&buf) > 0) + virBufferAddLit(&buf, "\n\n"); + + notes = (char **) virtualSystemSettingData->data.v2->Notes.data; + virBufferAdd(&buf, *notes, -1); + notes++; + }
This reset notes in each iteration of the for loop to the start of the array, then adds the first item to the buffer and then advances the pointer by one item. This will add the first item of the notes array count times to the buffer instead of adding each item to the buffer once. I think you need to move the "notes = (char **) virtualSystemSettingData->data.v2->Notes.data;" line before the for loop to make this work correctly. -- Matthias Bolte http://photron.blogspot.com
When hyperv code generator for WMI classes identifies common properties, it needs to take into account array type as a distinct type, i.e string != string[]. This is the case where v1 of the Msvm_VirtualSystemSettingData has Notes property as string whereas v2 uses Notes[], therefore they have to be treated as different fields and cannot be placed in the "common" struct. --- changes in v2: * move notes variable assignment outside of the loop so that it's not reset at each iteration. src/hyperv/hyperv_driver.c | 25 +++++++++++++++++++++++-- src/hyperv/hyperv_wmi_generator.py | 2 +- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 090ea24..350f123 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -894,8 +894,29 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0) goto cleanup; - if (VIR_STRDUP(def->description, virtualSystemSettingData->data.common->Notes) < 0) - goto cleanup; + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + if (VIR_STRDUP(def->description, + virtualSystemSettingData->data.v1->Notes) < 0) + goto cleanup; + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 && + virtualSystemSettingData->data.v2->Notes.data != NULL) { + char **notes = (char **) virtualSystemSettingData->data.v2->Notes.data; + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i = 0; + + /* in practice Notes has 1 element */ + for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++) { + /* but if there's more than 1, separate by double new line */ + if (virBufferUse(&buf) > 0) + virBufferAddLit(&buf, "\n\n"); + + virBufferAdd(&buf, *notes, -1); + notes++; + } + + def->description = virBufferContentAndReset(&buf); + } + virDomainDefSetMemoryTotal(def, memorySettingData->data.common->Limit * 1024); /* megabyte to kilobyte */ def->mem.cur_balloon = memorySettingData->data.common->VirtualQuantity * 1024; /* megabyte to kilobyte */ diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py index c15d97a..9aee0b9 100755 --- a/src/hyperv/hyperv_wmi_generator.py +++ b/src/hyperv/hyperv_wmi_generator.py @@ -251,7 +251,7 @@ class WmiClass: for cls in self.versions: for prop in cls.properties: # consdered same if matches by name AND type - key = "%s_%s" % (prop.name, prop.type) + key = "%s_%s_%s" % (prop.name, prop.type, prop.is_array) if key in property_info: property_info[key][1] += 1 -- 2.9.3
2017-04-11 21:42 GMT+02:00 Dawid Zamirski <dzamirski@datto.com>:
When hyperv code generator for WMI classes identifies common properties, it needs to take into account array type as a distinct type, i.e string != string[]. This is the case where v1 of the Msvm_VirtualSystemSettingData has Notes property as string whereas v2 uses Notes[], therefore they have to be treated as different fields and cannot be placed in the "common" struct. ---
changes in v2: * move notes variable assignment outside of the loop so that it's not reset at each iteration.
src/hyperv/hyperv_driver.c | 25 +++++++++++++++++++++++-- src/hyperv/hyperv_wmi_generator.py | 2 +- 2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 090ea24..350f123 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -894,8 +894,29 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0) goto cleanup;
- if (VIR_STRDUP(def->description, virtualSystemSettingData->data.common->Notes) < 0) - goto cleanup; + if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) { + if (VIR_STRDUP(def->description, + virtualSystemSettingData->data.v1->Notes) < 0) + goto cleanup; + } else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 && + virtualSystemSettingData->data.v2->Notes.data != NULL) { + char **notes = (char **) virtualSystemSettingData->data.v2->Notes.data; + virBuffer buf = VIR_BUFFER_INITIALIZER; + size_t i = 0; + + /* in practice Notes has 1 element */ + for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++) { + /* but if there's more than 1, separate by double new line */ + if (virBufferUse(&buf) > 0) + virBufferAddLit(&buf, "\n\n"); + + virBufferAdd(&buf, *notes, -1); + notes++; + }
virBufferContentAndReset will silently return NULL if an error (e.g. OOM) occurred while building the buffer content. You current code will silently set description to NULL instead of properly reporting an error. Use virBufferCheckError as shown here: http://libvirt.org/hacking.html#strbuf virBufferCheckError will report and error for you if there is one. if (virBufferCheckError(&buf)) goto cleanup;
+ + def->description = virBufferContentAndReset(&buf); + } +
-- Matthias Bolte http://photron.blogspot.com
participants (2)
-
Dawid Zamirski -
Matthias Bolte