[libvirt] [PATCH] hyperv: recognize array property as distinct type.

Dawid Zamirski posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170411150236.2746-1-dzamirski@datto.com
There is a newer version of this series
src/hyperv/hyperv_driver.c         | 26 ++++++++++++++++++++++++--
src/hyperv/hyperv_wmi_generator.py |  2 +-
2 files changed, 25 insertions(+), 3 deletions(-)
[libvirt] [PATCH] hyperv: recognize array property as distinct type.
Posted by Dawid Zamirski 6 years, 11 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hyperv: recognize array property as distinct type.
Posted by Matthias Bolte 6 years, 11 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] hyperv: recognize array property as distinct type.
Posted by Dawid Zamirski 6 years, 11 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] hyperv: recognize array property as distinct type.
Posted by Matthias Bolte 6 years, 11 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list