[libvirt] [PATCH 07/16] hyperv: implement ability to send xml soap requests

[Matthias Bolte]

2016-08-09 14:39 GMT+02:00 Jason Miesionczek <jmiesionczek at datto.com>:
> also added ability to get/set auto start
> ---
>  src/hyperv/hyperv_driver.c | 101 +++++++
>  src/hyperv/hyperv_wmi.c    | 670 ++++++++++++++++++++++++++++++++++++++++++++-
>  src/hyperv/hyperv_wmi.h    |  58 ++++
>  src/hyperv/openwsman.h     |   4 +
>  4 files changed, 827 insertions(+), 6 deletions(-)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 861d5ab..aea7837 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1604,6 +1604,105 @@ hypervNodeGetFreeMemory(virConnectPtr conn)
>      return res;
>  }
>
> +static int
> +hypervDomainSetAutostart(virDomainPtr domain, int autostart)
> +{
> +    int result = -1;
> +    invokeXmlParam *params = NULL;
> +    hypervPrivate *priv = domain->conn->privateData;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    virBuffer queryVssd = VIR_BUFFER_INITIALIZER;
> +    Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL;
> +    properties_t *tab_props = NULL;
> +    eprParam eprparam;
> +    embeddedParam embeddedparam;

These types are not properly named. The all need a hyperv prefix and
should not have _t suffixed.

> +    int nb_params;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService";
> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +
> +    /* Prepare EPR param */
> +    virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
> +    virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string);
> +    eprparam.query = &query;
> +    eprparam.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> +    /* Prepare EMBEDDED param */
> +    virBufferAsprintf(&queryVssd,
> +                      "associators of "
> +                      "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> +                      "Name=\"%s\"} "
> +                      "where AssocClass = Msvm_SettingsDefineState "
> +                      "ResultClass = Msvm_VirtualSystemSettingData",
> +                      uuid_string);
> +
> +    if (hypervGetMsvmVirtualSystemSettingDataList(priv, &queryVssd, &virtualSystemSettingData) < 0)
> +        goto cleanup;
> +
> +    embeddedparam.nbProps = 2;
> +    if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0)
> +        goto cleanup;
> +    (*tab_props).name = "AutomaticStartupAction";
> +    (*tab_props).val = autostart ? "2" : "0";
> +    (*(tab_props+1)).name = "InstanceID";
> +    (*(tab_props+1)).val = virtualSystemSettingData->data->InstanceID;

Why do you do a VIR_ALLOC_N here? You know the number of elements at
compile time and the array doesn't have to life after the call to
hypervDomainSetAutostart.

You can turn this into a stack allocation.

Also, as pointed out earlier, the names should be converted from
strings to enum/int values. Those can then be used for an O(1) index
based lookup instead of an O(n) string based lookup.

-- 
Matthias Bolte
http://photron.blogspot.com