- Devel - libvirt List Archives

Re: [libvirt] virt-top version bump in EPEL5
by Richard W.M. Jones 06 Sep '10

06 Sep '10

On Mon, Sep 06, 2010 at 05:46:03PM +0200, Wolfram Schlich wrote: > * Richard W.M. Jones <rjones(a)redhat.com> [2010-09-06 17:23]: > > On Mon, Sep 06, 2010 at 05:06:55PM +0200, Wolfram Schlich wrote: > > > We are experiencing a problem with virt-top on our RHEL5 virthosts. > > > > > > On a heavily loaded virthost, virt-top is damn slow -- it takes 4-5s > > > to update and imposes a noticeable load on the dom0 as well > > > (xenstored shows up in top eating all CPU for some seconds). > > > xentop seems to impose *much* less load on the dom0. > > > Is there something I can do about it? > > > > > > Another problem: 0.3.3.1 shows RDRQ/WRRQ/RXBY/TXBY after some seconds > > > whereas 1.0.4 does not...? > > > > I bet both of these will be libvirt issues. > > > > Try running: > > > > virsh list --all > > Takes around 2-4s depending on dom0 load. CC-ing to libvir-list. There may be a better way now for virt-top to get the list of domains, but if 'virsh list --all' is also slow, then it's probably a generic libvirt problem. > > virsh domblkstat DomainName hda > > s/hda/xvda/ I guess :) > This one is quite fast, below 0.2s. > > > virsh domifstat DomainName vnet0 > > This takes around 0.1s most of the time, but sometimes around 1.0s. > > > etc. > > > > If those commands (done in a suitable loop) also cause load on the > > dom0, and if domblkstat/domifstat don't show stats, then it's down to > > libvirt. > > > > virt-top is a simple little program that just exercises those libvirt > > APIs ... > > Hmm :/ So I guess we're better off with xentop... We should be able to have performance very close to xentop. After all, libvirt makes exactly the same direct hypervisor calls. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

3 6

[libvirt] [PATCH] Default to qemu:///system if accessible
by Soren Hansen 06 Sep '10

06 Sep '10

If no uri is passed to one of the virConnectOpen-ish calls, libvirt attempts to autoprobe a sensible URI. Part of the current logic checks for getuid() > 0, and if that check is succesful, a libvirtd daemon is spawned. This patch replaces this check with a call to access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK) so that users with access to the qemu:///system UNIX socket connect to that one by default instead of spawning a new libvirtd daemon. Signed-off-by: Soren Hansen <soren(a)linux2go.dk> --- src/remote/remote_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a945710..17e6ead 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1086,7 +1086,7 @@ remoteOpen (virConnectPtr conn, if (!conn->uri) { DEBUG0("Auto-probe remote URI"); #ifndef __sun - if (getuid() > 0) { + if (access(LIBVIRTD_PRIV_UNIX_SOCKET, W_OK)) { DEBUG0("Auto-spawn user daemon instance"); rflags |= VIR_DRV_OPEN_REMOTE_USER; if (!autostart || -- 1.7.1

6 16

[libvirt] [PATCH] esx: Use the VirtualDisk UUID as storage volume key
by Matthias Bolte 04 Sep '10

04 Sep '10

VirtualDisks are .vmdk file based. Other files in a datastore like .iso or .flp files don't have a UUID attached, fall back to the path as key for them. --- src/esx/esx_storage_driver.c | 190 ++++++++++++++++++++++++++++++++++++---- src/esx/esx_util.c | 19 ++++ src/esx/esx_util.h | 2 + src/esx/esx_vi.c | 53 +++++++++++ src/esx/esx_vi.h | 4 + src/esx/esx_vi_generator.input | 7 ++ 6 files changed, 256 insertions(+), 19 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 3b959c2..a9a5633 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -697,7 +697,7 @@ esxStorageVolumeLookupByName(virStoragePoolPtr pool, const char *name) virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->storagePrivateData; char *datastorePath = NULL; - esxVI_FileInfo *fileInfo = NULL; + char *key = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; @@ -708,17 +708,16 @@ esxStorageVolumeLookupByName(virStoragePoolPtr pool, const char *name) goto cleanup; } - if (esxVI_LookupFileInfoByDatastorePath(priv->primary, datastorePath, - false, &fileInfo, - esxVI_Occurrence_RequiredItem) < 0) { + if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, + datastorePath, &key) < 0) { goto cleanup; } - volume = virGetStorageVol(pool->conn, pool->name, name, datastorePath); + volume = virGetStorageVol(pool->conn, pool->name, name, key); cleanup: VIR_FREE(datastorePath); - esxVI_FileInfo_Free(&fileInfo); + VIR_FREE(key); return volume; } @@ -726,36 +725,170 @@ esxStorageVolumeLookupByName(virStoragePoolPtr pool, const char *name) static virStorageVolPtr -esxStorageVolumeLookupByKeyOrPath(virConnectPtr conn, const char *keyOrPath) +esxStorageVolumeLookupByPath(virConnectPtr conn, const char *path) { virStorageVolPtr volume = NULL; esxPrivate *priv = conn->storagePrivateData; char *datastoreName = NULL; char *directoryAndFileName = NULL; - esxVI_FileInfo *fileInfo = NULL; + char *key = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; } - if (esxUtil_ParseDatastorePath(keyOrPath, &datastoreName, NULL, + if (esxUtil_ParseDatastorePath(path, &datastoreName, NULL, &directoryAndFileName) < 0) { goto cleanup; } - if (esxVI_LookupFileInfoByDatastorePath(priv->primary, keyOrPath, - false, &fileInfo, - esxVI_Occurrence_RequiredItem) < 0) { + if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path, + &key) < 0) { goto cleanup; } - volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, - keyOrPath); + volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, key); cleanup: VIR_FREE(datastoreName); VIR_FREE(directoryAndFileName); - esxVI_FileInfo_Free(&fileInfo); + VIR_FREE(key); + + return volume; +} + + + +static virStorageVolPtr +esxStorageVolumeLookupByKey(virConnectPtr conn, const char *key) +{ + virStorageVolPtr volume = NULL; + esxPrivate *priv = conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastoreList = NULL; + esxVI_ObjectContent *datastore = NULL; + char *datastoreName = NULL; + esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; + esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; + char *directoryAndFileName = NULL; + size_t length; + char *datastorePath = NULL; + char *volumeName = NULL; + esxVI_FileInfo *fileInfo = NULL; + char *uuid_string = NULL; + char key_candidate[VIR_UUID_STRING_BUFLEN] = ""; + + if (STRPREFIX(key, "[")) { + /* Key is probably a datastore path */ + return esxStorageVolumeLookupByPath(conn, key); + } + + if (esxVI_EnsureSession(priv->primary) < 0) { + return NULL; + } + + /* Lookup all datastores */ + if (esxVI_String_AppendValueToList(&propertyNameList, "summary.name") < 0 || + esxVI_LookupDatastoreList(priv->primary, propertyNameList, + &datastoreList) < 0) { + goto cleanup; + } + + for (datastore = datastoreList; datastore != NULL; + datastore = datastore->_next) { + datastoreName = NULL; + + if (esxVI_GetStringValue(datastore, "summary.name", &datastoreName, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + /* Lookup datastore content */ + esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); + + if (esxVI_LookupDatastoreContentByDatastoreName + (priv->primary, datastoreName, &searchResultsList) < 0) { + goto cleanup; + } + + /* Interpret search result */ + for (searchResults = searchResultsList; searchResults != NULL; + searchResults = searchResults->_next) { + VIR_FREE(directoryAndFileName); + + if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, + NULL, &directoryAndFileName) < 0) { + goto cleanup; + } + + /* Strip trailing separators */ + length = strlen(directoryAndFileName); + + while (length > 0 && directoryAndFileName[length - 1] == '/') { + directoryAndFileName[length - 1] = '\0'; + --length; + } + + /* Build datastore path and query the UUID */ + for (fileInfo = searchResults->file; fileInfo != NULL; + fileInfo = fileInfo->_next) { + VIR_FREE(datastorePath); + + if (length < 1) { + if (virAsprintf(&volumeName, "%s", + fileInfo->path) < 0) { + virReportOOMError(); + goto cleanup; + } + } else if (virAsprintf(&volumeName, "%s/%s", + directoryAndFileName, + fileInfo->path) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&datastorePath, "[%s] %s", datastoreName, + volumeName) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (esxVI_VmDiskFileInfo_DynamicCast(fileInfo) == NULL) { + /* Only a VirtualDisk has a UUID */ + continue; + } + + VIR_FREE(uuid_string); + + if (esxVI_QueryVirtualDiskUuid + (priv->primary, datastorePath, + priv->primary->datacenter->_reference, + &uuid_string) < 0) { + goto cleanup; + } + + if (esxUtil_ReformatUuid(uuid_string, key_candidate) < 0) { + goto cleanup; + } + + if (STREQ(key, key_candidate)) { + /* Found matching UUID */ + volume = virGetStorageVol(conn, datastoreName, + volumeName, key); + goto cleanup; + } + } + } + } + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastoreList); + esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); + VIR_FREE(directoryAndFileName); + VIR_FREE(datastorePath); + VIR_FREE(volumeName); + VIR_FREE(uuid_string); return volume; } @@ -783,6 +916,8 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileBackedVirtualDiskSpec *virtualDiskSpec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *uuid_string = NULL; + char key[VIR_UUID_STRING_BUFLEN] = ""; virCheckFlags(0, NULL); @@ -935,6 +1070,16 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create volume")); goto cleanup; } + + if (esxVI_QueryVirtualDiskUuid(priv->primary, datastorePath, + priv->primary->datacenter->_reference, + &uuid_string) < 0) { + goto cleanup; + } + + if (esxUtil_ReformatUuid(uuid_string, key) < 0) { + goto cleanup; + } } else { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Creation of %s volumes is not supported"), @@ -942,7 +1087,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, goto cleanup; } - volume = virGetStorageVol(pool->conn, pool->name, def->name, datastorePath); + volume = virGetStorageVol(pool->conn, pool->name, def->name, key); cleanup: if (virtualDiskSpec != NULL) { @@ -960,6 +1105,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileInfo_Free(&fileInfo); esxVI_FileBackedVirtualDiskSpec_Free(&virtualDiskSpec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(uuid_string); return volume; } @@ -1090,7 +1236,12 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) floppyImageFileInfo = esxVI_FloppyImageFileInfo_DynamicCast(fileInfo); def.name = volume->name; - def.key = datastorePath; + + if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, datastorePath, + &def.key) < 0) { + goto cleanup; + } + def.type = VIR_STORAGE_VOL_FILE; def.target.path = datastorePath; @@ -1123,6 +1274,7 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) esxVI_DatastoreInfo_Free(&datastoreInfo); VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); + VIR_FREE(def.key); return xml; } @@ -1189,8 +1341,8 @@ static virStorageDriver esxStorageDriver = { esxStoragePoolNumberOfStorageVolumes, /* poolNumOfVolumes */ esxStoragePoolListStorageVolumes, /* poolListVolumes */ esxStorageVolumeLookupByName, /* volLookupByName */ - esxStorageVolumeLookupByKeyOrPath, /* volLookupByKey */ - esxStorageVolumeLookupByKeyOrPath, /* volLookupByPath */ + esxStorageVolumeLookupByKey, /* volLookupByKey */ + esxStorageVolumeLookupByPath, /* volLookupByPath */ esxStorageVolumeCreateXML, /* volCreateXML */ NULL, /* volCreateXMLFrom */ NULL, /* volDelete */ diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 08c6c46..99cc4f6 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -602,3 +602,22 @@ esxUtil_GetConfigBoolean(virConfPtr conf, const char *name, bool *boolean_, return 0; } + + + +int +esxUtil_ReformatUuid(const char *input, char *output) +{ + unsigned char uuid[VIR_UUID_BUFLEN]; + + if (virUUIDParse(input, uuid) < 0) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + input); + return -1; + } + + virUUIDFormat(uuid, output); + + return 0; +} diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index 4d92333..650f145 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -69,4 +69,6 @@ int esxUtil_GetConfigLong(virConfPtr conf, const char *name, long long *number, int esxUtil_GetConfigBoolean(virConfPtr conf, const char *name, bool *boolean_, bool default_, bool optional); +int esxUtil_ReformatUuid(const char *input, char *output); + #endif /* __ESX_UTIL_H__ */ diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 8d83eac..fb2a499 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -3254,6 +3254,59 @@ esxVI_LookupDatastoreContentByDatastoreName int +esxVI_LookupStorageVolumeKeyByDatastorePath(esxVI_Context *ctx, + const char *datastorePath, + char **key) +{ + int result = -1; + esxVI_FileInfo *fileInfo = NULL; + char *uuid_string = NULL; + + if (key == NULL || *key != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (esxVI_LookupFileInfoByDatastorePath(ctx, datastorePath, false, &fileInfo, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + if (esxVI_VmDiskFileInfo_DynamicCast(fileInfo) != NULL) { + /* VirtualDisks have a UUID, use it as key */ + if (esxVI_QueryVirtualDiskUuid(ctx, datastorePath, + ctx->datacenter->_reference, + &uuid_string) < 0) { + goto cleanup; + } + + if (VIR_ALLOC_N(*key, VIR_UUID_STRING_BUFLEN) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (esxUtil_ReformatUuid(uuid_string, *key) < 0) { + goto cleanup; + } + } else { + /* Other files don't have a UUID, fall back to the path as key */ + if (esxVI_String_DeepCopyValue(key, datastorePath) < 0) { + goto cleanup; + } + } + + result = 0; + + cleanup: + esxVI_FileInfo_Free(&fileInfo); + VIR_FREE(uuid_string); + + return result; +} + + + +int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, esxVI_VirtualMachineQuestionInfo *questionInfo, diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 773a1c6..bd37b05 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -409,6 +409,10 @@ int esxVI_LookupDatastoreContentByDatastoreName (esxVI_Context *ctx, const char *datastoreName, esxVI_HostDatastoreBrowserSearchResults **searchResultsList); +int esxVI_LookupStorageVolumeKeyByDatastorePath(esxVI_Context *ctx, + const char *datastorePath, + char **key); + int esxVI_HandleVirtualMachineQuestion (esxVI_Context *ctx, esxVI_ManagedObjectReference *virtualMachine, diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index b911c22..2ee513a 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -801,6 +801,13 @@ method QueryPerfCounter returns PerfCounterInfo ol end +method QueryVirtualDiskUuid returns String r + ManagedObjectReference _this:VirtualDiskManager r + String name r + ManagedObjectReference datacenter o +end + + method RebootGuest ManagedObjectReference _this r end -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Add .vmdk storage volume creation
by Matthias Bolte 03 Sep '10

03 Sep '10

--- 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

2 2

[libvirt] [PATCH] OpenVZ: add ethernet interface type support
by Jean-Baptiste Rouault 03 Sep '10

03 Sep '10

Hi all, This patch adds support for ethernet interface type to OpenVZ domains as stated in this previous message: http://www.redhat.com/archives/libvir- list/2010-July/msg00658.html Regards, Jean-Baptiste

2 2

[libvirt] libvirt-guests and ./configure --without-libvirtd
by Eric Blake 03 Sep '10

03 Sep '10

On mingw, libvirt is typically configured --without-libvirtd, because the mingw build is designed to target other hosts, but cannot natively run as a hypervisor. Additionally, mingw doesn't really have a notion of /etc/sysconf, so installing /etc/sysconfig/libvirt-guests doesn't work too well. Should tools/Makefile.am make the build and installation of libvirt-guests dependent on the already-existing WITH_LIBVIRTD Makefile conditional, or would it ever make sense to build libvirt-guests but not libvirtd? I guess the answer to that boils down to whether it ever makes sense to use libvirt-guests on a system that is not running libvirtd, in which case it would only be able to control remote URIs. But the whole point of the script is to cleanly save state for local VMs, so I don't see using libvirt-guests as a management tool for remote URIs. Unless anyone speaks up with another opinion, I'll prepare a patch that makes libvirt-guests depend on WITH_LIBVIRTD. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2 2

[libvirt] [PATCH] build: avoid unused variable warning
by Eric Blake 03 Sep '10

03 Sep '10

* src/vbox/vbox_tmpl.c (vboxAttachDrives): Capture return value. --- Pushing under the build-breaker rule. Gcc didn't catch this under '-g -O0', but with -Werror and -O2, it dies with a warning that rc is used uninitialized. I suspect that clang would have also reported this, although I haven't tested that suspicion. This was a pre-existing bug; my refactoring exposed it (before the refactoring, rc contained stale contents, unrelated to the actual message being printed; after the refactoring, rc was indeed used without initialization). src/vbox/vbox_tmpl.c | 34 ++++++++++++++++++---------------- 1 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 3e8ff23..b5cd2e4 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -3788,40 +3788,42 @@ vboxAttachDrives(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) VBOX_UTF16_FREE(mediumFileUtf16); continue; } if (!medium) { PRUnichar *mediumEmpty = NULL; VBOX_UTF8_TO_UTF16("", &mediumEmpty); if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - data->vboxObj->vtbl->OpenHardDisk(data->vboxObj, - mediumFileUtf16, - AccessMode_ReadWrite, - false, - mediumEmpty, - false, - mediumEmpty, - &medium); + rc = data->vboxObj->vtbl->OpenHardDisk(data->vboxObj, + mediumFileUtf16, + AccessMode_ReadWrite, + false, + mediumEmpty, + false, + mediumEmpty, + &medium); } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - data->vboxObj->vtbl->OpenDVDImage(data->vboxObj, - mediumFileUtf16, - mediumEmpty, - &medium); + rc = data->vboxObj->vtbl->OpenDVDImage(data->vboxObj, + mediumFileUtf16, + mediumEmpty, + &medium); } else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - data->vboxObj->vtbl->OpenFloppyImage(data->vboxObj, - mediumFileUtf16, - mediumEmpty, - &medium); + rc = data->vboxObj->vtbl->OpenFloppyImage(data->vboxObj, + mediumFileUtf16, + mediumEmpty, + &medium); + } else { + rc = 0; } VBOX_UTF16_FREE(mediumEmpty); } if (!medium) { vboxError(VIR_ERR_INTERNAL_ERROR, _("Failed to attach the following disk/dvd/floppy " "to the machine: %s, rc=%08x"), def->disks[i]->src, (unsigned)rc); -- 1.7.2.2

1 0

[libvirt] [PATCH Java] Remove non-thread-safe error reporting
by Matthias Bolte 03 Sep '10

03 Sep '10

virConnCopyLastError is not thread-safe, don't use it. Reported by Ravi Pawar. --- src/main/java/org/libvirt/Connect.java | 2 +- src/main/java/org/libvirt/ErrorHandler.java | 17 ----------------- 2 files changed, 1 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/libvirt/Connect.java b/src/main/java/org/libvirt/Connect.java index fb8ea89..7761c1f 100644 --- a/src/main/java/org/libvirt/Connect.java +++ b/src/main/java/org/libvirt/Connect.java @@ -1319,7 +1319,7 @@ public class Connect { * @throws LibvirtException */ protected void processError() throws LibvirtException { - ErrorHandler.processError(libvirt, VCP); + ErrorHandler.processError(libvirt); } /** diff --git a/src/main/java/org/libvirt/ErrorHandler.java b/src/main/java/org/libvirt/ErrorHandler.java index 7b723bb..e30291b 100644 --- a/src/main/java/org/libvirt/ErrorHandler.java +++ b/src/main/java/org/libvirt/ErrorHandler.java @@ -28,21 +28,4 @@ public class ErrorHandler { throw new LibvirtException(error); } } - - /** - * Look for the latest error from libvirt tied to a connection - * - * @param libvirt - * the active connection - * @throws LibvirtException - */ - public static void processError(Libvirt libvirt, ConnectionPointer conn) throws LibvirtException { - virError vError = new virError(); - int errorCode = libvirt.virConnCopyLastError(conn, vError); - if (errorCode > 0) { - Error error = new Error(vError); - libvirt.virConnResetLastError(conn); - throw new LibvirtException(error); - } - } } -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Rework datastore path parsing and handling
by Matthias Bolte 03 Sep '10

03 Sep '10

Instead of splitting the path part of a datastore path into directory and file name, keep this in one piece. An example: "[datastore] directory/file" was split into this before: datastoreName = "datastore" directoryName = "directory" fileName = "file" Now it's split into this: datastoreName = "datastore" directoryName = "directory" directoryAndFileName = "directory/file" This simplifies code using esxUtil_ParseDatastorePath, because directoryAndFileName is used more often than fileName. Also the old approach expected the datastore path to reference a actual file, but this isn't always correct, especially when listing volume. In that case esxUtil_ParseDatastorePath is used to parse a path that references a directory. This fails for a vpx:// connection because the vCenter returns directory paths with a trailing '/'. The new approach is robust against this and the actual decision if the datastore path should reference a file or a directory is up to the caller of esxUtil_ParseDatastorePath. Update the tests accordingly. --- src/esx/esx_driver.c | 90 +++++++++++++++++++----------------------- src/esx/esx_storage_driver.c | 70 +++++++++----------------------- src/esx/esx_util.c | 71 +++++++++++++++++++-------------- src/esx/esx_util.h | 2 +- src/esx/esx_vi.c | 30 +++++++++++++- tests/esxutilstest.c | 32 ++++++++------ tests/xml2vmxtest.c | 19 +++------ 7 files changed, 155 insertions(+), 159 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 50d5bc0..eda3fc2 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -52,8 +52,7 @@ typedef struct _esxVMX_Data esxVMX_Data; struct _esxVMX_Data { esxVI_Context *ctx; - const char *datastoreName; - const char *directoryName; + char *datastorePathWithoutFileName; }; @@ -117,8 +116,8 @@ esxParseVMXFileName(const char *fileName, void *opaque) if (strchr(fileName, '/') == NULL && strchr(fileName, '\\') == NULL) { /* Plain file name, use same directory as for the .vmx file */ - if (virAsprintf(&datastorePath, "[%s] %s/%s", data->datastoreName, - data->directoryName, fileName) < 0) { + if (virAsprintf(&datastorePath, "%s/%s", + data->datastorePathWithoutFileName, fileName) < 0) { virReportOOMError(); goto cleanup; } @@ -254,8 +253,7 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque) bool success = false; esxVMX_Data *data = opaque; char *datastoreName = NULL; - char *directoryName = NULL; - char *fileName = NULL; + char *directoryAndFileName = NULL; esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; char separator = '/'; @@ -265,13 +263,12 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque) char *absolutePath = NULL; /* Parse datastore path and lookup datastore */ - if (esxUtil_ParseDatastorePath(datastorePath, &datastoreName, - &directoryName, &fileName) < 0) { + if (esxUtil_ParseDatastorePath(datastorePath, &datastoreName, NULL, + &directoryAndFileName) < 0) { goto cleanup; } - if (esxVI_LookupDatastoreByName(data->ctx, datastoreName, - NULL, &datastore, + if (esxVI_LookupDatastoreByName(data->ctx, datastoreName, NULL, &datastore, esxVI_Occurrence_RequiredItem) < 0 || esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj, &hostMount) < 0) { @@ -290,29 +287,23 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque) --length; } - /* Format as <mount>[/<directory>]/<file> */ + /* Format as <mount>[/<directory>]/<file>, convert / to \ when necessary */ virBufferAdd(&buffer, hostMount->mountInfo->path, length); - if (directoryName != NULL) { - /* Convert / to \ when necessary */ - if (separator != '/') { - tmp = directoryName; - - while (*tmp != '\0') { - if (*tmp == '/') { - *tmp = separator; - } + if (separator != '/') { + tmp = directoryAndFileName; - ++tmp; + while (*tmp != '\0') { + if (*tmp == '/') { + *tmp = separator; } - } - virBufferAddChar(&buffer, separator); - virBufferAdd(&buffer, directoryName, -1); + ++tmp; + } } virBufferAddChar(&buffer, separator); - virBufferAdd(&buffer, fileName, -1); + virBufferAdd(&buffer, directoryAndFileName, -1); if (virBufferError(&buffer)) { virReportOOMError(); @@ -332,8 +323,7 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque) } VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(fileName); + VIR_FREE(directoryAndFileName); esxVI_ObjectContent_Free(&datastore); esxVI_DatastoreHostMount_Free(&hostMount); @@ -2527,7 +2517,7 @@ esxDomainDumpXML(virDomainPtr domain, int flags) char *vmPathName = NULL; char *datastoreName = NULL; char *directoryName = NULL; - char *fileName = NULL; + char *directoryAndFileName = NULL; virBuffer buffer = VIR_BUFFER_INITIALIZER; char *url = NULL; char *vmx = NULL; @@ -2554,19 +2544,13 @@ esxDomainDumpXML(virDomainPtr domain, int flags) } if (esxUtil_ParseDatastorePath(vmPathName, &datastoreName, &directoryName, - &fileName) < 0) { + &directoryAndFileName) < 0) { goto cleanup; } virBufferVSprintf(&buffer, "%s://%s:%d/folder/", priv->transport, domain->conn->uri->server, domain->conn->uri->port); - - if (directoryName != NULL) { - virBufferURIEncodeString(&buffer, directoryName); - virBufferAddChar(&buffer, '/'); - } - - virBufferURIEncodeString(&buffer, fileName); + virBufferURIEncodeString(&buffer, directoryAndFileName); virBufferAddLit(&buffer, "?dcPath="); virBufferURIEncodeString(&buffer, priv->primary->datacenter->name); virBufferAddLit(&buffer, "&dsName="); @@ -2584,8 +2568,20 @@ esxDomainDumpXML(virDomainPtr domain, int flags) } data.ctx = priv->primary; - data.datastoreName = datastoreName; - data.directoryName = directoryName; + + if (directoryName == NULL) { + if (virAsprintf(&data.datastorePathWithoutFileName, "[%s]", + datastoreName) < 0) { + virReportOOMError(); + goto cleanup; + } + } else { + if (virAsprintf(&data.datastorePathWithoutFileName, "[%s] %s", + datastoreName, directoryName) < 0) { + virReportOOMError(); + goto cleanup; + } + } ctx.opaque = &data; ctx.parseFileName = esxParseVMXFileName; @@ -2612,8 +2608,9 @@ esxDomainDumpXML(virDomainPtr domain, int flags) esxVI_ObjectContent_Free(&virtualMachine); VIR_FREE(datastoreName); VIR_FREE(directoryName); - VIR_FREE(fileName); + VIR_FREE(directoryAndFileName); VIR_FREE(url); + VIR_FREE(data.datastorePathWithoutFileName); VIR_FREE(vmx); virDomainDefFree(def); @@ -2640,8 +2637,7 @@ esxDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, } data.ctx = priv->primary; - data.datastoreName = "?"; - data.directoryName = "?"; + data.datastorePathWithoutFileName = (char *)"[?] ?"; ctx.opaque = &data; ctx.parseFileName = esxParseVMXFileName; @@ -2686,8 +2682,7 @@ esxDomainXMLToNative(virConnectPtr conn, const char *nativeFormat, } data.ctx = priv->primary; - data.datastoreName = NULL; - data.directoryName = NULL; + data.datastorePathWithoutFileName = NULL; ctx.opaque = &data; ctx.parseFileName = NULL; @@ -2887,7 +2882,6 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) esxVMX_Data data; char *datastoreName = NULL; char *directoryName = NULL; - char *fileName = NULL; virBuffer buffer = VIR_BUFFER_INITIALIZER; char *url = NULL; char *datastoreRelatedPath = NULL; @@ -2927,8 +2921,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) /* Build VMX from domain XML */ data.ctx = priv->primary; - data.datastoreName = NULL; - data.directoryName = NULL; + data.datastorePathWithoutFileName = NULL; ctx.opaque = &data; ctx.parseFileName = NULL; @@ -2979,11 +2972,11 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) } if (esxUtil_ParseDatastorePath(disk->src, &datastoreName, &directoryName, - &fileName) < 0) { + NULL) < 0) { goto cleanup; } - if (! virFileHasSuffix(fileName, ".vmdk")) { + if (! virFileHasSuffix(disk->src, ".vmdk")) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Expecting source '%s' of first file-based harddisk to " "be a VMDK image"), disk->src); @@ -3067,7 +3060,6 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml ATTRIBUTE_UNUSED) VIR_FREE(vmx); VIR_FREE(datastoreName); VIR_FREE(directoryName); - VIR_FREE(fileName); VIR_FREE(url); VIR_FREE(datastoreRelatedPath); esxVI_ObjectContent_Free(&virtualMachine); diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index af8876a..d2d8f22 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -611,10 +611,8 @@ esxStoragePoolListStorageVolumes(virStoragePoolPtr pool, char **const names, esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; esxVI_FileInfo *fileInfo = NULL; - char *datastoreName = NULL; - char *directoryName = NULL; - char *fileName = NULL; - char *prefix = NULL; + char *directoryAndFileName = NULL; + int length; int count = 0; int i; @@ -639,40 +637,32 @@ esxStoragePoolListStorageVolumes(virStoragePoolPtr pool, char **const names, /* Interpret search result */ for (searchResults = searchResultsList; searchResults != NULL; searchResults = searchResults->_next) { - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(fileName); - VIR_FREE(prefix); + VIR_FREE(directoryAndFileName); - if (esxUtil_ParseDatastorePath(searchResults->folderPath, &datastoreName, - &directoryName, &fileName) < 0) { + if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL, + &directoryAndFileName) < 0) { goto cleanup; } - if (directoryName != NULL) { - if (virAsprintf(&prefix, "%s/%s", directoryName, fileName) < 0) { - virReportOOMError(); - goto cleanup; - } - } else { - prefix = strdup(fileName); + /* Strip trailing separators */ + length = strlen(directoryAndFileName); - if (prefix == NULL) { - virReportOOMError(); - goto cleanup; - } + while (length > 0 && directoryAndFileName[length - 1] == '/') { + directoryAndFileName[length - 1] = '\0'; + --length; } + /* Build volume names */ for (fileInfo = searchResults->file; fileInfo != NULL; fileInfo = fileInfo->_next) { - if (*prefix == '\0') { + if (length < 1) { names[count] = strdup(fileInfo->path); if (names[count] == NULL) { virReportOOMError(); goto cleanup; } - } else if (virAsprintf(&names[count], "%s/%s", prefix, + } else if (virAsprintf(&names[count], "%s/%s", directoryAndFileName, fileInfo->path) < 0) { virReportOOMError(); goto cleanup; @@ -694,10 +684,7 @@ esxStoragePoolListStorageVolumes(virStoragePoolPtr pool, char **const names, } esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); - VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(fileName); - VIR_FREE(prefix); + VIR_FREE(directoryAndFileName); return count; } @@ -744,46 +731,29 @@ esxStorageVolumeLookupByKeyOrPath(virConnectPtr conn, const char *keyOrPath) virStorageVolPtr volume = NULL; esxPrivate *priv = conn->storagePrivateData; char *datastoreName = NULL; - char *directoryName = NULL; - char *fileName = NULL; - char *volumeName = NULL; + char *directoryAndFileName = NULL; esxVI_FileInfo *fileInfo = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; } - if (esxUtil_ParseDatastorePath(keyOrPath, &datastoreName, &directoryName, - &fileName) < 0) { + if (esxUtil_ParseDatastorePath(keyOrPath, &datastoreName, NULL, + &directoryAndFileName) < 0) { goto cleanup; } - if (directoryName != NULL) { - if (virAsprintf(&volumeName, "%s/%s", directoryName, fileName) < 0) { - virReportOOMError(); - goto cleanup; - } - } else { - volumeName = strdup(fileName); - - if (volumeName == NULL) { - virReportOOMError(); - goto cleanup; - } - } - if (esxVI_LookupFileInfoByDatastorePath(priv->primary, keyOrPath, &fileInfo, esxVI_Occurrence_RequiredItem) < 0) { goto cleanup; } - volume = virGetStorageVol(conn, datastoreName, volumeName, keyOrPath); + volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, + keyOrPath); cleanup: VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(fileName); - VIR_FREE(volumeName); + VIR_FREE(directoryAndFileName); esxVI_FileInfo_Free(&fileInfo); return volume; diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index 7c7c841..08c6c46 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -275,19 +275,19 @@ esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id) int esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, - char **directoryName, char **fileName) + char **directoryName, char **directoryAndFileName) { int result = -1; char *copyOfDatastorePath = NULL; char *tmp = NULL; char *saveptr = NULL; char *preliminaryDatastoreName = NULL; - char *directoryAndFileName = NULL; - char *separator = NULL; + char *preliminaryDirectoryAndFileName = NULL; + char *preliminaryFileName = NULL; - if (datastoreName == NULL || *datastoreName != NULL || - directoryName == NULL || *directoryName != NULL || - fileName == NULL || *fileName != NULL) { + if ((datastoreName != NULL && *datastoreName != NULL) || + (directoryName != NULL && *directoryName != NULL) || + (directoryAndFileName != NULL && *directoryAndFileName != NULL)) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); return -1; } @@ -296,43 +296,46 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, goto cleanup; } - /* Expected format: '[<datastore>] <path>' */ - if ((tmp = STRSKIP(copyOfDatastorePath, "[")) == NULL || - (preliminaryDatastoreName = strtok_r(tmp, "]", &saveptr)) == NULL || - (directoryAndFileName = strtok_r(NULL, "", &saveptr)) == NULL) { + /* Expected format: '[<datastore>] <path>' where <path> is optional */ + if ((tmp = STRSKIP(copyOfDatastorePath, "[")) == NULL || *tmp == ']' || + (preliminaryDatastoreName = strtok_r(tmp, "]", &saveptr)) == NULL) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Datastore path '%s' doesn't have expected format " "'[<datastore>] <path>'"), datastorePath); goto cleanup; } - if (esxVI_String_DeepCopyValue(datastoreName, + if (datastoreName != NULL && + esxVI_String_DeepCopyValue(datastoreName, preliminaryDatastoreName) < 0) { goto cleanup; } - directoryAndFileName += strspn(directoryAndFileName, " "); + preliminaryDirectoryAndFileName = strtok_r(NULL, "", &saveptr); - /* Split <path> into <directory>/<file>, where <directory> is optional */ - separator = strrchr(directoryAndFileName, '/'); + if (preliminaryDirectoryAndFileName == NULL) { + preliminaryDirectoryAndFileName = (char *)""; + } else { + preliminaryDirectoryAndFileName += + strspn(preliminaryDirectoryAndFileName, " "); + } + + if (directoryAndFileName != NULL && + esxVI_String_DeepCopyValue(directoryAndFileName, + preliminaryDirectoryAndFileName) < 0) { + goto cleanup; + } - if (separator != NULL) { - *separator++ = '\0'; + if (directoryName != NULL) { + /* Split <path> into <directory>/<file> */ + preliminaryFileName = strrchr(preliminaryDirectoryAndFileName, '/'); - if (*separator == '\0') { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Datastore path '%s' doesn't reference a file"), - datastorePath); - goto cleanup; + if (preliminaryFileName != NULL) { + *preliminaryFileName++ = '\0'; } if (esxVI_String_DeepCopyValue(directoryName, - directoryAndFileName) < 0 || - esxVI_String_DeepCopyValue(fileName, separator) < 0) { - goto cleanup; - } - } else { - if (esxVI_String_DeepCopyValue(fileName, directoryAndFileName) < 0) { + preliminaryDirectoryAndFileName) < 0) { goto cleanup; } } @@ -341,9 +344,17 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, cleanup: if (result < 0) { - VIR_FREE(*datastoreName); - VIR_FREE(*directoryName); - VIR_FREE(*fileName); + if (datastoreName != NULL) { + VIR_FREE(*datastoreName); + } + + if (directoryName != NULL) { + VIR_FREE(*directoryName); + } + + if (directoryAndFileName != NULL) { + VIR_FREE(*directoryAndFileName); + } } VIR_FREE(copyOfDatastorePath); diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index 5799730..4d92333 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -52,7 +52,7 @@ void esxUtil_FreeParsedUri(esxUtil_ParsedUri **parsedUri); int esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id); int esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName, - char **directoryName, char **fileName); + char **directoryName, char **directoryAndFileName); 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 2359e8f..a6950fd 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2946,7 +2946,9 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, int result = -1; char *datastoreName = NULL; char *directoryName = NULL; + char *directoryAndFileName = NULL; char *fileName = NULL; + int length; char *datastorePathWithoutFileName = NULL; esxVI_String *propertyNameList = NULL; esxVI_ObjectContent *datastore = NULL; @@ -2966,22 +2968,45 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, } if (esxUtil_ParseDatastorePath(datastorePath, &datastoreName, - &directoryName, &fileName) < 0) { + &directoryName, &directoryAndFileName) < 0) { goto cleanup; } - if (directoryName == NULL) { + if (STREQ(directoryName, directoryAndFileName)) { + /* + * The <path> part of the datatore path didn't contain a '/', assume + * that the <path> part is actually the file name. + */ if (virAsprintf(&datastorePathWithoutFileName, "[%s]", datastoreName) < 0) { virReportOOMError(); goto cleanup; } + + if (esxVI_String_DeepCopyValue(&fileName, directoryAndFileName) < 0) { + goto cleanup; + } } else { if (virAsprintf(&datastorePathWithoutFileName, "[%s] %s", datastoreName, directoryName) < 0) { virReportOOMError(); goto cleanup; } + + length = strlen(directoryName); + + if (directoryAndFileName[length] != '/' || + directoryAndFileName[length + 1] == '\0') { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Datastore path '%s' doesn't reference a file"), + datastorePath); + goto cleanup; + } + + if (esxVI_String_DeepCopyValue(&fileName, + directoryAndFileName + length + 1) < 0) { + goto cleanup; + } } /* Lookup HostDatastoreBrowser */ @@ -3087,6 +3112,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, VIR_FREE(datastoreName); VIR_FREE(directoryName); + VIR_FREE(directoryAndFileName); VIR_FREE(fileName); VIR_FREE(datastorePathWithoutFileName); esxVI_String_Free(&propertyNameList); diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c index 09470ed..4906ad4 100644 --- a/tests/esxutilstest.c +++ b/tests/esxutilstest.c @@ -99,14 +99,18 @@ struct testPath { int result; const char *datastoreName; const char *directoryName; - const char *fileName; + const char *directoryAndFileName; }; static struct testPath paths[] = { - { "[datastore] directory/file", 0, "datastore", "directory", "file" }, - { "[datastore] file", 0, "datastore", NULL, "file" }, + { "[datastore] directory/file", 0, "datastore", "directory", + "directory/file" }, + { "[datastore] directory1/directory2/file", 0, "datastore", + "directory1/directory2", "directory1/directory2/file" }, + { "[datastore] file", 0, "datastore", "file", "file" }, + { "[datastore] directory/", 0, "datastore", "directory", "directory/" }, + { "[datastore]", 0, "datastore", "", "" }, { "[] directory/file", -1, NULL, NULL, NULL }, - { "[datastore] directory/", -1, NULL, NULL, NULL }, { "directory/file", -1, NULL, NULL, NULL }, }; @@ -116,16 +120,16 @@ testParseDatastorePath(const void *data ATTRIBUTE_UNUSED) int i, result = 0; char *datastoreName = NULL; char *directoryName = NULL; - char *fileName = NULL; + char *directoryAndFileName = NULL; for (i = 0; i < ARRAY_CARDINALITY(paths); ++i) { VIR_FREE(datastoreName); VIR_FREE(directoryName); - VIR_FREE(fileName); + VIR_FREE(directoryAndFileName); - if (esxUtil_ParseDatastorePath(paths[i].datastorePath, - &datastoreName, &directoryName, - &fileName) != paths[i].result) { + if (esxUtil_ParseDatastorePath + (paths[i].datastorePath, &datastoreName, &directoryName, + &directoryAndFileName) != paths[i].result) { goto failure; } @@ -138,14 +142,14 @@ testParseDatastorePath(const void *data ATTRIBUTE_UNUSED) goto failure; } - if (paths[i].directoryName != NULL && - STRNEQ(paths[i].directoryName, directoryName)) { + if (STRNEQ(paths[i].directoryName, directoryName)) { virtTestDifference(stderr, paths[i].directoryName, directoryName); goto failure; } - if (STRNEQ(paths[i].fileName, fileName)) { - virtTestDifference(stderr, paths[i].fileName, fileName); + if (STRNEQ(paths[i].directoryAndFileName, directoryAndFileName)) { + virtTestDifference(stderr, paths[i].directoryAndFileName, + directoryAndFileName); goto failure; } } @@ -153,7 +157,7 @@ testParseDatastorePath(const void *data ATTRIBUTE_UNUSED) cleanup: VIR_FREE(datastoreName); VIR_FREE(directoryName); - VIR_FREE(fileName); + VIR_FREE(directoryAndFileName); return result; diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index eed3ac0..71ce14a 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -148,24 +148,18 @@ testFormatVMXFileName(const char *src, void *opaque ATTRIBUTE_UNUSED) { bool success = false; char *datastoreName = NULL; - char *directoryName = NULL; - char *fileName = NULL; + char *directoryAndFileName = NULL; char *absolutePath = NULL; if (STRPREFIX(src, "[")) { /* Found potential datastore path */ - if (esxUtil_ParseDatastorePath(src, &datastoreName, &directoryName, - &fileName) < 0) { + if (esxUtil_ParseDatastorePath(src, &datastoreName, NULL, + &directoryAndFileName) < 0) { goto cleanup; } - if (directoryName == NULL) { - virAsprintf(&absolutePath, "/vmfs/volumes/%s/%s", datastoreName, - fileName); - } else { - virAsprintf(&absolutePath, "/vmfs/volumes/%s/%s/%s", datastoreName, - directoryName, fileName); - } + virAsprintf(&absolutePath, "/vmfs/volumes/%s/%s", datastoreName, + directoryAndFileName); } else if (STRPREFIX(src, "/")) { /* Found absolute path */ absolutePath = strdup(src); @@ -182,8 +176,7 @@ testFormatVMXFileName(const char *src, void *opaque ATTRIBUTE_UNUSED) } VIR_FREE(datastoreName); - VIR_FREE(directoryName); - VIR_FREE(fileName); + VIR_FREE(directoryAndFileName); return absolutePath; } -- 1.7.0.4

2 4

[libvirt] [PATCH 0/7] round 2 of sprintf cleanups
by Eric Blake 03 Sep '10

03 Sep '10

I don't think all snprintf uses are bad - those that format a number into an appropriately-sized maximum-possible array should be fine. For example, there are some fixed-size buffers whose size is determined by the kernel, where snprintf was the appropriate course of action in nwfilter_ebiptables_driver.c. But sprintf doesn't have quite the coverage as snprintf, so this continuation of *printf cleanups converts some of these. I still have more patches to go, but this has taken me long enough to get some more review rather than slamming a 20-patch series all at the end. More comments within individual patches. Eric Blake (7): build: add some modules vbox: factor a large function vbox: avoid problematic uses of sprintf network: use virAsprintf when appropriate lxc: avoid large stacks with veth creation openvz: formatting cleanups openvz: use virAsprintf to avoid large stacks bootstrap.conf | 2 + src/conf/network_conf.c | 15 +- src/lxc/lxc_driver.c | 32 +- src/lxc/veth.c | 84 ++- src/lxc/veth.h | 6 +- src/openvz/openvz_conf.c | 142 ++-- src/openvz/openvz_driver.c | 23 +- src/vbox/vbox_tmpl.c | 2108 +++++++++++++++++++++++--------------------- 8 files changed, 1269 insertions(+), 1143 deletions(-) -- 1.7.2.2

2 21