[libvirt] [PATCH] esx: Add .vmdk storage volume creation

--- src/esx/esx_driver.c | 3 +- src/esx/esx_storage_driver.c | 215 +++++++++++++++++++++++++++++++++++++++- src/esx/esx_vi.c | 57 +++++++----- src/esx/esx_vi.h | 1 + src/esx/esx_vi_generator.input | 32 ++++++ src/esx/esx_vi_generator.py | 7 +- src/esx/esx_vi_methods.c | 12 +++ 7 files changed, 296 insertions(+), 31 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index eda3fc2..f8d4771 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -353,7 +353,8 @@ esxAutodetectSCSIControllerModel(virDomainDiskDefPtr def, int *model, return 0; } - if (esxVI_LookupFileInfoByDatastorePath(data->ctx, def->src, &fileInfo, + if (esxVI_LookupFileInfoByDatastorePath(data->ctx, def->src, + false, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index d2d8f22..3b959c2 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -709,7 +709,7 @@ esxStorageVolumeLookupByName(virStoragePoolPtr pool, const char *name) } if (esxVI_LookupFileInfoByDatastorePath(priv->primary, datastorePath, - &fileInfo, + false, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } @@ -743,7 +743,8 @@ esxStorageVolumeLookupByKeyOrPath(virConnectPtr conn, const char *keyOrPath) goto cleanup; } - if (esxVI_LookupFileInfoByDatastorePath(priv->primary, keyOrPath, &fileInfo, + if (esxVI_LookupFileInfoByDatastorePath(priv->primary, keyOrPath, + false, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } @@ -761,6 +762,210 @@ esxStorageVolumeLookupByKeyOrPath(virConnectPtr conn, const char *keyOrPath) +static virStorageVolPtr +esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, + unsigned int flags) +{ + virStorageVolPtr volume = NULL; + esxPrivate *priv = pool->conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_DatastoreInfo *datastoreInfo = NULL; + virStoragePoolDef poolDef; + virStorageVolDefPtr def = NULL; + char *tmp1; + char *tmp2; + char *datastorePath = NULL; + char *directoryName = NULL; + char *datastorePathWithoutFileName = NULL; + esxVI_FileInfo *fileInfo = NULL; + esxVI_FileBackedVirtualDiskSpec *virtualDiskSpec = NULL; + esxVI_ManagedObjectReference *task = NULL; + esxVI_TaskInfoState taskInfoState; + + virCheckFlags(0, NULL); + + memset(&poolDef, 0, sizeof (poolDef)); + + if (esxVI_EnsureSession(priv->primary) < 0) { + return NULL; + } + + /* Lookup storage pool type */ + if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || + esxVI_LookupDatastoreByName(priv->primary, pool->name, + propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "info")) { + if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, + &datastoreInfo) < 0) { + goto cleanup; + } + + break; + } + } + + if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + poolDef.type = VIR_STORAGE_POOL_DIR; + } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + poolDef.type = VIR_STORAGE_POOL_NETFS; + } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + poolDef.type = VIR_STORAGE_POOL_FS; + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("DatastoreInfo has unexpected type")); + goto cleanup; + } + + /* Parse config */ + def = virStorageVolDefParseString(&poolDef, xmldesc); + + if (def == NULL) { + goto cleanup; + } + + if (def->type != VIR_STORAGE_VOL_FILE) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Creating non-file volumes is not supported")); + goto cleanup; + } + + /* Validate config */ + tmp1 = strchr(def->name, '/'); + tmp2 = strrchr(def->name, '/'); + + if (tmp1 == NULL || tmp1 == def->name || + tmp2 == NULL || tmp2[1] == '\0') { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Volume name '%s' doesn't have expected format " + "'<directory>/<file>'"), def->name); + goto cleanup; + } + + if (! virFileHasSuffix(def->name, ".vmdk")) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Volume name '%s' has unsupported suffix, expecting '.vmdk'"), + def->name); + goto cleanup; + } + + if (virAsprintf(&datastorePath, "[%s] %s", pool->name, def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (def->target.format == VIR_STORAGE_FILE_VMDK) { + /* Create directory, if it doesn't exist yet */ + if (esxUtil_ParseDatastorePath(datastorePath, NULL, &directoryName, + NULL) < 0) { + goto cleanup; + } + + if (virAsprintf(&datastorePathWithoutFileName, "[%s] %s", pool->name, + directoryName) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (esxVI_LookupFileInfoByDatastorePath + (priv->primary, datastorePathWithoutFileName, true, &fileInfo, + esxVI_Occurrence_OptionalItem) < 0) { + goto cleanup; + } + + if (fileInfo == NULL) { + if (esxVI_MakeDirectory(priv->primary, datastorePathWithoutFileName, + priv->primary->datacenter->_reference, + esxVI_Boolean_True) < 0) { + goto cleanup; + } + } + + /* Create VirtualDisk */ + if (esxVI_FileBackedVirtualDiskSpec_Alloc(&virtualDiskSpec) < 0 || + esxVI_Long_Alloc(&virtualDiskSpec->capacityKb) < 0) { + goto cleanup; + } + + /* From the vSphere API documentation about VirtualDiskType ... */ + if (def->allocation == def->capacity) { + /* + * "A preallocated disk has all space allocated at creation time + * and the space is zeroed on demand as the space is used." + */ + virtualDiskSpec->diskType = (char *)"preallocated"; + } else if (def->allocation == 0) { + /* + * "Space required for thin-provisioned virtual disk is allocated + * and zeroed on demand as the space is used." + */ + virtualDiskSpec->diskType = (char *)"thin"; + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unsupported capacity-to-allocation relation")); + goto cleanup; + } + + /* + * FIXME: The adapter type is a required parameter, but there is no + * way to let the user specifiy it in the volume XML config. Therefore, + * default to 'busLogic' here. + */ + virtualDiskSpec->adapterType = (char *)"busLogic"; + + virtualDiskSpec->capacityKb->value = def->capacity / 1024; /* Scale from byte to kilobyte */ + + if (esxVI_CreateVirtualDisk_Task + (priv->primary, datastorePath, priv->primary->datacenter->_reference, + esxVI_VirtualDiskSpec_DynamicCast(virtualDiskSpec), &task) < 0 || + esxVI_WaitForTaskCompletion(priv->primary, task, NULL, + esxVI_Occurrence_None, + priv->autoAnswer, &taskInfoState) < 0) { + goto cleanup; + } + + if (taskInfoState != esxVI_TaskInfoState_Success) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create volume")); + goto cleanup; + } + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Creation of %s volumes is not supported"), + virStorageFileFormatTypeToString(def->target.format)); + goto cleanup; + } + + volume = virGetStorageVol(pool->conn, pool->name, def->name, datastorePath); + + cleanup: + if (virtualDiskSpec != NULL) { + virtualDiskSpec->diskType = NULL; + virtualDiskSpec->adapterType = NULL; + } + + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + esxVI_DatastoreInfo_Free(&datastoreInfo); + virStorageVolDefFree(def); + VIR_FREE(datastorePath); + VIR_FREE(directoryName); + VIR_FREE(datastorePathWithoutFileName); + esxVI_FileInfo_Free(&fileInfo); + esxVI_FileBackedVirtualDiskSpec_Free(&virtualDiskSpec); + esxVI_ManagedObjectReference_Free(&task); + + return volume; +} + + + static int esxStorageVolumeGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) { @@ -782,7 +987,7 @@ esxStorageVolumeGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) } if (esxVI_LookupFileInfoByDatastorePath(priv->primary, datastorePath, - &fileInfo, + false, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } @@ -875,7 +1080,7 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) } if (esxVI_LookupFileInfoByDatastorePath(priv->primary, datastorePath, - &fileInfo, + false, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } @@ -986,7 +1191,7 @@ static virStorageDriver esxStorageDriver = { esxStorageVolumeLookupByName, /* volLookupByName */ esxStorageVolumeLookupByKeyOrPath, /* volLookupByKey */ esxStorageVolumeLookupByKeyOrPath, /* volLookupByPath */ - NULL, /* volCreateXML */ + esxStorageVolumeCreateXML, /* volCreateXML */ NULL, /* volCreateXMLFrom */ NULL, /* volDelete */ NULL, /* volWipe */ diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index a6950fd..8d83eac 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2940,6 +2940,7 @@ esxVI_LookupCurrentSnapshotTree int esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, const char *datastorePath, + bool lookupFolder, esxVI_FileInfo **fileInfo, esxVI_Occurrence occurrence) { @@ -2954,6 +2955,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_ObjectContent *datastore = NULL; esxVI_ManagedObjectReference *hostDatastoreBrowser = NULL; esxVI_HostDatastoreBrowserSearchSpec *searchSpec = NULL; + esxVI_FolderFileQuery *folderFileQuery = NULL; esxVI_VmDiskFileQuery *vmDiskFileQuery = NULL; esxVI_IsoImageFileQuery *isoImageFileQuery = NULL; esxVI_FloppyImageFileQuery *floppyImageFileQuery = NULL; @@ -3030,32 +3032,41 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, searchSpec->details->fileSize = esxVI_Boolean_True; searchSpec->details->modification = esxVI_Boolean_False; - if (esxVI_VmDiskFileQuery_Alloc(&vmDiskFileQuery) < 0 || - esxVI_VmDiskFileQueryFlags_Alloc(&vmDiskFileQuery->details) < 0 || - esxVI_FileQuery_AppendToList - (&searchSpec->query, - esxVI_FileQuery_DynamicCast(vmDiskFileQuery)) < 0) { - goto cleanup; - } + if (lookupFolder) { + if (esxVI_FolderFileQuery_Alloc(&folderFileQuery) < 0 || + esxVI_FileQuery_AppendToList + (&searchSpec->query, + esxVI_FileQuery_DynamicCast(folderFileQuery)) < 0) { + goto cleanup; + } + } else { + if (esxVI_VmDiskFileQuery_Alloc(&vmDiskFileQuery) < 0 || + esxVI_VmDiskFileQueryFlags_Alloc(&vmDiskFileQuery->details) < 0 || + esxVI_FileQuery_AppendToList + (&searchSpec->query, + esxVI_FileQuery_DynamicCast(vmDiskFileQuery)) < 0) { + goto cleanup; + } - vmDiskFileQuery->details->diskType = esxVI_Boolean_False; - vmDiskFileQuery->details->capacityKb = esxVI_Boolean_True; - vmDiskFileQuery->details->hardwareVersion = esxVI_Boolean_False; - vmDiskFileQuery->details->controllerType = esxVI_Boolean_True; - vmDiskFileQuery->details->diskExtents = esxVI_Boolean_False; + vmDiskFileQuery->details->diskType = esxVI_Boolean_False; + vmDiskFileQuery->details->capacityKb = esxVI_Boolean_True; + vmDiskFileQuery->details->hardwareVersion = esxVI_Boolean_False; + vmDiskFileQuery->details->controllerType = esxVI_Boolean_True; + vmDiskFileQuery->details->diskExtents = esxVI_Boolean_False; - if (esxVI_IsoImageFileQuery_Alloc(&isoImageFileQuery) < 0 || - esxVI_FileQuery_AppendToList - (&searchSpec->query, - esxVI_FileQuery_DynamicCast(isoImageFileQuery)) < 0) { - goto cleanup; - } + if (esxVI_IsoImageFileQuery_Alloc(&isoImageFileQuery) < 0 || + esxVI_FileQuery_AppendToList + (&searchSpec->query, + esxVI_FileQuery_DynamicCast(isoImageFileQuery)) < 0) { + goto cleanup; + } - if (esxVI_FloppyImageFileQuery_Alloc(&floppyImageFileQuery) < 0 || - esxVI_FileQuery_AppendToList - (&searchSpec->query, - esxVI_FileQuery_DynamicCast(floppyImageFileQuery)) < 0) { - goto cleanup; + if (esxVI_FloppyImageFileQuery_Alloc(&floppyImageFileQuery) < 0 || + esxVI_FileQuery_AppendToList + (&searchSpec->query, + esxVI_FileQuery_DynamicCast(floppyImageFileQuery)) < 0) { + goto cleanup; + } } if (esxVI_String_Alloc(&searchSpec->matchPattern) < 0) { diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 3d1aee0..773a1c6 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -401,6 +401,7 @@ int esxVI_LookupCurrentSnapshotTree int esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, const char *datastorePath, + bool lookupFolder, esxVI_FileInfo **fileInfo, esxVI_Occurrence occurrence); diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 0fb9448..b911c22 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -166,6 +166,11 @@ object Description end +object DeviceBackedVirtualDiskSpec extends VirtualDiskSpec + String device r +end + + object DynamicProperty String name r AnyType val r @@ -190,6 +195,11 @@ object Event end +object FileBackedVirtualDiskSpec extends VirtualDiskSpec + Long capacityKb r +end + + object FileInfo String path r Long fileSize o @@ -528,6 +538,12 @@ object UserSession end +object VirtualDiskSpec + String diskType r + String adapterType r +end + + object VirtualMachineConfigSpec String changeVersion o String name o @@ -694,6 +710,14 @@ method CreateSnapshot_Task returns ManagedObjectReference r end +method CreateVirtualDisk_Task returns ManagedObjectReference r + ManagedObjectReference _this:VirtualDiskManager r + String name r + ManagedObjectReference datacenter o + VirtualDiskSpec spec r +end + + method DestroyPropertyFilter ManagedObjectReference _this r end @@ -728,6 +752,14 @@ method Logout end +method MakeDirectory + ManagedObjectReference _this:FileManager r + String name r + ManagedObjectReference datacenter o + Boolean createParentDirectories o +end + + method MigrateVM_Task returns ManagedObjectReference r ManagedObjectReference _this r ManagedObjectReference pool o diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 411fd80..be96a03 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -45,10 +45,12 @@ valid_occurrences = [OCCURRENCE__REQUIRED_ITEM, class Parameter: - autobind_map = { "PerformanceManager" : "perfManager", + autobind_map = { "FileManager" : "fileManager", + "PerformanceManager" : "perfManager", "PropertyCollector" : "propertyCollector", "SearchIndex" : "searchIndex", - "SessionManager" : "sessionManager" } + "SessionManager" : "sessionManager", + "VirtualDiskManager" : "virtualDiskManager" } def __init__(self, type, name, occurrence): self.type = type @@ -1146,6 +1148,7 @@ additional_object_features = { "DatastoreHostMount" : Object.FEATURE__DE "SharesInfo" : Object.FEATURE__ANY_TYPE, "TaskInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "UserSession" : Object.FEATURE__ANY_TYPE, + "VirtualDiskSpec" : Object.FEATURE__DYNAMIC_CAST, "VirtualMachineQuestionInfo" : Object.FEATURE__ANY_TYPE, "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | Object.FEATURE__ANY_TYPE } diff --git a/src/esx/esx_vi_methods.c b/src/esx/esx_vi_methods.c index 56d2e58..a00561f 100644 --- a/src/esx/esx_vi_methods.c +++ b/src/esx/esx_vi_methods.c @@ -173,6 +173,12 @@ +#define ESX_VI__METHOD__PARAMETER__THIS__fileManager \ + ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ + fileManager) + + + #define ESX_VI__METHOD__PARAMETER__THIS__perfManager \ ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ perfManager) @@ -197,6 +203,12 @@ +#define ESX_VI__METHOD__PARAMETER__THIS__virtualDiskManager \ + ESX_VI__METHOD__PARAMETER__THIS_FROM_SERVICE(ManagedObjectReference, \ + virtualDiskManager) + + + /* * A required parameter must be != 0 (NULL for pointers, "undefined" == 0 for * enumeration values). -- 1.7.0.4

On 08/28/2010 01:53 PM, Matthias Bolte wrote:
+ /* Validate config */ + tmp1 = strchr(def->name, '/'); + tmp2 = strrchr(def->name, '/'); + + if (tmp1 == NULL || tmp1 == def->name || + tmp2 == NULL || tmp2[1] == '\0') {
tmp2 cannot be NULL if tmp1 is not NULL, since if the string contains a / in a forward search, it will also contain one in a reverse search. Also, checking that tmp1 != def->name can be done with a single-byte probe. What about using a single temporary for a faster test: tmp = strrchr(def->name, '/'); if (tmp == NULL || *def->name == '/' || tmp[1] == '\0') {
+ /* + * FIXME: The adapter type is a required parameter, but there is no + * way to let the user specifiy it in the volume XML config. Therefore,
s/specifiy/specify/
+ * default to 'busLogic' here. + */
Sounds like a good followup patch, but doesn't impact whether this one is okay as-is.
+ virtualDiskSpec->adapterType = (char *)"busLogic"; + + virtualDiskSpec->capacityKb->value = def->capacity / 1024; /* Scale from byte to kilobyte */
Do you need to round up? That is, should def->capacity of 1 result in 0 or 1024 bytes? I think those points are minor enough to not need a v2, so: ACK with those points addressed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2010/8/30 Eric Blake <eblake@redhat.com>:
On 08/28/2010 01:53 PM, Matthias Bolte wrote:
+ /* Validate config */ + tmp1 = strchr(def->name, '/'); + tmp2 = strrchr(def->name, '/'); + + if (tmp1 == NULL || tmp1 == def->name || + tmp2 == NULL || tmp2[1] == '\0') {
tmp2 cannot be NULL if tmp1 is not NULL, since if the string contains a / in a forward search, it will also contain one in a reverse search. Also, checking that tmp1 != def->name can be done with a single-byte probe. What about using a single temporary for a faster test:
tmp = strrchr(def->name, '/'); if (tmp == NULL || *def->name == '/' || tmp[1] == '\0') {
Yes, that's fine too. I'll fold that in.
+ /* + * FIXME: The adapter type is a required parameter, but there is no + * way to let the user specifiy it in the volume XML config. Therefore,
s/specifiy/specify/
+ * default to 'busLogic' here. + */
Sounds like a good followup patch, but doesn't impact whether this one is okay as-is.
+ virtualDiskSpec->adapterType = (char *)"busLogic"; + + virtualDiskSpec->capacityKb->value = def->capacity / 1024; /* Scale from byte to kilobyte */
Do you need to round up? That is, should def->capacity of 1 result in 0 or 1024 bytes?
I truncate here on purpose. The rest of the ESX driver does it this way, also the rest of the libvirt codebase truncates in situations like this. Therefore, I'll leave this as it is.
I think those points are minor enough to not need a v2, so:
ACK with those points addressed.
Thanks, pushed. Matthias
participants (2)
-
Eric Blake
-
Matthias Bolte