[libvirt] [PATCH v3 05/12] hyperv: make hypervEnumAndPull use hypervWqlQuery
Dawid Zamirski
dzamirski at datto.com
Tue Apr 4 21:31:16 UTC 2017
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);
>
>
More information about the libvir-list
mailing list