[libvirt] [PATCH v3 05/12] hyperv: make hypervEnumAndPull use hypervWqlQuery

[Dawid Zamirski]

On Tue, 2017-04-04 at 23:10 +0200, Matthias Bolte wrote:
> 2017-04-04 1:52 GMT+02:00 Dawid Zamirski <dzamirski at 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);
> 
>