- Devel - libvirt List Archives

[libvirt] [PATCH 0/2] Fix race in qemu driver wrt vnc port allocation
by Jim Fehlig 18 May '10

18 May '10

The qemu driver contains a subtle race in the logic to find next available vnc port. Currently it iterates through all available ports and returns the first for which bind(2) succeeds. However it is possible that a previously issued port has not yet been bound by qemu, resulting in the same port used for a subsequent domain. The issue was briefly discussed on IRC and the consensus there was to track port allocation with a bitmap. The first patch adds some simple bitmap operations to utils. I added them to the generic util.[ch] for lack of a more suitable file. Alternate suggestions welcome. The second patch uses the bitmap to track vnc port allocations. The race was consistently encountered in HA environments [1]. This patch series has been successfully tested in such environment. [1] http://www.linux-ha.org/wiki/VirtualDomain_%28resource_agent%29#VNC_port_au… Jim Fehlig (2): Add simple bitmap operations to utils Fix race in finding available vnc port src/qemu/qemu_driver.c | 22 ++++++++++++++++++++++ src/util/util.c | 38 ++++++++++++++++++++++++++++++++++++++ src/util/util.h | 10 ++++++++++ 3 files changed, 70 insertions(+), 0 deletions(-)

4 9

[libvirt] [PATCH] qemudDomainSetVcpus: avoid NULL-deref on failed uuid look-up
by Jim Meyering 18 May '10

18 May '10

Here's the fix, followed by the endjob/cleanup code: >From d696f6067d6e802714adbf3e36bf53c9fcf3ec76 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 19:28:44 +0200 Subject: [PATCH] qemudDomainSetVcpus: avoid NULL-deref on failed uuid look-up * src/qemu/qemu_driver.c (qemudDomainSetVcpus): Upon look-up failure, i.e., vm==NULL, goto cleanup, rather than to "endjob", superficially since the latter would dereference vm, but more fundamentally because we certainly don't want to call qemuDomainObjEndJob before we've even attempted qemuDomainObjBeginJob. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 582fdee..16a9646 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5526,11 +5526,11 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto endjob; + goto cleanup; } if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; -- 1.7.1.250.g7d1e8 Here's the end of that function. Note that in endjob's code, vm is dereferenced: ... ret = qemudDomainHotplugVcpus(vm, nvcpus); endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; cleanup: if (vm) virDomainObjUnlock(vm); return ret; }

2 2

[libvirt] [PATCH] docs: mention domain <clock> improvements
by Eric Blake 18 May '10

18 May '10

Add documentation for features added a while ago. * docs/formatdomain.html.in (Time keeping): Update documentation of <clock> element to match 0.8.0 addition. --- Please double-check me. The conversion from schemas/domain.rng to text, coupled with list traffic about the patches, was pretty overwhelming. I hope I haven't over-trivialized the various options, but am pretty sure I didn't omit any. docs/formatdomain.html.in | 76 ++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 71 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a23663a..be0446d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -395,13 +395,18 @@ <pre> ... - <clock offset="localtime"/> + <clock offset="localtime"> + <timer name="rtc" tickpolicy="catchup" track="guest"> + <catchup threshold=123 slew=120 limit=10000/> + </timer> + <timer name="pit" tickpolicy="none"/> + </clock> ...</pre> <dl> <dt><code>clock</code></dt> <dd> - The <code>offset</code> attribute takes three possible + The <code>offset</code> attribute takes four possible values, allowing fine grained control over how the guest clock is synchronized to the host. NB, not all hypervisors support all modes. @@ -419,6 +424,7 @@ <dd> The guest clock will be synchronized to the requested timezone using the <code>timezone</code> attribute. + Since 0.7.7 </dd> <dt><code>variable</code></dt> <dd> @@ -428,14 +434,74 @@ The guest is free to adjust the RTC over time an expect that it will be honoured at next reboot. This is in contrast to 'utc' mode, where the RTC adjustments are - lost at each reboot. + lost at each reboot. Since 0.7.7 </dd> </dl> - NB, at time of writing, only QEMU supports the variable - clock mode, or custom timezones. + A <code>clock</code> may have zero or more + <code>timer</code>sub-elements. Since + 0.8.0 </dd> + <dt><code>timer</code></dt> + <dd> + + Each timer element requires a <code>name</code> attribute, + and has other optional attributes that depend on + the <code>name</code> specified. Various hypervisors + support different combinations of attributes. + + <dl> + <dt><code>name</code></dt> + <dd> + The <code>name</code> attribute selects which timer is + being modified, and can be one of "platform", "pit", + "rtc", "hpet", or "tsc". + </dd> + <dt><code>track</code></dt> + <dd> + The <code>track</code> attribute specifies what the timer + tracks, and can be "boot", "guest", or "wall". + Only valid for <code>name="rtc"</code> + or <code>name="platform"</code>. + </dd> + <dt><code>tickpolicy</code></dt> + <dd> + The <code>tickpolicy</code> attribute determines how + missed ticks in the guest are handled, and can be "delay", + "catchup", "merge", or "discard". If the policy is + "catchup", there can be further details in + the <code>catchup</code> sub-element. + <dl> + <dt><code>catchup</code></dt> + <dd> + The <code>catchup</code> element has three optional + attributes, each a positive integer. The attributes + are <code>threshold</code>, <code>slew</code>, + and <code>limit</code>. + </dd> + </dl> + </dd> + <dt><code>frequency</code></dt> + <dd> + The <code>frequency</code> attribute is an unsigned + integer specifying the frequency at + which <code>name="tsc"</code> runs. + </dd> + <dt><code>mode</code></dt> + <dd> + The <code>mode</code> attribute controls how + the <code>name="tsc"</code> timer is managed, and can be + "auto", "native", "emulate", "paravirt", or "smpsafe". + Other timers are always emulated. + </dd> + <dt><code>present</code></dt> + <dd> + The <code>present</code> attribute can be "yes" or "no" to + specify whether a particular timer is available to the guest. + </dd> + </dl> + </dd> </dl> <h3><a name="elementsDevices">Devices</a></h3> -- 1.7.0.1

3 3

[libvirt] [PATCH] esx: Add read-only storage pool access
by Matthias Bolte 18 May '10

18 May '10

Add VIR_STORAGE_POOL_INACCESSIBLE to denote a running but inaccessible storage pool. For example an NFS pool is inaccessible when the NFS server is currently unreachable. Add CIFS to the list of network file systems because ESX distinguishes between NFS and CIFS. Alter the esxVI_ProductVersion enum in a way that allows to check for product type by masking. Make esxVI_*_CastFromAnyType dynamically dispatched in order to handle the DatastoreInfo type and inheriting types properly. Allow esxVI_X_DynamicCast to be called successfully on objects with type X. This is necessary for handling DatastoreInfo and inheriting types properly. --- include/libvirt/libvirt.h.in | 1 + po/POTFILES.in | 1 + src/conf/storage_conf.c | 2 +- src/conf/storage_conf.h | 1 + src/esx/esx_storage_driver.c | 606 +++++++++++++++++++++++++++++++++++++++- src/esx/esx_vi.c | 126 ++++++++- src/esx/esx_vi.h | 26 ++- src/esx/esx_vi_generator.input | 59 ++++ src/esx/esx_vi_generator.py | 20 ++- src/esx/esx_vi_types.c | 44 ++- tools/virsh.c | 4 + 11 files changed, 845 insertions(+), 45 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index db107cc..838028d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1118,6 +1118,7 @@ typedef enum { VIR_STORAGE_POOL_BUILDING = 1, /* Initializing pool, not available */ VIR_STORAGE_POOL_RUNNING = 2, /* Running normally */ VIR_STORAGE_POOL_DEGRADED = 3, /* Running degraded */ + VIR_STORAGE_POOL_INACCESSIBLE = 4, /* Running, but not accessible */ } virStoragePoolState; diff --git a/po/POTFILES.in b/po/POTFILES.in index 88218bd..e047b1b 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -19,6 +19,7 @@ src/cpu/cpu_map.c src/cpu/cpu_x86.c src/datatypes.c src/esx/esx_driver.c +src/esx/esx_storage_driver.c src/esx/esx_util.c src/esx/esx_vi.c src/esx/esx_vi_methods.c diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6218e02..c1595aa 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -61,7 +61,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatFileSystem, VIR_ENUM_IMPL(virStoragePoolFormatFileSystemNet, VIR_STORAGE_POOL_NETFS_LAST, - "auto", "nfs", "glusterfs") + "auto", "nfs", "glusterfs", "cifs") VIR_ENUM_IMPL(virStoragePoolFormatDisk, VIR_STORAGE_POOL_DISK_LAST, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 1c9ba04..5813489 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -405,6 +405,7 @@ enum virStoragePoolFormatFileSystemNet { VIR_STORAGE_POOL_NETFS_AUTO = 0, VIR_STORAGE_POOL_NETFS_NFS, VIR_STORAGE_POOL_NETFS_GLUSTERFS, + VIR_STORAGE_POOL_NETFS_CIFS, VIR_STORAGE_POOL_NETFS_LAST, }; VIR_ENUM_DECL(virStoragePoolFormatFileSystemNet) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 97b92a5..c82667b 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -29,6 +29,7 @@ #include "memory.h" #include "logging.h" #include "uuid.h" +#include "storage_conf.h" #include "esx_private.h" #include "esx_storage_driver.h" #include "esx_vi.h" @@ -65,17 +66,596 @@ esxStorageClose(virConnectPtr conn) +static int +esxNumberOfStoragePools(virConnectPtr conn) +{ + int result = 0; + esxPrivate *priv = conn->storagePrivateData; + esxVI_ObjectContent *datastoreList = NULL; + esxVI_ObjectContent *datastore = NULL; + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_LookupObjectContentByType(priv->host, priv->host->datacenter, + "Datastore", NULL, esxVI_Boolean_True, + &datastoreList) < 0) { + goto failure; + } + + for (datastore = datastoreList; datastore != NULL; + datastore = datastore->_next) { + ++result; + } + + cleanup: + esxVI_ObjectContent_Free(&datastoreList); + + return result; + + failure: + result = -1; + + goto cleanup; +} + + + +static int +esxListStoragePools(virConnectPtr conn, char **const names, int maxnames) +{ + esxPrivate *priv = conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_ObjectContent *datastoreList = NULL; + esxVI_ObjectContent *datastore = NULL; + int count = 0; + int i; + + if (names == NULL || maxnames < 0) { + ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", _("Invalid argument")); + return -1; + } + + if (maxnames == 0) { + return 0; + } + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_String_AppendValueToList(&propertyNameList, + "summary.name") < 0 || + esxVI_LookupObjectContentByType(priv->host, priv->host->datacenter, + "Datastore", propertyNameList, + esxVI_Boolean_True, + &datastoreList) < 0) { + goto failure; + } + + for (datastore = datastoreList; datastore != NULL; + datastore = datastore->_next) { + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "summary.name")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + goto failure; + } + + names[count] = strdup(dynamicProperty->val->string); + + if (names[count] == NULL) { + virReportOOMError(); + goto failure; + } + + ++count; + break; + } else { + VIR_WARN("Unexpected '%s' property", dynamicProperty->name); + } + } + } + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastoreList); + + return count; + + failure: + for (i = 0; i < count; ++i) { + VIR_FREE(names[i]); + } + + count = -1; + + goto cleanup; +} + + + +static int +esxNumberOfDefinedStoragePools(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + /* ESX storage pools are always active */ + return 0; +} + + + +static int +esxListDefinedStoragePools(virConnectPtr conn ATTRIBUTE_UNUSED, + char **const names ATTRIBUTE_UNUSED, + int maxnames ATTRIBUTE_UNUSED) +{ + /* ESX storage pools are always active */ + return 0; +} + + + +static virStoragePoolPtr +esxStoragePoolLookupByName(virConnectPtr conn, const char *name) +{ + esxPrivate *priv = conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + esxVI_Boolean accessible = esxVI_Boolean_Undefined; + char *summaryUrl = NULL; + char *suffix = NULL; + int suffixLength; + char uuid_string[VIR_UUID_STRING_BUFLEN] = "00000000-00000000-0000-000000000000"; + unsigned char uuid[VIR_UUID_BUFLEN]; + char *realName = NULL; + virStoragePoolPtr pool = NULL; + + if (esxVI_EnsureSession(priv->host) < 0) { + goto cleanup; + } + + if (esxVI_String_AppendValueListToList(&propertyNameList, + "summary.accessible\0" + "summary.name\0" + "summary.url\0") < 0 || + esxVI_LookupDatastoreByName(priv->host, name, + propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0 || + esxVI_GetBoolean(datastore, "summary.accessible", + &accessible, esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + /* + * Datastores don't have a UUID. We can use the 'summary.url' property as + * source for a "UUID" on ESX, because the property value has this format: + * + * summary.url = /vmfs/volumes/4b0beca7-7fd401f3-1d7f-000ae484a6a3 + * summary.url = /vmfs/volumes/b24b7a78-9d82b4f5 (short format) + * + * The 'summary.url' property comes in two forms, with a complete "UUID" + * and a short "UUID". + * + * But this trailing "UUID" is not guaranteed to be there. On the other + * hand we already rely on another implementation detail of the ESX server: + * The object name of virtual machine contains an integer, we use that as + * domain ID. + * + * The 'summary.url' property of an inaccessible datastore is invalid. + */ + if (accessible == esxVI_Boolean_True && + priv->host->productVersion & esxVI_ProductVersion_ESX) { + if (esxVI_GetStringValue(datastore, "summary.url", &summaryUrl, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + if ((suffix = STRSKIP(summaryUrl, "/vmfs/volumes/")) == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Datastore URL '%s' has unexpected prefix, " + "expecting '/vmfs/volumes/' prefix"), summaryUrl); + goto cleanup; + } + + suffixLength = strlen(suffix); + + if ((suffixLength == 35 && /* = strlen("4b0beca7-7fd401f3-1d7f-000ae484a6a3") */ + suffix[8] == '-' && suffix[17] == '-' && suffix[22] == '-') || + (suffixLength == 17 && /* = strlen("b24b7a78-9d82b4f5") */ + suffix[8] == '-')) { + /* + * Intentionally use memcpy here, because we want to be able to + * replace a prefix of the initial Zero-UUID. virStrncpy would + * null-terminate the string in an unwanted place. + */ + memcpy(uuid_string, suffix, suffixLength); + } else { + VIR_WARN("Datastore URL suffix '%s' has unexpected format, " + "cannot deduce a UUID from it", suffix); + } + } + + if (virUUIDParse(uuid_string, uuid) < 0) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not parse UUID from string '%s'"), + uuid_string); + goto cleanup; + } + + if (esxVI_GetStringValue(datastore, "summary.name", &realName, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + pool = virGetStoragePool(conn, realName, uuid); + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + + return pool; +} + + + +static virStoragePoolPtr +esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) +{ + esxPrivate *priv = conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; + char *name = NULL; + virStoragePoolPtr pool = NULL; + + if (! (priv->host->productVersion & esxVI_ProductVersion_ESX)) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Lookup by UUID is supported on ESX only")); + return NULL; + } + + if (esxVI_EnsureSession(priv->host) < 0) { + goto cleanup; + } + + /* + * Convert from UUID to datastore URL form by stripping the second '-': + * + * <---- 14 ----><-------- 22 --------> <---- 13 ---><-------- 22 --------> + * 4b0beca7-7fd4-01f3-1d7f-000ae484a6a3 -> 4b0beca7-7fd401f3-1d7f-000ae484a6a3 + */ + virUUIDFormat(uuid, uuid_string); + memmove(uuid_string + 13, uuid_string + 14, 22 + 1); + + /* + * Use esxVI_LookupDatastoreByName because it also does try to match "UUID" + * part of the 'summary.url' property if there is no name match. + */ + if (esxVI_String_AppendValueToList(&propertyNameList, "summary.name") < 0 || + esxVI_LookupDatastoreByName(priv->host, uuid_string, + propertyNameList, &datastore, + esxVI_Occurrence_OptionalItem) < 0) { + goto cleanup; + } + + /* + * If the first try didn't succeed and the trailing 16 digits are zero then + * the "UUID" could be a short one. Strip the 16 zeros and try again: + * + * <------ 17 -----> <------ 17 -----> + * b24b7a78-9d82b4f5-0000-000000000000 -> b24b7a78-9d82b4f5 + */ + if (datastore == NULL && STREQ(uuid_string + 17, "-0000-000000000000")) { + uuid_string[17] = '\0'; + + if (esxVI_LookupDatastoreByName(priv->host, uuid_string, + propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + } + + if (datastore == NULL) { + virUUIDFormat(uuid, uuid_string); + + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not find datastore with UUID '%s'"), + uuid_string); + + goto cleanup; + } + + if (esxVI_GetStringValue(datastore, "summary.name", &name, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + pool = virGetStoragePool(conn, name, uuid); + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + + return pool; +} + + + +static int +esxStoragePoolRefresh(virStoragePoolPtr pool, + unsigned int flags ATTRIBUTE_UNUSED) +{ + int result = 0; + esxPrivate *priv = pool->conn->storagePrivateData; + esxVI_ObjectContent *datastore = NULL; + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_LookupDatastoreByName(priv->host, pool->name, NULL, &datastore, + esxVI_Occurrence_RequiredItem) < 0 || + esxVI_RefreshDatastore(priv->host, datastore->obj) < 0) { + goto failure; + } + + cleanup: + esxVI_ObjectContent_Free(&datastore); + + return result; + + failure: + result = -1; + + goto cleanup; +} + + + +static int +esxStoragePoolGetInfo(virStoragePoolPtr pool, virStoragePoolInfoPtr info) +{ + int result = 0; + esxPrivate *priv = pool->conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_Boolean accessible = esxVI_Boolean_Undefined; + + memset(info, 0, sizeof (*info)); + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_String_AppendValueListToList(&propertyNameList, + "summary.accessible\0" + "summary.capacity\0" + "summary.freeSpace\0") < 0 || + esxVI_LookupDatastoreByName(priv->host, pool->name, + propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0 || + esxVI_GetBoolean(datastore, "summary.accessible", + &accessible, esxVI_Occurrence_RequiredItem) < 0) { + goto failure; + } + + if (accessible == esxVI_Boolean_True) { + info->state = VIR_STORAGE_POOL_RUNNING; + + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "summary.capacity")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_Long) < 0) { + goto failure; + } + + info->capacity = dynamicProperty->val->int64; + } else if (STREQ(dynamicProperty->name, "summary.freeSpace")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_Long) < 0) { + goto failure; + } + + info->available = dynamicProperty->val->int64; + } + } + + info->allocation = info->capacity - info->available; + } else { + info->state = VIR_STORAGE_POOL_INACCESSIBLE; + } + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + + return result; + + failure: + memset(info, 0, sizeof (*info)); + + result = -1; + + goto cleanup; +} + + + +static char * +esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) +{ + esxPrivate *priv = pool->conn->storagePrivateData; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_Boolean accessible = esxVI_Boolean_Undefined; + virStoragePoolDef def; + esxVI_DatastoreInfo *info = NULL; + esxVI_LocalDatastoreInfo *localInfo = NULL; + esxVI_NasDatastoreInfo *nasInfo = NULL; + esxVI_VmfsDatastoreInfo *vmfsInfo = NULL; + char *xml = NULL; + + virCheckFlags(0, NULL); + + memset(&def, 0, sizeof (def)); + + if (esxVI_EnsureSession(priv->host) < 0) { + goto failure; + } + + if (esxVI_String_AppendValueListToList(&propertyNameList, + "summary.accessible\0" + "summary.capacity\0" + "summary.freeSpace\0" + "info\0") < 0 || + esxVI_LookupDatastoreByName(priv->host, pool->name, + propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0 || + esxVI_GetBoolean(datastore, "summary.accessible", + &accessible, esxVI_Occurrence_RequiredItem) < 0) { + goto failure; + } + + def.name = pool->name; + memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + + if (accessible == esxVI_Boolean_True) { + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "summary.capacity")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_Long) < 0) { + goto failure; + } + + def.capacity = dynamicProperty->val->int64; + } else if (STREQ(dynamicProperty->name, "summary.freeSpace")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_Long) < 0) { + goto failure; + } + + def.available = dynamicProperty->val->int64; + } else if (STREQ(dynamicProperty->name, "info")) { + if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, + &info) < 0) { + goto failure; + } + } + } + + def.allocation = def.capacity - def.available; + + /* See vSphere API documentation about HostDatastoreSystem for details */ + if ((localInfo = esxVI_LocalDatastoreInfo_DynamicCast(info)) != NULL) { + def.type = VIR_STORAGE_POOL_DIR; + def.target.path = localInfo->path; + } else if ((nasInfo = esxVI_NasDatastoreInfo_DynamicCast(info)) != NULL) { + def.type = VIR_STORAGE_POOL_NETFS; + def.source.host.name = nasInfo->nas->remoteHost; + def.source.dir = nasInfo->nas->remotePath; + + if (STRCASEEQ(nasInfo->nas->type, "NFS")) { + def.source.format = VIR_STORAGE_POOL_NETFS_NFS; + } else if (STRCASEEQ(nasInfo->nas->type, "CIFS")) { + def.source.format = VIR_STORAGE_POOL_NETFS_CIFS; + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Datastore has unexpected type '%s'"), + nasInfo->nas->type); + goto failure; + } + } else if ((vmfsInfo = esxVI_VmfsDatastoreInfo_DynamicCast(info)) != NULL) { + def.type = VIR_STORAGE_POOL_FS; + /* FIXME */ + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("DatastoreInfo has unexpected type")); + goto failure; + } + } + + xml = virStoragePoolDefFormat(&def); + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + esxVI_DatastoreInfo_Free(&info); + + return xml; + + failure: + VIR_FREE(xml); + + goto cleanup; +} + + + +static int +esxStoragePoolGetAutostart(virStoragePoolPtr pool ATTRIBUTE_UNUSED, + int *autostart) +{ + /* ESX storage pools are always active */ + *autostart = 1; + + return 0; +} + + + +static int +esxStoragePoolSetAutostart(virStoragePoolPtr pool ATTRIBUTE_UNUSED, + int autostart) +{ + /* Just accept autostart activation, but fail on autostart deactivation */ + autostart = (autostart != 0); + + if (! autostart) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot deactivate storage pool autostart")); + return -1; + } + + return 0; +} + + + +static int +esxStoragePoolIsActive(virStoragePoolPtr pool ATTRIBUTE_UNUSED) +{ + /* ESX storage pools are always active */ + return 1; +} + + + +static int +esxStoragePoolIsPersistent(virStoragePoolPtr pool ATTRIBUTE_UNUSED) +{ + /* ESX has no concept of transient pools, so all of them are persistent */ + return 1; +} + + static virStorageDriver esxStorageDriver = { "ESX", /* name */ esxStorageOpen, /* open */ esxStorageClose, /* close */ - NULL, /* numOfPools */ - NULL, /* listPools */ - NULL, /* numOfDefinedPools */ - NULL, /* listDefinedPools */ + esxNumberOfStoragePools, /* numOfPools */ + esxListStoragePools, /* listPools */ + esxNumberOfDefinedStoragePools, /* numOfDefinedPools */ + esxListDefinedStoragePools, /* listDefinedPools */ NULL, /* findPoolSources */ - NULL, /* poolLookupByName */ - NULL, /* poolLookupByUUID */ + esxStoragePoolLookupByName, /* poolLookupByName */ + esxStoragePoolLookupByUUID, /* poolLookupByUUID */ NULL, /* poolLookupByVolume */ NULL, /* poolCreateXML */ NULL, /* poolDefineXML */ @@ -84,11 +664,11 @@ static virStorageDriver esxStorageDriver = { NULL, /* poolCreate */ NULL, /* poolDestroy */ NULL, /* poolDelete */ - NULL, /* poolRefresh */ - NULL, /* poolGetInfo */ - NULL, /* poolGetXMLDesc */ - NULL, /* poolGetAutostart */ - NULL, /* poolSetAutostart */ + esxStoragePoolRefresh, /* poolRefresh */ + esxStoragePoolGetInfo, /* poolGetInfo */ + esxStoragePoolGetXMLDesc, /* poolGetXMLDesc */ + esxStoragePoolGetAutostart, /* poolGetAutostart */ + esxStoragePoolSetAutostart, /* poolSetAutostart */ NULL, /* poolNumOfVolumes */ NULL, /* poolListVolumes */ NULL, /* volLookupByName */ @@ -101,8 +681,8 @@ static virStorageDriver esxStorageDriver = { NULL, /* volGetInfo */ NULL, /* volGetXMLDesc */ NULL, /* volGetPath */ - NULL, /* poolIsActive */ - NULL, /* poolIsPersistent */ + esxStoragePoolIsActive, /* poolIsActive */ + esxStoragePoolIsPersistent, /* poolIsPersistent */ }; diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 966ef85..7a39fc2 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -1530,6 +1530,114 @@ esxVI_GetVirtualMachineQuestionInfo int +esxVI_GetBoolean(esxVI_ObjectContent *objectContent, const char *propertyName, + esxVI_Boolean *value, esxVI_Occurrence occurence) +{ + esxVI_DynamicProperty *dynamicProperty; + + if (value == NULL || *value != esxVI_Boolean_Undefined) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, propertyName)) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_Boolean) < 0) { + return -1; + } + + *value = dynamicProperty->val->boolean; + break; + } + } + + if (*value == esxVI_Boolean_Undefined && + occurence == esxVI_Occurrence_RequiredItem) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Missing '%s' property"), propertyName); + return -1; + } + + return 0; +} + + + +int +esxVI_GetStringValue(esxVI_ObjectContent *objectContent, + const char *propertyName, + char **value, esxVI_Occurrence occurence) +{ + esxVI_DynamicProperty *dynamicProperty; + + if (value == NULL || *value != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, propertyName)) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + return -1; + } + + *value = dynamicProperty->val->string; + break; + } + } + + if (*value == NULL && occurence == esxVI_Occurrence_RequiredItem) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Missing '%s' property"), propertyName); + return -1; + } + + return 0; +} + + + +int +esxVI_GetManagedObjectReference(esxVI_ObjectContent *objectContent, + const char *propertyName, + esxVI_ManagedObjectReference **value, + esxVI_Occurrence occurence) +{ + esxVI_DynamicProperty *dynamicProperty; + + if (value == NULL || *value != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + for (dynamicProperty = objectContent->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, propertyName)) { + if (esxVI_ManagedObjectReference_CastFromAnyType + (dynamicProperty->val, value) < 0) { + return -1; + } + + break; + } + } + + if (*value == NULL && occurence == esxVI_Occurrence_RequiredItem) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Missing '%s' property"), propertyName); + return -1; + } + + return 0; +} + + + +int esxVI_LookupNumberOfDomainsByPowerState(esxVI_Context *ctx, esxVI_VirtualMachinePowerState powerState, esxVI_Boolean inverse) @@ -2161,7 +2269,7 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, esxVI_ObjectContent *candidate = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; esxVI_Boolean accessible = esxVI_Boolean_Undefined; - size_t offset = strlen("/vmfs/volumes/"); + int offset = 14; /* = strlen("/vmfs/volumes/") */ int numInaccessibleDatastores = 0; if (datastore == NULL || *datastore != NULL) { @@ -2227,9 +2335,7 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, for (dynamicProperty = candidate->propSet; dynamicProperty != NULL; dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "summary.accessible")) { - /* Ignore it */ - } else if (STREQ(dynamicProperty->name, "summary.name")) { + if (STREQ(dynamicProperty->name, "summary.name")) { if (esxVI_AnyType_ExpectType(dynamicProperty->val, esxVI_Type_String) < 0) { goto failure; @@ -2244,7 +2350,8 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, /* Found datastore with matching name */ goto cleanup; } - } else if (STREQ(dynamicProperty->name, "summary.url")) { + } else if (STREQ(dynamicProperty->name, "summary.url") && + ctx->productVersion & esxVI_ProductVersion_ESX) { if (accessible == esxVI_Boolean_False) { /* * The 'summary.url' property of an inaccessible datastore @@ -2276,8 +2383,6 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, /* Found datastore with matching URL suffix */ goto cleanup; } - } else { - VIR_WARN("Unexpected '%s' property", dynamicProperty->name); } } } @@ -2309,9 +2414,10 @@ esxVI_LookupDatastoreByName(esxVI_Context *ctx, const char *name, -int esxVI_LookupTaskInfoByTask(esxVI_Context *ctx, - esxVI_ManagedObjectReference *task, - esxVI_TaskInfo **taskInfo) +int +esxVI_LookupTaskInfoByTask(esxVI_Context *ctx, + esxVI_ManagedObjectReference *task, + esxVI_TaskInfo **taskInfo) { int result = 0; esxVI_String *propertyNameList = NULL; diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index d581a59..e84f62b 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -98,11 +98,14 @@ enum _esxVI_APIVersion { enum _esxVI_ProductVersion { esxVI_ProductVersion_Undefined = 0, - esxVI_ProductVersion_GSX20, - esxVI_ProductVersion_ESX35, - esxVI_ProductVersion_ESX40, - esxVI_ProductVersion_VPX25, - esxVI_ProductVersion_VPX40 + esxVI_ProductVersion_GSX = 0x1000, + esxVI_ProductVersion_GSX20 = 0x1001, + esxVI_ProductVersion_ESX = 0x2000, + esxVI_ProductVersion_ESX35 = 0x2001, + esxVI_ProductVersion_ESX40 = 0x2002, + esxVI_ProductVersion_VPX = 0x4000, + esxVI_ProductVersion_VPX25 = 0x4001, + esxVI_ProductVersion_VPX40 = 0x4002 }; enum _esxVI_Occurrence { @@ -272,6 +275,19 @@ int esxVI_GetVirtualMachineQuestionInfo (esxVI_ObjectContent *virtualMachine, esxVI_VirtualMachineQuestionInfo **questionInfo); +int esxVI_GetBoolean(esxVI_ObjectContent *objectContent, + const char *propertyName, + esxVI_Boolean *value, esxVI_Occurrence occurence); + +int esxVI_GetStringValue(esxVI_ObjectContent *objectContent, + const char *propertyName, + char **value, esxVI_Occurrence occurence); + +int esxVI_GetManagedObjectReference(esxVI_ObjectContent *objectContent, + const char *propertyName, + esxVI_ManagedObjectReference **value, + esxVI_Occurrence occurence); + int esxVI_LookupNumberOfDomainsByPowerState (esxVI_Context *ctx, esxVI_VirtualMachinePowerState powerState, esxVI_Boolean inverse); diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 5e5e6ba..ff65178 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -146,6 +146,14 @@ object ChoiceOption extends OptionType end +object DatastoreInfo + String name r + String url r + Long freeSpace r + Long maxFileSize r +end + + object Description String label r String summary r @@ -186,6 +194,47 @@ object HostCpuIdInfo end +object HostFileSystemVolume + String type r + String name r + Long capacity r +end + + +object HostNasVolume extends HostFileSystemVolume + String remoteHost r + String remotePath r + String userName o +end + + +object HostScsiDiskPartition + String diskName r + Int partition r +end + + +object HostVmfsVolume extends HostFileSystemVolume + Int blockSizeMb r + Int maxBlocks r + Int majorVersion r + String version r + String uuid r + HostScsiDiskPartition extent rl + Boolean vmfsUpgradable r +end + + +object LocalDatastoreInfo extends DatastoreInfo + String path o +end + + +object NasDatastoreInfo extends DatastoreInfo + HostNasVolume nas o +end + + object ObjectContent ManagedObjectReference obj r DynamicProperty propSet ol @@ -453,6 +502,11 @@ object VirtualMachineSnapshotTree end +object VmfsDatastoreInfo extends DatastoreInfo + HostVmfsVolume vmfs o +end + + # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # Methods # @@ -571,6 +625,11 @@ method ReconfigVM_Task returns ManagedObjectReference r end +method RefreshDatastore + ManagedObjectReference _this r +end + + method RegisterVM_Task returns ManagedObjectReference r ManagedObjectReference _this r String path r diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index d249e2d..8df0e80 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -382,6 +382,9 @@ class Object: self.properties = properties self.extended_by = extended_by + if self.extended_by is not None: + self.extended_by.sort(); + def generate_struct_members(self, add_banner = False, struct_gap = False): global objects_by_name @@ -765,7 +768,18 @@ class Object: # cast from any type if self.features & Object.FEATURE__ANY_TYPE: source += "/* esxVI_%s_CastFromAnyType */\n" % self.name - source += "ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(%s)\n" % self.name + source += "ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(%s,\n" % self.name + + if self.extended_by is None: + source += "{\n" + source += "})\n\n" + else: + source += "{\n" + + for extended_by in self.extended_by: + source += " ESX_VI__TEMPLATE__DISPATCH__CAST_FROM_ANY_TYPE(%s)\n" % extended_by + + source += "})\n\n" if self.features & Object.FEATURE__LIST: source += "/* esxVI_%s_CastListFromAnyType */\n" % self.name @@ -1084,7 +1098,8 @@ additional_enum_features = { "ManagedEntityStatus" : Enum.FEATURE__ANY_TYPE "VirtualMachinePowerState" : Enum.FEATURE__ANY_TYPE } -additional_object_features = { "Event" : Object.FEATURE__LIST, +additional_object_features = { "DatastoreInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, + "Event" : Object.FEATURE__LIST, "HostCpuIdInfo" : Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, "ManagedObjectReference" : Object.FEATURE__ANY_TYPE, "ObjectContent" : Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST, @@ -1224,6 +1239,7 @@ for obj in objects_by_name.values(): extended_obj.extended_by = [obj.name] else: extended_obj.extended_by.append(obj.name) + extended_obj.extended_by.sort() diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 7d2c02c..8334efd 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -183,7 +183,7 @@ -#define ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(_type) \ +#define ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(_type, _dispatch) \ int \ esxVI_##_type##_CastFromAnyType(esxVI_AnyType *anyType, \ esxVI_##_type **ptrptr) \ @@ -194,11 +194,16 @@ return -1; \ } \ \ - if (anyType->type != esxVI_Type_##_type) { \ + switch (anyType->type) { \ + _dispatch \ + \ + case esxVI_Type_##_type: \ + break; \ + \ + default: \ ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \ - "Expecting type '%s' but found '%s'", \ - esxVI_Type_ToString(esxVI_Type_##_type), \ - anyType->other); \ + _("Call to %s for unexpected type '%s'"), \ + __FUNCTION__, anyType->other); \ return -1; \ } \ \ @@ -505,7 +510,7 @@ \ default: \ ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, \ - "Call to %s for unexpected type '%s'", __FUNCTION__, \ + _("Call to %s for unexpected type '%s'"), __FUNCTION__, \ esxVI_Type_ToString(item->_type)); \ return _error_return; \ } @@ -526,6 +531,13 @@ +#define ESX_VI__TEMPLATE__DISPATCH__CAST_FROM_ANY_TYPE(_type) \ + case esxVI_Type_##_type: \ + return esxVI_##_type##_Deserialize(anyType->node, \ + (esxVI_##_type **)ptrptr); + + + #define ESX_VI__TEMPLATE__DISPATCH__SERIALIZE(_type) \ case esxVI_Type_##_type: \ return esxVI_##_type##_Serialize((esxVI_##_type *)item, element, \ @@ -540,6 +552,13 @@ +#define ESX_VI__TEMPLATE__DYNAMIC_CAST__ACCEPT(__type) \ + if (((esxVI_Object *)item)->_type == esxVI_Type_##__type) { \ + return item; \ + } + + + #define ESX_VI__TEMPLATE__DYNAMIC_CAST(__type, _accept) \ esxVI_##__type * \ esxVI_##__type##_DynamicCast(void *item) \ @@ -550,6 +569,8 @@ return NULL; \ } \ \ + ESX_VI__TEMPLATE__DYNAMIC_CAST__ACCEPT(__type) \ + \ _accept \ \ return NULL; \ @@ -557,13 +578,6 @@ -#define ESX_VI__TEMPLATE__DYNAMIC_CAST__ACCEPT(__type) \ - if (((esxVI_Object *)item)->_type == esxVI_Type_##__type) { \ - return item; \ - } - - - #define ESX_VI__TEMPLATE__DYNAMIC_SERIALIZE(__type, _dispatch, _serialize) \ ESX_VI__TEMPLATE__SERIALIZE_EXTRA(__type, \ ESX_VI__TEMPLATE__DISPATCH(__type, _dispatch, -1), \ @@ -1357,7 +1371,9 @@ ESX_VI__TEMPLATE__DEEP_COPY(ManagedObjectReference, ESX_VI__TEMPLATE__LIST__APPEND(ManagedObjectReference) /* esxVI_ManagedObjectReference_CastFromAnyType */ -ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(ManagedObjectReference) +ESX_VI__TEMPLATE__CAST_FROM_ANY_TYPE(ManagedObjectReference, +{ +}) /* esxVI_ManagedObjectReference_CastListFromAnyType */ ESX_VI__TEMPLATE__LIST__CAST_FROM_ANY_TYPE(ManagedObjectReference) diff --git a/tools/virsh.c b/tools/virsh.c index 0a63f1b..4108e1e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5151,6 +5151,10 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %s\n", _("State:"), _("degraded")); break; + case VIR_STORAGE_POOL_INACCESSIBLE: + vshPrint(ctl, "%-15s %s\n", _("State:"), + _("inaccessible")); + break; } if (info.state == VIR_STORAGE_POOL_RUNNING || -- 1.7.0.4

3 3

[libvirt] [PATCH] qemu_conf: fix flag value
by Eric Blake 18 May '10

18 May '10

(gdb) p/x QEMUD_CMD_FLAG_VNET_HOST $7 = 0xffffffff80000000 Oops - that meant we were incorrectly setting QEMU_CMD_FLAG_RTC_TD_HACK for qemu-kvm-0.12.3 (and probably botching a few other settings as well). * src/qemu/qemu_conf.h (QEMUD_CMD_FLAG_VNET_HOST): Avoid sign extension. * tests/qemuhelpdata/qemu-kvm-0.12.3: New file. * tests/qemuhelptest.c (mymain): Add another case. --- It didn't help that the test was truncating output to 31 bits, and getting confused with sign extension as well. src/qemu/qemu_conf.h | 8 +- tests/qemuhelpdata/qemu-kvm-0.12.3 | 216 ++++++++++++++++++++++++++++++++++++ tests/qemuhelptest.c | 39 ++++++- 3 files changed, 255 insertions(+), 8 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.3 diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index b2820f0..a101e47 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -1,7 +1,7 @@ /* * qemu_conf.h: QEMU configuration management * - * Copyright (C) 2006, 2007, 2009, 2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -64,7 +64,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_DRIVE_FORMAT = (1 << 14), /* Is -drive format= avail */ QEMUD_CMD_FLAG_VGA = (1 << 15), /* Is -vga avail */ - /* features added in qemu-0.10.0 */ + /* features added in qemu-0.10.0 or later */ QEMUD_CMD_FLAG_0_10 = (1 << 16), QEMUD_CMD_FLAG_NET_NAME = QEMUD_CMD_FLAG_0_10, /* -net ...,name=str */ QEMUD_CMD_FLAG_HOST_NET_ADD = QEMUD_CMD_FLAG_0_10, /* host_net_add monitor command */ @@ -83,8 +83,8 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_SMP_TOPOLOGY = (1 << 28), /* Is sockets=s,cores=c,threads=t available for -smp? */ QEMUD_CMD_FLAG_NETDEV = (1 << 29), /* The -netdev flag & netdev_add/remove monitor commands */ QEMUD_CMD_FLAG_RTC = (1 << 30), /* The -rtc flag for clock options */ - QEMUD_CMD_FLAG_VNET_HOST = (1 << 31), /* vnet-host support is available in qemu */ - QEMUD_CMD_FLAG_RTC_TD_HACK = (1LL << 32), /* -rtd-td-hack available */ + QEMUD_CMD_FLAG_VNET_HOST = (1LL << 31), /* vnet-host support is available in qemu */ + QEMUD_CMD_FLAG_RTC_TD_HACK = (1LL << 32), /* -rtc-td-hack available */ QEMUD_CMD_FLAG_NO_HPET = (1LL << 33), /* -no-hpet flag is supported */ QEMUD_CMD_FLAG_NO_KVM_PIT = (1LL << 34), /* -no-kvm-pit-reinjection supported */ QEMUD_CMD_FLAG_TDF = (1LL << 35), /* -tdf flag (user-mode pit catchup) */ diff --git a/tests/qemuhelpdata/qemu-kvm-0.12.3 b/tests/qemuhelpdata/qemu-kvm-0.12.3 new file mode 100644 index 0000000..54a804f --- /dev/null +++ b/tests/qemuhelpdata/qemu-kvm-0.12.3 @@ -0,0 +1,216 @@ +QEMU PC emulator version 0.12.3 (qemu-kvm-0.12.3), Copyright (c) 2003-2008 Fabrice Bellard +usage: qemu [options] [disk_image] + +'disk_image' is a raw hard image image for IDE hard disk 0 + +Standard options: +-h or -help display this help and exit +-version display version information and exit +-M machine select emulated machine (-M ? for list) +-cpu cpu select CPU (-cpu ? for list) +-smp n[,maxcpus=cpus][,cores=cores][,threads=threads][,sockets=sockets] + set the number of CPUs to 'n' [default=1] + maxcpus= maximum number of total cpus, including + offline CPUs for hotplug etc. + cores= number of CPU cores on one socket + threads= number of threads on one CPU core + sockets= number of discrete sockets in the system +-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node] +-fda/-fdb file use 'file' as floppy disk 0/1 image +-hda/-hdb file use 'file' as IDE hard disk 0/1 image +-hdc/-hdd file use 'file' as IDE hard disk 2/3 image +-cdrom file use 'file' as IDE cdrom image (cdrom is ide1 master) +-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i] + [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off] + [,cache=writethrough|writeback|none][,format=f][,serial=s] + [,addr=A][,id=name][,aio=threads|native] + [,boot=on|off] + use 'file' as a drive image +-set group.id.arg=value + set <arg> parameter for item <id> of type <group> + i.e. -set drive.$id.file=/path/to/image +-global driver.property=value + set a global default for a driver property +-mtdblock file use 'file' as on-board Flash memory image +-sd file use 'file' as SecureDigital card image +-pflash file use 'file' as a parallel flash image +-boot [order=drives][,once=drives][,menu=on|off] + 'drives': floppy (a), hard disk (c), CD-ROM (d), network (n) +-snapshot write to temporary files instead of disk image files +-m megs set virtual RAM size to megs MB [default=128] +-k language use keyboard layout (for example 'fr' for French) +-audio-help print list of audio drivers and their options +-soundhw c1,... enable audio support + and only specified sound cards (comma separated list) + use -soundhw ? to get the list of supported cards + use -soundhw all to enable all of them +-usb enable the USB driver (will be the default soon) +-usbdevice name add the host or guest USB device 'name' +-device driver[,options] add device +-name string1[,process=string2] set the name of the guest + string1 sets the window title and string2 the process name (on Linux) +-uuid %08x-%04x-%04x-%04x-%012x + specify machine UUID + +Display options: +-nographic disable graphical output and redirect serial I/Os to console +-curses use a curses/ncurses interface instead of SDL +-no-frame open SDL window without a frame and window decorations +-alt-grab use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt) +-ctrl-grab use Right-Ctrl to grab mouse (instead of Ctrl-Alt) +-no-quit disable SDL window close capability +-sdl enable SDL +-portrait rotate graphical output 90 deg left (only PXA LCD) +-vga [std|cirrus|vmware|xenfb|none] + select video card type +-full-screen start in full screen +-vnc display start a VNC server on display + +1 target only: +-win2k-hack use it when installing Windows 2000 to avoid a disk full bug +-no-fd-bootchk disable boot signature checking for floppy disks +-no-acpi disable ACPI +-no-hpet disable HPET +-balloon none disable balloon device +-balloon virtio[,addr=str] + enable virtio balloon device (default) +-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,data=file1[:file2]...] + ACPI table description +-smbios file=binary + Load SMBIOS entry from binary file +-smbios type=0[,vendor=str][,version=str][,date=str][,release=%d.%d] + Specify SMBIOS type 0 fields +-smbios type=1[,manufacturer=str][,product=str][,version=str][,serial=str] + [,uuid=uuid][,sku=str][,family=str] + Specify SMBIOS type 1 fields + +Network options: +-net nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v] + create a new Network Interface Card and connect it to VLAN 'n' +-net user[,vlan=n][,name=str][,net=addr[/mask]][,host=addr][,restrict=y|n] + [,hostname=host][,dhcpstart=addr][,dns=addr][,tftp=dir][,bootfile=f] + [,hostfwd=rule][,guestfwd=rule][,smb=dir[,smbserver=addr]] + connect the user mode network stack to VLAN 'n', configure its + DHCP server and enabled optional services +-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h] + connect the host TAP network interface to VLAN 'n' and use the + network scripts 'file' (default=/etc/qemu-ifup) + and 'dfile' (default=/etc/qemu-ifdown); + use '[down]script=no' to disable script execution; + use 'fd=h' to connect to an already opened TAP interface + use 'sndbuf=nbytes' to limit the size of the send buffer; the + default of 'sndbuf=1048576' can be disabled using 'sndbuf=0' + use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag; use + vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition + use vhost=on to enable experimental in kernel accelerator + use 'vhostfd=h' to connect to an already opened vhost net device +-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port] + connect the vlan 'n' to another VLAN using a socket connection +-net socket[,vlan=n][,name=str][,fd=h][,mcast=maddr:port] + connect the vlan 'n' to multicast maddr and port +-net dump[,vlan=n][,file=f][,len=n] + dump traffic on vlan 'n' to file 'f' (max n bytes per packet) +-net none use it alone to have zero network devices; if no -net option + is provided, the default is '-net nic -net user' +-netdev [user|tap|socket],id=str[,option][,option][,...] + +Character device options: +-chardev null,id=id +-chardev socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay] + [,server][,nowait][,telnet] (tcp) +-chardev socket,id=id,path=path[,server][,nowait][,telnet] (unix) +-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr] + [,localport=localport][,ipv4][,ipv6] +-chardev msmouse,id=id +-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]] +-chardev file,id=id,path=path +-chardev pipe,id=id,path=path +-chardev pty,id=id +-chardev stdio,id=id +-chardev tty,id=id,path=path +-chardev parport,id=id,path=path + +Bluetooth(R) options: +-bt hci,null dumb bluetooth HCI - doesn't respond to commands +-bt hci,host[:id] + use host's HCI with the given name +-bt hci[,vlan=n] + emulate a standard HCI in virtual scatternet 'n' +-bt vhci[,vlan=n] + add host computer to virtual scatternet 'n' using VHCI +-bt device:dev[,vlan=n] + emulate a bluetooth device 'dev' in scatternet 'n' + +Linux/Multiboot boot specific: +-kernel bzImage use 'bzImage' as kernel image +-append cmdline use 'cmdline' as kernel command line +-initrd file use 'file' as initial ram disk + +Debug/Expert options: +-serial dev redirect the serial port to char device 'dev' +-parallel dev redirect the parallel port to char device 'dev' +-monitor dev redirect the monitor to char device 'dev' +-qmp dev like -monitor but opens in 'control' mode. +-mon chardev=[name][,mode=readline|control][,default] +-pidfile file write PID to 'file' +-singlestep always run in singlestep mode +-S freeze CPU at startup (use 'c' to start execution) +-gdb dev wait for gdb connection on 'dev' +-s shorthand for -gdb tcp::1234 +-d item1,... output log to /tmp/qemu.log (use -d ? for a list of log items) +-hdachs c,h,s[,t] + force hard disk 0 physical geometry and the optional BIOS + translation (t=none or lba) (usually qemu can guess them) +-L path set the directory for the BIOS, VGA BIOS and keymaps +-bios file set the filename for the BIOS +-no-reboot exit instead of rebooting +-no-shutdown stop before shutdown +-loadvm [tag|id] + start right away with a saved state (loadvm in monitor) +-daemonize daemonize QEMU after initializing +-option-rom rom load a file, rom, into the option ROM space +-clock force the use of the given methods for timer alarm. + To see what timers are available use -clock ? +-rtc [base=utc|localtime|date][,clock=host|vm][,driftfix=none|slew] + set the RTC base and clock, enable drift fix for clock ticks +-icount [N|auto] + enable virtual instruction counter with 2^N clock ticks per + instruction +-watchdog i6300esb|ib700 + enable virtual hardware watchdog [default=none] +-watchdog-action reset|shutdown|poweroff|pause|debug|none + action when watchdog fires [default=reset] +-echr chr set terminal escape character instead of ctrl-a +-virtioconsole c + set virtio console +-show-cursor show cursor +-tb-size n set TB size +-incoming p prepare for incoming migration, listen on port p +-nodefaults don't create default devices. +-chroot dir Chroot to dir just before starting the VM. +-runas user Change to user id user just before starting the VM. +-readconfig <file> +-writeconfig <file> + read/write config file +-no-kvm disable KVM hardware virtualization +-no-kvm-irqchip disable KVM kernel mode PIC/IOAPIC/LAPIC +-no-kvm-pit disable KVM kernel mode PIT +-no-kvm-pit-reinjection disable KVM kernel mode PIT interrupt reinjection +-pcidevice host=bus:dev.func[,dma=none][,name=string] + expose a PCI device to the guest OS. + dma=none: don't perform any dma translations (default is to use an iommu) + 'string' is used in log output. +-enable-nesting enable support for running a VM inside the VM (AMD only) +-nvram FILE provide ia64 nvram contents +-tdf enable guest time drift compensation +-kvm-shadow-memory MEGABYTES + allocate MEGABYTES for kvm mmu shadowing +-mem-path FILE provide backing storage for guest RAM +-mem-prealloc preallocate guest memory (use with -mempath) + +During emulation, the following keys are useful: +ctrl-alt-f toggle full screen +ctrl-alt-n switch to virtual console 'n' +ctrl-alt toggle mouse and keyboard grab + +When using -nographic, press 'ctrl-a h' to get some help. diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index b8c4b91..517a8fe 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -21,13 +21,14 @@ struct testInfo { static char *progname; static char *abs_srcdir; -static void printMismatchedFlags(int got, int expect) +static void printMismatchedFlags(unsigned long long got, + unsigned long long expect) { int i; - for (i = 0 ; i < (sizeof(got)*8) ; i++) { - int gotFlag = (got & (1 << i)); - int expectFlag = (expect & (1 << i)); + for (i = 0 ; i < (sizeof(got)*CHAR_BIT) ; i++) { + unsigned long long gotFlag = (got & (1LL << i)); + unsigned long long expectFlag = (expect & (1LL << i)); if (gotFlag && !expectFlag) fprintf(stderr, "Extra flag %i\n", i); if (!gotFlag && expectFlag) @@ -245,6 +246,36 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_RTC | QEMUD_CMD_FLAG_NO_HPET, 12001, 0, 0); + DO_TEST("qemu-kvm-0.12.3", + QEMUD_CMD_FLAG_VNC_COLON | + QEMUD_CMD_FLAG_NO_REBOOT | + QEMUD_CMD_FLAG_DRIVE | + QEMUD_CMD_FLAG_DRIVE_BOOT | + QEMUD_CMD_FLAG_NAME | + QEMUD_CMD_FLAG_UUID | + QEMUD_CMD_FLAG_VNET_HDR | + QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP | + QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC | + QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | + QEMUD_CMD_FLAG_KVM | + QEMUD_CMD_FLAG_DRIVE_FORMAT | + QEMUD_CMD_FLAG_DRIVE_SERIAL | + QEMUD_CMD_FLAG_VGA | + QEMUD_CMD_FLAG_0_10 | + QEMUD_CMD_FLAG_PCIDEVICE | + QEMUD_CMD_FLAG_MEM_PATH | + QEMUD_CMD_FLAG_SDL | + QEMUD_CMD_FLAG_MIGRATE_QEMU_UNIX | + QEMUD_CMD_FLAG_CHARDEV | + QEMUD_CMD_FLAG_BALLOON | + QEMUD_CMD_FLAG_DEVICE | + QEMUD_CMD_FLAG_SMP_TOPOLOGY | + QEMUD_CMD_FLAG_RTC | + QEMUD_CMD_FLAG_VNET_HOST | + QEMUD_CMD_FLAG_NO_HPET | + QEMUD_CMD_FLAG_NO_KVM_PIT | + QEMUD_CMD_FLAG_TDF, + 12003, 1, 0); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.0.1

2 2

[libvirt] [PATCH] qemu: Clarify a couple error messages
by Cole Robinson 17 May '10

17 May '10

A fedora translator filed: https://bugzilla.redhat.com/show_bug.cgi?id=580816 Pointing out these two error messages as unclear: "write save" sounds like a typo without context, and lack of a colon made the second message difficult to parse. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/qemu/qemu_driver.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 582fdee..9ae8307 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4805,7 +4805,8 @@ static int qemudDomainSaveFileOpHook(int fd, void *data) { if (safewrite(fd, hdata->header, sizeof(*hdata->header)) != sizeof(*hdata->header)) { ret = errno; qemuReportError(VIR_ERR_OPERATION_FAILED, - _("failed to write save header to '%s'"), hdata->path); + _("failed to write domain save file header to '%s'"), + hdata->path); goto endjob; } @@ -5005,8 +5006,9 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path, if (statfs_ret == -1) { virReportSystemError(errno, - _("Failed to create domain save file '%s'" - " statfs of all elements of path failed."), + _("Failed to create domain save file " + "'%s': statfs of all elements of path " + "failed"), path); VIR_FREE(dirpath); goto endjob; -- 1.6.6.1

2 2

[libvirt] [PATCH] virFileResolveLink: fix return value
by Eric Blake 17 May '10

17 May '10

virFileResolveLink was returning a positive value on error, thus confusing callers that assumed failure was < 0. The confusion is further evidenced by callers that would have ended up calling virReportSystemError with a negative value instead of a valid errno. Fixes Red Hat BZ #591363. * src/util/util.c (virFileResolveLink): Live up to documentation. * src/qemu/qemu_security_dac.c (qemuSecurityDACRestoreSecurityFileLabel): Adjust callers. * src/security/security_selinux.c (SELinuxRestoreSecurityFileLabel): Likewise. * src/storage/storage_backend_disk.c (virStorageBackendDiskDeleteVol): Likewise. --- Thanks to Daniel P. Berrange for helping isolate the cause of the bugzilla failure. I validated that the only instance of 'cannot stat %s' in libvirt.po comes from SELinuxRestoreSecurityFileLabel, and that it does indeed have the ability to dereference a NULL pointer before this fix. src/qemu/qemu_security_dac.c | 5 ++--- src/security/security_selinux.c | 7 +++---- src/storage/storage_backend_disk.c | 7 +++---- src/util/util.c | 6 +++--- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index 364227d..a816441 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -75,13 +75,12 @@ qemuSecurityDACRestoreSecurityFileLabel(const char *path) { struct stat buf; int rc = -1; - int err; char *newpath = NULL; VIR_INFO("Restoring DAC user and group on '%s'", path); - if ((err = virFileResolveLink(path, &newpath)) < 0) { - virReportSystemError(err, + if (virFileResolveLink(path, &newpath) < 0) { + virReportSystemError(errno, _("cannot resolve symlink %s"), path); goto err; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 47534df..669ef42 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008,2009 Red Hat, Inc. + * Copyright (C) 2008-2010 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -353,13 +353,12 @@ SELinuxRestoreSecurityFileLabel(const char *path) struct stat buf; security_context_t fcon = NULL; int rc = -1; - int err; char *newpath = NULL; VIR_INFO("Restoring SELinux context on '%s'", path); - if ((err = virFileResolveLink(path, &newpath)) < 0) { - virReportSystemError(err, + if (virFileResolveLink(path, &newpath) < 0) { + virReportSystemError(errno, _("cannot resolve symlink %s"), path); goto err; } diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 836d1ca..7188386 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -1,7 +1,7 @@ /* * storage_backend_disk.c: storage backend for disk handling * - * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008, 2010 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -612,13 +612,12 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags ATTRIBUTE_UNUSED) { char *part_num = NULL; - int err; char *devpath = NULL; char *devname, *srcname; int rc = -1; - if ((err = virFileResolveLink(vol->target.path, &devpath)) < 0) { - virReportSystemError(err, + if (virFileResolveLink(vol->target.path, &devpath) < 0) { + virReportSystemError(errno, _("Couldn't read volume target path '%s'"), vol->target.path); goto cleanup; diff --git a/src/util/util.c b/src/util/util.c index 26ac6ba..e937d39 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1182,7 +1182,7 @@ int virFileLinkPointsTo(const char *checkLink, * real path * * Return 0 if path was not a symbolic, or the link was - * resolved. Return -1 upon error + * resolved. Return -1 with errno set upon error */ int virFileResolveLink(const char *linkpath, char **resultpath) @@ -1192,11 +1192,11 @@ int virFileResolveLink(const char *linkpath, *resultpath = NULL; if (lstat(linkpath, &st) < 0) - return errno; + return -1; if (!S_ISLNK(st.st_mode)) { if (!(*resultpath = strdup(linkpath))) - return -ENOMEM; + return -1; return 0; } -- 1.7.0.1

2 2

[libvirt] [PATCH] lxcFreezeContainer: avoid test-after-deref of never-NULL pointer
by Jim Meyering 17 May '10

17 May '10

This addresses another coverity-spotted "flaw". However, since "cgroup" is never NULL after that initial "if" stmt, the only penalty is that the useless cleanup test would make a reviewer try to figure out how cgroup could be NULL there. >From d89098801d4e5011e07994cf0391ace2363d8971 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 19:18:12 +0200 Subject: [PATCH] lxcFreezeContainer: avoid test-after-deref of never-NULL pointer * src/lxc/lxc_driver.c (lxcFreezeContainer): Remove test-after-deref. Correct indentation in expression. --- src/lxc/lxc_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index fc0df37..8c3bbd3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2276,69 +2276,71 @@ static int lxcDomainSetAutostart(virDomainPtr dom, } } else { if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { virReportSystemError(errno, _("Failed to delete symlink '%s'"), autostartLink); goto cleanup; } } vm->autostart = autostart; ret = 0; cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); if (vm) virDomainObjUnlock(vm); lxcDriverUnlock(driver); return ret; } static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm) { int timeout = 1000; /* In milliseconds */ int check_interval = 1; /* In milliseconds */ int exp = 10; int waited_time = 0; int ret = -1; char *state = NULL; virCgroupPtr cgroup = NULL; if (!(driver->cgroup && - virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) + virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) == 0)) return -1; + /* From here on, we know that cgroup != NULL. */ + while (waited_time < timeout) { int r; /* * Writing "FROZEN" to the "freezer.state" freezes the group, * i.e., the container, temporarily transiting "FREEZING" state. * Once the freezing is completed, the state of the group transits * to "FROZEN". * (see linux-2.6/Documentation/cgroups/freezer-subsystem.txt) */ r = virCgroupSetFreezerState(cgroup, "FROZEN"); /* * Returning EBUSY explicitly indicates that the group is * being freezed but incomplete and other errors are true * errors. */ if (r < 0 && r != -EBUSY) { VIR_DEBUG("Writing freezer.state failed with errno: %d", r); goto error; } if (r == -EBUSY) VIR_DEBUG0("Writing freezer.state gets EBUSY"); /* * Unfortunately, returning 0 (success) is likely to happen * even when the freezing has not been completed. Sometimes * the state of the group remains "FREEZING" like when * returning -EBUSY and even worse may never transit to * "FROZEN" even if writing "FROZEN" again. * * So we don't trust the return value anyway and always * decide that the freezing has been complete only with * the state actually transit to "FROZEN". @@ -2351,68 +2353,67 @@ static int lxcFreezeContainer(lxc_driver_t *driver, virDomainObjPtr vm) VIR_DEBUG("Reading freezer.state failed with errno: %d", r); goto error; } VIR_DEBUG("Read freezer.state: %s", state); if (STREQ(state, "FROZEN")) { ret = 0; goto cleanup; } waited_time += check_interval; /* * Increasing check_interval exponentially starting with * small initial value treats nicely two cases; One is * a container is under no load and waiting for long period * makes no sense. The other is under heavy load. The container * may stay longer time in FREEZING or never transit to FROZEN. * In that case, eager polling will just waste CPU time. */ check_interval *= exp; VIR_FREE(state); } VIR_DEBUG0("lxcFreezeContainer timeout"); error: /* * If timeout or an error on reading the state occurs, * activate the group again and return an error. * This is likely to fall the group back again gracefully. */ virCgroupSetFreezerState(cgroup, "THAWED"); ret = -1; cleanup: - if (cgroup) - virCgroupFree(&cgroup); + virCgroupFree(&cgroup); VIR_FREE(state); return ret; } static int lxcDomainSuspend(virDomainPtr dom) { lxc_driver_t *driver = dom->conn->privateData; virDomainObjPtr vm; virDomainEventPtr event = NULL; int ret = -1; lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); lxcError(VIR_ERR_NO_DOMAIN, _("No domain with matching uuid '%s'"), uuidstr); goto cleanup; } if (!virDomainObjIsActive(vm)) { lxcError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); goto cleanup; } if (vm->state != VIR_DOMAIN_PAUSED) { if (lxcFreezeContainer(driver, vm) < 0) { lxcError(VIR_ERR_OPERATION_FAILED, "%s", _("Suspend operation failed")); goto cleanup; -- 1.7.1.250.g7d1e8

2 11

[libvirt] [PATCH 0/2] Fix multiple NULL pointer problems in monitor usage
by Daniel P. Berrange 17 May '10

17 May '10

These two patches address multiple problems in usage of the monitor which lead to a crash on NULL pointer. The first patch fixes all the actual root cause problems. The second patch provides a safety net in case I missed any, or people introduce similar bugs in the future

2 9

[libvirt] [PATCH] tests: Skip daemon-conf test if dir exceeds UNIX_PATH_MAX
by Cole Robinson 17 May '10

17 May '10

The max path length for unix sockets is pretty small (108, see man 7 unix). If 'make check' is run from a directory that exceeds this, one of the tests will fail, and in such a way that requires manually editting the test to determine why. There are certainly other ways to handle this, but I've chosen just to skip the offending test if we will exceed the length limitation. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- tests/daemon-conf | 22 ++++++++++++++-------- 1 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/daemon-conf b/tests/daemon-conf index 10c1628..8eed4c4 100755 --- a/tests/daemon-conf +++ b/tests/daemon-conf @@ -76,16 +76,22 @@ sed 's,^log_outputs.*,log_outputs="3:file:'"$(pwd)/log"'",' tmp.conf > k \ || fail=1 mv k tmp.conf || fail=1 -$abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=tmp.conf > log 2>&1 & pid=$! -sleep $sleep_secs -kill $pid +# Unix socket max path size is 108 on linux. If the generated sock path +# exceeds this, the test will fail, so skip it if CWD is too long +SOCKPATH=`pwd`/libvirt-sock +if test ${#SOCKPATH} -lt 108 ; then -RET=0 -# Expect an orderly shut-down and successful exit. -wait $pid || RET=1 + $abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=tmp.conf > log 2>&1 & pid=$! + sleep $sleep_secs + kill $pid -test_result $i "valid config file (sleeping $sleep_secs seconds)" $RET -test $RET = 0 || fail=1 + RET=0 + # Expect an orderly shut-down and successful exit. + wait $pid || RET=1 + + test_result $i "valid config file (sleeping $sleep_secs seconds)" $RET + test $RET = 0 || fail=1 +fi test_final $i $fail -- 1.6.6.1

3 4

[libvirt] [PATCH v3] pci: Give an explicit error if device not found
by Cole Robinson 17 May '10

17 May '10

v2: Use intended F_OK. Drop devdir param, just check dev->path for device existence. v3: Use virReportSystemError, include dev->path in error message. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/util/pci.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 81193b7..a1bd8e3 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1046,6 +1046,14 @@ pciGetDevice(unsigned domain, snprintf(dev->path, sizeof(dev->path), PCI_SYSFS "devices/%s/config", dev->name); + if (access(dev->path, F_OK) != 0) { + virReportSystemError(errno, + _("Device %s not found: could not access %s"), + dev->name, dev->path); + pciFreeDevice(dev); + return NULL; + } + vendor = pciReadDeviceID(dev, "vendor"); product = pciReadDeviceID(dev, "device"); -- 1.6.6.1

2 2

[libvirt] [PATCH v2] tests: Skip daemon-conf test if dir exceeds UNIX_PATH_MAX
by Cole Robinson 17 May '10

17 May '10

The max path length for unix sockets is pretty small (108, see man 7 unix). If 'make check' is run from a directory that exceeds this, one of the tests will fail, and in such a way that requires manually editting the test to determine why. There are certainly other ways to handle this, but I've chosen just to skip the offending test if we will exceed the length limitation. v2: Drop bashism, use test infrastructure to warn and skip Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- tests/daemon-conf | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/tests/daemon-conf b/tests/daemon-conf index 10c1628..14d4ced 100755 --- a/tests/daemon-conf +++ b/tests/daemon-conf @@ -76,6 +76,13 @@ sed 's,^log_outputs.*,log_outputs="3:file:'"$(pwd)/log"'",' tmp.conf > k \ || fail=1 mv k tmp.conf || fail=1 +# Unix socket max path size is 108 on linux. If the generated sock path +# exceeds this, the test will fail, so skip it if CWD is too long +SOCKPATH=`pwd`/libvirt-sock +if test 108 -lt `echo $SOCKPATH | wc -c`; then + skip_test_ "CWD too long" +fi + $abs_top_builddir/daemon/libvirtd --pid-file=pid-file --config=tmp.conf > log 2>&1 & pid=$! sleep $sleep_secs kill $pid -- 1.6.6.1

2 2

[libvirt] [PATCH v2] pci: Give an explicit error if device not found
by Cole Robinson 17 May '10

17 May '10

v2: Use intended F_OK. Drop devdir param, just check dev->path for device existence. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/util/pci.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 81193b7..3fcc658 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1046,6 +1046,13 @@ pciGetDevice(unsigned domain, snprintf(dev->path, sizeof(dev->path), PCI_SYSFS "devices/%s/config", dev->name); + if (access(dev->path, F_OK) != 0) { + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Device %s not found"), dev->name); + pciFreeDevice(dev); + return NULL; + } + vendor = pciReadDeviceID(dev, "vendor"); product = pciReadDeviceID(dev, "device"); -- 1.6.6.1

2 2

[libvirt] [PATCH] build: fix cygwin build
by Eric Blake 17 May '10

17 May '10

make[3]: *** No rule to make target `-lxml2', needed by `libvirt.la'. Stop. Due to treating the wrong string as a dependency. * src/Makefile.am (libvirt_la_DEPENDENCIES): Depend only on locally-built file, not on strings that might resolve as '-lxml2'. --- Pushing under the obvious rule, since this was breaking a build. src/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 15bc8fc..72f23a7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -974,13 +974,13 @@ libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS) libvirt_la_LIBADD += $(LIBXML_LIBS) \ $(LIBPCAP_LIBS) \ $(DRIVER_MODULE_LIBS) \ $(CYGWIN_EXTRA_LIBADD) ../gnulib/lib/libgnu.la libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT -libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) $(LIBVIRT_SYMBOL_FILE) +libvirt_la_DEPENDENCIES = ../gnulib/lib/libgnu.la $(LIBVIRT_SYMBOL_FILE) # Create an automake "convenience library" version of libvirt_la, # just for testing, since the test harness requires access to internal # bits and pieces that we don't want to make publicly accessible. noinst_LTLIBRARIES += libvirt_test.la -- 1.7.0.1

2 7

[libvirt] [PATCH] Protect against NULL pointer flaws in monitor usage (v2)
by Daniel P. Berrange 17 May '10

17 May '10

History has shown that there are frequent bugs in the QEMU driver code leading to the monitor being invoked with a NULL pointer. Although the QEMU driver code should always report an error in this case before invoking the monitor, as a safety net put in a generic check in the monitor code entry points. * src/qemu/qemu_monitor.c: Safety net to check for NULL monitor object --- src/qemu/qemu_monitor.c | 436 +++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 368 insertions(+), 68 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index efaf74a..ec0c3fe 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -891,7 +891,13 @@ int qemuMonitorEmitGraphics(qemuMonitorPtr mon, int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSetCapabilities(mon); @@ -906,7 +912,13 @@ qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONStartCPUs(mon, conn); @@ -920,7 +932,13 @@ int qemuMonitorStopCPUs(qemuMonitorPtr mon) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONStopCPUs(mon); @@ -933,7 +951,13 @@ qemuMonitorStopCPUs(qemuMonitorPtr mon) int qemuMonitorSystemPowerdown(qemuMonitorPtr mon) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSystemPowerdown(mon); @@ -947,7 +971,13 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int **pids) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetCPUInfo(mon, pids); @@ -960,7 +990,13 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, unsigned long *currmem) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetBalloonInfo(mon, currmem); @@ -975,7 +1011,13 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, unsigned int nr_stats) { int ret; - DEBUG("mon=%p, fd=%d stats=%p nstats=%u", mon, mon->fd, stats, nr_stats); + DEBUG("mon=%p stats=%p nstats=%u", mon, stats, nr_stats); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetMemoryStats(mon, stats, nr_stats); @@ -994,7 +1036,13 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *errs) { int ret; - DEBUG("mon=%p, fd=%d dev=%s", mon, mon->fd, devname); + DEBUG("mon=%p dev=%s", mon, devname); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetBlockStatsInfo(mon, devname, @@ -1014,8 +1062,14 @@ int qemuMonitorGetBlockExtent(qemuMonitorPtr mon, unsigned long long *extent) { int ret; - DEBUG("mon=%p, fd=%d, devname=%p", - mon, mon->fd, devname); + DEBUG("mon=%p, devname=%p", + mon, devname); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetBlockExtent(mon, devname, extent); @@ -1030,8 +1084,14 @@ int qemuMonitorSetVNCPassword(qemuMonitorPtr mon, const char *password) { int ret; - DEBUG("mon=%p, fd=%d, password=%p", - mon, mon->fd, password); + DEBUG("mon=%p, password=%p", + mon, password); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (!password) password = ""; @@ -1052,8 +1112,14 @@ int qemuMonitorSetGraphicsPassword(qemuMonitorPtr mon, unsigned int expiry) { int ret; - DEBUG("mon=%p, fd=%d type=%d, password=%p, expiry=%u", - mon, mon->fd, type, password, expiry); + DEBUG("mon=%p type=%d, password=%p, expiry=%u", + mon, type, password, expiry); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (!password) password = ""; @@ -1071,7 +1137,13 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon, unsigned long newmem) { int ret; - DEBUG("mon=%p, fd=%d newmem=%lu", mon, mon->fd, newmem); + DEBUG("mon=%p newmem=%lu", mon, newmem); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSetBalloon(mon, newmem); @@ -1084,7 +1156,13 @@ int qemuMonitorSetBalloon(qemuMonitorPtr mon, int qemuMonitorSetCPU(qemuMonitorPtr mon, int cpu, int online) { int ret; - DEBUG("mon=%p, fd=%d cpu=%d online=%d", mon, mon->fd, cpu, online); + DEBUG("mon=%p cpu=%d online=%d", mon, cpu, online); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSetCPU(mon, cpu, online); @@ -1098,7 +1176,13 @@ int qemuMonitorEjectMedia(qemuMonitorPtr mon, const char *devname) { int ret; - DEBUG("mon=%p, fd=%d devname=%s", mon, mon->fd, devname); + DEBUG("mon=%p devname=%s", mon, devname); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONEjectMedia(mon, devname); @@ -1114,8 +1198,14 @@ int qemuMonitorChangeMedia(qemuMonitorPtr mon, const char *format) { int ret; - DEBUG("mon=%p, fd=%d devname=%s newmedia=%s format=%s", - mon, mon->fd, devname, newmedia, format); + DEBUG("mon=%p devname=%s newmedia=%s format=%s", + mon, devname, newmedia, format); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONChangeMedia(mon, devname, newmedia, format); @@ -1131,8 +1221,14 @@ int qemuMonitorSaveVirtualMemory(qemuMonitorPtr mon, const char *path) { int ret; - DEBUG("mon=%p, fd=%d offset=%llu length=%zu path=%s", - mon, mon->fd, offset, length, path); + DEBUG("mon=%p offset=%llu length=%zu path=%s", + mon, offset, length, path); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSaveVirtualMemory(mon, offset, length, path); @@ -1147,8 +1243,14 @@ int qemuMonitorSavePhysicalMemory(qemuMonitorPtr mon, const char *path) { int ret; - DEBUG("mon=%p, fd=%d offset=%llu length=%zu path=%s", - mon, mon->fd, offset, length, path); + DEBUG("mon=%p offset=%llu length=%zu path=%s", + mon, offset, length, path); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSavePhysicalMemory(mon, offset, length, path); @@ -1162,7 +1264,13 @@ int qemuMonitorSetMigrationSpeed(qemuMonitorPtr mon, unsigned long bandwidth) { int ret; - DEBUG("mon=%p, fd=%d bandwidth=%lu", mon, mon->fd, bandwidth); + DEBUG("mon=%p bandwidth=%lu", mon, bandwidth); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSetMigrationSpeed(mon, bandwidth); @@ -1176,7 +1284,13 @@ int qemuMonitorSetMigrationDowntime(qemuMonitorPtr mon, unsigned long long downtime) { int ret; - DEBUG("mon=%p, fd=%d downtime=%llu", mon, mon->fd, downtime); + DEBUG("mon=%p downtime=%llu", mon, downtime); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSetMigrationDowntime(mon, downtime); @@ -1193,7 +1307,13 @@ int qemuMonitorGetMigrationStatus(qemuMonitorPtr mon, unsigned long long *total) { int ret; - DEBUG("mon=%p, fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetMigrationStatus(mon, status, @@ -1215,8 +1335,14 @@ int qemuMonitorMigrateToHost(qemuMonitorPtr mon, int port) { int ret; - DEBUG("mon=%p, fd=%d hostname=%s port=%d", - mon, mon->fd, hostname, port); + DEBUG("mon=%p hostname=%s port=%d", + mon, hostname, port); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONMigrateToHost(mon, background, hostname, port); @@ -1231,8 +1357,14 @@ int qemuMonitorMigrateToCommand(qemuMonitorPtr mon, const char * const *argv) { int ret; - DEBUG("mon=%p, fd=%d argv=%p", - mon, mon->fd, argv); + DEBUG("mon=%p argv=%p", + mon, argv); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONMigrateToCommand(mon, background, argv); @@ -1248,8 +1380,14 @@ int qemuMonitorMigrateToFile(qemuMonitorPtr mon, unsigned long long offset) { int ret; - DEBUG("mon=%p, fd=%d argv=%p target=%s offset=%llu", - mon, mon->fd, argv, target, offset); + DEBUG("mon=%p argv=%p target=%s offset=%llu", + mon, argv, target, offset); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (offset % QEMU_MONITOR_MIGRATE_TO_FILE_BS) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -1270,8 +1408,14 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, const char *unixfile) { int ret; - DEBUG("mon=%p fd=%d unixfile=%s", - mon, mon->fd, unixfile); + DEBUG("mon=%p, unixfile=%s", + mon, unixfile); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONMigrateToUnix(mon, background, unixfile); @@ -1283,7 +1427,13 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, int qemuMonitorMigrateCancel(qemuMonitorPtr mon) { int ret; - DEBUG("mon=%p fd=%d", mon, mon->fd); + DEBUG("mon=%p", mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONMigrateCancel(mon); @@ -1304,6 +1454,12 @@ int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon, DEBUG("mon=%p type=%d hostname=%s port=%d tlsPort=%d tlsSubject=%s", mon, type, hostname, port, tlsPort, NULLSTR(tlsSubject)); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONGraphicsRelocate(mon, type, @@ -1327,7 +1483,13 @@ int qemuMonitorAddUSBDisk(qemuMonitorPtr mon, const char *path) { int ret; - DEBUG("mon=%p, fd=%d path=%s", mon, mon->fd, path); + DEBUG("mon=%p path=%s", mon, path); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddUSBDisk(mon, path); @@ -1342,7 +1504,13 @@ int qemuMonitorAddUSBDeviceExact(qemuMonitorPtr mon, int dev) { int ret; - DEBUG("mon=%p, fd=%d bus=%d dev=%d", mon, mon->fd, bus, dev); + DEBUG("mon=%p bus=%d dev=%d", mon, bus, dev); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddUSBDeviceExact(mon, bus, dev); @@ -1356,8 +1524,14 @@ int qemuMonitorAddUSBDeviceMatch(qemuMonitorPtr mon, int product) { int ret; - DEBUG("mon=%p, fd=%d vendor=%d product=%d", - mon, mon->fd, vendor, product); + DEBUG("mon=%p vendor=%d product=%d", + mon, vendor, product); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddUSBDeviceMatch(mon, vendor, product); @@ -1372,10 +1546,16 @@ int qemuMonitorAddPCIHostDevice(qemuMonitorPtr mon, virDomainDevicePCIAddress *guestAddr) { int ret; - DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d function=%d", - mon, mon->fd, + DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d", + mon, hostAddr->domain, hostAddr->bus, hostAddr->slot, hostAddr->function); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONAddPCIHostDevice(mon, hostAddr, guestAddr); else @@ -1390,8 +1570,14 @@ int qemuMonitorAddPCIDisk(qemuMonitorPtr mon, virDomainDevicePCIAddress *guestAddr) { int ret; - DEBUG("mon=%p, fd=%d path=%s bus=%s", - mon, mon->fd, path, bus); + DEBUG("mon=%p path=%s bus=%s", + mon, path, bus); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddPCIDisk(mon, path, bus, guestAddr); @@ -1406,7 +1592,13 @@ int qemuMonitorAddPCINetwork(qemuMonitorPtr mon, virDomainDevicePCIAddress *guestAddr) { int ret; - DEBUG("mon=%p, fd=%d nicstr=%s", mon, mon->fd, nicstr); + DEBUG("mon=%p nicstr=%s", mon, nicstr); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddPCINetwork(mon, nicstr, guestAddr); @@ -1420,10 +1612,16 @@ int qemuMonitorRemovePCIDevice(qemuMonitorPtr mon, virDomainDevicePCIAddress *guestAddr) { int ret; - DEBUG("mon=%p, fd=%d domain=%d bus=%d slot=%d function=%d", - mon, mon->fd, guestAddr->domain, guestAddr->bus, + DEBUG("mon=%p domain=%d bus=%d slot=%d function=%d", + mon, guestAddr->domain, guestAddr->bus, guestAddr->slot, guestAddr->function); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONRemovePCIDevice(mon, guestAddr); else @@ -1437,8 +1635,14 @@ int qemuMonitorSendFileHandle(qemuMonitorPtr mon, int fd) { int ret; - DEBUG("mon=%p, fd=%d fdname=%s fd=%d", - mon, mon->fd, fdname, fd); + DEBUG("mon=%p, fdname=%s fd=%d", + mon, fdname, fd); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONSendFileHandle(mon, fdname, fd); @@ -1452,8 +1656,14 @@ int qemuMonitorCloseFileHandle(qemuMonitorPtr mon, const char *fdname) { int ret; - DEBUG("mon=%p, fd=%d fdname=%s", - mon, mon->fd, fdname); + DEBUG("mon=%p fdname=%s", + mon, fdname); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONCloseFileHandle(mon, fdname); @@ -1467,8 +1677,14 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, const char *netstr) { int ret; - DEBUG("mon=%p, fd=%d netstr=%s", - mon, mon->fd, netstr); + DEBUG("mon=%p netstr=%s", + mon, netstr); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddHostNetwork(mon, netstr); @@ -1483,8 +1699,14 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, const char *netname) { int ret; - DEBUG("mon=%p, fd=%d netname=%s", - mon, mon->fd, netname); + DEBUG("mon=%p netname=%s", + mon, netname); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONRemoveHostNetwork(mon, vlan, netname); @@ -1498,8 +1720,14 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, const char *netdevstr) { int ret; - DEBUG("mon=%p, fd=%d netdevstr=%s", - mon, mon->fd, netdevstr); + DEBUG("mon=%p netdevstr=%s", + mon, netdevstr); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONAddNetdev(mon, netdevstr); @@ -1513,8 +1741,14 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias) { int ret; - DEBUG("mon=%p, fd=%d alias=%s", - mon, mon->fd, alias); + DEBUG("mon=%p alias=%s", + mon, alias); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONRemoveNetdev(mon, alias); @@ -1528,8 +1762,14 @@ int qemuMonitorGetPtyPaths(qemuMonitorPtr mon, virHashTablePtr paths) { int ret; - DEBUG("mon=%p, fd=%d", - mon, mon->fd); + DEBUG("mon=%p", + mon); + + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } if (mon->json) ret = qemuMonitorJSONGetPtyPaths(mon, paths); @@ -1543,9 +1783,15 @@ int qemuMonitorAttachPCIDiskController(qemuMonitorPtr mon, const char *bus, virDomainDevicePCIAddress *guestAddr) { - DEBUG("mon=%p, fd=%d type=%s", mon, mon->fd, bus); + DEBUG("mon=%p type=%s", mon, bus); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONAttachPCIDiskController(mon, bus, guestAddr); else @@ -1560,12 +1806,18 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon, virDomainDevicePCIAddress *controllerAddr, virDomainDeviceDriveAddress *driveAddr) { - DEBUG("mon=%p, fd=%d drivestr=%s domain=%d bus=%d slot=%d function=%d", - mon, mon->fd, drivestr, + DEBUG("mon=%p drivestr=%s domain=%d bus=%d slot=%d function=%d", + mon, drivestr, controllerAddr->domain, controllerAddr->bus, controllerAddr->slot, controllerAddr->function); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr, driveAddr); else @@ -1577,9 +1829,15 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon, int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, qemuMonitorPCIAddress **addrs) { - DEBUG("mon=%p, fd=%d addrs=%p", mon, mon->fd, addrs); + DEBUG("mon=%p addrs=%p", mon, addrs); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONGetAllPCIAddresses(mon, addrs); else @@ -1590,9 +1848,15 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorDelDevice(qemuMonitorPtr mon, const char *devalias) { - DEBUG("mon=%p, fd=%d devalias=%s", mon, mon->fd, devalias); + DEBUG("mon=%p devalias=%s", mon, devalias); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONDelDevice(mon, devalias); else @@ -1604,9 +1868,15 @@ int qemuMonitorDelDevice(qemuMonitorPtr mon, int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr) { - DEBUG("mon=%p, fd=%d device=%s", mon, mon->fd, devicestr); + DEBUG("mon=%p device=%s", mon, devicestr); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONAddDevice(mon, devicestr); else @@ -1617,9 +1887,15 @@ int qemuMonitorAddDevice(qemuMonitorPtr mon, int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr) { - DEBUG("mon=%p, fd=%d drive=%s", mon, mon->fd, drivestr); + DEBUG("mon=%p drive=%s", mon, drivestr); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONAddDrive(mon, drivestr); else @@ -1632,9 +1908,15 @@ int qemuMonitorSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) { - DEBUG("mon=%p, fd=%d alias=%s passphrase=%p(value hidden)", mon, mon->fd, alias, passphrase); + DEBUG("mon=%p alias=%s passphrase=%p(value hidden)", mon, alias, passphrase); int ret; + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONSetDrivePassphrase(mon, alias, passphrase); else @@ -1648,6 +1930,12 @@ int qemuMonitorCreateSnapshot(qemuMonitorPtr mon, const char *name) DEBUG("mon=%p, name=%s",mon,name); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONCreateSnapshot(mon, name); else @@ -1661,6 +1949,12 @@ int qemuMonitorLoadSnapshot(qemuMonitorPtr mon, const char *name) DEBUG("mon=%p, name=%s",mon,name); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONLoadSnapshot(mon, name); else @@ -1674,6 +1968,12 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name) DEBUG("mon=%p, name=%s",mon,name); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + if (mon->json) ret = qemuMonitorJSONDeleteSnapshot(mon, name); else -- 1.6.6.1

2 2

[libvirt] [PATCH] pci: Give an explicit error if device not found
by Cole Robinson 17 May '10

17 May '10

Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/util/pci.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 81193b7..e1e11bc 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1028,6 +1028,7 @@ pciGetDevice(unsigned domain, unsigned function) { pciDevice *dev; + char devdir[PATH_MAX]; char *vendor, *product; if (VIR_ALLOC(dev) < 0) { @@ -1043,8 +1044,17 @@ pciGetDevice(unsigned domain, snprintf(dev->name, sizeof(dev->name), "%.4x:%.2x:%.2x.%.1x", dev->domain, dev->bus, dev->slot, dev->function); + snprintf(devdir, sizeof(devdir), + PCI_SYSFS "devices/%s", dev->name); snprintf(dev->path, sizeof(dev->path), - PCI_SYSFS "devices/%s/config", dev->name); + "%s/%s/config", devdir, dev->name); + + if (access(devdir, X_OK) != 0) { + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Device %s not found"), dev->name); + pciFreeDevice(dev); + return NULL; + } vendor = pciReadDeviceID(dev, "vendor"); product = pciReadDeviceID(dev, "device"); -- 1.7.0.1

2 2

[libvirt] [PATCH] umlAutostartDomain: avoid NULL-deref upon virGetLastError failure
by Jim Meyering 17 May '10

17 May '10

Another... >From 09579adb73f04c5b940ad28cdfc242cff0726e0e Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 14:38:14 +0200 Subject: [PATCH] umlAutostartDomain: avoid NULL-deref upon virGetLastError failure * src/uml/uml_driver.c (umlAutostartDomain): Handle a NULL return from virGetLastError. --- src/uml/uml_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 63fe808..ffb87c8 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -157,7 +157,7 @@ umlAutostartDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaqu if (umlStartVMDaemon(data->conn, data->driver, vm) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), - vm->def->name, err->message); + vm->def->name, err ? err->message : ""); } } virDomainObjUnlock(vm); -- 1.7.1.250.g7d1e8

2 4

[libvirt] [PATCH] build: fix up some compiler flags
by Eric Blake 17 May '10

17 May '10

Matthias noted that the line: virt_aa_helper_LDFLAGS = $(WARN_CFLAGS) looks inconsistent, so I did an audit. Currently, the set of compiler warning flags passed to gcc as $CC are equally permitted as the set of linker flags passed to gcc as $LD, so there was no problem with that usage. But if we ever get in a situation where $CC and $LD treat particular flags differently, using the right variable form will make it easier. In the process, I spotted a couple of typos that were omitting useful flags, as well as specifying a -l under the wrong variable. * acinclude.m4 (LIBVIRT_COMPILE_WARNINGS): Define WARN_LDFLAGS as an alias for WARN_CFLAGS. * tools/Makefile.am (virsh_LDFLAGS): Use more canonical spelling. * proxy/Makefile.am (libvirt_proxy_LDFLAGS): Likewise. Move library... (libvirt_proxy_LDADD): ...here. * src/Makefile.am (virt_aa_helper_LDFLAGS): Use more canonical spelling of WARN_LDFLAGS. (libvirt_parthelper_LDFLAGS, libvirt_lxc_LDFLAGS): Likewise. Use correct spelling of COVERAGE_LDFLAGS. Reported by Matthias Bolte. --- acinclude.m4 | 3 ++- proxy/Makefile.am | 4 ++-- src/Makefile.am | 6 +++--- tools/Makefile.am | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index 7fa2d67..f048879 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -78,7 +78,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_MSG_RESULT($complCFLAGS) WARN_CFLAGS="$COMPILER_FLAGS $complCFLAGS" - AC_SUBST(WARN_CFLAGS) + AC_SUBST([WARN_CFLAGS]) + AC_SUBST([WARN_LDFLAGS]) dnl Needed to keep compile quiet on python 2.4 COMPILER_FLAGS= diff --git a/proxy/Makefile.am b/proxy/Makefile.am index bee47d0..4716683 100644 --- a/proxy/Makefile.am +++ b/proxy/Makefile.am @@ -32,9 +32,9 @@ libvirt_proxy_SOURCES = libvirt_proxy.c \ @top_srcdir@/src/xen/xen_hypervisor.c \ @top_srcdir@/src/xen/sexpr.c \ @top_srcdir@/src/xen/xs_internal.c -libvirt_proxy_LDFLAGS = $(WARN_CFLAGS) $(XEN_LIBS) +libvirt_proxy_LDFLAGS = $(WARN_LDFLAGS) libvirt_proxy_DEPENDENCIES = -libvirt_proxy_LDADD = ../gnulib/lib/libgnu.la $(LIB_PTHREAD) +libvirt_proxy_LDADD = ../gnulib/lib/libgnu.la $(XEN_LIBS) $(LIB_PTHREAD) install-exec-hook: chmod u+s $(DESTDIR)$(libexecdir)/libvirt_proxy diff --git a/src/Makefile.am b/src/Makefile.am index 72f23a7..034feec 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1004,7 +1004,7 @@ if WITH_LIBVIRTD libexec_PROGRAMS += libvirt_parthelper libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) -libvirt_parthelper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) +libvirt_parthelper_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS) libvirt_parthelper_LDADD = $(LIBPARTED_LIBS) libvirt_parthelper_CFLAGS = $(LIBPARTED_CFLAGS) endif @@ -1024,7 +1024,7 @@ libvirt_lxc_SOURCES = \ $(DOMAIN_CONF_SOURCES) \ $(CPU_CONF_SOURCES) \ $(NWFILTER_PARAM_CONF_SOURCES) -libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) +libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ $(LIBXML_LIBS) $(NUMACTL_LIBS) $(LIB_PTHREAD) \ ../gnulib/lib/libgnu.la @@ -1044,7 +1044,7 @@ libexec_PROGRAMS += virt-aa-helper virt_aa_helper_SOURCES = $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) -virt_aa_helper_LDFLAGS = $(WARN_CFLAGS) +virt_aa_helper_LDFLAGS = $(WARN_LDFLAGS) virt_aa_helper_LDADD = \ $(LIBXML_LIBS) \ libvirt_conf.la \ diff --git a/tools/Makefile.am b/tools/Makefile.am index 33a3216..fd05e8b 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -34,14 +34,14 @@ virsh_SOURCES = \ console.c console.h \ virsh.c -virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) +virsh_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS) virsh_LDADD = \ $(STATIC_BINARIES) \ $(WARN_CFLAGS) \ ../src/libvirt.la \ ../gnulib/lib/libgnu.la \ $(VIRSH_LIBS) -virsh_CFLAGS = \ +virsh_CFLAGS = \ -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ -I../include -I$(top_srcdir)/include \ -I$(top_srcdir)/src \ -- 1.7.0.1

2 4

[libvirt] [PATCH] build: avoid compile failure on linux kernels older than 2.6.19
by Jim Meyering 17 May '10

17 May '10

This is required for any kernel prior to 2.6.19, since <linux/magic.h> didn't exist back then. Now that file is provided by the kernel-headers package. >From d14ef1669968ffeb65076b007e318934ed99aa61 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 16:17:08 +0200 Subject: [PATCH] build: avoid compile failure on linux kernels older than 2.6.19 * configure.ac: Check for <linux/magic.h>. * src/util/storage_file.c: Include <linux/magic.h> only if present. Linux kernels prior to 2.6.19 lacked it. [__linux__] (NFS_SUPER_MAGIC): Define if not already defined. --- configure.ac | 2 +- src/util/storage_file.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c187420..ebd2082 100644 --- a/configure.ac +++ b/configure.ac @@ -108,7 +108,7 @@ LIBS=$old_libs dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h \ - termios.h sys/poll.h syslog.h mntent.h net/ethernet.h]) + termios.h sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h]) dnl Where are the XDR functions? dnl If portablexdr is installed, prefer that. diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 5f15a64..a07bedc 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -27,7 +27,9 @@ #include <unistd.h> #include <fcntl.h> #ifdef __linux__ -# include <linux/magic.h> +# if HAVE_LINUX_MAGIC_H +# include <linux/magic.h> +# endif # include <sys/statfs.h> #endif #include "dirname.h" @@ -416,6 +418,9 @@ virStorageFileGetMetadata(const char *path, #ifdef __linux__ +# ifndef NFS_SUPER_MAGIC +# define NFS_SUPER_MAGIC 0x6969 +# endif # ifndef OCFS2_SUPER_MAGIC # define OCFS2_SUPER_MAGIC 0x7461636f # endif -- 1.7.1.250.g7d1e8

3 6

[libvirt] [PATCH] x86ModelHasFeature: avoid NULL-dereference for unmatched CPU "feature"
by Jim Meyering 17 May '10

17 May '10

Here's another fix for a potential NULL-deref. x86cpuidFind can return NULL, yet this caller would dereference that pointer (via x86cpuidMatchMasked) without first checking. >From 9e759e2714b67ea98b18aafb66b5a99ad6361086 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 14:06:13 +0200 Subject: [PATCH] x86ModelHasFeature: avoid NULL-dereference for unmatched CPU "feature" * src/cpu/cpu_x86.c (x86ModelHasFeature): Do not dereference the pointer returned by x86cpuidFind without first ensuring it is non-NULL. --- src/cpu/cpu_x86.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 633eb69..f7473bf 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -602,31 +602,31 @@ x86ModelMergeFeature(struct x86_model *model, static bool x86ModelHasFeature(struct x86_model *model, const struct x86_feature *feature) { unsigned int i; struct cpuX86cpuid *cpuid; struct cpuX86cpuid *model_cpuid; if (feature == NULL) return false; for (i = 0; i < feature->ncpuid; i++) { cpuid = feature->cpuid + i; model_cpuid = x86cpuidFind(model->cpuid, model->ncpuid, cpuid->function); - if (!x86cpuidMatchMasked(model_cpuid, cpuid)) + if (!model_cpuid || !x86cpuidMatchMasked(model_cpuid, cpuid)) return false; } return true; } static struct x86_model * x86ModelFromCPU(const virCPUDefPtr cpu, const struct x86_map *map, int policy) { struct x86_model *model = NULL; int i; -- 1.7.1.250.g7d1e8

2 2

[libvirt] [PATCH] qemu: Report cmdline output if VM dies early
by Cole Robinson 17 May '10

17 May '10

qemuReadLogOutput early VM death detection is racy and won't always work. Startup then errors when connecting to the VM monitor. This won't report the emulator cmdline output which is typically the most useful diagnostic. Check if the VM has died at the very end of the monitor connection step, and if so, report the cmdline output. See also: https://bugzilla.redhat.com/show_bug.cgi?id=581381 Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/qemu/qemu_driver.c | 59 ++++++++++++++++++++++++++++++++++------------- 1 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 42a653a..c446028 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2029,39 +2029,47 @@ static void qemudFreePtyPath(void *payload, const char *name ATTRIBUTE_UNUSED) VIR_FREE(payload); } +static void +qemuReadLogFD(int logfd, char *buf, int maxlen, int off) +{ + int ret; + char *tmpbuf = buf+off; + + ret = saferead(logfd, tmpbuf, maxlen-off-1); + if (ret < 0) { + ret = 0; + } + + *(tmpbuf+ret) = '\0'; +} + static int qemudWaitForMonitor(struct qemud_driver* driver, virDomainObjPtr vm, off_t pos) { - char buf[4096]; /* Plenty of space to get startup greeting */ + char buf[4096] = "\0"; /* Plenty of space to get startup greeting */ int logfd; int ret = -1; + virHashTablePtr paths = NULL; - if ((logfd = qemudLogReadFD(driver->logDir, vm->def->name, pos)) - < 0) + if ((logfd = qemudLogReadFD(driver->logDir, vm->def->name, pos)) < 0) return -1; - ret = qemudReadLogOutput(vm, logfd, buf, sizeof(buf), - qemudFindCharDevicePTYs, - "console", 30); - if (close(logfd) < 0) { - char ebuf[4096]; - VIR_WARN(_("Unable to close logfile: %s"), - virStrerror(errno, ebuf, sizeof ebuf)); - } - - if (ret < 0) - return -1; + if (qemudReadLogOutput(vm, logfd, buf, sizeof(buf), + qemudFindCharDevicePTYs, + "console", 30) < 0) + goto closelog; VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); - if (qemuConnectMonitor(driver, vm) < 0) - return -1; + if (qemuConnectMonitor(driver, vm) < 0) { + goto cleanup; + } /* Try to get the pty path mappings again via the monitor. This is much more * reliable if it's available. * Note that the monitor itself can be on a pty, so we still need to try the * log output method. */ - virHashTablePtr paths = virHashCreate(0); + paths = virHashCreate(0); if (paths == NULL) { virReportOOMError(); goto cleanup; @@ -2082,6 +2090,23 @@ cleanup: virHashFree(paths, qemudFreePtyPath); } + if (kill(vm->pid, 0) == -1 && errno == ESRCH) { + /* VM is dead, any other error raised in the interim is probably + * not as important as the qemu cmdline output */ + qemuReadLogFD(logfd, buf, sizeof(buf), strlen(buf)); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("process exited while connecting to monitor: %s"), + buf); + ret = -1; + } + +closelog: + if (close(logfd) < 0) { + char ebuf[4096]; + VIR_WARN(_("Unable to close logfile: %s"), + virStrerror(errno, ebuf, sizeof ebuf)); + } + return ret; } -- 1.7.0.1

2 2

[libvirt] [PATCH] qemu_driver: avoid NULL dereference
by Jim Meyering 17 May '10

17 May '10

The following theoretical possibility of a NULL dereference has been in the code since April 1 (commit 6e41f30efcac08e50b21d9c943d6d27e90555951). It's theoretical, because if that vm = NULL statement is ever executed, the very next one, calling virDomainObjUnlock would dereference that now-NULL "vm". Hence, I think we can conclude the vm = NULL statement is effectively dead code. That conclusion is in line with the "should" in the preceding comment. At first, it seemed like it would deserve an sa_assert. But without the assert, "n_refs" would be unused, (so this first patch is solely FYI -- not proposed) I solved it differently in the 2nd patch. >From 524aec3ebed613f86b64584d2f461f4a18d2e618 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 12:10:52 +0200 Subject: [PATCH] qemu_driver: avoid NULL dereference * src/qemu/qemu_driver.c (qemudDomainStart): Rather than trying to handle a "can't happen" case, simply sa_assert that it won't happen. --- src/qemu/qemu_driver.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8f69b5a..819ea17 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6489,8 +6489,9 @@ static int qemudDomainStart(virDomainPtr dom) { * We should still have a reference left to vm but * one should check for 0 anyway */ - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + int n_refs = qemuDomainObjEndJob(vm); + sa_assert (n_refs); + virDomainObjUnlock(vm); qemuDriverUnlock(driver); ret = qemudDomainRestore(dom->conn, managed_save); -- 1.7.1.250.g7d1e8 >From f88969b986a1c88985671c9d6fa9cb1dc449ed74 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 12:10:52 +0200 Subject: [PATCH] qemu_driver: avoid NULL dereference * src/qemu/qemu_driver.c (qemudDomainStart): After setting vm to NULL, goto cleanup, rather than dereferencing the NULL pointer. --- src/qemu/qemu_driver.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8f69b5a..3559e36 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6489,8 +6489,11 @@ static int qemudDomainStart(virDomainPtr dom) { * We should still have a reference left to vm but * one should check for 0 anyway */ - if (qemuDomainObjEndJob(vm) == 0) + if (qemuDomainObjEndJob(vm) = 0) { vm = NULL; + goto cleanup; + } + virDomainObjUnlock(vm); qemuDriverUnlock(driver); ret = qemudDomainRestore(dom->conn, managed_save); -- 1.7.1.250.g7d1e8

3 4

[libvirt] [PATCH] Don't reset user/group/security label on shared filesystems during migrate
by Daniel P. Berrange 17 May '10

17 May '10

When QEMU runs with its disk on NFS, and as a non-root user, the disk is chownd to that non-root user. When migration completes the last step is shutting down the QEMU on the source host. THis normally resets user/group/security label. This is bad when the VM was just migrated because the file is still in use on the dest host. It is thus neccessary to skip the reset step for any files found to be on a shared filesystem * src/libvirt_private.syms: Export virStorageFileIsSharedFS * src/util/storage_file.c, src/util/storage_file.h: Add a new method virStorageFileIsSharedFS() to determine if a file is on a shared filesystem (NFS, GFS, OCFS2, etc) * src/qemu/qemu_driver.c: Tell security driver not to reset disk labels on migration completion * src/qemu/qemu_security_dac.c, src/qemu/qemu_security_stacked.c, src/security/security_selinux.c, src/security/security_driver.h, src/security/security_apparmor.c: Add ability to skip disk restore step for files on shared filesystems. --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 34 ++++++++++++++----------- src/qemu/qemu_security_dac.c | 40 +++++++++++++++++++++++++---- src/qemu/qemu_security_stacked.c | 7 +++-- src/security/security_apparmor.c | 3 +- src/security/security_driver.h | 3 +- src/security/security_selinux.c | 38 +++++++++++++++++++++++++---- src/util/storage_file.c | 50 ++++++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 2 + 9 files changed, 147 insertions(+), 31 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5713b2c..7d7ee14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -617,6 +617,7 @@ virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; +virStorageFileIsSharedFS; # threads.h virMutexInit; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b478464..702a652 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -174,7 +174,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, int stdin_fd); static void qemudShutdownVMDaemon(struct qemud_driver *driver, - virDomainObjPtr vm); + virDomainObjPtr vm, + int migrated); static int qemudDomainGetMaxVcpus(virDomainPtr dom); @@ -1105,7 +1106,7 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_DOMAIN_EVENT_STOPPED_FAILED : VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN); - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); else @@ -1636,7 +1637,7 @@ error: /* We can't get the monitor back, so must kill the VM * to remove danger of it ending up running twice if * user tries to start it again later */ - qemudShutdownVMDaemon(driver, obj); + qemudShutdownVMDaemon(driver, obj, 0); if (!obj->persistent) virDomainRemoveInactive(&driver->domains, obj); else @@ -4009,7 +4010,7 @@ cleanup: if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityAllLabel) - driver->securityDriver->domainRestoreSecurityAllLabel(vm); + driver->securityDriver->domainRestoreSecurityAllLabel(vm, 0); if (driver->securityDriver && driver->securityDriver->domainReleaseSecurityLabel) driver->securityDriver->domainReleaseSecurityLabel(vm); @@ -4032,7 +4033,7 @@ cleanup: abort: /* We jump here if we failed to initialize the now running VM * killing it off and pretend we never started it */ - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); if (logfile != -1) close(logfile); @@ -4042,7 +4043,8 @@ abort: static void qemudShutdownVMDaemon(struct qemud_driver *driver, - virDomainObjPtr vm) { + virDomainObjPtr vm, + int migrated) { int ret; int retries = 0; qemuDomainObjPrivatePtr priv = vm->privateData; @@ -4053,7 +4055,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, if (!virDomainObjIsActive(vm)) return; - VIR_DEBUG("Shutting down VM '%s'", vm->def->name); + VIR_DEBUG("Shutting down VM '%s' migrated=%d", vm->def->name, migrated); /* This method is routinely used in clean up paths. Disable error * reporting so we don't squash a legit error. */ @@ -4111,7 +4113,7 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver, /* Reset Security Labels */ if (driver->securityDriver && driver->securityDriver->domainRestoreSecurityAllLabel) - driver->securityDriver->domainRestoreSecurityAllLabel(vm); + driver->securityDriver->domainRestoreSecurityAllLabel(vm, migrated); if (driver->securityDriver && driver->securityDriver->domainReleaseSecurityLabel) driver->securityDriver->domainReleaseSecurityLabel(vm); @@ -4835,7 +4837,7 @@ static int qemudDomainDestroy(virDomainPtr dom) { goto endjob; } - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); @@ -5546,7 +5548,7 @@ static int qemudDomainSaveFlag(virDomainPtr dom, const char *path, ret = 0; /* Shut it down */ - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); @@ -5856,7 +5858,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, endjob: if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); @@ -10365,7 +10367,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, qemust = qemuStreamMigOpen(st, unixfile); if (qemust == NULL) { - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); if (!vm->persistent) { if (qemuDomainObjEndJob(vm) > 0) virDomainRemoveInactive(&driver->domains, vm); @@ -11093,7 +11095,9 @@ qemudDomainMigratePerform (virDomainPtr dom, } /* Clean up the source domain. */ - qemudShutdownVMDaemon(driver, vm); + fprintf(stderr, "******************* MIG \n"); + qemudShutdownVMDaemon(driver, vm, 1); + fprintf(stderr, "******************* YEEHAAA\n"); resume = 0; event = virDomainEventNewFromObj(vm, @@ -11235,7 +11239,7 @@ qemudDomainMigrateFinish2 (virConnectPtr dconn, } virDomainSaveStatus(driver->caps, driver->stateDir, vm); } else { - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); @@ -12081,7 +12085,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, */ if (virDomainObjIsActive(vm)) { - qemudShutdownVMDaemon(driver, vm); + qemudShutdownVMDaemon(driver, vm, 0); event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index e408dbf..2d42ce2 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -142,8 +142,9 @@ qemuSecurityDACSetSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, static int -qemuSecurityDACRestoreSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, - virDomainDiskDefPtr disk) +qemuSecurityDACRestoreSecurityImageLabelInt(virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk, + int migrated) { if (!driver->privileged || !driver->dynamicOwnership) return 0; @@ -162,11 +163,35 @@ qemuSecurityDACRestoreSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, if (!disk->src) return 0; + /* If we have a shared FS & doing migrated, we must not + * change ownership, because that kills access on the + * destination host which is sub-optimal for the guest + * VM's I/O attempts :-) + */ + if (migrated) { + int rc = virStorageFileIsSharedFS(disk->src); + if (rc < 0) + return -1; + if (rc == 1) { + VIR_DEBUG("Skipping image label restore on %s because FS is shared", + disk->src); + return 0; + } + } + return qemuSecurityDACRestoreSecurityFileLabel(disk->src); } static int +qemuSecurityDACRestoreSecurityImageLabel(virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + return qemuSecurityDACRestoreSecurityImageLabelInt(vm, disk, 0); +} + + +static int qemuSecurityDACSetSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque ATTRIBUTE_UNUSED) @@ -306,7 +331,8 @@ done: static int -qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm) +qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm, + int migrated) { int i; int rc = 0; @@ -314,7 +340,8 @@ qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm) if (!driver->privileged || !driver->dynamicOwnership) return 0; - VIR_DEBUG("Restoring security label on %s", vm->def->name); + VIR_DEBUG("Restoring security label on %s migrated=%d", + vm->def->name, migrated); for (i = 0 ; i < vm->def->nhostdevs ; i++) { if (qemuSecurityDACRestoreSecurityHostdevLabel(vm, @@ -322,8 +349,9 @@ qemuSecurityDACRestoreSecurityAllLabel(virDomainObjPtr vm) rc = -1; } for (i = 0 ; i < vm->def->ndisks ; i++) { - if (qemuSecurityDACRestoreSecurityImageLabel(vm, - vm->def->disks[i]) < 0) + if (qemuSecurityDACRestoreSecurityImageLabelInt(vm, + vm->def->disks[i], + migrated) < 0) rc = -1; } diff --git a/src/qemu/qemu_security_stacked.c b/src/qemu/qemu_security_stacked.c index c0258ce..04c1f10 100644 --- a/src/qemu/qemu_security_stacked.c +++ b/src/qemu/qemu_security_stacked.c @@ -215,18 +215,19 @@ qemuSecurityStackedSetSecurityAllLabel(virDomainObjPtr vm) static int -qemuSecurityStackedRestoreSecurityAllLabel(virDomainObjPtr vm) +qemuSecurityStackedRestoreSecurityAllLabel(virDomainObjPtr vm, + int migrated) { int rc = 0; if (driver->securitySecondaryDriver && driver->securitySecondaryDriver->domainRestoreSecurityAllLabel && - driver->securitySecondaryDriver->domainRestoreSecurityAllLabel(vm) < 0) + driver->securitySecondaryDriver->domainRestoreSecurityAllLabel(vm, migrated) < 0) rc = -1; if (driver->securityPrimaryDriver && driver->securityPrimaryDriver->domainRestoreSecurityAllLabel && - driver->securityPrimaryDriver->domainRestoreSecurityAllLabel(vm) < 0) + driver->securityPrimaryDriver->domainRestoreSecurityAllLabel(vm, migrated) < 0) rc = -1; return rc; diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index c0c91cc..87f8a1b 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -444,7 +444,8 @@ AppArmorReleaseSecurityLabel(virDomainObjPtr vm) static int -AppArmorRestoreSecurityAllLabel(virDomainObjPtr vm) +AppArmorRestoreSecurityAllLabel(virDomainObjPtr vm, + int migrated ATTRIBUTE_UNUSED) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; int rc = 0; diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 3de234a..39edc6d 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -46,7 +46,8 @@ typedef int (*virSecurityDomainGenLabel) (virDomainObjPtr sec); typedef int (*virSecurityDomainReserveLabel) (virDomainObjPtr sec); typedef int (*virSecurityDomainReleaseLabel) (virDomainObjPtr sec); typedef int (*virSecurityDomainSetAllLabel) (virDomainObjPtr sec); -typedef int (*virSecurityDomainRestoreAllLabel) (virDomainObjPtr vm); +typedef int (*virSecurityDomainRestoreAllLabel) (virDomainObjPtr vm, + int migrated); typedef int (*virSecurityDomainGetProcessLabel) (virDomainObjPtr vm, virSecurityLabelPtr sec); typedef int (*virSecurityDomainSetProcessLabel) (virSecurityDriverPtr drv, diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 1aabb20..47534df 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -385,8 +385,9 @@ err: } static int -SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm, - virDomainDiskDefPtr disk) +SELinuxRestoreSecurityImageLabelInt(virDomainObjPtr vm, + virDomainDiskDefPtr disk, + int migrated) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; @@ -407,9 +408,34 @@ SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm, if (!disk->src) return 0; + /* If we have a shared FS & doing migrated, we must not + * change ownership, because that kills access on the + * destination host which is sub-optimal for the guest + * VM's I/O attempts :-) + */ + if (migrated) { + int rc = virStorageFileIsSharedFS(disk->src); + if (rc < 0) + return -1; + if (rc == 1) { + VIR_DEBUG("Skipping image label restore on %s because FS is shared", + disk->src); + return 0; + } + } + return SELinuxRestoreSecurityFileLabel(disk->src); } + +static int +SELinuxRestoreSecurityImageLabel(virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + return SELinuxRestoreSecurityImageLabelInt(vm, disk, 0); +} + + static int SELinuxSetSecurityImageLabel(virDomainObjPtr vm, virDomainDiskDefPtr disk) @@ -603,7 +629,8 @@ done: } static int -SELinuxRestoreSecurityAllLabel(virDomainObjPtr vm) +SELinuxRestoreSecurityAllLabel(virDomainObjPtr vm, + int migrated ATTRIBUTE_UNUSED) { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; int i; @@ -619,8 +646,9 @@ SELinuxRestoreSecurityAllLabel(virDomainObjPtr vm) rc = -1; } for (i = 0 ; i < vm->def->ndisks ; i++) { - if (SELinuxRestoreSecurityImageLabel(vm, - vm->def->disks[i]) < 0) + if (SELinuxRestoreSecurityImageLabelInt(vm, + vm->def->disks[i], + migrated) < 0) rc = -1; } diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 76ebb98..d4aad86 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -26,9 +26,14 @@ #include <unistd.h> #include <fcntl.h> +#ifdef __linux__ +#include <linux/magic.h> +#include <sys/statfs.h> +#endif #include "dirname.h" #include "memory.h" #include "virterror_internal.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -407,3 +412,48 @@ virStorageFileGetMetadata(const char *path, return ret; } + + +#ifdef __linux__ + +#ifndef OCFS2_SUPER_MAGIC +#define OCFS2_SUPER_MAGIC 0x7461636f +#endif +#ifndef GFS2_MAGIC +#define GFS2_MAGIC 0x01161970 +#endif +#ifndef AFS_FS_MAGIC +#define AFS_FS_MAGIC 0x6B414653 +#endif + + +int virStorageFileIsSharedFS(const char *path) +{ + struct statfs sb; + + if (statfs(path, &sb) < 0) { + virReportSystemError(errno, + _("cannot determine filesystem for '%s'"), + path); + return -1; + } + + VIR_DEBUG("Check if path %s with FS magic %lld is shared", + path, (long long int)sb.f_type); + + if (sb.f_type == NFS_SUPER_MAGIC || + sb.f_type == GFS2_MAGIC || + sb.f_type == OCFS2_SUPER_MAGIC || + sb.f_type == AFS_FS_MAGIC) { + return 1; + } + + return 0; +} +#else +int virStorageFileIsSharedFS(const char *path ATTRIBUTE_UNUSED) +{ + /* XXX implement me :-) */ + return 0; +} +#endif diff --git a/src/util/storage_file.h b/src/util/storage_file.h index c761dc6..58533ee 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -61,4 +61,6 @@ int virStorageFileGetMetadataFromFD(const char *path, int fd, virStorageFileMetadata *meta); +int virStorageFileIsSharedFS(const char *path); + #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.6.6.1

4 5

[libvirt] [PATCH] nwfilter: Add missing driver lock in qemu driver
by Stefan Berger 17 May '10

17 May '10

This adds a missing driver lock in the qemu driver to protect the list of domains. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com> --- src/qemu/qemu_driver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -11869,11 +11869,15 @@ static virStateDriver qemuStateDriver = }; static int -qemudVMFilterRebuild(virConnectPtr conn, +qemudVMFilterRebuild(virConnectPtr conn ATTRIBUTE_UNUSED, virHashIterator iter, void *data) { - (void)conn; + struct qemud_driver *driver = qemu_driver; + + qemuDriverLock(driver); virHashForEach(qemu_driver->domains.objs, iter, data); + qemuDriverUnlock(driver); + return 0; }

3 3

[libvirt] [PATCH] qemudDomainSetVcpus: avoid NULL-deref
by Jim Meyering 15 May '10

15 May '10

Here's a patch for a coverity-spotted bug: >From 12160fa54bc948e5de3fecff6a9552995e9595b0 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 14 May 2010 12:38:43 +0200 Subject: [PATCH] qemudDomainSetVcpus: avoid NULL-deref * src/qemu/qemu_driver.c (qemudDomainSetVcpus): Avoid NULL-deref upon unknown UUID. Call qemuDomainObjBeginJob(vm) only after ensuring that vm != NULL, not before. This potential NULL-deref was introduced by commit 2c555d87b0041e0d1ec4742386d2161d1b2f0600. --- src/qemu/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bb1079e..cbddb96 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5410,9 +5410,6 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { vm = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); - if (qemuDomainObjBeginJob(vm) < 0) - goto cleanup; - if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); @@ -5421,6 +5418,9 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto endjob; } + if (qemuDomainObjBeginJob(vm) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); -- 1.7.1.460.gf3c4c

2 2