- Devel - libvirt List Archives

[libvirt] [PATCH] esx: Make storage pool lookup by name and UUID more robust
by Matthias Bolte 02 Aug '10

02 Aug '10

Don't rely on summary.url anymore, because its value is different between an esx:// and vpx:// connection. Use host.mountInfo.path instead. Don't fallback to lookup by UUID (actually lookup by absolute path) in esxVI_LookupDatastoreByName when lookup by name fails. Add a seperate function for this: esxVI_LookupDatastoreByAbsolutePath --- src/esx/esx_driver.c | 8 +- src/esx/esx_storage_driver.c | 107 ++++++++++++++------------ src/esx/esx_util.c | 22 +++--- src/esx/esx_util.h | 5 +- src/esx/esx_vi.c | 171 ++++++++++++++++++++-------------------- src/esx/esx_vi.h | 6 ++ src/esx/esx_vi_generator.input | 13 +++ src/esx/esx_vi_generator.py | 3 +- src/esx/esx_vmx.c | 65 +++++++-------- src/esx/esx_vmx.h | 3 +- tests/esxutilstest.c | 14 ++-- 11 files changed, 221 insertions(+), 196 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index fd87078..d824371 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2193,8 +2193,8 @@ esxDomainDumpXML(virDomainPtr domain, int flags) goto cleanup; } - if (esxUtil_ParseDatastoreRelatedPath(vmPathName, &datastoreName, - &directoryName, &fileName) < 0) { + if (esxUtil_ParseDatastorePath(vmPathName, &datastoreName, &directoryName, + &fileName) < 0) { goto cleanup; } @@ -2572,8 +2572,8 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) goto cleanup; } - if (esxUtil_ParseDatastoreRelatedPath(disk->src, &datastoreName, - &directoryName, &fileName) < 0) { + if (esxUtil_ParseDatastorePath(disk->src, &datastoreName, &directoryName, + &fileName) < 0) { goto cleanup; } diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index b0ccc32..e0680a1 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -196,61 +196,64 @@ esxStoragePoolLookupByName(virConnectPtr conn, const char *name) esxPrivate *priv = conn->storagePrivateData; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastore = NULL; - esxVI_Boolean accessible = esxVI_Boolean_Undefined; - char *summaryUrl = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_DatastoreHostMount *datastoreHostMountList = NULL; + esxVI_DatastoreHostMount *datastoreHostMount = NULL; char *suffix = NULL; int suffixLength; char uuid_string[VIR_UUID_STRING_BUFLEN] = "00000000-00000000-0000-000000000000"; unsigned char uuid[VIR_UUID_BUFLEN]; - char *realName = NULL; virStoragePoolPtr pool = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; } - if (esxVI_String_AppendValueListToList(&propertyNameList, - "summary.accessible\0" - "summary.name\0" - "summary.url\0") < 0 || + if (esxVI_String_AppendValueToList(&propertyNameList, "host") < 0 || esxVI_LookupDatastoreByName(priv->primary, name, propertyNameList, &datastore, - esxVI_Occurrence_RequiredItem) < 0 || - esxVI_GetBoolean(datastore, "summary.accessible", - &accessible, esxVI_Occurrence_RequiredItem) < 0) { + esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } /* - * Datastores don't have a UUID. We can use the 'summary.url' property as - * source for a "UUID" on ESX, because the property value has this format: + * Datastores don't have a UUID. We can use the 'host.mountInfo.path' + * property as source for a "UUID" on ESX, because the property value has + * this format: * - * summary.url = /vmfs/volumes/4b0beca7-7fd401f3-1d7f-000ae484a6a3 - * summary.url = /vmfs/volumes/b24b7a78-9d82b4f5 (short format) + * host.mountInfo.path = /vmfs/volumes/4b0beca7-7fd401f3-1d7f-000ae484a6a3 + * host.mountInfo.path = /vmfs/volumes/b24b7a78-9d82b4f5 (short format) * - * The 'summary.url' property comes in two forms, with a complete "UUID" - * and a short "UUID". + * The 'host.mountInfo.path' property comes in two forms, with a complete + * "UUID" and a short "UUID". * * But this trailing "UUID" is not guaranteed to be there. On the other * hand we already rely on another implementation detail of the ESX server: * The object name of virtual machine contains an integer, we use that as * domain ID. - * - * The 'summary.url' property of an inaccessible datastore is invalid. */ - /* FIXME: Need to handle this for a vpx:// connection */ - if (accessible == esxVI_Boolean_True && priv->host != NULL && - priv->host->productVersion & esxVI_ProductVersion_ESX) { - if (esxVI_GetStringValue(datastore, "summary.url", &summaryUrl, - esxVI_Occurrence_RequiredItem) < 0) { - goto cleanup; + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "host")) { + if (esxVI_DatastoreHostMount_CastListFromAnyType + (dynamicProperty->val, &datastoreHostMountList) < 0) { + goto cleanup; + } + + break; } + } - if ((suffix = STRSKIP(summaryUrl, "/vmfs/volumes/")) == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Datastore URL '%s' has unexpected prefix, " - "expecting '/vmfs/volumes/' prefix"), summaryUrl); - goto cleanup; + for (datastoreHostMount = datastoreHostMountList; datastoreHostMount != NULL; + datastoreHostMount = datastoreHostMount->_next) { + if (STRNEQ(priv->primary->hostSystem->_reference->value, + datastoreHostMount->key->value)) { + continue; + } + + if ((suffix = STRSKIP(datastoreHostMount->mountInfo->path, + "/vmfs/volumes/")) == NULL) { + break; } suffixLength = strlen(suffix); @@ -266,8 +269,8 @@ esxStoragePoolLookupByName(virConnectPtr conn, const char *name) */ memcpy(uuid_string, suffix, suffixLength); } else { - VIR_WARN("Datastore URL suffix '%s' has unexpected format, " - "cannot deduce a UUID from it", suffix); + VIR_WARN("Datastore host mount path suffix '%s' has unexpected " + "format, cannot deduce a UUID from it", suffix); } } @@ -278,16 +281,12 @@ esxStoragePoolLookupByName(virConnectPtr conn, const char *name) goto cleanup; } - if (esxVI_GetStringValue(datastore, "summary.name", &realName, - esxVI_Occurrence_RequiredItem) < 0) { - goto cleanup; - } - - pool = virGetStoragePool(conn, realName, uuid); + pool = virGetStoragePool(conn, name, uuid); cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastore); + esxVI_DatastoreHostMount_Free(&datastoreHostMountList); return pool; } @@ -301,6 +300,7 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastore = NULL; char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; + char *absolutePath = NULL; char *name = NULL; virStoragePoolPtr pool = NULL; @@ -309,7 +309,7 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) } /* - * Convert from UUID to datastore URL form by stripping the second '-': + * Convert UUID to 'host.mountInfo.path' form by stripping the second '-': * * <---- 14 ----><-------- 22 --------> <---- 13 ---><-------- 22 --------> * 4b0beca7-7fd4-01f3-1d7f-000ae484a6a3 -> 4b0beca7-7fd401f3-1d7f-000ae484a6a3 @@ -317,14 +317,15 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virUUIDFormat(uuid, uuid_string); memmove(uuid_string + 13, uuid_string + 14, 22 + 1); - /* - * Use esxVI_LookupDatastoreByName because it also does try to match "UUID" - * part of the 'summary.url' property if there is no name match. - */ + if (virAsprintf(&absolutePath, "/vmfs/volumes/%s", uuid_string) < 0) { + virReportOOMError(); + goto cleanup; + } + if (esxVI_String_AppendValueToList(&propertyNameList, "summary.name") < 0 || - esxVI_LookupDatastoreByName(priv->primary, uuid_string, - propertyNameList, &datastore, - esxVI_Occurrence_OptionalItem) < 0) { + esxVI_LookupDatastoreByAbsolutePath(priv->primary, absolutePath, + propertyNameList, &datastore, + esxVI_Occurrence_OptionalItem) < 0) { goto cleanup; } @@ -338,9 +339,16 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (datastore == NULL && STREQ(uuid_string + 17, "-0000-000000000000")) { uuid_string[17] = '\0'; - if (esxVI_LookupDatastoreByName(priv->primary, uuid_string, - propertyNameList, &datastore, - esxVI_Occurrence_RequiredItem) < 0) { + VIR_FREE(absolutePath); + + if (virAsprintf(&absolutePath, "/vmfs/volumes/%s", uuid_string) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (esxVI_LookupDatastoreByAbsolutePath(priv->primary, absolutePath, + propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } } @@ -348,7 +356,7 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (datastore == NULL) { virUUIDFormat(uuid, uuid_string); - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + ESX_VI_ERROR(VIR_ERR_NO_STORAGE_POOL, _("Could not find datastore with UUID '%s'"), uuid_string); @@ -363,6 +371,7 @@ esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) pool = virGetStoragePool(conn, name, uuid); cleanup: + VIR_FREE(absolutePath); esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastore); diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 75a9aaf..7c7c841 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -274,12 +274,11 @@ esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id) int -esxUtil_ParseDatastoreRelatedPath(const char *datastoreRelatedPath, - char **datastoreName, - char **directoryName, char **fileName) +esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, + char **directoryName, char **fileName) { int result = -1; - char *copyOfDatastoreRelatedPath = NULL; + char *copyOfDatastorePath = NULL; char *tmp = NULL; char *saveptr = NULL; char *preliminaryDatastoreName = NULL; @@ -293,18 +292,17 @@ esxUtil_ParseDatastoreRelatedPath(const char *datastoreRelatedPath, return -1; } - if (esxVI_String_DeepCopyValue(&copyOfDatastoreRelatedPath, - datastoreRelatedPath) < 0) { + if (esxVI_String_DeepCopyValue(&copyOfDatastorePath, datastorePath) < 0) { goto cleanup; } /* Expected format: '[<datastore>] <path>' */ - if ((tmp = STRSKIP(copyOfDatastoreRelatedPath, "[")) == NULL || + if ((tmp = STRSKIP(copyOfDatastorePath, "[")) == NULL || (preliminaryDatastoreName = strtok_r(tmp, "]", &saveptr)) == NULL || (directoryAndFileName = strtok_r(NULL, "", &saveptr)) == NULL) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Datastore related path '%s' doesn't have expected format " - "'[<datastore>] <path>'"), datastoreRelatedPath); + _("Datastore path '%s' doesn't have expected format " + "'[<datastore>] <path>'"), datastorePath); goto cleanup; } @@ -323,8 +321,8 @@ esxUtil_ParseDatastoreRelatedPath(const char *datastoreRelatedPath, if (*separator == '\0') { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Datastore related path '%s' doesn't reference a file"), - datastoreRelatedPath); + _("Datastore path '%s' doesn't reference a file"), + datastorePath); goto cleanup; } @@ -348,7 +346,7 @@ esxUtil_ParseDatastoreRelatedPath(const char *datastoreRelatedPath, VIR_FREE(*fileName); } - VIR_FREE(copyOfDatastoreRelatedPath); + VIR_FREE(copyOfDatastorePath); return result; } diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index 99ce81d..5799730 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -51,9 +51,8 @@ void esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri); int esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id); -int esxUtil_ParseDatastoreRelatedPath(const char *datastoreRelatedPath, - char **datastoreName, - char **directoryName, char **fileName); +int esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, + char **directoryName, char **fileName); int esxUtil_ResolveHostname(const char *hostname, char *ipAddress, size_t ipAddress_length); diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 9f2ac36..f421502 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2422,10 +2422,7 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, esxVI_String *completePropertyNameList = NULL; esxVI_ObjectContent *datastoreList = NULL; esxVI_ObjectContent *candidate = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; - esxVI_Boolean accessible = esxVI_Boolean_Undefined; - size_t offset = 14; /* = strlen("/vmfs/volumes/") */ - int numInaccessibleDatastores = 0; + char *name_candidate; if (datastore == NULL || *datastore != NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); @@ -2435,118 +2432,123 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, /* Get all datastores */ if (esxVI_String_DeepCopyList(&completePropertyNameList, propertyNameList) < 0 || - esxVI_String_AppendValueListToList(&completePropertyNameList, - "summary.accessible\0" - "summary.name\0" - "summary.url\0") < 0 || + esxVI_String_AppendValueToList(&completePropertyNameList, + "summary.name") < 0 || esxVI_LookupDatastoreList(ctx, completePropertyNameList, &datastoreList) < 0) { goto cleanup; } - if (datastoreList == NULL) { - if (occurrence == esxVI_Occurrence_OptionalItem) { - goto success; - } else { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("No datastores available")); + /* Search for a matching datastore */ + for (candidate = datastoreList; candidate != NULL; + candidate = candidate->_next) { + name_candidate = NULL; + + if (esxVI_GetStringValue(candidate, "summary.name", &name_candidate, + esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } + + if (STREQ(name_candidate, name)) { + if (esxVI_ObjectContent_DeepCopy(datastore, candidate) < 0) { + goto cleanup; + } + + // Found datastore with matching name + goto success; + } + } + + if (*datastore == NULL && occurrence != esxVI_Occurrence_OptionalItem) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not find datastore with name '%s'"), name); + goto cleanup; + } + + success: + result = 0; + + cleanup: + esxVI_String_Free(&completePropertyNameList); + esxVI_ObjectContent_Free(&datastoreList); + + return result; +} + + +int +esxVI_LookupDatastoreByAbsolutePath(esxVI_Context *ctx, + const char *absolutePath, + esxVI_String *propertyNameList, + esxVI_ObjectContent **datastore, + esxVI_Occurrence occurrence) +{ + int result = -1; + esxVI_String *completePropertyNameList = NULL; + esxVI_ObjectContent *datastoreList = NULL; + esxVI_ObjectContent *candidate = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_DatastoreHostMount *datastoreHostMountList = NULL; + esxVI_DatastoreHostMount *datastoreHostMount = NULL; + + if (datastore == NULL || *datastore != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + /* Get all datastores */ + if (esxVI_String_DeepCopyList(&completePropertyNameList, + propertyNameList) < 0 || + esxVI_String_AppendValueToList(&completePropertyNameList, "host") < 0 || + esxVI_LookupDatastoreList(ctx, completePropertyNameList, + &datastoreList) < 0) { + goto cleanup; } /* Search for a matching datastore */ for (candidate = datastoreList; candidate != NULL; candidate = candidate->_next) { - accessible = esxVI_Boolean_Undefined; + esxVI_DatastoreHostMount_Free(&datastoreHostMountList); for (dynamicProperty = candidate->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "summary.accessible")) { - if (esxVI_AnyType_ExpectType(dynamicProperty->val, - esxVI_Type_Boolean) < 0) { + if (STREQ(dynamicProperty->name, "host")) { + if (esxVI_DatastoreHostMount_CastListFromAnyType + (dynamicProperty->val, &datastoreHostMountList) < 0) { goto cleanup; } - accessible = dynamicProperty->val->boolean; break; } } - if (accessible == esxVI_Boolean_Undefined) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Got incomplete response while querying for the " - "datastore 'summary.accessible' property")); - goto cleanup; - } - - if (accessible == esxVI_Boolean_False) { - ++numInaccessibleDatastores; + if (datastoreHostMountList == NULL) { + continue; } - for (dynamicProperty = candidate->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "summary.name")) { - if (esxVI_AnyType_ExpectType(dynamicProperty->val, - esxVI_Type_String) < 0) { - goto cleanup; - } - - if (STREQ(dynamicProperty->val->string, name)) { - if (esxVI_ObjectContent_DeepCopy(datastore, - candidate) < 0) { - goto cleanup; - } - - /* Found datastore with matching name */ - goto success; - } - } else if (STREQ(dynamicProperty->name, "summary.url") && - ctx->productVersion & esxVI_ProductVersion_ESX) { - if (accessible == esxVI_Boolean_False) { - /* - * The 'summary.url' property of an inaccessible datastore - * is invalid and cannot be used to identify the datastore. - */ - continue; - } + for (datastoreHostMount = datastoreHostMountList; + datastoreHostMount != NULL; + datastoreHostMount = datastoreHostMount->_next) { + if (STRNEQ(ctx->hostSystem->_reference->value, + datastoreHostMount->key->value)) { + continue; + } - if (esxVI_AnyType_ExpectType(dynamicProperty->val, - esxVI_Type_String) < 0) { + if (STRPREFIX(absolutePath, datastoreHostMount->mountInfo->path)) { + if (esxVI_ObjectContent_DeepCopy(datastore, candidate) < 0) { goto cleanup; } - if (! STRPREFIX(dynamicProperty->val->string, - "/vmfs/volumes/")) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Datastore URL '%s' has unexpected prefix, " - "expecting '/vmfs/volumes/' prefix"), - dynamicProperty->val->string); - goto cleanup; - } - - if (STREQ(dynamicProperty->val->string + offset, name)) { - if (esxVI_ObjectContent_DeepCopy(datastore, - candidate) < 0) { - goto cleanup; - } - - /* Found datastore with matching URL suffix */ - goto success; - } + /* Found datastore with matching mount path */ + goto success; } } } - if (occurrence != esxVI_Occurrence_OptionalItem) { - if (numInaccessibleDatastores > 0) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not find datastore '%s', maybe it's " - "inaccessible"), name); - } else { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not find datastore '%s'"), name); - } - + if (*datastore == NULL && occurrence != esxVI_Occurrence_OptionalItem) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not find datastore containing absolute path '%s'"), + absolutePath); goto cleanup; } @@ -2556,6 +2558,7 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, cleanup: esxVI_String_Free(&completePropertyNameList); esxVI_ObjectContent_Free(&datastoreList); + esxVI_DatastoreHostMount_Free(&datastoreHostMountList); return result; } diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index a23c56d..fdd15f1 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -364,6 +364,12 @@ int esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, esxVI_ObjectContent **datastore, esxVI_Occurrence occurrence); +int esxVI_LookupDatastoreByAbsolutePath(esxVI_Context *ctx, + const char *absolutePath, + esxVI_String *propertyNameList, + esxVI_ObjectContent **datastore, + esxVI_Occurrence occurrence); + int esxVI_LookupTaskInfoByTask(esxVI_Context *ctx, esxVI_ManagedObjectReference *task, esxVI_TaskInfo **taskInfo); diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index b4b33f6..ab50ea5 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -146,6 +146,12 @@ object ChoiceOption extends OptionType end +object DatastoreHostMount + ManagedObjectReference key r + HostMountInfo mountInfo r +end + + object DatastoreInfo String name r String url r @@ -251,6 +257,13 @@ object HostFileSystemVolume end +object HostMountInfo + String path o + String accessMode r + Boolean accessible o +end + + object HostNasVolume extends HostFileSystemVolume String remoteHost r String remotePath r diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 44c3493..e3c3d14 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1127,7 +1127,8 @@ additional_enum_features = { "ManagedEntityStatus" : Enum.FEATURE__ANY_TYPE "VirtualMachinePowerState" : Enum.FEATURE__ANY_TYPE } -additional_object_features = { "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, +additional_object_features = { "DatastoreHostMount" : Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, + "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, "Event" : Object.FEATURE__LIST, "FileInfo" : Object.FEATURE__DYNAMIC_CAST, "FileQuery" : Object.FEATURE__DYNAMIC_CAST, diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 807c6db..1a5e8d3 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -749,8 +749,8 @@ esxVMX_AutodetectSCSIControllerModel(esxVI_Context *ctx, return 0; } - if (esxUtil_ParseDatastoreRelatedPath(def->src, &datastoreName, - &directoryName, &fileName) < 0) { + if (esxUtil_ParseDatastorePath(def->src, &datastoreName, &directoryName, + &fileName) < 0) { goto cleanup; } @@ -986,19 +986,19 @@ esxVMX_GatherSCSIControllers(esxVI_Context *ctx, virDomainDefPtr def, char * -esxVMX_AbsolutePathToDatastoreRelatedPath(esxVI_Context *ctx, - const char *absolutePath) +esxVMX_AbsolutePathToDatastorePath(esxVI_Context *ctx, const char *absolutePath) { bool success = false; char *copyOfAbsolutePath = NULL; char *tmp = NULL; char *saveptr = NULL; - char *datastoreRelatedPath = NULL; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + + char *datastorePath = NULL; char *preliminaryDatastoreName = NULL; char *directoryAndFileName = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; - esxVI_ObjectContent *datastore = NULL; - const char *datastoreName = NULL; + char *datastoreName = NULL; if (esxVI_String_DeepCopyValue(&copyOfAbsolutePath, absolutePath) < 0) { return NULL; @@ -1015,30 +1015,26 @@ esxVMX_AbsolutePathToDatastoreRelatedPath(esxVI_Context *ctx, } if (ctx != NULL) { - if (esxVI_LookupDatastoreByName(ctx, preliminaryDatastoreName, - NULL, &datastore, - esxVI_Occurrence_OptionalItem) < 0) { + if (esxVI_String_AppendValueToList(&propertyNameList, + "summary.name") < 0 || + esxVI_LookupDatastoreByAbsolutePath(ctx, absolutePath, + propertyNameList, &datastore, + esxVI_Occurrence_OptionalItem) < 0) { goto cleanup; } - if (datastore != NULL) { - for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "summary.accessible")) { - /* Ignore it */ - } else if (STREQ(dynamicProperty->name, "summary.name")) { - if (esxVI_AnyType_ExpectType(dynamicProperty->val, - esxVI_Type_String) < 0) { - goto cleanup; - } + if (datastore == NULL) { + if (esxVI_LookupDatastoreByName(ctx, preliminaryDatastoreName, + propertyNameList, &datastore, + esxVI_Occurrence_OptionalItem) < 0) { + goto cleanup; + } + } - datastoreName = dynamicProperty->val->string; - break; - } else if (STREQ(dynamicProperty->name, "summary.url")) { - /* Ignore it */ - } else { - VIR_WARN("Unexpected '%s' property", dynamicProperty->name); - } + if (datastore != NULL) { + if (esxVI_GetStringValue(datastore, "summary.name", &datastoreName, + esxVI_Occurrence_RequiredItem)) { + goto cleanup; } } @@ -1053,7 +1049,7 @@ esxVMX_AbsolutePathToDatastoreRelatedPath(esxVI_Context *ctx, datastoreName = preliminaryDatastoreName; } - if (virAsprintf(&datastoreRelatedPath, "[%s] %s", datastoreName, + if (virAsprintf(&datastorePath, "[%s] %s", datastoreName, directoryAndFileName) < 0) { virReportOOMError(); goto cleanup; @@ -1065,13 +1061,14 @@ esxVMX_AbsolutePathToDatastoreRelatedPath(esxVI_Context *ctx, cleanup: if (! success) { - VIR_FREE(datastoreRelatedPath); + VIR_FREE(datastorePath); } VIR_FREE(copyOfAbsolutePath); + esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastore); - return datastoreRelatedPath; + return datastorePath; } @@ -1088,7 +1085,7 @@ esxVMX_ParseFileName(esxVI_Context *ctx, const char *fileName, if (STRPREFIX(fileName, "/vmfs/volumes/")) { /* Found absolute path referencing a file inside a datastore */ - return esxVMX_AbsolutePathToDatastoreRelatedPath(ctx, fileName); + return esxVMX_AbsolutePathToDatastorePath(ctx, fileName); } else if (STRPREFIX(fileName, "/")) { /* Found absolute path referencing a file outside a datastore */ src = strdup(fileName); @@ -2625,8 +2622,8 @@ esxVMX_FormatFileName(esxVI_Context *ctx ATTRIBUTE_UNUSED, const char *src) if (STRPREFIX(src, "[")) { /* Found potential datastore related path */ - if (esxUtil_ParseDatastoreRelatedPath(src, &datastoreName, - &directoryName, &fileName) < 0) { + if (esxUtil_ParseDatastorePath(src, &datastoreName, &directoryName, + &fileName) < 0) { goto cleanup; } diff --git a/src/esx/esx_vmx.h b/src/esx/esx_vmx.h index 3ccae7a..bda98f0 100644 --- a/src/esx/esx_vmx.h +++ b/src/esx/esx_vmx.h @@ -56,8 +56,7 @@ esxVMX_GatherSCSIControllers(esxVI_Context *ctx, virDomainDefPtr def, int virtualDev[4], bool present[4]); char * -esxVMX_AbsolutePathToDatastoreRelatedPath(esxVI_Context *ctx, - const char *absolutePath); +esxVMX_AbsolutePathToDatastorePath(esxVI_Context *ctx, const char *absolutePath); diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c index 2a13282..09470ed 100644 --- a/tests/esxutilstest.c +++ b/tests/esxutilstest.c @@ -95,7 +95,7 @@ testDiskNameToIndex(const void *data ATTRIBUTE_UNUSED) struct testPath { - const char *datastoreRelatedPath; + const char *datastorePath; int result; const char *datastoreName; const char *directoryName; @@ -111,7 +111,7 @@ static struct testPath paths[] = { }; static int -testParseDatastoreRelatedPath(const void *data ATTRIBUTE_UNUSED) +testParseDatastorePath(const void *data ATTRIBUTE_UNUSED) { int i, result = 0; char *datastoreName = NULL; @@ -123,9 +123,9 @@ testParseDatastoreRelatedPath(const void *data ATTRIBUTE_UNUSED) VIR_FREE(directoryName); VIR_FREE(fileName); - if (esxUtil_ParseDatastoreRelatedPath(paths[i].datastoreRelatedPath, - &datastoreName, &directoryName, - &fileName) != paths[i].result) { + if (esxUtil_ParseDatastorePath(paths[i].datastorePath, + &datastoreName, &directoryName, + &fileName) != paths[i].result) { goto failure; } @@ -242,7 +242,7 @@ mymain(int argc, char **argv) virSetErrorFunc(NULL, testQuietError); -# define DO_TEST(_name) \ +# define DO_TEST(_name) \ do { \ if (virtTestRun("VMware "#_name, 1, test##_name, \ NULL) < 0) { \ @@ -252,7 +252,7 @@ mymain(int argc, char **argv) DO_TEST(IndexToDiskName); DO_TEST(DiskNameToIndex); - DO_TEST(ParseDatastoreRelatedPath); + DO_TEST(ParseDatastorePath); DO_TEST(ConvertDateTimeToCalendarTime); return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Map some managed object types
by Matthias Bolte 02 Aug '10

02 Aug '10

Datacenter, ComputeResource and HostSystem will be used for simplified handling and caching. --- src/esx/esx_vi_types.c | 283 ++++++++++++++++++++++++++++++++++++++++++++++++ src/esx/esx_vi_types.h | 96 ++++++++++++++++ 2 files changed, 379 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 5cf30b1..bb791af 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -704,6 +704,15 @@ esxVI_Type_ToString(esxVI_Type type) case esxVI_Type_ManagedObjectReference: return "ManagedObjectReference"; + case esxVI_Type_Datacenter: + return "Datacenter"; + + case esxVI_Type_ComputeResource: + return "ComputeResource"; + + case esxVI_Type_HostSystem: + return "HostSystem"; + #include "esx_vi_types.generated.typetostring" case esxVI_Type_Other: @@ -734,8 +743,15 @@ esxVI_Type_FromString(const char *type) return esxVI_Type_Fault; } else if (STREQ(type, "ManagedObjectReference")) { return esxVI_Type_ManagedObjectReference; + } else if (STREQ(type, "Datacenter")) { + return esxVI_Type_Datacenter; + } else if (STREQ(type, "ComputeResource")) { + return esxVI_Type_ComputeResource; + } else if (STREQ(type, "HostSystem")) { + return esxVI_Type_HostSystem; } + #include "esx_vi_types.generated.typefromstring" else { @@ -1532,6 +1548,273 @@ esxVI_ManagedObjectReference_Deserialize /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Managed Object: Datacenter + * extends ManagedEntity + */ + +/* esxVI_Datacenter_Alloc */ +ESX_VI__TEMPLATE__ALLOC(Datacenter) + +/* esxVI_Datacenter_Free */ +ESX_VI__TEMPLATE__FREE(Datacenter, +{ + esxVI_Datacenter_Free(&item->_next); + esxVI_ManagedObjectReference_Free(&item->_reference); + + /* ManagedEntity */ + VIR_FREE(item->name); + + /* Datacenter */ + esxVI_ManagedObjectReference_Free(&item->hostFolder); + esxVI_ManagedObjectReference_Free(&item->vmFolder); +}) + +/* esxVI_Datacenter_Validate */ +ESX_VI__TEMPLATE__VALIDATE(Datacenter, +{ + /* ManagedEntity */ + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(name); + + /* Datacenter */ + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(hostFolder); + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(vmFolder); +}) + +int +esxVI_Datacenter_CastFromObjectContent(esxVI_ObjectContent *objectContent, + esxVI_Datacenter **datacenter) +{ + esxVI_DynamicProperty *dynamicProperty = NULL; + + if (objectContent == NULL || datacenter == NULL || *datacenter != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (esxVI_Datacenter_Alloc(datacenter) < 0) { + return -1; + } + + if (esxVI_ManagedObjectReference_DeepCopy(&(*datacenter)->_reference, + objectContent->obj) < 0) { + goto failure; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "name")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + goto failure; + } + + (*datacenter)->name = strdup(dynamicProperty->val->string); + + if ((*datacenter)->name == NULL) { + virReportOOMError(); + goto failure; + } + } else if (STREQ(dynamicProperty->name, "hostFolder")) { + if (esxVI_ManagedObjectReference_CastFromAnyType + (dynamicProperty->val, &(*datacenter)->hostFolder) < 0) { + goto failure; + } + } else if (STREQ(dynamicProperty->name, "vmFolder")) { + if (esxVI_ManagedObjectReference_CastFromAnyType + (dynamicProperty->val, &(*datacenter)->vmFolder) < 0) { + goto failure; + } + } + } + + if (esxVI_Datacenter_Validate(*datacenter) < 0) { + goto failure; + } + + return 0; + + failure: + esxVI_Datacenter_Free(datacenter); + + return -1; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Managed Object: ComputeResource + * extends ManagedEntity + */ + +/* esxVI_ComputeResource_Alloc */ +ESX_VI__TEMPLATE__ALLOC(ComputeResource) + +/* esxVI_ComputeResource_Free */ +ESX_VI__TEMPLATE__FREE(ComputeResource, +{ + esxVI_ComputeResource_Free(&item->_next); + esxVI_ManagedObjectReference_Free(&item->_reference); + + /* ManagedEntity */ + VIR_FREE(item->name); + + /* ComputeResource */ + esxVI_ManagedObjectReference_Free(&item->host); + esxVI_ManagedObjectReference_Free(&item->resourcePool); +}) + +/* esxVI_ComputeResource_Validate */ +ESX_VI__TEMPLATE__VALIDATE(ComputeResource, +{ + /* ManagedEntity */ + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(name); + + /* ComputeResource */ +}) + +int +esxVI_ComputeResource_CastFromObjectContent + (esxVI_ObjectContent *objectContent, esxVI_ComputeResource **computeResource) +{ + esxVI_DynamicProperty *dynamicProperty = NULL; + + if (objectContent == NULL || computeResource == NULL || + *computeResource != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (esxVI_ComputeResource_Alloc(computeResource) < 0) { + return -1; + } + + if (esxVI_ManagedObjectReference_DeepCopy(&(*computeResource)->_reference, + objectContent->obj) < 0) { + goto failure; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "name")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + goto failure; + } + + (*computeResource)->name = strdup(dynamicProperty->val->string); + + if ((*computeResource)->name == NULL) { + virReportOOMError(); + goto failure; + } + } else if (STREQ(dynamicProperty->name, "host")) { + if (esxVI_ManagedObjectReference_CastListFromAnyType + (dynamicProperty->val, &(*computeResource)->host) < 0) { + goto failure; + } + } else if (STREQ(dynamicProperty->name, "resourcePool")) { + if (esxVI_ManagedObjectReference_CastFromAnyType + (dynamicProperty->val, &(*computeResource)->resourcePool) < 0) { + goto failure; + } + } + } + + if (esxVI_ComputeResource_Validate(*computeResource) < 0) { + goto failure; + } + + return 0; + + failure: + esxVI_ComputeResource_Free(computeResource); + + return -1; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Managed Object: HostSystem + * extends ManagedEntity + */ + +/* esxVI_HostSystem_Alloc */ +ESX_VI__TEMPLATE__ALLOC(HostSystem) + +/* esxVI_HostSystem_Free */ +ESX_VI__TEMPLATE__FREE(HostSystem, +{ + esxVI_HostSystem_Free(&item->_next); + esxVI_ManagedObjectReference_Free(&item->_reference); + + /* ManagedEntity */ + VIR_FREE(item->name); + + /* HostSystem */ +}) + +/* esxVI_HostSystem_Validate */ +ESX_VI__TEMPLATE__VALIDATE(HostSystem, +{ + /* ManagedEntity */ + ESX_VI__TEMPLATE__PROPERTY__REQUIRE(name); + + /* HostSystem */ +}) + +int +esxVI_HostSystem_CastFromObjectContent(esxVI_ObjectContent *objectContent, + esxVI_HostSystem **hostSystem) +{ + esxVI_DynamicProperty *dynamicProperty = NULL; + + if (objectContent == NULL || hostSystem == NULL || *hostSystem != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (esxVI_HostSystem_Alloc(hostSystem) < 0) { + return -1; + } + + if (esxVI_ManagedObjectReference_DeepCopy(&(*hostSystem)->_reference, + objectContent->obj) < 0) { + goto failure; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "name")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + goto failure; + } + + (*hostSystem)->name = strdup(dynamicProperty->val->string); + + if ((*hostSystem)->name == NULL) { + virReportOOMError(); + goto failure; + } + } + } + + if (esxVI_HostSystem_Validate(*hostSystem) < 0) { + goto failure; + } + + return 0; + + failure: + esxVI_HostSystem_Free(hostSystem); + + return -1; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Enum: VirtualMachinePowerState (Additions) */ diff --git a/src/esx/esx_vi_types.h b/src/esx/esx_vi_types.h index 4bedca9..f659361 100644 --- a/src/esx/esx_vi_types.h +++ b/src/esx/esx_vi_types.h @@ -26,6 +26,7 @@ typedef enum _esxVI_Type esxVI_Type; typedef struct _esxVI_Object esxVI_Object; +typedef struct _esxVI_ManagedObject esxVI_ManagedObject; @@ -48,6 +49,9 @@ typedef struct _esxVI_DateTime esxVI_DateTime; typedef struct _esxVI_Fault esxVI_Fault; typedef struct _esxVI_ManagedObjectReference esxVI_ManagedObjectReference; +typedef struct _esxVI_Datacenter esxVI_Datacenter; +typedef struct _esxVI_ComputeResource esxVI_ComputeResource; +typedef struct _esxVI_HostSystem esxVI_HostSystem; # include "esx_vi_types.generated.typedef" @@ -68,6 +72,9 @@ enum _esxVI_Type { esxVI_Type_DateTime, esxVI_Type_Fault, esxVI_Type_ManagedObjectReference, + esxVI_Type_Datacenter, + esxVI_Type_ComputeResource, + esxVI_Type_HostSystem, # include "esx_vi_types.generated.typeenum" @@ -91,6 +98,18 @@ struct _esxVI_Object { /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * ManagedObject extends Object + */ + +struct _esxVI_ManagedObject { + esxVI_ManagedObject *_next; /* optional */ + esxVI_Type _type; /* required */ + esxVI_ManagedObjectReference *_reference; /* required */ +}; + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * XSD: Boolean */ @@ -301,6 +320,83 @@ int esxVI_ManagedObjectReference_Deserialize /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Managed Object: Datacenter + * extends ManagedEntity + */ + +struct _esxVI_Datacenter { + esxVI_Datacenter *_next; /* optional */ + esxVI_Type _type; /* required */ + esxVI_ManagedObjectReference *_reference; /* required */ + + /* ManagedEntity */ + char *name; /* required */ + + /* Datacenter */ + esxVI_ManagedObjectReference *hostFolder; /* required */ + esxVI_ManagedObjectReference *vmFolder; /* required */ +}; + +int esxVI_Datacenter_Alloc(esxVI_Datacenter **datacenter); +void esxVI_Datacenter_Free(esxVI_Datacenter **datacenter); +int esxVI_Datacenter_Validate(esxVI_Datacenter *datacenter); +int esxVI_Datacenter_CastFromObjectContent(esxVI_ObjectContent *objectContent, + esxVI_Datacenter **datacenter); + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Managed Object: ComputeResource + * extends ManagedEntity + */ + +struct _esxVI_ComputeResource { + esxVI_ComputeResource *_next; /* optional */ + esxVI_Type _type; /* required */ + esxVI_ManagedObjectReference *_reference; /* required */ + + /* ManagedEntity */ + char *name; /* required */ + + /* ComputeResource */ + esxVI_ManagedObjectReference *host; /* optional, list */ + esxVI_ManagedObjectReference *resourcePool; /* optional */ +}; + +int esxVI_ComputeResource_Alloc(esxVI_ComputeResource **computeResource); +void esxVI_ComputeResource_Free(esxVI_ComputeResource **computeResource); +int esxVI_ComputeResource_Validate(esxVI_ComputeResource *computeResource); +int esxVI_ComputeResource_CastFromObjectContent + (esxVI_ObjectContent *objectContent, + esxVI_ComputeResource **computeResource); + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Managed Object: HostSystem + * extends ManagedEntity + */ + +struct _esxVI_HostSystem { + esxVI_HostSystem *_next; /* optional */ + esxVI_Type _type; /* required */ + esxVI_ManagedObjectReference *_reference; /* required */ + + /* ManagedEntity */ + char *name; /* required */ + + /* HostSystem */ +}; + +int esxVI_HostSystem_Alloc(esxVI_HostSystem **hostSystem); +void esxVI_HostSystem_Free(esxVI_HostSystem **hostSystem); +int esxVI_HostSystem_Validate(esxVI_HostSystem *hostSystem); +int esxVI_HostSystem_CastFromObjectContent(esxVI_ObjectContent *objectContent, + esxVI_HostSystem **hostSystem); + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Enum: VirtualMachinePowerState (Additions) */ -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Parse the path of the URI
by Matthias Bolte 02 Aug '10

02 Aug '10

The path will be used to specify the datacenter, compute resource and host system to be used with a vpx:// connection. --- src/esx/esx_driver.c | 49 ++++++++++------------- src/esx/esx_util.c | 104 +++++++++++++++++++++++++++++++------------------- src/esx/esx_util.h | 11 +++-- src/esx/esx_vi.c | 14 +++--- src/esx/esx_vi.h | 3 +- 5 files changed, 102 insertions(+), 79 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 913420c..3bdc551 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -294,7 +294,7 @@ static int esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth, const char *hostname, int port, const char *predefinedUsername, - esxUtil_ParsedQuery *parsedQuery, + esxUtil_ParsedUri *parsedUri, esxVI_ProductVersion expectedProductVersion, char **vCenterIpAddress) { @@ -347,7 +347,7 @@ esxConnectToHost(esxPrivate *priv, virConnectAuthPtr auth, if (esxVI_Context_Alloc(&priv->host) < 0 || esxVI_Context_Connect(priv->host, url, ipAddress, username, password, - parsedQuery) < 0) { + parsedUri) < 0) { goto cleanup; } @@ -416,7 +416,7 @@ static int esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr auth, const char *hostname, int port, const char *predefinedUsername, - esxUtil_ParsedQuery *parsedQuery) + esxUtil_ParsedUri *parsedUri) { int result = -1; char ipAddress[NI_MAXHOST] = ""; @@ -459,7 +459,7 @@ esxConnectToVCenter(esxPrivate *priv, virConnectAuthPtr auth, if (esxVI_Context_Alloc(&priv->vCenter) < 0 || esxVI_Context_Connect(priv->vCenter, url, ipAddress, username, - password, parsedQuery) < 0) { + password, parsedUri) < 0) { goto cleanup; } @@ -528,7 +528,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; esxPrivate *priv = NULL; - esxUtil_ParsedQuery *parsedQuery = NULL; + esxUtil_ParsedUri *parsedUri = NULL; char *potentialVCenterIpAddress = NULL; char vCenterIpAddress[NI_MAXHOST] = ""; @@ -545,29 +545,24 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) return VIR_DRV_OPEN_DECLINED; } - if (conn->uri->path != NULL && STRNEQ(conn->uri->path, "") && - STRNEQ(conn->uri->path, "/")) { - VIR_WARN("Ignoring unexpected path '%s' in URI", conn->uri->path); - } - /* Allocate per-connection private data */ if (VIR_ALLOC(priv) < 0) { virReportOOMError(); goto cleanup; } - if (esxUtil_ParseQuery(&parsedQuery, conn->uri) < 0) { + if (esxUtil_ParseUri(&parsedUri, conn->uri) < 0) { goto cleanup; } - priv->transport = parsedQuery->transport; - parsedQuery->transport = NULL; + priv->transport = parsedUri->transport; + parsedUri->transport = NULL; priv->maxVcpus = -1; priv->supportsVMotion = esxVI_Boolean_Undefined; priv->supportsLongMode = esxVI_Boolean_Undefined; - priv->autoAnswer = parsedQuery->autoAnswer ? esxVI_Boolean_True - : esxVI_Boolean_False; + priv->autoAnswer = parsedUri->autoAnswer ? esxVI_Boolean_True + : esxVI_Boolean_False; priv->usedCpuTimeCounterId = -1; /* @@ -597,7 +592,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) STRCASEEQ(conn->uri->scheme, "gsx")) { /* Connect to host */ if (esxConnectToHost(priv, auth, conn->uri->server, conn->uri->port, - conn->uri->user, parsedQuery, + conn->uri->user, parsedUri, STRCASEEQ(conn->uri->scheme, "esx") ? esxVI_ProductVersion_ESX : esxVI_ProductVersion_GSX, @@ -606,8 +601,8 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) } /* Connect to vCenter */ - if (parsedQuery->vCenter != NULL) { - if (STREQ(parsedQuery->vCenter, "*")) { + if (parsedUri->vCenter != NULL) { + if (STREQ(parsedUri->vCenter, "*")) { if (potentialVCenterIpAddress == NULL) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("This host is not managed by a vCenter")); @@ -622,7 +617,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) goto cleanup; } } else { - if (esxUtil_ResolveHostname(parsedQuery->vCenter, + if (esxUtil_ResolveHostname(parsedUri->vCenter, vCenterIpAddress, NI_MAXHOST) < 0) { goto cleanup; } @@ -633,14 +628,14 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) _("This host is managed by a vCenter with IP " "address %s, but a mismachting vCenter '%s' " "(%s) has been specified"), - potentialVCenterIpAddress, parsedQuery->vCenter, + potentialVCenterIpAddress, parsedUri->vCenter, vCenterIpAddress); goto cleanup; } } if (esxConnectToVCenter(priv, auth, vCenterIpAddress, - conn->uri->port, NULL, parsedQuery) < 0) { + conn->uri->port, NULL, parsedUri) < 0) { goto cleanup; } } @@ -649,7 +644,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) } else { /* VPX */ /* Connect to vCenter */ if (esxConnectToVCenter(priv, auth, conn->uri->server, conn->uri->port, - conn->uri->user, parsedQuery) < 0) { + conn->uri->user, parsedUri) < 0) { goto cleanup; } @@ -678,7 +673,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags ATTRIBUTE_UNUSED) VIR_FREE(priv); } - esxUtil_FreeParsedQuery(&parsedQuery); + esxUtil_FreeParsedUri(&parsedUri); VIR_FREE(potentialVCenterIpAddress); return result; @@ -3113,14 +3108,14 @@ esxDomainMigratePrepare(virConnectPtr dconn, unsigned long resource ATTRIBUTE_UNUSED) { int result = -1; - esxUtil_ParsedQuery *parsedQuery = NULL; + esxUtil_ParsedUri *parsedUri = NULL; if (uri_in == NULL) { - if (esxUtil_ParseQuery(&parsedQuery, dconn->uri) < 0) { + if (esxUtil_ParseUri(&parsedUri, dconn->uri) < 0) { return -1; } - if (virAsprintf(uri_out, "%s://%s:%d/sdk", parsedQuery->transport, + if (virAsprintf(uri_out, "%s://%s:%d/sdk", parsedUri->transport, dconn->uri->server, dconn->uri->port) < 0) { virReportOOMError(); goto cleanup; @@ -3130,7 +3125,7 @@ esxDomainMigratePrepare(virConnectPtr dconn, result = 0; cleanup: - esxUtil_FreeParsedQuery(&parsedQuery); + esxUtil_FreeParsedUri(&parsedUri); return result; } diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index d79de2c..75a9aaf 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -41,7 +41,7 @@ int -esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) +esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, xmlURIPtr uri) { int result = -1; struct qparam_set *queryParamSet = NULL; @@ -50,13 +50,14 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) int noVerify; int autoAnswer; char *tmp; + char *saveptr; - if (parsedQuery == NULL || *parsedQuery != NULL) { + if (parsedUri == NULL || *parsedUri != NULL) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; } - if (VIR_ALLOC(*parsedQuery) < 0) { + if (VIR_ALLOC(*parsedUri) < 0) { virReportOOMError(); return -1; } @@ -75,29 +76,29 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) queryParam = &queryParamSet->p[i]; if (STRCASEEQ(queryParam->name, "transport")) { - VIR_FREE((*parsedQuery)->transport); + VIR_FREE((*parsedUri)->transport); - (*parsedQuery)->transport = strdup(queryParam->value); + (*parsedUri)->transport = strdup(queryParam->value); - if ((*parsedQuery)->transport == NULL) { + if ((*parsedUri)->transport == NULL) { virReportOOMError(); goto cleanup; } - if (STRNEQ((*parsedQuery)->transport, "http") && - STRNEQ((*parsedQuery)->transport, "https")) { + if (STRNEQ((*parsedUri)->transport, "http") && + STRNEQ((*parsedUri)->transport, "https")) { ESX_ERROR(VIR_ERR_INVALID_ARG, _("Query parameter 'transport' has unexpected value " "'%s' (should be http|https)"), - (*parsedQuery)->transport); + (*parsedUri)->transport); goto cleanup; } } else if (STRCASEEQ(queryParam->name, "vcenter")) { - VIR_FREE((*parsedQuery)->vCenter); + VIR_FREE((*parsedUri)->vCenter); - (*parsedQuery)->vCenter = strdup(queryParam->value); + (*parsedUri)->vCenter = strdup(queryParam->value); - if ((*parsedQuery)->vCenter == NULL) { + if ((*parsedUri)->vCenter == NULL) { virReportOOMError(); goto cleanup; } @@ -110,7 +111,7 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) goto cleanup; } - (*parsedQuery)->noVerify = noVerify != 0; + (*parsedUri)->noVerify = noVerify != 0; } else if (STRCASEEQ(queryParam->name, "auto_answer")) { if (virStrToLong_i(queryParam->value, NULL, 10, &autoAnswer) < 0 || (autoAnswer != 0 && autoAnswer != 1)) { @@ -120,23 +121,23 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) goto cleanup; } - (*parsedQuery)->autoAnswer = autoAnswer != 0; + (*parsedUri)->autoAnswer = autoAnswer != 0; } else if (STRCASEEQ(queryParam->name, "proxy")) { /* Expected format: [<type>://]<hostname>[:<port>] */ - (*parsedQuery)->proxy = true; - (*parsedQuery)->proxy_type = CURLPROXY_HTTP; - VIR_FREE((*parsedQuery)->proxy_hostname); - (*parsedQuery)->proxy_port = 1080; + (*parsedUri)->proxy = true; + (*parsedUri)->proxy_type = CURLPROXY_HTTP; + VIR_FREE((*parsedUri)->proxy_hostname); + (*parsedUri)->proxy_port = 1080; if ((tmp = STRSKIP(queryParam->value, "http://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_HTTP; + (*parsedUri)->proxy_type = CURLPROXY_HTTP; } else if ((tmp = STRSKIP(queryParam->value, "socks://")) != NULL || (tmp = STRSKIP(queryParam->value, "socks5://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_SOCKS5; + (*parsedUri)->proxy_type = CURLPROXY_SOCKS5; } else if ((tmp = STRSKIP(queryParam->value, "socks4://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_SOCKS4; + (*parsedUri)->proxy_type = CURLPROXY_SOCKS4; } else if ((tmp = STRSKIP(queryParam->value, "socks4a://")) != NULL) { - (*parsedQuery)->proxy_type = CURLPROXY_SOCKS4A; + (*parsedUri)->proxy_type = CURLPROXY_SOCKS4A; } else if ((tmp = strstr(queryParam->value, "://")) != NULL) { *tmp = '\0'; @@ -149,15 +150,15 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) tmp = queryParam->value; } - (*parsedQuery)->proxy_hostname = strdup(tmp); + (*parsedUri)->proxy_hostname = strdup(tmp); - if ((*parsedQuery)->proxy_hostname == NULL) { + if ((*parsedUri)->proxy_hostname == NULL) { virReportOOMError(); goto cleanup; } - if ((tmp = strchr((*parsedQuery)->proxy_hostname, ':')) != NULL) { - if (tmp == (*parsedQuery)->proxy_hostname) { + if ((tmp = strchr((*parsedUri)->proxy_hostname, ':')) != NULL) { + if (tmp == (*parsedUri)->proxy_hostname) { ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", _("Query parameter 'proxy' doesn't contain a " "hostname")); @@ -167,9 +168,9 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) *tmp++ = '\0'; if (virStrToLong_i(tmp, NULL, 10, - &(*parsedQuery)->proxy_port) < 0 || - (*parsedQuery)->proxy_port < 1 || - (*parsedQuery)->proxy_port > 65535) { + &(*parsedUri)->proxy_port) < 0 || + (*parsedUri)->proxy_port < 1 || + (*parsedUri)->proxy_port > 65535) { ESX_ERROR(VIR_ERR_INVALID_ARG, _("Query parameter 'proxy' has unexpected port" "value '%s' (should be [1..65535])"), tmp); @@ -182,10 +183,32 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) } } - if ((*parsedQuery)->transport == NULL) { - (*parsedQuery)->transport = strdup("https"); + /* Expected format: [/]<datacenter>/<computeresource>[/<hostsystem>] */ + if (uri->path != NULL) { + tmp = strdup(uri->path); - if ((*parsedQuery)->transport == NULL) { + if (tmp == NULL) { + virReportOOMError(); + goto cleanup; + } + + if (esxVI_String_DeepCopyValue(&(*parsedUri)->path_datacenter, + strtok_r(tmp, "/", &saveptr)) < 0 || + esxVI_String_DeepCopyValue(&(*parsedUri)->path_computeResource, + strtok_r(NULL, "/", &saveptr)) < 0 || + esxVI_String_DeepCopyValue(&(*parsedUri)->path_hostSystem, + strtok_r(NULL, "", &saveptr)) < 0) { + VIR_FREE(tmp); + goto cleanup; + } + + VIR_FREE(tmp); + } + + if ((*parsedUri)->transport == NULL) { + (*parsedUri)->transport = strdup("https"); + + if ((*parsedUri)->transport == NULL) { virReportOOMError(); goto cleanup; } @@ -195,7 +218,7 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) cleanup: if (result < 0) { - esxUtil_FreeParsedQuery(parsedQuery); + esxUtil_FreeParsedUri(parsedUri); } if (queryParamSet != NULL) { @@ -209,17 +232,20 @@ esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri) void -esxUtil_FreeParsedQuery(esxUtil_ParsedQuery **parsedQuery) +esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri) { - if (parsedQuery == NULL || *parsedQuery == NULL) { + if (parsedUri == NULL || *parsedUri == NULL) { return; } - VIR_FREE((*parsedQuery)->transport); - VIR_FREE((*parsedQuery)->vCenter); - VIR_FREE((*parsedQuery)->proxy_hostname); + VIR_FREE((*parsedUri)->transport); + VIR_FREE((*parsedUri)->vCenter); + VIR_FREE((*parsedUri)->proxy_hostname); + VIR_FREE((*parsedUri)->path_datacenter); + VIR_FREE((*parsedUri)->path_computeResource); + VIR_FREE((*parsedUri)->path_hostSystem); - VIR_FREE(*parsedQuery); + VIR_FREE(*parsedUri); } diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index a1927a6..99ce81d 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -29,9 +29,9 @@ # include "internal.h" # include "conf.h" -typedef struct _esxUtil_ParsedQuery esxUtil_ParsedQuery; +typedef struct _esxUtil_ParsedUri esxUtil_ParsedUri; -struct _esxUtil_ParsedQuery { +struct _esxUtil_ParsedUri { char *transport; char *vCenter; bool noVerify; @@ -40,11 +40,14 @@ struct _esxUtil_ParsedQuery { int proxy_type; char *proxy_hostname; int proxy_port; + char *path_datacenter; + char *path_computeResource; + char *path_hostSystem; }; -int esxUtil_ParseQuery(esxUtil_ParsedQuery **parsedQuery, xmlURIPtr uri); +int esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, xmlURIPtr uri); -void esxUtil_FreeParsedQuery(esxUtil_ParsedQuery **parsedQuery); +void esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri); int esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id); diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 98b8bcd..5695881 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -277,7 +277,7 @@ esxVI_CURL_Perform(esxVI_Context *ctx, const char *url) int esxVI_Context_Connect(esxVI_Context *ctx, const char *url, const char *ipAddress, const char *username, - const char *password, esxUtil_ParsedQuery *parsedQuery) + const char *password, esxUtil_ParsedUri *parsedUri) { int result = -1; esxVI_String *propertyNameList = NULL; @@ -329,9 +329,9 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, curl_easy_setopt(ctx->curl_handle, CURLOPT_HEADER, 0); curl_easy_setopt(ctx->curl_handle, CURLOPT_FOLLOWLOCATION, 0); curl_easy_setopt(ctx->curl_handle, CURLOPT_SSL_VERIFYPEER, - parsedQuery->noVerify ? 0 : 1); + parsedUri->noVerify ? 0 : 1); curl_easy_setopt(ctx->curl_handle, CURLOPT_SSL_VERIFYHOST, - parsedQuery->noVerify ? 0 : 2); + parsedUri->noVerify ? 0 : 2); curl_easy_setopt(ctx->curl_handle, CURLOPT_COOKIEFILE, ""); curl_easy_setopt(ctx->curl_handle, CURLOPT_HTTPHEADER, ctx->curl_headers); curl_easy_setopt(ctx->curl_handle, CURLOPT_READFUNCTION, @@ -345,13 +345,13 @@ esxVI_Context_Connect(esxVI_Context *ctx, const char *url, curl_easy_setopt(ctx->curl_handle, CURLOPT_VERBOSE, 1); #endif - if (parsedQuery->proxy) { + if (parsedUri->proxy) { curl_easy_setopt(ctx->curl_handle, CURLOPT_PROXY, - parsedQuery->proxy_hostname); + parsedUri->proxy_hostname); curl_easy_setopt(ctx->curl_handle, CURLOPT_PROXYTYPE, - parsedQuery->proxy_type); + parsedUri->proxy_type); curl_easy_setopt(ctx->curl_handle, CURLOPT_PROXYPORT, - parsedQuery->proxy_port); + parsedUri->proxy_port); } if (virMutexInit(&ctx->curl_lock) < 0) { diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index ef9c6d9..325ba69 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -168,8 +168,7 @@ int esxVI_Context_Alloc(esxVI_Context **ctx); void esxVI_Context_Free(esxVI_Context **ctx); int esxVI_Context_Connect(esxVI_Context *ctx, const char *ipAddress, const char *url, const char *username, - const char *password, - esxUtil_ParsedQuery *parsedQuery); + const char *password, esxUtil_ParsedUri *parsedUri); int esxVI_Context_DownloadFile(esxVI_Context *ctx, const char *url, char **content); int esxVI_Context_UploadFile(esxVI_Context *ctx, const char *url, -- 1.7.0.4

2 2

[libvirt] [PATCH] Add iptables rule to fixup DHCP response checksum.
by Laine Stump 02 Aug '10

02 Aug '10

From: Laine Stump <laine(a)redhat.com> (I don't know whether or not we want to commit this upstream yet - the proposed iptables and kernel module backend for the changes have been posted but not yet committed upstream. On the other hand, the new libvirt code ends up simply printing a warning message if the necessary iptables support isn't yet in place.) This patch attempts to take advantage of a newly added netfilter module to correct for a problem with some guest DHCP client implementations when used in conjunction with a DHCP server run on the host systems with packet checksum offloading enabled. The problem is that, when the guest uses a RAW socket to read the DHCP response packets, the checksum hasn't yet been fixed by the IP stack, so it is incorrect. The fix implemented here is to add a rule to the POSTROUTING chain of the mangle table in iptables that fixes up the checksum for packets on the virtual network's bridge that are destined for the bootpc port (ie "dhcpc", ie port 68) port on the guest. Only very new versions of iptables will have this support (it has been submitted upstream, but not yet committed), so a failure to add this rule only results in a warning message. The iptables patch is here: http://patchwork.ozlabs.org/patch/58525/ A corresponding kernel module patch is also required (the backend of the iptables patch) and has been submitted, but I don't have the details for that (I tested using a pre-built image I received from the developer, Michael Tsirkin). --- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 17 ++++++++++ src/util/iptables.c | 71 +++++++++++++++++++++++++++++++++++++++++++ src/util/iptables.h | 6 ++++ 4 files changed, 96 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 778ceb1..d81d4cf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -328,6 +328,7 @@ iptablesAddForwardAllowRelatedIn; iptablesAddForwardMasquerade; iptablesAddForwardRejectIn; iptablesAddForwardRejectOut; +iptablesAddOutputFixUdpChecksum; iptablesAddTcpInput; iptablesAddUdpInput; iptablesContextFree; @@ -339,6 +340,7 @@ iptablesRemoveForwardAllowRelatedIn; iptablesRemoveForwardMasquerade; iptablesRemoveForwardRejectIn; iptablesRemoveForwardRejectOut; +iptablesRemoveOutputFixUdpChecksum; iptablesRemoveTcpInput; iptablesRemoveUdpInput; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 72255c1..c4480ff 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -781,6 +781,19 @@ networkAddIptablesRules(struct network_driver *driver, !networkAddRoutingIptablesRules(driver, network)) goto err8; + /* If we are doing local DHCP service on this network, attempt to + * add a rule that will fixup the checksum of DHCP response + * packets back to the guests (but report failure without + * aborting, since not all iptables implementations support it). + */ + + if ((network->def->ipAddress || network->def->nranges) && + (iptablesAddOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68) != 0)) { + VIR_WARN("Could not add rule to fixup DHCP response checksums " + "on network '%s'", network->def->name); + } + return 1; err8: @@ -811,6 +824,10 @@ networkAddIptablesRules(struct network_driver *driver, static void networkRemoveIptablesRules(struct network_driver *driver, virNetworkObjPtr network) { + if (network->def->ipAddress || network->def->nranges) { + iptablesRemoveOutputFixUdpChecksum(driver->iptables, + network->def->bridge, 68); + } if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { iptablesRemoveForwardMasquerade(driver->iptables, diff --git a/src/util/iptables.c b/src/util/iptables.c index d06b857..9b888e5 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -60,6 +60,7 @@ struct _iptablesContext iptRules *input_filter; iptRules *forward_filter; iptRules *nat_postrouting; + iptRules *mangle_postrouting; }; static void @@ -188,6 +189,9 @@ iptablesContextNew(void) if (!(ctx->nat_postrouting = iptRulesNew("nat", "POSTROUTING"))) goto error; + if (!(ctx->mangle_postrouting = iptRulesNew("mangle", "POSTROUTING"))) + goto error; + return ctx; error: @@ -210,6 +214,8 @@ iptablesContextFree(iptablesContext *ctx) iptRulesFree(ctx->forward_filter); if (ctx->nat_postrouting) iptRulesFree(ctx->nat_postrouting); + if (ctx->mangle_postrouting) + iptRulesFree(ctx->mangle_postrouting); VIR_FREE(ctx); } @@ -753,3 +759,68 @@ iptablesRemoveForwardMasquerade(iptablesContext *ctx, { return iptablesForwardMasquerade(ctx, network, physdev, REMOVE); } + + +static int +iptablesOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port, + int action) +{ + char portstr[32]; + + snprintf(portstr, sizeof(portstr), "%d", port); + portstr[sizeof(portstr) - 1] = '\0'; + + return iptablesAddRemoveRule(ctx->mangle_postrouting, + action, + "--out-interface", iface, + "--protocol", "udp", + "--destination-port", portstr, + "--jump", "CHECKSUM", "--checksum-fill", + NULL); +} + +/** + * iptablesAddOutputFixUdpChecksum: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port to match + * + * Add an rule to the mangle table's POSTROUTING chain that fixes up the + * checksum of packets with the given destination @port. + * the given @iface interface for TCP packets. + * + * Returns 0 in case of success or an error code in case of error. + * (NB: if the system's iptables does not support checksum mangling, + * this will return an error, which should be ignored.) + */ + +int +iptablesAddOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port) +{ + return iptablesOutputFixUdpChecksum(ctx, iface, port, ADD); +} + +/** + * iptablesRemoveOutputFixUdpChecksum: + * @ctx: pointer to the IP table context + * @iface: the interface name + * @port: the UDP port of the rule to remove + * + * Removes the checksum fixup rule that was previous added with + * iptablesAddOutputFixUdpChecksum. + * + * Returns 0 in case of success or an error code in case of error + * (again, if iptables doesn't support checksum fixup, this will + * return an error, which should be ignored) + */ +int +iptablesRemoveOutputFixUdpChecksum(iptablesContext *ctx, + const char *iface, + int port) +{ + return iptablesOutputFixUdpChecksum(ctx, iface, port, REMOVE); +} diff --git a/src/util/iptables.h b/src/util/iptables.h index 7d55a6d..5c2e553 100644 --- a/src/util/iptables.h +++ b/src/util/iptables.h @@ -89,5 +89,11 @@ int iptablesAddForwardMasquerade (iptablesContext *ctx, int iptablesRemoveForwardMasquerade (iptablesContext *ctx, const char *network, const char *physdev); +int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, + const char *iface, + int port); +int iptablesRemoveOutputFixUdpChecksum (iptablesContext *ctx, + const char *iface, + int port); #endif /* __QEMUD_IPTABLES_H__ */ -- 1.7.1.1

2 3

[libvirt] [PATCH] qemu: kill some dead stores
by Eric Blake 02 Aug '10

02 Aug '10

Spotted by clang. The qemuConnectMonitor one is an outright bug, the other two are cosmetic. * src/qemu/qemu_monitor.c (qemuMonitorClose): Kill dead store. * src/qemu/qemu_driver.c (qemudDomainSaveImageStartVM): Likewise. (qemuConnectMonitor): Don't lose error status. --- src/qemu/qemu_driver.c | 2 -- src/qemu/qemu_monitor.c | 4 +--- 2 files changed, 1 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 098f4da..57b8271 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1416,7 +1416,6 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) ret = qemuMonitorSetCapabilities(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); - ret = 0; error: if (ret < 0) qemuMonitorClose(priv->mon); @@ -6535,7 +6534,6 @@ qemudDomainSaveImageStartVM(virConnectPtr conn, wait_ret = qemudDomainSaveImageClose(fd, read_pid, &status); fd = -1; if (read_pid != -1) { - read_pid = -1; if (wait_ret == -1) { virReportSystemError(errno, _("failed to wait for process reading '%s'"), diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f1494ff..2366fdb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -685,8 +685,6 @@ cleanup: void qemuMonitorClose(qemuMonitorPtr mon) { - int refs; - if (!mon) return; @@ -706,7 +704,7 @@ void qemuMonitorClose(qemuMonitorPtr mon) mon->closed = 1; } - if ((refs = qemuMonitorUnref(mon)) > 0) + if (qemuMonitorUnref(mon) > 0) qemuMonitorUnlock(mon); } -- 1.7.2

2 4

[libvirt] [PATCH] Check that virsh -d argument is numeric
by Daniel Veillard 02 Aug '10

02 Aug '10

Having been bitten one more time by the use of -d to pass the hypervisor URI instead of -c (confusion coming from CVS using -d to specify the root), I suggest to drop atoi and use the function with checking and error out with proper explanation instead of silently failing ! paphio:~/libvirt -> /usr/bin/virsh -d qemu+ssh://test2/system list Id Name State ---------------------------------- paphio:~/libvirt -> tools/virsh -d qemu+ssh://test2/system list error: option -d take a numeric argument paphio:~/libvirt -> tools/virsh -d 5 list command: "list " Id Name State ---------------------------------- paphio:~/libvirt -> Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

3 6

[libvirt] [PATCH] Fix the ACS checking in the PCI code.
by Chris Lalancette 02 Aug '10

02 Aug '10

When trying to assign a PCI device to a guest, we have to check that all bridges upstream of that device support ACS. That means that we have to find the parent bridge of the current device, check for ACS, then find the parent bridge of that device, check for ACS, etc. As it currently stands, the code to do this iterates through all PCI devices on the system, looking for a device that has a range of busses that included the current device's bus. That check is not restrictive enough, though. Depending on how we iterated through the list of PCI devices, we could first find the *topmost* bridge in the system; since it necessarily had a range of busses including the current device's bus, we would only ever check the topmost bridge, and not check any of the intermediate bridges. Note that this also caused a fairly serious bug in the secondary bus reset code, where we could erroneously find and reset the topmost bus instead of the inner bus. This patch changes pciGetParentDevice() so that it first checks if a bridge device's secondary bus exactly matches the bus of the device we are looking for. If it does, we've found the correct parent bridge and we are done. If it does not, then we check to see if this bridge device's busses *include* the bus of the device we care about. If so, we mark this bridge device as best, and go on. If we later find another bridge device whose busses include this device, but is more restrictive, then we free up the previous best and mark the new one as best. This algorithm ensures that in the normal case we find the direct parent, but in the case that the parent bridge secondary bus is not exactly the same as the device, we still find the correct bridge. This patch was tested by me on a 4-port NIC with a bridge without ACS (where assignment failed), a 4-port NIC with a bridge with ACS (where assignment succeeded), and a 2-port NIC with no bridges (where assignment succeeded). Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/util/pci.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 26d55b8..f2890bd 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -283,6 +283,7 @@ pciIterDevices(pciIterPredicate predicate, DIR *dir; struct dirent *entry; int ret = 0; + int rc; *matched = NULL; @@ -322,11 +323,20 @@ pciIterDevices(pciIterPredicate predicate, break; } - if (predicate(dev, check, data)) { + rc = predicate(dev, check, data); + if (rc < 0) { + /* the predicate returned an error, bail */ + pciFreeDevice(check); + ret = -1; + break; + } + else if (rc == 1) { VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name, check->name); *matched = check; + ret = 1; break; } + pciFreeDevice(check); } closedir(dir); @@ -510,10 +520,11 @@ pciBusContainsActiveDevices(pciDevice *dev, /* Is @check the parent of @dev ? */ static int -pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED) +pciIsParent(pciDevice *dev, pciDevice *check, void *data) { uint16_t device_class; uint8_t header_type, secondary, subordinate; + pciDevice **best = data; if (dev->domain != check->domain) return 0; @@ -533,16 +544,54 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED) VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name); - /* No, it's superman! */ - return (dev->bus >= secondary && dev->bus <= subordinate); + /* if the secondary bus exactly equals the device's bus, then we found + * the direct parent. No further work is necessary + */ + if (dev->bus == secondary) + return 1; + + /* otherwise, SRIOV allows VFs to be on different busses then their PFs. + * In this case, what we need to do is look for the "best" match; i.e. + * the most restrictive match that still satisfies all of the conditions. + */ + if (dev->bus > secondary && dev->bus <= subordinate) { + if (*best == NULL) { + *best = pciGetDevice(check->domain, check->bus, check->slot, + check->function); + if (*best == NULL) + return -1; + } + else { + /* OK, we had already recorded a previous "best" match for the + * parent. See if the current device is more restrictive than the + * best, and if so, make it the new best + */ + if (secondary > pciRead8(*best, PCI_SECONDARY_BUS)) { + pciFreeDevice(*best); + *best = pciGetDevice(check->domain, check->bus, check->slot, + check->function); + if (*best == NULL) + return -1; + } + } + } + + return 0; } -static pciDevice * -pciGetParentDevice(pciDevice *dev) +static int +pciGetParentDevice(pciDevice *dev, pciDevice **parent) { - pciDevice *parent = NULL; - pciIterDevices(pciIsParent, dev, &parent, NULL); - return parent; + pciDevice *best = NULL; + int ret; + + *parent = NULL; + ret = pciIterDevices(pciIsParent, dev, parent, &best); + if (ret == 1) + pciFreeDevice(best); + else if (ret == 0) + *parent = best; + return ret; } /* Secondary Bus Reset is our sledgehammer - it resets all @@ -570,7 +619,8 @@ pciTrySecondaryBusReset(pciDevice *dev, } /* Find the parent bus */ - parent = pciGetParentDevice(dev); + if (pciGetParentDevice(dev, &parent) < 0) + return -1; if (!parent) { pciReportError(VIR_ERR_NO_SUPPORT, _("Failed to find parent device for %s"), @@ -1377,7 +1427,8 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) { pciDevice *parent; - parent = pciGetParentDevice(dev); + if (pciGetParentDevice(dev, &parent) < 0) + return -1; if (!parent) { /* if we have no parent, and this is the root bus, ACS doesn't come * into play since devices on the root bus can't P2P without going @@ -1400,6 +1451,7 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) do { pciDevice *tmp; int acs; + int ret; acs = pciDeviceDownstreamLacksACS(parent); @@ -1412,8 +1464,10 @@ pciDeviceIsBehindSwitchLackingACS(pciDevice *dev) } tmp = parent; - parent = pciGetParentDevice(parent); + ret = pciGetParentDevice(parent, &parent); pciFreeDevice(tmp); + if (ret < 0) + return -1; } while (parent); return 0; -- 1.7.1.1

3 3

[libvirt] [PATCH] Free up memballoon def.
by Chris Lalancette 02 Aug '10

02 Aug '10

Forgetting to do this was causing a memory leak. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ca4bc6e..1ddea0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -768,6 +768,8 @@ void virDomainDefFree(virDomainDefPtr def) virDomainWatchdogDefFree(def->watchdog); + virDomainMemballoonDefFree(def->memballoon); + virSecurityLabelDefFree(def); virCPUDefFree(def->cpu); -- 1.7.2

2 2

[libvirt] [PATCH] Don't put a semicolon on the end of a VIR_ENUM_IMPL.
by Chris Lalancette 02 Aug '10

02 Aug '10

Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 04c417e..ca4bc6e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -99,7 +99,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", "pci", "drive", - "virtio-serial"); + "virtio-serial") VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", -- 1.7.2

2 2

[libvirt] [PATCH] Fix a bogus warning when parsing <hostdev>
by Chris Lalancette 02 Aug '10

02 Aug '10

When parsing hostdev, the following message would be emitted: 10:17:19.052: error : virDomainHostdevDefParseXML:3748 : internal error unknown node alias However, alias is appropriately parsed in virDomainDeviceInfoParseXML anyway. Disable the error message in the initial XML parsing loop. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/conf/domain_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8721dd1..04c417e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3723,6 +3723,9 @@ virDomainHostdevDefParseXML(const xmlNodePtr node, goto error; } } else if (xmlStrEqual(cur->name, BAD_CAST "address")) { + /* address is parsed as part of virDomainDeviceInfoParseXML */ + } else if (xmlStrEqual(cur->name, BAD_CAST "alias")) { + /* alias is parsed as part of virDomainDeviceInfoParseXML */ } else { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unknown node %s"), cur->name); -- 1.7.2

2 2