On 08/09/2016 08:39 AM, Jason Miesionczek wrote:
> ---
> src/hyperv/hyperv_driver.c | 807 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 807 insertions(+)
>
This is where the optxml first shows up and where it should be defined.
Although I'm still a bit unclear why it's being used with the NULL
parameters...
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index bd028ed..716fadb 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -2275,6 +2275,809 @@ hypervDomainUndefine(virDomainPtr domain)
> return hypervDomainUndefineFlags(domain, 0);
> }
>
> +/*
> + * Creates the attribute __PATH for the RASD object
> + * The attribute is build like this:
s/build/built
> + * \\<host_name>\root\virtualization:Msvm_ResourceAllocationSettingData.InstanceID="<rasdInstanceID>"
> + * where backslashes in rasdInstanceID are doubled
> + */
> +static int
> +hypervGetResourceAllocationSettingDataPATH(virDomainPtr domain, char *rasdInstanceID, char **__path)
> +{
> + char uuid_string[VIR_UUID_STRING_BUFLEN];
> + hypervPrivate *priv = domain->conn->privateData;
> + virBuffer query = VIR_BUFFER_INITIALIZER;
> + Msvm_ComputerSystem *computerSystem = NULL;
> + char *strTemp = NULL;
> + int result = -1, i = 0, j = 0, n = 0;
> +
> + virUUIDFormat(domain->uuid, uuid_string);
> +
> + /* Get host name */
> + virBufferAsprintf(&query,
> + "associators of "
> + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> + "Name=\"%s\"} "
> + "where AssocClass = Msvm_HostedDependency "
> + "ResultClass = Msvm_ComputerSystem",
> + uuid_string);
> + if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) {
> + goto cleanup;
> + }
> + if (computerSystem == NULL) {
> + virReportError(VIR_ERR_NO_DOMAIN, _("No domain with UUID %s"), uuid_string);
> + goto cleanup;
> + }
Repeatable sequence (comment from earlier patch 12 I think)
> +
> + /* Count the number of backslash character */
> + strTemp = strchr(rasdInstanceID, '\\');
> + while (strTemp != NULL) {
> + n++;
> + strTemp = strchr(++strTemp, '\\');
> + }
> + /* Double the blackslashes */
> + if (VIR_ALLOC_N(strTemp, strlen(rasdInstanceID) + 1 + n) < 0)
> + goto cleanup;
> + while (rasdInstanceID[i] != '\0') {
> + strTemp[j] = rasdInstanceID[i];
> + if (rasdInstanceID[i] == '\\') {
> + j++;
> + strTemp[j] = '\\';
> + }
> + i++;
> + j++;
> + }
> + strTemp[j] = '\0';
I think you may want to look at virBufferEscape
> +
> + /* Create the attribute __PATH */
> + /* FIXME: *__path allocated with 255 characters (static value) */
> + if (VIR_ALLOC_N(*__path, 255) < 0)
> + goto cleanup;
> + sprintf(*__path, "\\\\");
> + strcat(*__path, computerSystem->data->ElementName);
> + strcat(*__path, "\\root\\virtualization:Msvm_ResourceAllocationSettingData.InstanceID=\"");
> + strcat(*__path, strTemp);
> + strcat(*__path, "\"");
Why not return __path instead of passing char **_path and returning
where the caller would then being able succeed/fail based on NULL return.
Perhaps even virAsprintf will help you out with your quadary.
There's even a virBufferStrcat
> +
> + result = 0;
> +
> + cleanup:
> + VIR_FREE(strTemp);
> + hypervFreeObject(priv, (hypervObject *)computerSystem);
> + virBufferFreeAndReset(&query);
> +
> + return result;
> +}
> +
> +
> +
> +/* hypervDomainAttachDisk
> + * FIXME:
> + * - added ressources must me removed in case of error
> + * - allow attaching disks on iSCSI (implemented only on IDE)
> + * - allow attaching ISO images (on DVD devices)
> + * - implement associated detach method
> + */
> +ATTRIBUTE_UNUSED static int
> +hypervDomainAttachDisk(virDomainPtr domain, virDomainDiskDefPtr disk)
> +{
> + int result = -1, nb_params;
> + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService";
> + char uuid_string[VIR_UUID_STRING_BUFLEN];
> + char *ideRasdPath = NULL, *newDiskDrivePath = NULL;
> + char ideControler[2], ideControlerAddr[2];
A type A person would tell you it's "controller" ;-)
> + hypervPrivate *priv = domain->conn->privateData;
> + virBuffer query = VIR_BUFFER_INITIALIZER;
> + Msvm_VirtualSystemSettingData *virtualSystemSettingData = NULL;
> + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData = NULL;
> + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData2 = NULL;
> + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData3 = NULL;
> + Msvm_ResourceAllocationSettingData *resourceAllocationSettingData4 = NULL;
> + Msvm_ResourceAllocationSettingData *ideRasd = NULL; /* resourceAllocationSettingData subtree -> do not disallocate */
> + Msvm_ResourceAllocationSettingData *diskRasd = NULL; /* resourceAllocationSettingData2 subtree -> do not disallocate */
> + Msvm_ResourceAllocationSettingData *newDiskDrive = NULL; /* resourceAllocationSettingData3 subtree -> do not disallocate */
> + Msvm_AllocationCapabilities *allocationCapabilities = NULL;
> + Msvm_AllocationCapabilities *allocationCapabilities2 = NULL;
> + invokeXmlParam *params = NULL;
> + properties_t *tab_props = NULL;
> + eprParam eprparam1, eprparam2;
> + embeddedParam embeddedparam1, embeddedparam2;
> +
> + /* Initialization */
> + virUUIDFormat(domain->uuid, uuid_string);
> +
> + /* Set IDE Controler 0 or 1 and address 0 or 1 */
> + if (STREQ(disk->dst, "hda")) {
> + sprintf(ideControler, "%d", 0);
> + sprintf(ideControlerAddr, "%d", 0);
> + } else if (STREQ(disk->dst, "hdb")) {
> + sprintf(ideControler, "%d", 0);
> + sprintf(ideControlerAddr, "%d", 1);
> + } else if (STREQ(disk->dst, "hdc")) {
> + sprintf(ideControler, "%d", 1);
> + sprintf(ideControlerAddr, "%d", 0);
> + } else if (STREQ(disk->dst, "hdd")) {
> + sprintf(ideControler, "%d", 1);
> + sprintf(ideControlerAddr, "%d", 1);
> + } else {
> + /* IDE Controler 0 and address 0 choosen by default */
> + sprintf(ideControler, "%d", 0);
> + sprintf(ideControlerAddr, "%d", 0);
> + }
That's an odd set of lines to read... So no such thing as "hde" - just
4 recognized types. What if someone tries 'hda' and 'hde'?
> +
> + VIR_DEBUG("src=%s, dst=IDE Controller %s:%s, uuid=%s",
> + disk->src->path, ideControler, ideControlerAddr, uuid_string);
> +
> + /* Get the current VM settings object */
> + 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 the settings for IDE Controller on the VM */
> + virBufferFreeAndReset(&query);
> + virBufferAsprintf(&query,
> + "associators of "
> + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} "
> + "where AssocClass = Msvm_VirtualSystemSettingDataComponent "
> + "ResultClass = Msvm_ResourceAllocationSettingData",
> + virtualSystemSettingData->data->InstanceID);
> + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData) < 0) {
> + goto cleanup;
> + }
While sometimes it takes a few moments to figure out the acronym, it
sure beats typing ResourceAllocationSettingData (or copy/cut - paste).
> + ideRasd = resourceAllocationSettingData;
> + while (ideRasd != NULL) {
> + if (ideRasd->data->ResourceType == 5 && STREQ(ideRasd->data->Address, ideControler)) {
5 is a magic number?
> + /* IDE Controller 0 or 1 */
> + break;
> + }
> + ideRasd = ideRasd->next;
> + }
> + if (ideRasd == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not find IDE Controller %s"), ideControler);
> + goto cleanup;
> + }
> +
> + /* Get the settings for 'Microsoft Synthetic Disk Drive' */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_ALLOCATIONCAPABILITIES_WQL_SELECT);
> + virBufferAddLit(&query, "WHERE ResourceSubType = 'Microsoft Synthetic Disk Drive'");
> + if (hypervGetMsvmAllocationCapabilitiesList(priv, &query, &allocationCapabilities) < 0) {
> + goto cleanup;
> + }
> +
> + /* Get default values for 'Microsoft Synthetic Disk Drive' */
> + virBufferFreeAndReset(&query);
> + virBufferAsprintf(&query,
> + "associators of "
> + "{Msvm_AllocationCapabilities.InstanceID=\"%s\"} "
> + "where AssocClass = Msvm_SettingsDefineCapabilities "
> + "ResultClass = Msvm_ResourceAllocationSettingData",
> + allocationCapabilities->data->InstanceID);
> + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData2) < 0) {
> + goto cleanup;
> + }
> + diskRasd = resourceAllocationSettingData2;
> + while (diskRasd != NULL) {
> + if (strstr(diskRasd->data->InstanceID, "Default") != NULL) {
> + /* Default values */
> + break;
> + }
> + diskRasd = diskRasd->next;
> + }
> + if (diskRasd == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not get default values for 'Microsoft Synthetic Disk Drive'"));
> + goto cleanup;
> + }
> +
> + /* Create the attribute _PATH for the RASD object */
> + if (hypervGetResourceAllocationSettingDataPATH(domain, ideRasd->data->InstanceID, &ideRasdPath) < 0) {
> + goto cleanup;
> + }
> +
> + /* Add default disk drive */
> + /* Prepare EPR param */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
> + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string);
> + eprparam1.query = &query;
> + eprparam1.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> + /* Prepare EMBEDDED param 1 */
> + embeddedparam1.nbProps = 4;
> + if (VIR_ALLOC_N(tab_props, embeddedparam1.nbProps) < 0)
> + goto cleanup;
> + (*tab_props).name = "Parent";
> + (*tab_props).val = ideRasdPath;
> + (*(tab_props+1)).name = "Address";
> + (*(tab_props+1)).val = ideControlerAddr;
> + (*(tab_props+2)).name = "ResourceType";
> + (*(tab_props+2)).val = "22";
22 is a magic number
> + (*(tab_props+3)).name = "ResourceSubType";
> + (*(tab_props+3)).val = diskRasd->data->ResourceSubType;
> + embeddedparam1.instanceName = MSVM_RESOURCEALLOCATIONSETTINGDATA_CLASSNAME;
> + embeddedparam1.prop_t = tab_props;
> +
I still cannot get used to the construct (*tab_props)
> + /* Create invokeXmlParam tab */
> + nb_params = 2;
> + if (VIR_ALLOC_N(params, nb_params) < 0)
> + goto cleanup;
> + (*params).name = "TargetSystem";
> + (*params).type = EPR_PARAM;
> + (*params).param = &eprparam1;
> + (*(params+1)).name = "ResourceSettingData";
> + (*(params+1)).type = EMBEDDED_PARAM;
> + (*(params+1)).param = &embeddedparam1;
> +
> + if (hypervInvokeMethod(priv, params, nb_params, "AddVirtualSystemResources",
> + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not add default disk drive"));
> + goto cleanup;
> + }
> +
> + /* Get the instance of the new default drive disk */
> + virBufferFreeAndReset(&query);
> + virBufferAsprintf(&query,
> + "associators of "
> + "{Msvm_VirtualSystemSettingData.InstanceID=\"%s\"} "
> + "where AssocClass = Msvm_VirtualSystemSettingDataComponent "
> + "ResultClass = Msvm_ResourceAllocationSettingData",
> + virtualSystemSettingData->data->InstanceID);
> + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData3) < 0) {
> + goto cleanup;
> + }
> + newDiskDrive = resourceAllocationSettingData3;
> + while (newDiskDrive != NULL) {
> + if (newDiskDrive->data->ResourceType == 22 &&
> + STREQ(newDiskDrive->data->ResourceSubType, "Microsoft Synthetic Disk Drive") &&
> + STREQ(newDiskDrive->data->Parent, ideRasdPath) &&
> + STREQ(newDiskDrive->data->Address, ideControlerAddr)) {
> + break;
> + }
> + newDiskDrive = newDiskDrive->next;
> + }
> + if (newDiskDrive == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not find 'Microsoft Synthetic Disk Drive'"));
> + goto cleanup;
> + }
> +
> + /* Get the settings for 'Microsoft Virtual Hard Disk' */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_ALLOCATIONCAPABILITIES_WQL_SELECT);
> + virBufferAddLit(&query, "WHERE ResourceSubType = 'Microsoft Virtual Hard Disk'");
> + if (hypervGetMsvmAllocationCapabilitiesList(priv, &query, &allocationCapabilities2) < 0) {
> + goto cleanup;
> + }
> +
> + /* Get default values for 'Microsoft Virtual Hard Drive' */
> + virBufferFreeAndReset(&query);
> + virBufferAsprintf(&query,
> + "associators of "
> + "{Msvm_AllocationCapabilities.InstanceID=\"%s\"} "
> + "where AssocClass = Msvm_SettingsDefineCapabilities "
> + "ResultClass = Msvm_ResourceAllocationSettingData",
> + allocationCapabilities2->data->InstanceID);
> + if (hypervGetMsvmResourceAllocationSettingDataList(priv, &query, &resourceAllocationSettingData4) < 0) {
> + goto cleanup;
> + }
> + diskRasd = resourceAllocationSettingData4;
> + while (diskRasd != NULL) {
> + if (strstr(diskRasd->data->InstanceID, "Default") != NULL) {
> + /* Default values */
> + break;
> + }
> + diskRasd = diskRasd->next;
> + }
> + if (diskRasd == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not get default values for 'Microsoft Virtual Hard Drive'"));
> + goto cleanup;
> + }
> +
> + /* Create the attribute _PATH for the RASD object */
> + if (hypervGetResourceAllocationSettingDataPATH(domain, newDiskDrive->data->InstanceID, &newDiskDrivePath) < 0) {
> + goto cleanup;
> + }
> +
> + /* Add the new VHD */
> + /* Prepare EPR param 2 */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
> + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string);
> + eprparam2.query = &query;
> + eprparam2.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> + /* Prepare EMBEDDED param 2 */
> + VIR_FREE(tab_props);
> + embeddedparam2.nbProps = 4;
> + if (VIR_ALLOC_N(tab_props, embeddedparam2.nbProps) < 0)
> + goto cleanup;
> + (*tab_props).name = "Parent";
> + (*tab_props).val = newDiskDrivePath;
> + (*(tab_props+1)).name = "Connection";
> + (*(tab_props+1)).val = disk->src->path;
> + (*(tab_props+2)).name = "ResourceType";
> + (*(tab_props+2)).val = "21";
> + (*(tab_props+3)).name = "ResourceSubType";
> + (*(tab_props+3)).val = diskRasd->data->ResourceSubType;
> + embeddedparam2.instanceName = MSVM_RESOURCEALLOCATIONSETTINGDATA_CLASSNAME;
> + embeddedparam2.prop_t = tab_props;
> +
> + /* Create invokeXmlParam tab */
> + VIR_FREE(params);
> + nb_params = 2;
> + if (VIR_ALLOC_N(params, nb_params) < 0)
> + goto cleanup;
> + (*params).name = "TargetSystem";
> + (*params).type = EPR_PARAM;
> + (*params).param = &eprparam2;
> + (*(params+1)).name = "ResourceSettingData";
> + (*(params+1)).type = EMBEDDED_PARAM;
> + (*(params+1)).param = &embeddedparam2;
> +
> + if (hypervInvokeMethod(priv, params, nb_params, "AddVirtualSystemResources",
> + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not attach hard disk drive"));
> + goto cleanup;
> + }
> +
> + result = 0;
Oh my eyes and brain hurt badly by this point.
I think this needs to be broken up into multiple helpers
1. To do that IDE stuff
2. To do that Microsoft Synthetic Disk Drive stuff
3. To do that Microsoft Virtual Hard Disk
Combining everything in one function with all those consonants and
vowels is painful.
> +
> + cleanup:
> + VIR_FREE(ideRasdPath);
> + VIR_FREE(newDiskDrivePath);
> + VIR_FREE(tab_props);
> + VIR_FREE(params);
> + hypervFreeObject(priv, (hypervObject *)virtualSystemSettingData);
> + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData);
> + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData2);
> + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData3);
> + hypervFreeObject(priv, (hypervObject *)resourceAllocationSettingData4);
> + hypervFreeObject(priv, (hypervObject *)allocationCapabilities);
> + hypervFreeObject(priv, (hypervObject *)allocationCapabilities2);
> + virBufferFreeAndReset(&query);
> +
> + return result;
> +}
> +
OK so far the above is DISK... Can we make it it's own patch separate
from the following NETWORK stuff?
> +
> +/*
> + * Create the attribute __PATH for the SwitchPort object.
> + * The attribute is build like this:
> + * \\<host_name>\root\virtualization:Msvm_SwitchPort.CreationClassName="Msvm_SwitchPort",
> + * Name="<switchPortName>",SystemCreationClassName="Msvm_VirtualSwitch",
> + * SystemName="<virtualSwitchSystemName>"
> + */
> +static int
> +hypervGetSwitchPortPATH(virDomainPtr domain, char *switchPortName, char *virtualSwitchSystemName, char **__path)
> +{
> + char uuid_string[VIR_UUID_STRING_BUFLEN];
> + hypervPrivate *priv = domain->conn->privateData;
> + virBuffer query = VIR_BUFFER_INITIALIZER;
> + Msvm_ComputerSystem *computerSystem = NULL;
> + int result = -1;
> +
> + virUUIDFormat(domain->uuid, uuid_string);
> +
> + /* Get host name */
> + virBufferAsprintf(&query,
> + "associators of "
> + "{Msvm_ComputerSystem.CreationClassName=\"Msvm_ComputerSystem\","
> + "Name=\"%s\"} "
> + "where AssocClass = Msvm_HostedDependency "
> + "ResultClass = Msvm_ComputerSystem",
> + uuid_string);
> + if (hypervGetMsvmComputerSystemList(priv, &query, &computerSystem) < 0) {
> + goto cleanup;
> + }
> + if (computerSystem == NULL) {
> + virReportError(VIR_ERR_NO_DOMAIN,
> + _("No domain with UUID %s"), uuid_string);
> + goto cleanup;
> + }
> +
> + /* Create the attribute __PATH */
> + /* FIXME: *__path is allocated with 512 characters (static value) */
> + if (VIR_ALLOC_N(*__path, 512) < 0)
> + goto cleanup;
> + sprintf(*__path,
> + "\\\\%s\\root\\virtualization:Msvm_SwitchPort.CreationClassName=\"Msvm_SwitchPort\","
> + "Name=\"%s\",SystemCreationClassName=\"Msvm_VirtualSwitch\",SystemName=\"%s\"",
> + computerSystem->data->ElementName, switchPortName, virtualSwitchSystemName);
> +
virAsprintf
Holy Shinola that is one fugly construct.
> + result = 0;
> +
> + cleanup:
> + hypervFreeObject(priv, (hypervObject *) computerSystem);
> + virBufferFreeAndReset(&query);
> + return result;
> +}
> +
> +
> +
> +/* hypervDomainAttachNetwork
> + * FIXME:
> + * - implement associated detach method
> + */
> +ATTRIBUTE_UNUSED static int
> +hypervDomainAttachNetwork(virDomainPtr domain, virDomainNetDefPtr net)
> +{
> + int result = -1, nb_params;
> + const char *selector1 = "CreationClassName=Msvm_VirtualSwitchManagementService";
> + const char *selector2 = "CreationClassName=Msvm_VirtualSystemManagementService";
> + char uuid_string[VIR_UUID_STRING_BUFLEN], guid_string[VIR_UUID_STRING_BUFLEN];
> + unsigned char guid[VIR_UUID_BUFLEN];
> + char *virtualSystemIdentifiers = NULL, *switchPortPATH = NULL;
> + hypervPrivate *priv = domain->conn->privateData;
> + virBuffer query = VIR_BUFFER_INITIALIZER;
> + eprParam eprparam1, eprparam2;
> + simpleParam simpleparam1, simpleparam2, simpleparam3;
> + embeddedParam embeddedparam;
> + properties_t *tab_props = NULL;
> + invokeXmlParam *params = NULL;
> + Msvm_SwitchPort *switchPort = NULL;
> + Msvm_VirtualSwitch *virtualSwitch = NULL;
> +
> + /* Initialization */
> + virUUIDFormat(domain->uuid, uuid_string);
> +
> + VIR_DEBUG("network=%s, uuid=%s", net->data.network.name, uuid_string);
> +
> + /* Create virtual switch port */
> + /* Prepare EPR param 1 */
> + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT);
> + virBufferAsprintf(&query, "where ElementName = \"%s\"", net->data.network.name);
> + eprparam1.query = &query;
> + eprparam1.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> + /* Prepare SIMPLE params */
> + virUUIDGenerate(guid);
Coverity notes, this needs to check the status like other such calls
> + virUUIDFormat(guid, guid_string);
> + simpleparam1.value = guid_string;
> + simpleparam2.value = "Dynamic Ethernet Switch Port";
> + simpleparam3.value = "";
> +
> + /* Create invokeXmlParam tab */
> + nb_params = 4;
> + if (VIR_ALLOC_N(params, nb_params) < 0)
> + goto cleanup;
> + (*params).name = "VirtualSwitch";
> + (*params).type = EPR_PARAM;
> + (*params).param = &eprparam1;
> + (*(params+1)).name = "Name";
> + (*(params+1)).type = SIMPLE_PARAM;
> + (*(params+1)).param = &simpleparam1;
> + (*(params+2)).name = "FriendlyName";
> + (*(params+2)).type = SIMPLE_PARAM;
> + (*(params+2)).param = &simpleparam2;
> + (*(params+3)).name = "ScopeOfResidence";
> + (*(params+3)).type = SIMPLE_PARAM;
> + (*(params+3)).param = &simpleparam3;
> +
> + if (hypervInvokeMethod(priv, params, nb_params, "CreateSwitchPort",
> + MSVM_VIRTUALSWITCHMANAGEMENTSERVICE_RESOURCE_URI, selector1) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not create port for virtual switch '%s'"), net->data.network.name);
> + goto cleanup;
> + }
> +
> + /* Get a reference of the switch port created previously */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_SWITCHPORT_WQL_SELECT);
> + virBufferAsprintf(&query, "where Name = \"%s\"", guid_string);
> + if (hypervGetMsvmSwitchPortList(priv, &query, &switchPort) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Method hypervGetMsvmSwitchPortList failed with query=%s"), query.e);
> + goto cleanup;
> + }
> + if (switchPort == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not get switch port with Name=%s"), guid_string);
> + goto cleanup;
> + }
> +
> + /* Get a reference of the given virtual switch */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_VIRTUALSWITCH_WQL_SELECT);
> + virBufferAsprintf(&query, "where ElementName = \"%s\"", net->data.network.name);
> + if (hypervGetMsvmVirtualSwitchList(priv, &query, &virtualSwitch) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Method hypervGetMsvmVirtualSwitchList failed with query=%s"), query.e);
> + goto cleanup;
> + }
> + if (virtualSwitch == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not get virtual switch '%s'"), net->data.network.name);
> + goto cleanup;
> + }
> +
> + /* Add the synthetic ethernet port to the VM */
> + /* Prepare EPR param 2 */
> + virBufferFreeAndReset(&query);
> + virBufferAddLit(&query, MSVM_COMPUTERSYSTEM_WQL_SELECT);
> + virBufferAsprintf(&query, "where Name = \"%s\"", uuid_string);
> + eprparam2.query = &query;
> + eprparam2.wmiProviderURI = ROOT_VIRTUALIZATION;
> +
> + /* Prepare EMBEDDED param */
> + virUUIDGenerate(guid);
> + virUUIDFormat(guid, guid_string);
> + virtualSystemIdentifiers = (char *) malloc((strlen(guid_string)+3) * sizeof(char));
> + sprintf(virtualSystemIdentifiers, "{%s}", guid_string);
> + if (hypervGetSwitchPortPATH(domain, switchPort->data->Name, virtualSwitch->data->Name, &switchPortPATH) < 0) {
> + goto cleanup;
> + }
> +
> + embeddedparam.nbProps = 5;
> + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0)
> + goto cleanup;
> + (*tab_props).name = "Connection";
> + (*tab_props).val = switchPortPATH;
> + (*(tab_props+1)).name = "ElementName";
> + (*(tab_props+1)).val = "Network Adapter";
> + (*(tab_props+2)).name = "VirtualSystemIdentifiers";
> + (*(tab_props+2)).val = virtualSystemIdentifiers;
> + (*(tab_props+3)).name = "ResourceType";
> + (*(tab_props+3)).val = "10";
> + (*(tab_props+4)).name = "ResourceSubType";
> + (*(tab_props+4)).val = "Microsoft Synthetic Ethernet Port";
> + embeddedparam.instanceName = MSVM_SYNTHETICETHERNETPORTSETTINGDATA_CLASSNAME;
> + embeddedparam.prop_t = tab_props;
> +
> + /* Create invokeXmlParam tab */
> + VIR_FREE(params);
> + nb_params = 2;
> + if (VIR_ALLOC_N(params, nb_params) < 0)
> + goto cleanup;
> + (*params).name = "TargetSystem";
> + (*params).type = EPR_PARAM;
> + (*params).param = &eprparam2;
> + (*(params+1)).name = "ResourceSettingData";
> + (*(params+1)).type = EMBEDDED_PARAM;
> + (*(params+1)).param = &embeddedparam;
> +
The drum broke from the continual beating...
> + if (hypervInvokeMethod(priv, params, nb_params, "AddVirtualSystemResources",
> + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector2) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not attach the network"));
> + goto cleanup;
> + }
> +
> + result = 0;
> +
> + cleanup:
> + VIR_FREE(virtualSystemIdentifiers);
> + VIR_FREE(switchPortPATH);
> + VIR_FREE(tab_props);
> + VIR_FREE(params);
> + hypervFreeObject(priv, (hypervObject *)switchPort);
> + hypervFreeObject(priv, (hypervObject *)virtualSwitch);
> + virBufferFreeAndReset(&query);
> +
> + return result;
> +}
> +
So the above is NETWORK and below is DEVICE - separate patches.
> +
> +static int
> +hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml,
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
> + int result = -1;
> + hypervPrivate *priv = domain->conn->privateData;
> + virDomainDefPtr def = NULL;
> + virDomainDeviceDefPtr dev = NULL;
> + char *xmlDomain = NULL;
> +
> + /* Get domain definition */
> + if ((xmlDomain = hypervDomainGetXMLDesc(domain, 0)) == NULL) {
> + goto cleanup;
> + }
> + if ((def = virDomainDefParseString(xmlDomain, priv->caps, priv->xmlopt,
> + 1 << VIR_DOMAIN_VIRT_HYPERV | VIR_DOMAIN_XML_INACTIVE)) == NULL) {
> + goto cleanup;
> + }
> +
> + /* Get domain device definition */
> + if ((dev = virDomainDeviceDefParse(xml, def, priv->caps,
> + priv->xmlopt, VIR_DOMAIN_XML_INACTIVE)) == NULL) {
> + goto cleanup;
> + }
> +
> + switch (dev->type) {
> + /* Device = disk */
> + case VIR_DOMAIN_DEVICE_DISK:
> + if (hypervDomainAttachDisk(domain, dev->data.disk) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not attach disk"));
> + goto cleanup;
> + }
> + VIR_DEBUG("Disk attached");
> + break;
> +
> + /* Device = network */
> + case VIR_DOMAIN_DEVICE_NET:
> + if (hypervDomainAttachNetwork(domain, dev->data.net) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not attach network"));
> + goto cleanup;
> + }
> + VIR_DEBUG("Network attached");
> + break;
> +
> + /* Unsupported device type */
> + default:
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Device attachment of type %d is not implemented"), dev->type);
> + goto cleanup;
> + }
> +
> + result = 0;
> +
> + cleanup:
> + virDomainDefFree(def);
> + virDomainDeviceDefFree(dev);
> + VIR_FREE(xmlDomain);
> +
> + return result;
> +}
> +
> +
> +
> +static int
> +hypervDomainAttachDevice(virDomainPtr domain, const char *xml)
> +{
> + return hypervDomainAttachDeviceFlags(domain, xml, 0);
> +}
> +
Above is AttachDevice (without any DetachDevice, but let's not take that
fork in the road yet)... Below is DomainDefine which is a separate patch.
> +static virDomainPtr
> +hypervDomainDefineXML(virConnectPtr conn, const char *xml)
> +{
> + hypervPrivate *priv = conn->privateData;
> + virDomainDefPtr def = NULL;
> + virDomainPtr domain = NULL;
> + invokeXmlParam *params = NULL;
> + properties_t *tab_props = NULL;
> + embeddedParam embeddedparam;
> + int nb_params, i;
> + const char *selector = "CreationClassName=Msvm_VirtualSystemManagementService";
> + char uuid_string[VIR_UUID_STRING_BUFLEN];
> +
> + /* Parse XML domain description */
> + if ((def = virDomainDefParseString(xml, priv->caps, priv->xmlopt,
> + 1 << VIR_DOMAIN_VIRT_HYPERV | VIR_DOMAIN_XML_INACTIVE)) == NULL) {
> + goto cleanup;
> + }
> +
> + /* Create the domain if does not exist */
> + if (def->uuid == NULL || (domain = hypervDomainLookupByUUID(conn, def->uuid)) == NULL) {
Coverity notes "
(1) Event array_null: Comparing an array to null is not useful:
"def->uuid == NULL", since the test will always evaluate as true.
(2) Event remediation: Was "def->uuid" formerly declared as a pointer?
IOW: uuid is defined as "unsigned char uuid[VIR_UUID_BUFLEN];" so
def->uuid will always be true
> + /* Prepare EMBEDDED param */
> + /* Edit only VM name */
> + /* FIXME: cannot edit VM UUID */
> + embeddedparam.nbProps = 1;
> + if (VIR_ALLOC_N(tab_props, embeddedparam.nbProps) < 0)
> + goto cleanup;
> + (*tab_props).name = "ElementName";
> + (*tab_props).val = def->name;
> + embeddedparam.instanceName = "Msvm_VirtualSystemGlobalSettingData";
> + embeddedparam.prop_t = tab_props;
> +
> + /* Create invokeXmlParam */
> + nb_params = 1;
> + if (VIR_ALLOC_N(params, nb_params) < 0)
> + goto cleanup;
> + (*params).name = "SystemSettingData";
> + (*params).type = EMBEDDED_PARAM;
> + (*params).param = &embeddedparam;
> +
> + /* Create VM */
> + if (hypervInvokeMethod(priv, params, nb_params, "DefineVirtualSystem",
> + MSVM_VIRTUALSYSTEMMANAGEMENTSERVICE_RESOURCE_URI, selector) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not create new domain %s"), def->name);
> + goto cleanup;
> + }
> +
> + /* Get domain pointer */
> + domain = hypervDomainLookupByName(conn, def->name);
> +
> + VIR_DEBUG("Domain created: name=%s, uuid=%s",
> + domain->name, virUUIDFormat(domain->uuid, uuid_string));
> + }
> +
> + /* Set VM maximum memory */
> + if (def->mem.max_memory > 0) {
> + if (hypervDomainSetMaxMemory(domain, def->mem.max_memory) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not set VM maximum memory"));
> + }
> + }
> +
> + /* Set VM memory */
> + if (def->mem.cur_balloon > 0) {
> + if (hypervDomainSetMemory(domain, def->mem.cur_balloon) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not set VM memory"));
> + }
> + }
> +
> + /* Set VM vcpus */
> + /*if ((int)def->vcpus > 0) {*/
> + /*if (hypervDomainSetVcpus(domain, def->vcpus) < 0) {*/
> + /*virReportError(VIR_ERR_INTERNAL_ERROR,*/
> + /*_("Could not set VM vCPUs"));*/
> + /*}*/
> + /*}*/
Ugly...
> +
> + /* Attach networks */
> + for (i = 0; i < def->nnets; i++) {
> + if (hypervDomainAttachNetwork(domain, def->nets[i]) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not attach network"));
> + }
> + }
> +
> + /* Attach disks */
> + for (i = 0; i < def->ndisks; i++) {
> + if (hypervDomainAttachDisk(domain, def->disks[i]) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not attach disk"));
> + }
> + }
> +
Define generally means define the domain, save it somewhere, so that in
the future we can edit/start/undefine it.
This would mean some amount of persistence. Doesn't seem you're quite
there yet.
> + cleanup:
> + virDomainDefFree(def);
> + VIR_FREE(tab_props);
> + VIR_FREE(params);
> +
> + return domain;
> +}
> +
> +
Having Create/Define in the same is OK, but really should separate.
> +
> +static virDomainPtr
> +hypervDomainCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags)
> +{
> + virDomainPtr domain;
> +
> + virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL);
> +
> + /* Create the new domain */
> + domain = hypervDomainDefineXML(conn, xmlDesc);
> + if (domain == NULL)
> + return NULL;
Hmmm... Well Create is more for a transient domains - ones that will be
created and started, but not saved.
I think you may need to rethink how you're handling define/create
> +
> + /* Start the domain */
> + if (hypervInvokeMsvmComputerSystemRequestStateChange(domain, MSVM_COMPUTERSYSTEM_REQUESTEDSTATE_ENABLED) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not start the domain %s"), domain->name);
> + return domain;
> + }
> +
> + /* If the VIR_DOMAIN_START_PAUSED flag is set,
> + the guest domain will be started, but its CPUs will remain paused */
> + if (flags & VIR_DOMAIN_START_PAUSED) {
> + if (hypervDomainSuspend(domain) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not pause the domain %s"), domain->name);
> + }
> + }
> +
> + /* FIXME: process autodestroy flag */
> +
> + return domain;
> +}
> +
> +
> static virHypervisorDriver hypervHypervisorDriver = {
> .name = "Hyper-V",
> .connectOpen = hypervConnectOpen, /* 0.9.5 */
> @@ -2330,6 +3133,10 @@ static virHypervisorDriver hypervHypervisorDriver = {
> .domainSetVcpusFlags = hypervDomainSetVcpusFlags, /* 1.2.10 */
> .domainUndefine = hypervDomainUndefine, /* 1.2.10 */
> .domainUndefineFlags = hypervDomainUndefineFlags, /* 1.2.10 */
> + .domainAttachDevice = hypervDomainAttachDevice, /* 1.2.10 */
> + .domainAttachDeviceFlags = hypervDomainAttachDeviceFlags, /* 1.2.10 */
> + .domainDefineXML = hypervDomainDefineXML, /* 1.2.10 */
> + .domainCreateXML = hypervDomainCreateXML, /* 1.2.10 */
2.3.0 at the earliest and will need to be separate patches as indicated
above.
John
I've really run out of steam on this effort.
> };
>
> /* Retrieves host system UUID */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list