On 08/09/2016 08:39 AM, Jason Miesionczek wrote:
> ---
>  src/hyperv/hyperv_driver.c  | 232 
> ++++++++++++++++++++++++++++++++++++++++++++
>  src/hyperv/hyperv_private.h |   2 +
>  2 files changed, 234 insertions(+)
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 348b39c..daae371 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -56,6 +56,9 @@ hypervFreePrivate(hypervPrivate **priv)
>      if ((*priv)->caps != NULL)
>          virObjectUnref((*priv)->caps);
>  
> +    if ((*priv)->xmlopt != NULL)
> +        virObjectUnref((*priv)->xmlopt);
> +
>      hypervFreeParsedUri(&(*priv)->parsedUri);
>      VIR_FREE(*priv);
>  }
> @@ -195,6 +198,9 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr 
> auth,
>          goto cleanup;
>      }
>  
> +    /* Init xmlopt to parse Domain XML */
> +    priv->xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL);
> +

Strange to see NULL, NULL, NULL

No MAC prefix needs?

No desire/need to have post parse callbacks?  IOW: Things to check after
all the parsing is done so that you can verify things work together?

Guess I'm unclear why this is being done here.

Looking forward - I see patch 14 makes use of it, so that's where it
should be moved to.


>      conn->privateData = priv;
>      priv = NULL;
>      result = VIR_DRV_OPEN_SUCCESS;
> @@ -1860,6 +1866,229 @@ hypervDomainGetSchedulerType(virDomainPtr domain 
> ATTRIBUTE_UNUSED, int *nparams)
>      return type;
>  }
>  
> +/* Format a number as a string value */
> +static char *num2str(unsigned long value)
> +{
> +    int sz;
> +    char *result;
> +
> +    sz = snprintf (NULL, 0, "%lu", value);
> +    if (VIR_ALLOC_N(result, sz + 1) < 0) {
> +      return NULL;
> +    }
> +
> +    sprintf(result, "%lu", value);
> +    return result;

open coding virAsprintf() I think...

> +}
> +
> +
> +
> +static int
> +hypervDomainSetMaxMemory(virDomainPtr domain, unsigned long memory)
> +{
> +    int result = -1;
> +    invokeXmlParam *params = NULL;
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    hypervPrivate *priv = domain->conn->privateData;
> +    properties_t *tab_props = NULL;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    virBuffer query2 = VIR_BUFFER_INITIALIZER;
> +    Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL;
> +    Msvm_MemorySettingData *memorySettingData = NULL;
> +    eprParam eprparam;
> +    embeddedParam embeddedparam;
> +    int nb_params;
> +    const char *selector = 
> "CreationClassName=Msvm_VirtualSystemManagementService";
> +    unsigned long memory_mb = memory/1024;
> +    char *memory_str = NULL;
> +
> +    /* Memory value must be a multiple of 2 MB; round up it accordingly if 
> necessary */
> +    if (memory_mb % 2) memory_mb++;

There's VIR_ROUND_UP or VIR_ROUND_UP_POWER_OF_TWO

Not sure if you meant power of 2 instead multiple of 2MB

> +
> +    /* Convert the memory value as a string */
> +    memory_str = num2str(memory_mb);
> +    if (memory_str == NULL)
> +        goto cleanup;

if (virAsprintf(memory_str, "%lu", memory_mb) < 0)
    goto cleanup;

> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +
> +    VIR_DEBUG("memory=%sMb, uuid=%s", memory_str, 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 1 */
> +    /* Get Msvm_VirtualSystemSettingData */
> +    virBufferAsprintf(&query2,
> +                      "associators of "
> +                      
> "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> +                      "Name=\"%s\"} "
> +                      "where AssocClass = Msvm_SettingsDefineState "
> +                      "ResultClass = Msvm_VirtualSystemSettingData",
> +                      uuid_string);
> +
> +    if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query2, 
> &virtualSystemSettingData) < 0)
> +        goto cleanup;
> +
> +    /* Get Msvm_MemorySettingData */
> +    virBufferFreeAndReset(&query2);
> +    virBufferAsprintf(&query2,
> +                      "associators of "
> +                      "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} "
> +                      "where AssocClass = 
> Msvm_VirtualSystemSettingDataComponent "
> +                      "ResultClass = Msvm_MemorySettingData",
> +                      virtualSystemSettingData->data->InstanceID);
> +
> +    if (hypervGetMsvmMemorySettingDataList(priv, &query2, 
> &memorySettingData) < 0)
> +        goto cleanup;
> +
> +    embeddedparam.nbProps = 2;
> +    if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0)
> +        goto cleanup;
> +    (*tab_props).name = "Limit";
> +    (*tab_props).val = memory_str;
> +    (*(tab_props+1)).name = "InstanceID";
> +    (*(tab_props+1)).val = memorySettingData->data->InstanceID;
> +    embeddedparam.instanceName =  "Msvm_MemorySettingData";
> +    embeddedparam.prop_t = tab_props;
> +    embeddedparam.nbProps = 2;
> +
> +    /* Create invokeXmlParam */
> +    nb_params = 2;
> +    if (VIR_ALLOC_N(params, nb_params) < 0)
> +        goto cleanup;
> +    (*params).name = "ComputerSystem";
> +    (*params).type = EPR_PARAM;
> +    (*params).param = &eprparam;
> +    (*(params+1)).name = "ResourceSettingData";
> +    (*(params+1)).type = EMBEDDED_PARAM;
> +    (*(params+1)).param = &embeddedparam;
> +

More of these strange constructs *tab_props, *params...  consider

tab_props[0]->
tab_props[1]->
params[0]->
params[1]->


> +    result = hypervInvokeMethod(priv, params, nb_params, 
> "ModifyVirtualSystemResources",
> +                             
> MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector);
> +
> + cleanup:
> +    VIR_FREE(tab_props);
> +    VIR_FREE(params);
> +    VIR_FREE(memory_str);
> +    hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData);
> +    hypervFreeObject(priv, (hypervObject *)memorySettingData);
> +    virBufferFreeAndReset(&query);
> +    virBufferFreeAndReset(&query2);
> +
> +    return result;
> +}
> +


Since these two are so similar - check out what the qemu driver code does:

static int qemuDomainSetMaxMemory(virDomainPtr dom, unsigned long memory)
{
    return qemuDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM);
}

Then in qemuDomainSetMemoryFlags, there's a check:

   if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
   } else {
   }

Of course there's also an extra flags check for VIR_DOMAIN_MEM_MAXIMUM
in qemuDomainSetMemoryFlags

It's up to you though to determine how similar and whether any merging
of the two could be done...

> +static int
> +hypervDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory,
> +                           unsigned int flags ATTRIBUTE_UNUSED)
> +{
> +    int result = -1, nb_params;
> +    const char *selector = 
> "CreationClassName=Msvm_VirtualSystemManagementService";
> +    char uuid_string[VIR_UUID_STRING_BUFLEN];
> +    hypervPrivate *priv = domain->conn->privateData;
> +    invokeXmlParam *params = NULL;
> +    properties_t *tab_props = NULL;
> +    eprParam eprparam;
> +    embeddedParam embeddedparam;
> +    virBuffer query = VIR_BUFFER_INITIALIZER;
> +    Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL;
> +    Msvm_MemorySettingData *memorySettingData = NULL;
> +    unsigned long memory_mb = memory / 1024;   /* Memory converted in MB */
> +    char *memory_str = NULL;
> +
> +    /* Memory value must be a multiple of 2 MB; round up it accordingly if 
> necessary */
> +    if (memory_mb % 2) memory_mb++;
> +
> +    /* Convert the memory value as a string */
> +    memory_str = num2str(memory_mb);
> +    if (memory_str == NULL)
> +        goto cleanup;

Simi
> +
> +    virUUIDFormat(domain->uuid, uuid_string);
> +
> +    VIR_DEBUG("memory=%sMb, uuid=%s", memory_str, uuid_string);
> +
> +    /* Get Msvm_VirtualSystemSettingData */
> +    virBufferAsprintf(&query,
> +                      "associators of "
> +                      
> "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> +                      "Name=\"%s\"} "
> +                      "where AssocClass = Msvm_SettingsDefineState "
> +                      "ResultClass = Msvm_VirtualSystemSettingData",
> +                      uuid_string);
> +    if (hypervGetMsvmVirtualSystemSettingDataList(priv, &query, 
> &virtualSystemSettingData) < 0)
> +        goto cleanup;
> +
> +    /* Get Msvm_MemorySettingData */
> +    virBufferFreeAndReset(&query);
> +    virBufferAsprintf(&query,
> +                      "associators of "
> +                      "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} "
> +                      "where AssocClass = 
> Msvm_VirtualSystemSettingDataComponent "
> +                      "ResultClass = Msvm_MemorySettingData",
> +                      virtualSystemSettingData->data->InstanceID);
> +    if (hypervGetMsvmMemorySettingDataList(priv, &query, &memorySettingData) 
> < 0)
> +        goto cleanup;
> +
> +    /* Prepare EPR param */
> +    virBufferFreeAndReset(&query);
> +    virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
> +    virBufferAsprintf(&query, "where Name = \"%s\"",uuid_string);
> +    eprparam.query = &query;
> +    eprparam.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> +    /* Prepare EMBEDDED param */
> +    embeddedparam.nbProps = 2;
> +    if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0)
> +        goto cleanup;
> +    (*tab_props).name = "VirtualQuantity";
> +    (*tab_props).val = memory_str;
> +    (*(tab_props+1)).name = "InstanceID";
> +    (*(tab_props+1)).val = memorySettingData->data->InstanceID;
> +    embeddedparam.instanceName =  "Msvm_MemorySettingData";
> +    embeddedparam.prop_t = tab_props;
> +
> +    /* Create invokeXmlParam */
> +    nb_params = 2;
> +    if (VIR_ALLOC_N(params, nb_params) < 0)
> +        goto cleanup;
> +    (*params).name = "ComputerSystem";
> +    (*params).type = EPR_PARAM;
> +    (*params).param = &eprparam;
> +    (*(params+1)).name = "ResourceSettingData";
> +    (*(params+1)).type = EMBEDDED_PARAM;
> +    (*(params+1)).param = &embeddedparam;
> +
> +    if (hypervInvokeMethod(priv, params, nb_params, 
> "ModifyVirtualSystemResources",
> +                           MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, 
> selector) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not set domain 
> memory"));
> +        goto cleanup;
> +    }
> +
> +    result = 0;
> +
> + cleanup:
> +    VIR_FREE(tab_props);
> +    VIR_FREE(params);
> +    VIR_FREE(memory_str);
> +    hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData);
> +    hypervFreeObject(priv, (hypervObject *)memorySettingData);
> +    virBufferFreeAndReset(&query);
> +
> +    return result;
> +}
> +
> +
> +
> +static int
> +hypervDomainSetMemory(virDomainPtr domain, unsigned long memory)
> +{
> +    return hypervDomainSetMemoryFlags(domain, memory, 0);
> +}
>  
>  static virHypervisorDriver hypervHypervisorDriver = {
>      .name = "Hyper-V",
> @@ -1909,6 +2138,9 @@ static virHypervisorDriver hypervHypervisorDriver = {
>      .domainGetSchedulerParametersFlags = 
> hypervDomainGetSchedulerParametersFlags, /* 1.2.10 */
>      .domainGetSchedulerParameters = hypervDomainGetSchedulerParameters, /* 
> 1.2.10 */
>      .domainGetSchedulerType = hypervDomainGetSchedulerType, /* 1.2.10 */
> +    .domainSetMaxMemory = hypervDomainSetMaxMemory, /* 1.2.10 */
> +    .domainSetMemory = hypervDomainSetMemory, /* 1.2.10 */
> +    .domainSetMemoryFlags = hypervDomainSetMemoryFlags, /* 1.2.10 */

2.3.0 at the earliest

>  };
>  
>  /* Retrieves host system UUID  */
> diff --git a/src/hyperv/hyperv_private.h b/src/hyperv/hyperv_private.h
> index d9aa0bd..2dfce6e 100644
> --- a/src/hyperv/hyperv_private.h
> +++ b/src/hyperv/hyperv_private.h
> @@ -27,6 +27,7 @@
>  # include "virerror.h"
>  # include "hyperv_util.h"
>  # include "capabilities.h"
> +# include "domain_conf.h"

Will this also need to move to patch 14?

>  # include "openwsman.h"
>  
>  typedef struct _hypervPrivate hypervPrivate;
> @@ -35,6 +36,7 @@ struct _hypervPrivate {
>      hypervParsedUri *parsedUri;
>      WsManClient *client;
>      virCapsPtr caps;
> +    virDomainXMLOptionPtr xmlopt;

Move to patch 14

John

>  };
>  
>  #endif /* __HYPERV_PRIVATE_H__ */
> 

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

Reply via email to