4:31 p.m.
On Tue, 2017-04-04 at 23:10 +0200, Matthias Bolte wrote:
2017-04-04 1:52 GMT+02:00 Dawid Zamirski
<dzamirski(a)datto.com>:
> This enables this function to handle "v1" and "v2" WMI
requests.
>
> Since this commit and the ones that follow should be squashed on
> previous one:
> * rename hypervObjectUnified -> hypervObject as we've already
> broken
> compilation here so there's no point in keeping those in parallel
> anymore.
> * do not mark hypervGetWmiClassInfo as unused
> ---
> src/hyperv/hyperv_wmi.c | 40 ++++++++++++++++++-------------------
> ---
> src/hyperv/hyperv_wmi.h | 17 ++++-------------
> 2 files changed, 22 insertions(+), 35 deletions(-)
>
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index 069bcc9..5cac58d 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -45,7 +45,7 @@
> #define VIR_FROM_THIS VIR_FROM_HYPERV
>
>
> -static int ATTRIBUTE_UNUSED
> +static int
> hypervGetWmiClassInfo(hypervPrivate *priv,
> hypervWmiClassInfoListPtr list,
> hypervWmiClassInfoPtr *info)
> {
> @@ -143,14 +143,13 @@ hypervVerifyResponse(WsManClient *client,
> WsXmlDocH response,
>
> /* This function guarantees that query is freed, even on failure
> */
You changes broke the contract of this function as stated in the
comment above. You're changes make hypervEnumAndPull leak the query
string now stored in hypervWqlQuery.
I'd change hypervWqlQuery to hold the query string as a virBuffer
(not
a virBufferPtr) and then instead of using virBufferContentAndReset to
create the query string before calling hypervEnumAndPull just make
all
the callers build the query in the virBuffer stored in the
hypervWqlQuery struct directly.
Then hypervEnumAndPull can keep all its virBuffer handling and ensure
again that the query string is freed properly in all cases.
> int
> -hypervEnumAndPull(hypervPrivate *priv, virBufferPtr query, const
> char *root,
> - XmlSerializerInfo *serializerInfo, const char
> *resourceUri,
> - const char *className, hypervObject **list)
> +hypervEnumAndPull(hypervPrivate *priv, hypervWqlQueryPtr wqlQuery,
> + hypervObject **list)
> {
> int result = -1;
> WsSerializerContextH serializerContext;
> client_opt_t *options = NULL;
> - char *query_string = NULL;
> + hypervWmiClassInfoPtr wmiInfo = NULL;
> filter_t *filter = NULL;
> WsXmlDocH response = NULL;
> char *enumContext = NULL;
> @@ -160,18 +159,14 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> XML_TYPE_PTR data = NULL;
> hypervObject *object;
>
> - if (virBufferCheckError(query) < 0) {
> - virBufferFreeAndReset(query);
> - return -1;
> - }
> - query_string = virBufferContentAndReset(query);
> -
Don't remove this but feed it with &wqlQuery->query instead of query
> if (list == NULL || *list != NULL) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid
> argument"));
> - VIR_FREE(query_string);
> return -1;
> }
>
> + if (hypervGetWmiClassInfo(priv, wqlQuery->info, &wmiInfo) < 0)
Need to free query_string here.
Shouldn't this goto cleanup to also free &wql->query with
virtBufferFreeAndReset?
> + return -1;
> +
> serializerContext = wsmc_get_serialization_context(priv-
> >client);
>
> options = wsmc_options_init();
> @@ -182,7 +177,7 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> goto cleanup;
> }
>
> - filter = filter_create_simple(WSM_WQL_FILTER_DIALECT,
> query_string);
> + filter = filter_create_simple(WSM_WQL_FILTER_DIALECT,
> wqlQuery->query);
> if (filter == NULL) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -190,7 +185,8 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> goto cleanup;
> }
>
> - response = wsmc_action_enumerate(priv->client, root, options,
> filter);
> + response = wsmc_action_enumerate(priv->client, wmiInfo-
> >rootUri, options,
> + filter);
>
> if (hypervVerifyResponse(priv->client, response,
> "enumeration") < 0)
> goto cleanup;
> @@ -201,7 +197,7 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> response = NULL;
>
> while (enumContext != NULL && *enumContext != '\0') {
> - response = wsmc_action_pull(priv->client, resourceUri,
> options,
> + response = wsmc_action_pull(priv->client, wmiInfo-
> >resourceUri, options,
> filter, enumContext);
>
> if (hypervVerifyResponse(priv->client, response, "pull") <
> 0)
> @@ -231,11 +227,12 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> goto cleanup;
> }
>
> - if (ws_xml_get_child(node, 0, resourceUri, className) ==
> NULL)
> + if (ws_xml_get_child(node, 0, wmiInfo->resourceUri,
> + wmiInfo->name) == NULL)
> break;
>
> - data = ws_deserialize(serializerContext, node,
> serializerInfo,
> - className, resourceUri, NULL, 0, 0);
> + data = ws_deserialize(serializerContext, node, wmiInfo-
> >serializerInfo,
> + wmiInfo->name, wmiInfo->resourceUri,
> NULL, 0, 0);
>
> if (data == NULL) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -246,8 +243,8 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> if (VIR_ALLOC(object) < 0)
> goto cleanup;
>
> - object->serializerInfo = serializerInfo;
> - object->data = data;
> + object->info = wmiInfo;
> + object->data.common = data;
>
> data = NULL;
>
> @@ -283,13 +280,12 @@ hypervEnumAndPull(hypervPrivate *priv,
> virBufferPtr query, const char *root,
> /* FIXME: ws_serializer_free_mem is broken in openwsman <=
> 2.2.6,
> * see hypervFreeObject for a detailed explanation.
> */
> if (ws_serializer_free_mem(serializerContext, data,
> - serializerInfo) < 0) {
> + wmiInfo->serializerInfo) < 0) {
> VIR_ERROR(_("Could not free deserialized data"));
> }
> #endif
> }
>
> - VIR_FREE(query_string);
> ws_xml_destroy_doc(response);
> VIR_FREE(enumContext);
> hypervFreeObject(priv, head);