September 2010 - Devel - libvirt List Archives

[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

[libvirt] [PATCH] esx: Fix generator for string return values
by Matthias Bolte 02 Sep '10

02 Sep '10

Distinguish between strings as parameters (const char *) and strings as return values (char **). --- src/esx/esx_vi_generator.py | 19 +++++++++++++------ src/esx/esx_vi_methods.c | 22 ++++++++++++---------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index be96a03..f9e8e11 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -52,9 +52,10 @@ class Parameter: "SessionManager" : "sessionManager", "VirtualDiskManager" : "virtualDiskManager" } - def __init__(self, type, name, occurrence): + def __init__(self, type, name, occurrence, is_return_value = False): self.type = type self.occurrence = occurrence + self.is_return_value = is_return_value if ':' in name and name.startswith("_this"): self.name, self.autobind_type = name.split(":") @@ -96,7 +97,7 @@ class Parameter: def generate_return(self, offset = 0, end_of_line = ";"): if self.occurrence == OCCURRENCE__IGNORED: - raise ValueError("invalid function parameteroccurrence value '%s'" % self.occurrence) + raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) else: string = " " string += " " * offset @@ -130,7 +131,10 @@ class Parameter: if self.type == "String" and \ self.occurrence not in [OCCURRENCE__REQUIRED_LIST, OCCURRENCE__OPTIONAL_LIST]: - return "const char *" + if self.is_return_value: + return "char *" + else: + return "const char *" elif self.is_enum(): return "esxVI_%s " % self.type else: @@ -227,9 +231,11 @@ class Method: source += "),\n" if self.returns is None: - source += " void, None,\n" + source += " void, /* nothing */, None,\n" + elif self.returns.type == "String": + source += " String, Value, %s,\n" % self.returns.get_occurrence_short_enum() else: - source += " %s, %s,\n" % (self.returns.type, self.returns.get_occurrence_short_enum()) + source += " %s, /* nothing */, %s,\n" % (self.returns.type, self.returns.get_occurrence_short_enum()) source += "{\n" @@ -1054,7 +1060,8 @@ def parse_method(block): report_error("line %d: invalid block header" % (number)) else: returns = Parameter(type = header_items[3], name = "output", - occurrence = header_items[4]) + occurrence = header_items[4], + is_return_value = True) parameters = [] diff --git a/src/esx/esx_vi_methods.c b/src/esx/esx_vi_methods.c index a00561f..5967088 100644 --- a/src/esx/esx_vi_methods.c +++ b/src/esx/esx_vi_methods.c @@ -67,34 +67,34 @@ -#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__None(_type) \ +#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__None(_type, _suffix) \ /* nothing */ -#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__RequiredItem(_type) \ - if (esxVI_##_type##_Deserialize(response->node, output) < 0) { \ +#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__RequiredItem(_type, _suffix) \ + if (esxVI_##_type##_Deserialize##_suffix(response->node, output) < 0) { \ goto cleanup; \ } -#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__RequiredList(_type) \ +#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__RequiredList(_type, _suffix) \ if (esxVI_##_type##_DeserializeList(response->node, output) < 0) { \ goto cleanup; \ } -#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__OptionalItem(_type) \ +#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__OptionalItem(_type, _suffix) \ if (response->node != NULL && \ - esxVI_##_type##_Deserialize(response->node, output) < 0) { \ + esxVI_##_type##_Deserialize##_suffix(response->node, output) < 0) { \ goto cleanup; \ } -#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__OptionalList(_type) \ +#define ESX_VI__METHOD__DESERIALIZE_OUTPUT__OptionalList(_type, _suffix) \ if (response->node != NULL && \ esxVI_##_type##_DeserializeList(response->node, output) < 0) { \ goto cleanup; \ @@ -103,7 +103,8 @@ #define ESX_VI__METHOD(_name, _this_from_service, _parameters, _output_type, \ - _occurrence, _validate, _serialize) \ + _deserialize_suffix, _occurrence, _validate, \ + _serialize) \ int \ esxVI_##_name _parameters \ { \ @@ -139,7 +140,8 @@ goto cleanup; \ } \ \ - ESX_VI__METHOD__DESERIALIZE_OUTPUT__##_occurrence(_output_type) \ + ESX_VI__METHOD__DESERIALIZE_OUTPUT__##_occurrence \ + (_output_type, _deserialize_suffix) \ \ result = 0; \ \ @@ -296,7 +298,7 @@ ESX_VI__METHOD(ValidateMigration, /* special _this */, esxVI_ManagedObjectReference *pool, /* optional */ esxVI_ManagedObjectReference *host, /* optional */ esxVI_Event **output), /* optional, list */ - Event, OptionalList, + Event, /* nothing */, OptionalList, { ESX_VI__METHOD__PARAMETER__REQUIRE(vm) }, -- 1.7.0.4

2 4

[libvirt] [PATCH] buf: Fix possible infinite loop in EscapeString, VSnprintf
by Cole Robinson 01 Sep '10

01 Sep '10

The current code will go into an infinite loop if the printf generated string is >= 1000, AND exactly 1 character smaller than the amount of free space in the buffer. When this happens, we are dropped into the loop body, but nothing will actually change, because count == (buf->size - buf->use - 1), and virBufferGrow returns unchanged if count < (buf->size - buf->use) Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle the NULL byte for us anyways, so we shouldn't need to manually accomodate for it. Here's a bug where we are actually hitting this issue: https://bugzilla.redhat.com/show_bug.cgi?id=602772 Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/util/buf.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/util/buf.c b/src/util/buf.c index fc1217b..734b8f6 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -236,11 +236,11 @@ virBufferVSprintf(const virBufferPtr buf, const char *format, ...) virBufferGrow(buf, 100) < 0) return; - size = buf->size - buf->use - 1; + size = buf->size - buf->use; va_start(argptr, format); va_copy(locarg, argptr); while (((count = vsnprintf(&buf->content[buf->use], size, format, - locarg)) < 0) || (count >= size - 1)) { + locarg)) < 0) || (count >= size)) { buf->content[buf->use] = 0; va_end(locarg); @@ -250,13 +250,12 @@ virBufferVSprintf(const virBufferPtr buf, const char *format, ...) return; } - size = buf->size - buf->use - 1; + size = buf->size - buf->use; va_copy(locarg, argptr); } va_end(argptr); va_end(locarg); buf->use += count; - buf->content[buf->use] = '\0'; } /** @@ -340,19 +339,18 @@ virBufferEscapeString(const virBufferPtr buf, const char *format, const char *st return; } - size = buf->size - buf->use - 1; + size = buf->size - buf->use; while (((count = snprintf(&buf->content[buf->use], size, format, - (char *)escaped)) < 0) || (count >= size - 1)) { + (char *)escaped)) < 0) || (count >= size)) { buf->content[buf->use] = 0; grow_size = (count > 1000) ? count : 1000; if (virBufferGrow(buf, grow_size) < 0) { VIR_FREE(escaped); return; } - size = buf->size - buf->use - 1; + size = buf->size - buf->use; } buf->use += count; - buf->content[buf->use] = '\0'; VIR_FREE(escaped); } -- 1.7.2.1

2 2

[libvirt] Next release 0.8.4 proposed schedule
by Daniel Veillard 01 Sep '10

01 Sep '10

So it's already September and while back from vacations I didn't yet fully scan all the list for potential patches. Still I'm tempted to try to get a release by the 10 which mean we would enter the freeze this week-end. I guess it would be 0.8.4 since there is no huge feature or change commited since 0.8.3. Is there anything important shich should land before the next release and that I may have overlooked ? There is also a number of patches which were sent but not ACK'ed or pushed but since I didn't finished the full list, I don't know yet if they got reposted, I will try to finish the scan tomorrow. But if you have patches blocked missing review please raise them ! thanks, 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/

1 0

[libvirt] command line chroot in a VM
by Mihamina Rakotomandimby 01 Sep '10

01 Sep '10

Manao ahoana, Hello, Bonjour, >From the host, I would like to perform a batch operation on the guests. For example: # virsh list 1 001 2 002 3 003 All guests are KVM, Debian Lenny I want to for GUEST in $(virsh list | awk '{<blah blah>}') do ????? ${GUEST} apt-get update ????? ${GUEST} apt-get upgrade done And then I just have to *read* and answer "yes" or "no" to the apt questions. The host is a Debian Lenny, but I can backport some packages if needed. What would be your suggestions? - libguestfs? (I see an unstable debian package existing) - ...? Misaotra, Thanks, Merci. -- Architecte Informatique chez Blueline/Gulfsat: Administration Systeme, Recherche & Developpement +261 34 56 000 19

2 1

[libvirt] Need help, how can I access to qemu-monitor with libvirt 0.8.1
by Lai Jiangshan 01 Sep '10

01 Sep '10

Hi, all I encounter this problem when I try to send NMI to guest. I don't know how. I think if I can access to the qemu-monitor, I can just do this command in the qemu-monitor: "nmi <cpu#>". But I don't know how to access to qemu-monitor, I think I can get help here. I noticed that the guest is started by libvirt with these parameters: -chardev socket,id=monitor,path=/var/lib/libvirt/qemu/vm~laijs.monitor,server,nowait \ -mon chardev=monitor,mode=control So I wrote a simple program and tried to access qemu-monitor by accessing the socket /var/lib/libvirt/qemu/vm~laijs.monitor. It failed, because I don't know how to send commands with "mode=control", I don't know the protocol for exchanging data with this file. (If I start guest with the same parameters as libvirt but using "mode=readline", I successfully sent commands to qemu-monitor by my simple program, I just send text commands) I noticed that libvirt 0.8.3 supports remote protocol and arbitrary qemu commands, but I did not found any document, how can I use it? Thanks, Lai, (I'm not in this email-list:libvir-list@redhat.com, please ensure my email address in your cc-list when you reply, thanks)

2 2