[libvirt] [PATCH] build: avoid unused variable warning
by Eric Blake
* 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
14 years, 3 months
[libvirt] [PATCH Java] Remove non-thread-safe error reporting
by Matthias Bolte
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
14 years, 3 months
[libvirt] [PATCH] esx: Rework datastore path parsing and handling
by Matthias Bolte
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
14 years, 3 months
[libvirt] [PATCH 0/7] round 2 of sprintf cleanups
by Eric Blake
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
14 years, 3 months
[libvirt] [PATCH] esx: Fix generator for string return values
by Matthias Bolte
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
14 years, 3 months
[libvirt] [PATCH] buf: Fix possible infinite loop in EscapeString, VSnprintf
by Cole Robinson
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
14 years, 3 months
[libvirt] Next release 0.8.4 proposed schedule
by Daniel Veillard
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/
14 years, 3 months
[libvirt] command line chroot in a VM
by Mihamina Rakotomandimby
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
14 years, 3 months
[libvirt] Need help, how can I access to qemu-monitor with libvirt 0.8.1
by Lai Jiangshan
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)
14 years, 3 months