- Devel - libvirt List Archives

[libvirt] [PATCH] storage: fix build with iscsi
by Eric Blake 30 Apr '12

30 Apr '12

The previous storage patch missed an instance affected by the struct member rename. * src/storage/storage_backend_iscsi.c (virStorageBackendISCSIFindPoolSources): Adjust to new struct. --- Pushing under the build-breaker rule. src/storage/storage_backend_iscsi.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 22a34af..4f57d85 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -1,7 +1,7 @@ /* * storage_backend_iscsi.c: storage backend for iSCSI handling * - * Copyright (C) 2007-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -589,11 +589,13 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, } for (i = 0 ; i < ntargets ; i++) { - if (VIR_ALLOC_N(list.sources[i].devices, 1) < 0) { + if (VIR_ALLOC_N(list.sources[i].devices, 1) < 0 || + VIR_ALLOC_N(list.sources[i].hosts, 1)) { virReportOOMError(); goto cleanup; } - list.sources[i].host = source->hosts[0]; + list.sources[i].nhost = 1; + list.sources[i].hosts[0] = source->hosts[0]; list.sources[i].initiator = source->initiator; list.sources[i].ndevice = 1; list.sources[i].devices[0].path = targets[i]; @@ -607,8 +609,10 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, cleanup: if (list.sources) { - for (i = 0 ; i < ntargets ; i++) + for (i = 0 ; i < ntargets ; i++) { + VIR_FREE(list.sources[i].hosts); VIR_FREE(list.sources[i].devices); + } VIR_FREE(list.sources); } for (i = 0 ; i < ntargets ; i++) -- 1.7.1

3 3

[libvirt] [PATCH] Yet more coverity fixes
by Stefan Berger 30 Apr '12

30 Apr '12

Addressing the following reports: Error: RESOURCE_LEAK: /libvirt/src/nodeinfo.c:631: alloc_fn: Calling allocation function "fopen". /libvirt/src/nodeinfo.c:631: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")". /libvirt/src/nodeinfo.c:640: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /libvirt/src/vmx/vmx.c:2431: alloc_fn: Calling allocation function "calloc". /libvirt/src/vmx/vmx.c:2431: var_assign: Assigning: "networkName" = storage returned from "calloc(1UL, 1UL)". /libvirt/src/vmx/vmx.c:2495: leaked_storage: Variable "networkName" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /libvirt/src/util/virnetlink.c:338: alloc_arg: Calling allocation function "virAlloc" on "srv". /libvirt/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /libvirt/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /libvirt/src/util/virnetlink.c:343: noescape: Variable "srv" is not freed or pointed-to in function "virMutexInit". /libvirt/src/util/threads-pthread.c:49:30: noescape: "virMutexInit" does not free or save its pointer parameter "m". /libvirt/src/util/virnetlink.c:404: leaked_storage: Variable "srv" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /libvirt/src/qemu/qemu_driver.c:6968: alloc_fn: Calling allocation function "calloc". /libvirt/src/qemu/qemu_driver.c:6968: var_assign: Assigning: "nodeset" = storage returned from "calloc(1UL, 1UL)". /libvirt/src/qemu/qemu_driver.c:6977: noescape: Variable "nodeset" is not freed or pointed-to in function "virTypedParameterAssign". /libvirt/src/qemu/qemu_driver.c:6997: leaked_storage: Variable "nodeset" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /libvirt/tests/qemuxml2argvtest.c:47: alloc_arg: Calling allocation function "virAlloc" on "ret". /libvirt/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /libvirt/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /libvirt/tests/qemuxml2argvtest.c:54: leaked_storage: Variable "ret" going out of scope leaks the storage it points to. Error: STRING_NULL: /libvirt/src/util/uuid.c:273: string_null_argument: Function "getDMISystemUUID" does not terminate string "*dmiuuid". /libvirt/src/util/uuid.c:241: string_null_argument: Function "saferead" fills array "*uuid" with a non-terminated string. /libvirt/src/util/util.c:101: string_null_argument: Function "read" fills array "*buf" with a non-terminated string. /libvirt/src/util/uuid.c:274: string_null: Passing unterminated string "dmiuuid" to a function expecting a null-terminated string. /libvirt/src/util/uuid.c:138: var_assign_parm: Assigning: "cur" = "uuidstr". They now point to the same thing. /libvirt/src/util/uuid.c:164: string_null_sink_loop: Searching for null termination in an unterminated array "cur". Error: STRING_NULL: /libvirt/src/node_device/node_device_linux_sysfs.c:80: string_null_argument: Function "saferead" does not terminate string "*buf". /libvirt/src/util/util.c:101: string_null_argument: Function "read" fills array "*buf" with a non-terminated string. /libvirt/src/node_device/node_device_linux_sysfs.c:87: string_null: Passing unterminated string "buf" to a function expecting a null-terminated string. --- daemon/libvirtd.c | 24 +++++++++++++++++++----- src/node_device/node_device_linux_sysfs.c | 9 +++++---- src/nodeinfo.c | 1 + src/qemu/qemu_driver.c | 4 ++++ src/util/uuid.c | 5 +++-- src/util/virnetlink.c | 7 ++++++- src/vmx/vmx.c | 1 + tests/qemuxml2argvtest.c | 4 +++- 8 files changed, 42 insertions(+), 13 deletions(-) Index: libvirt-acl/daemon/libvirtd.c =================================================================== --- libvirt-acl.orig/daemon/libvirtd.c +++ libvirt-acl/daemon/libvirtd.c @@ -141,6 +141,7 @@ static int daemonForkIntoBackground(cons int stdinfd = -1; int stdoutfd = -1; int nextpid; + int tmpfd; VIR_FORCE_CLOSE(statuspipe[0]); @@ -151,16 +152,16 @@ static int daemonForkIntoBackground(cons if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO) goto cleanup; if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO) - goto cleanup; + goto cleanup_close_stdin_fileno; if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO) - goto cleanup; + goto cleanup_close_stdout_fileno; if (stdinfd > STDERR_FILENO && VIR_CLOSE(stdinfd) < 0) - goto cleanup; + goto cleanup_close_stderr_fileno; if (stdoutfd > STDERR_FILENO && VIR_CLOSE(stdoutfd) < 0) - goto cleanup; + goto cleanup_close_stderr_fileno; if (setsid() < 0) - goto cleanup; + goto cleanup_close_stderr_fileno; nextpid = fork(); switch (nextpid) { @@ -172,6 +173,19 @@ static int daemonForkIntoBackground(cons _exit(EXIT_SUCCESS); } + + cleanup_close_stderr_fileno: + tmpfd = STDERR_FILENO; + VIR_FORCE_CLOSE(tmpfd); + + cleanup_close_stdout_fileno: + tmpfd = STDOUT_FILENO; + VIR_FORCE_CLOSE(tmpfd); + + cleanup_close_stdin_fileno: + tmpfd = STDIN_FILENO; + VIR_FORCE_CLOSE(tmpfd); + cleanup: VIR_FORCE_CLOSE(stdoutfd); VIR_FORCE_CLOSE(stdinfd); Index: libvirt-acl/src/node_device/node_device_linux_sysfs.c =================================================================== --- libvirt-acl.orig/src/node_device/node_device_linux_sysfs.c +++ libvirt-acl/src/node_device/node_device_linux_sysfs.c @@ -69,20 +69,21 @@ out: int read_wwn_linux(int host, const char *file, char **wwn) { char *p = NULL; - int fd = -1, retval = 0; - char buf[64]; + int fd = -1, retval = 0, len; + char buf[65]; if (open_wwn_file(LINUX_SYSFS_FC_HOST_PREFIX, host, file, &fd) < 0) { goto out; } - memset(buf, 0, sizeof(buf)); - if (saferead(fd, buf, sizeof(buf)) < 0) { + len = saferead(fd, buf, sizeof(buf) - 1); + if (len < 0) { retval = -1; VIR_DEBUG("Failed to read WWN for host%d '%s'", host, file); goto out; } + buf[len] = '\0'; p = strstr(buf, "0x"); if (p != NULL) { Index: libvirt-acl/src/nodeinfo.c =================================================================== --- libvirt-acl.orig/src/nodeinfo.c +++ libvirt-acl/src/nodeinfo.c @@ -636,6 +636,7 @@ int nodeGetInfo(virConnectPtr conn ATTRI } if (virAsprintf(&sysfs_cpuinfo, CPU_SYS_PATH) < 0) { + VIR_FORCE_FCLOSE(cpuinfo); virReportOOMError(); return -1; } Index: libvirt-acl/src/util/uuid.c =================================================================== --- libvirt-acl.orig/src/util/uuid.c +++ libvirt-acl/src/util/uuid.c @@ -269,8 +269,9 @@ virSetHostUUIDStr(const char *uuid) return EEXIST; if (!uuid) { - memset(dmiuuid, 0, sizeof(dmiuuid)); - if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid) - 1)) { + rc = getDMISystemUUID(dmiuuid, sizeof(dmiuuid) - 1); + dmiuuid[VIR_UUID_STRING_BUFLEN - 1] = '\0'; + if (!rc) { if (!virUUIDParse(dmiuuid, host_uuid)) return 0; } Index: libvirt-acl/src/util/virnetlink.c =================================================================== --- libvirt-acl.orig/src/util/virnetlink.c +++ libvirt-acl/src/util/virnetlink.c @@ -341,7 +341,7 @@ virNetlinkEventServiceStart(void) } if (virMutexInit(&srv->lock) < 0) - goto error; + goto error_free_srv; virNetlinkEventServerLock(srv); @@ -402,6 +402,11 @@ error_locked: } error: return ret; + +error_free_srv: + VIR_FREE(srv); + + return ret; } /** Index: libvirt-acl/src/vmx/vmx.c =================================================================== --- libvirt-acl.orig/src/vmx/vmx.c +++ libvirt-acl/src/vmx/vmx.c @@ -2485,6 +2485,7 @@ virVMXParseEthernet(virConfPtr conf, int *def = NULL; } + VIR_FREE(networkName); VIR_FREE(connectionType); VIR_FREE(addressType); VIR_FREE(generatedAddress); Index: libvirt-acl/tests/qemuxml2argvtest.c =================================================================== --- libvirt-acl.orig/tests/qemuxml2argvtest.c +++ libvirt-acl/tests/qemuxml2argvtest.c @@ -50,8 +50,10 @@ fakeSecretLookupByUsage(virConnectPtr co ret->magic = VIR_SECRET_MAGIC; ret->refs = 1; ret->usageID = strdup(usageID); - if (!ret->usageID) + if (!ret->usageID) { + VIR_FREE(ret); return NULL; + } ret->conn = conn; conn->refs++; return ret; Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -6984,6 +6984,9 @@ qemuDomainGetNumaParameters(virDomainPtr if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET, VIR_TYPED_PARAM_STRING, nodeset) < 0) goto cleanup; + + nodeset = NULL; + break; default: @@ -6997,6 +7000,7 @@ qemuDomainGetNumaParameters(virDomainPtr ret = 0; cleanup: + VIR_FREE(nodeset); virCgroupFree(&group); if (vm) virDomainObjUnlock(vm);

2 4

[libvirt] A couple of questions on snapshot-create/ snapshot-create-as
by Kashyap Chamarthy 30 Apr '12

30 Apr '12

Hi (Eric?), I got a couple of questions for 'snapshot-create' / 'snapshot-create-as' : 1/ can '--disk-only' option (when used with virsh snapshot-create-as) be run on an offline guest and generate an 'external snapshot' ? Or a running guest is needed? 2/ Can anyone point me to a bit more verbose info on these below options while creating snapshots? --quiesce quiesce guest's file systems --atomic require atomic operation --reuse-external reuse any existing external files Thanks. /kashyap

2 2

[libvirt] [PATCH] storage: Break out the loop if duplicate pool is found
by Osier Yang 30 Apr '12

30 Apr '12

It doesn't break out the "for" loop even if duplicate pool is found, and thus the "matchpool" could be overriden as NULL again if there is different pool afterwards. To address the problem in libvirt-user list: https://www.redhat.com/archives/libvirt-users/2012-April/msg00150.html --- src/conf/storage_conf.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7579327..9a42e34 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1710,6 +1710,9 @@ int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, break; } virStoragePoolObjUnlock(pool); + + if (matchpool) + break; } if (matchpool) { -- 1.7.7.3

2 2

[libvirt] [PATCH] storage: Allow multiple hosts for a storage pool
by Wido den Hollander 30 Apr '12

30 Apr '12

The current storage pools for NFS and iSCSI only require one host to connect to. Future storage pools like RBD and Sheepdog will require multiple hosts. This patch allows multiple source hosts and rewrites the current storage drivers. Signed-off-by: Wido den Hollander <wido(a)widodh.nl> --- src/conf/storage_conf.c | 59 ++++++++++++++++++++++++---------- src/conf/storage_conf.h | 5 ++- src/esx/esx_storage_driver.c | 6 +++- src/storage/storage_backend_fs.c | 12 +++--- src/storage/storage_backend_iscsi.c | 12 +++--- src/test/test_driver.c | 4 +- 6 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 2330fa1..aa23ff3 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -276,7 +276,11 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source) if (!source) return; - VIR_FREE(source->host.name); + for (i = 0 ; i < source->nhost ; i++) { + VIR_FREE(source->hosts[i].name); + } + VIR_FREE(source->hosts); + for (i = 0 ; i < source->ndevice ; i++) { VIR_FREE(source->devices[i].freeExtents); VIR_FREE(source->devices[i].path); @@ -404,6 +408,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, char *authType = NULL; int nsource, i; virStoragePoolOptionsPtr options; + char *name = NULL; char *port = NULL; relnode = ctxt->node; @@ -431,17 +436,34 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, VIR_FREE(format); } - source->host.name = virXPathString("string(./host/@name)", ctxt); - port = virXPathString("string(./host/@port)", ctxt); - if (port) { - if (virStrToLong_i(port, NULL, 10, &source->host.port) < 0) { - virStorageReportError(VIR_ERR_XML_ERROR, - _("Invalid port number: %s"), - port); + source->nhost = virXPathNodeSet("./host", ctxt, &nodeset); + + if (source->nhost) { + if (VIR_ALLOC_N(source->hosts, source->nhost) < 0) { + virReportOOMError(); goto cleanup; } - } + for (i = 0 ; i < source->nhost ; i++) { + name = virXMLPropString(nodeset[i], "name"); + if (name == NULL) { + virStorageReportError(VIR_ERR_XML_ERROR, + "%s", _("missing storage pool host name")); + goto cleanup; + } + source->hosts[i].name = name; + + port = virXMLPropString(nodeset[i], "port"); + if (port) { + if (virStrToLong_i(port, NULL, 10, &source->hosts[i].port) < 0) { + virStorageReportError(VIR_ERR_XML_ERROR, + _("Invalid port number: %s"), + port); + goto cleanup; + } + } + } + } source->initiator.iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); @@ -674,7 +696,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) { } if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) { - if (!ret->source.host.name) { + if (!ret->source.nhost) { virStorageReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source host name")); @@ -800,12 +822,13 @@ virStoragePoolSourceFormat(virBufferPtr buf, int i, j; virBufferAddLit(buf," <source>\n"); - if ((options->flags & VIR_STORAGE_POOL_SOURCE_HOST) && - src->host.name) { - virBufferAsprintf(buf, " <host name='%s'", src->host.name); - if (src->host.port) - virBufferAsprintf(buf, " port='%d'", src->host.port); - virBufferAddLit(buf, "/>\n"); + if ((options->flags & VIR_STORAGE_POOL_SOURCE_HOST) && src->nhost) { + for (i = 0; i < src->nhost; i++) { + virBufferAsprintf(buf, " <host name='%s'", src->hosts[i].name); + if (src->hosts[i].port) + virBufferAsprintf(buf, " port='%d'", src->hosts[i].port); + virBufferAddLit(buf, "/>\n"); + } } if ((options->flags & VIR_STORAGE_POOL_SOURCE_DEVICE) && @@ -1677,7 +1700,7 @@ int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, break; case VIR_STORAGE_POOL_NETFS: if ((STREQ(pool->def->source.dir, def->source.dir)) \ - && (STREQ(pool->def->source.host.name, def->source.host.name))) + && (STREQ(pool->def->source.hosts[0].name, def->source.hosts[0].name))) matchpool = pool; break; case VIR_STORAGE_POOL_SCSI: @@ -1688,7 +1711,7 @@ int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, { matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def); if (matchpool) { - if (STREQ(matchpool->def->source.host.name, def->source.host.name)) { + if (STREQ(matchpool->def->source.hosts[0].name, def->source.hosts[0].name)) { if ((matchpool->def->source.initiator.iqn) && (def->source.initiator.iqn)) { if (STREQ(matchpool->def->source.initiator.iqn, def->source.initiator.iqn)) break; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 1ef9295..9222c4a 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -212,8 +212,9 @@ struct _virStoragePoolSourceDevice { typedef struct _virStoragePoolSource virStoragePoolSource; typedef virStoragePoolSource *virStoragePoolSourcePtr; struct _virStoragePoolSource { - /* An optional host */ - virStoragePoolSourceHost host; + /* An optional (maybe multiple) host(s) */ + size_t nhost; + virStoragePoolSourceHostPtr hosts; /* And either one or more devices ... */ int ndevice; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index adf1edb..9b64891 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -544,8 +544,12 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) if (esxVI_LocalDatastoreInfo_DynamicCast(info) != NULL) { def.type = VIR_STORAGE_POOL_DIR; } else if ((nasInfo = esxVI_NasDatastoreInfo_DynamicCast(info)) != NULL) { + if (VIR_ALLOC_N(def.source.hosts, 1) < 0) { + virReportOOMError(); + goto cleanup; + } def.type = VIR_STORAGE_POOL_NETFS; - def.source.host.name = nasInfo->nas->remoteHost; + def.source.hosts[0].name = nasInfo->nas->remoteHost; def.source.dir = nasInfo->nas->remotePath; if (STRCASEEQ(nasInfo->nas->type, "NFS")) { diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 1af12e6..79eefd3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -206,7 +206,7 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virStoragePoolObjPtr pool ATTR if (!(src = virStoragePoolSourceListNewSource(&state->list))) goto cleanup; - if (!(src->host.name = strdup(state->host)) || + if (!(src->hosts[0].name = strdup(state->host)) || !(src->dir = strdup(path))) { virReportOOMError(); goto cleanup; @@ -260,8 +260,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE if (!source) goto cleanup; - state.host = source->host.name; - prog[3] = source->host.name; + state.host = source->hosts[0].name; + prog[3] = source->hosts[0].name; if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, @@ -387,7 +387,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { int ret; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { - if (pool->def->source.host.name == NULL) { + if (pool->def->source.hosts[0].name == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source host")); return -1; @@ -415,7 +415,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { if (pool->def->type == VIR_STORAGE_POOL_NETFS) { if (virAsprintf(&src, "%s:%s", - pool->def->source.host.name, + pool->def->source.hosts[0].name, pool->def->source.dir) == -1) { virReportOOMError(); return -1; @@ -452,7 +452,7 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { int ret; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { - if (pool->def->source.host.name == NULL) { + if (pool->def->source.hosts[0].name == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source host")); return -1; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index f04473d..3acc5fe 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -96,13 +96,13 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) char ipaddr[NI_MAXHOST]; char *portal; - if (virStorageBackendISCSITargetIP(source->host.name, + if (virStorageBackendISCSITargetIP(source->hosts[0].name, ipaddr, sizeof(ipaddr)) < 0) return NULL; if (virAsprintf(&portal, "%s:%d,1", ipaddr, - source->host.port ? - source->host.port : 3260) < 0) { + source->hosts[0].port ? + source->hosts[0].port : 3260) < 0) { virReportOOMError(); return NULL; } @@ -581,7 +581,7 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, virReportOOMError(); goto cleanup; } - list.sources[i].host = source->host; + list.sources[i].host = source->hosts[0]; list.sources[i].initiator = source->initiator; list.sources[i].ndevice = 1; list.sources[i].devices[0].path = targets[i]; @@ -617,7 +617,7 @@ virStorageBackendISCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, *isActive = false; - if (pool->def->source.host.name == NULL) { + if (pool->def->source.hosts[0].name == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source host")); return -1; @@ -649,7 +649,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, int ret = -1; const char *loginargv[] = { "--login", NULL }; - if (pool->def->source.host.name == NULL) { + if (pool->def->source.hosts[0].name == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source host")); return -1; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 39ac503..73a7cb1 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4053,14 +4053,14 @@ testStorageFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, break; case VIR_STORAGE_POOL_NETFS: - if (!source || !source->host.name) { + if (!source || !source->hosts[0].name) { testError(VIR_ERR_INVALID_ARG, "%s", "hostname must be specified for netfs sources"); goto cleanup; } if (virAsprintf(&ret, defaultPoolSourcesNetFSXML, - source->host.name) < 0) + source->hosts[0].name) < 0) virReportOOMError(); break; -- 1.7.0.4

3 4

[libvirt] [PATCH] qemu: Make sure qemu can access its directory in hugetlbfs
by Jiri Denemark 30 Apr '12

30 Apr '12

When libvirtd is started, we create "libvirt/qemu" directories under hugetlbfs mount point. Only the "qemu" subdirectory is chowned to qemu user and "libvirt" remains owned by root. If umask was too restrictive when libvirtd started, qemu user may lose access to "qemu" subdirectory. Let's explicitly grant search permissions to "libvirt" directory for all users. --- src/qemu/qemu_driver.c | 27 +++++++++++++++++---------- src/util/virfile.c | 37 +++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 4 ++++ 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ce31e09..29bab3c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -456,6 +456,8 @@ qemudStartup(int privileged) { int rc; virConnectPtr conn = NULL; char ebuf[1024]; + char *membase = NULL; + char *mempath = NULL; if (VIR_ALLOC(qemu_driver) < 0) return -1; @@ -660,23 +662,26 @@ qemudStartup(int privileged) { */ if (qemu_driver->hugetlbfs_mount && qemu_driver->hugetlbfs_mount[0] == '/') { - char *mempath = NULL; - if (virAsprintf(&mempath, "%s/libvirt/qemu", qemu_driver->hugetlbfs_mount) < 0) + if (virAsprintf(&membase, "%s/libvirt", + qemu_driver->hugetlbfs_mount) < 0 || + virAsprintf(&mempath, "%s/qemu", membase) < 0) goto out_of_memory; if (virFileMakePath(mempath) < 0) { virReportSystemError(errno, _("unable to create hugepage path %s"), mempath); - VIR_FREE(mempath); goto error; } - if (qemu_driver->privileged && - chown(mempath, qemu_driver->user, qemu_driver->group) < 0) { - virReportSystemError(errno, - _("unable to set ownership on %s to %d:%d"), - mempath, qemu_driver->user, qemu_driver->group); - VIR_FREE(mempath); - goto error; + if (qemu_driver->privileged) { + if (virFileUpdatePerm(membase, 0, S_IXGRP | S_IXOTH) < 0) + goto error; + if (chown(mempath, qemu_driver->user, qemu_driver->group) < 0) { + virReportSystemError(errno, + _("unable to set ownership on %s to %d:%d"), + mempath, qemu_driver->user, + qemu_driver->group); + goto error; + } } qemu_driver->hugepage_path = mempath; @@ -737,6 +742,8 @@ error: virConnectClose(conn); VIR_FREE(base); VIR_FREE(driverConf); + VIR_FREE(membase); + VIR_FREE(mempath); qemudShutdown(); return -1; } diff --git a/src/util/virfile.c b/src/util/virfile.c index 66160dc..db3d737 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -437,3 +437,40 @@ int virFileTouch(const char *path, mode_t mode) return 0; } + + +#define MODE_BITS (S_ISUID | S_ISGID | S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO) + +int virFileUpdatePerm(const char *path, + mode_t mode_remove, + mode_t mode_add) +{ + struct stat sb; + mode_t mode; + + if (mode_remove & ~MODE_BITS || mode_add & ~MODE_BITS) { + virFileError(VIR_ERR_INVALID_ARG, "%s", _("invalid mode")); + return -1; + } + + if (stat(path, &sb) < 0) { + virReportSystemError(errno, _("cannot stat '%s'"), path); + return -1; + } + + mode = sb.st_mode & MODE_BITS; + + if ((mode & mode_remove) == 0 && (mode & mode_add) == mode_add) + return 0; + + mode &= MODE_BITS ^ mode_remove; + mode |= mode_add; + + if (chmod(path, mode) < 0) { + virReportSystemError(errno, _("cannot change permission of '%s'"), + path); + return -1; + } + + return 0; +} diff --git a/src/util/virfile.h b/src/util/virfile.h index 184677c..05f5048 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -83,4 +83,8 @@ int virFileRewrite(const char *path, int virFileTouch(const char *path, mode_t mode); +int virFileUpdatePerm(const char *path, + mode_t mode_remove, + mode_t mode_add); + #endif /* __VIR_FILES_H */ -- 1.7.8.6

2 2

[libvirt] libvirt stable releases
by Cole Robinson 29 Apr '12

29 Apr '12

Hi all, An idea we've kicked around for awhile in Red Hat/Fedora land is doing official libvirt stable releases, but nothing ever took shape. The idea was brought up again recently and I've offered to help get something going. I've pushed an upstream v0.9.11-maint branch with a bunch of patches cherry-picked to libvirt 0.9.11. Shortly I'll be cutting a 0.9.11.1 and pushing it to the website, like other releases. Why 0.9.11? Because that's what we will be shipping in Fedora 17 :) Typically our policy with fedora is to stick with one libvirt version for the length of a release. Cutting stable releases should save us from having to backport patches, and hopefully get us more bug fixes than backporting only what our users report. So I don't plan on doing a similar branch for 0.9.12 or 0.9.13, but will probably do a branch in 6 months time for whatever libvirt version we are shipping in Fedora 18. While my primary motiviation at the moment is making Fedora maintenance easier, I hope that other distros will use the stable releases as well, where we can all benefit from each others QA and attention. Any feedback appreciated! Thanks, Cole

5 7

[libvirt] Schedule for next release libvirt-0.9.12
by Daniel Veillard 28 Apr '12

28 Apr '12

Assuming we try to stay on a monthly schedule, then I would suggest to enter the freeze around next Friday (27th April) and shoot for a release around the 4th of May. I'm supposed to be in vacation most of next week, but I think I will still be able to make the release candidate 1 We have a number of patch set which need further review to be pushed fully, and there are a few things in http://libvirt.org/pending.html which aren't false positive that we really ought to check and push :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2 2

[libvirt] [PATCH] build: Fix version of gettext macros
by Peter Krempa 28 Apr '12

28 Apr '12

Commit c9cd419caba9effa11ca53e8696e5f6a4b424d60 added copying of the makefile for translation files from gnulib. The makefile from gnulib is of version 0.18 but the build configuration cretes macros from version 0.17 which breaks the build with message: *** error: gettext infrastructure mismatch: using a Makefile.in.in from gettext version 0.18 but the autoconf macros are from gettext version 0.17 --- bootstrap.conf | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index c6620e5..56911e0 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -195,7 +195,7 @@ buildreq="\ autoconf 2.59 automake 1.9.6 autopoint - -gettext 0.17 +gettext 0.18 git 1.5.5 gzip - libtool - diff --git a/configure.ac b/configure.ac index 89fe818..5936d31 100644 --- a/configure.ac +++ b/configure.ac @@ -2300,7 +2300,7 @@ dnl compute the difference between save_CPPFLAGS and CPPFLAGS and append it dnl to INCLUDES in order to preserve changes made by gettext but in a place dnl that does not break the build save_CPPFLAGS="$CPPFLAGS" -AM_GNU_GETTEXT_VERSION([0.17]) +AM_GNU_GETTEXT_VERSION([0.18]) AM_GNU_GETTEXT([external]) GETTEXT_CPPFLAGS= if test "x$save_CPPFLAGS" != "x$CPPFLAGS"; then -- 1.7.3.4

5 14

[libvirt] [PATCH v2] qemu_agent: Report error class at least
by Michal Privoznik 28 Apr '12

28 Apr '12

Currently, qemu GA is not providing 'desc' field for errors like we are used to from qemu monitor. Therefore, we fall back to this general 'unknown error' string. However, GA is reporting 'class' which is not perfect, but much more helpful than generic error string. Thus we should fall back to class firstly and if even no class is presented, then we can fall back to that generic string. Before this patch: virsh # dompmsuspend --target mem f16 error: Domain f16 could not be suspended error: internal error unable to execute QEMU command 'guest-suspend-ram': unknown QEMU command error After this patch: virsh # dompmsuspend --target mem f16 error: Domain f16 could not be suspended error: internal error unable to execute QEMU command 'guest-suspend-ram': The command has not been found --- src/qemu/qemu_agent.c | 37 +++++++++++++++++++++++++++++++++++-- 1 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index b759b7f..bc4ceff 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1025,6 +1025,40 @@ cleanup: return ret; } +static const char * +qemuAgentStringifyErrorClass(const char *klass) +{ + if (STREQ_NULLABLE(klass, "BufferOverrun")) + return "Buffer overrun"; + else if (STREQ_NULLABLE(klass, "CommandDisabled")) + return "The command has been disabled for this instance"; + else if (STREQ_NULLABLE(klass, "CommandNotFound")) + return "The command has not been found"; + else if (STREQ_NULLABLE(klass, "FdNotFound")) + return "File descriptor not found"; + else if (STREQ_NULLABLE(klass, "InvalidParameter")) + return "Invalid parameter"; + else if (STREQ_NULLABLE(klass, "InvalidParameterType")) + return "Invalid parameter type"; + else if (STREQ_NULLABLE(klass, "InvalidParameterValue")) + return "Invalid parameter value"; + else if (STREQ_NULLABLE(klass, "OpenFileFailed")) + return "Cannot open file"; + else if (STREQ_NULLABLE(klass, "QgaCommandFailed")) + return "Guest agent command failed"; + else if (STREQ_NULLABLE(klass, "QMPBadInputObjectMember")) + return "Bad QMP input object member"; + else if (STREQ_NULLABLE(klass, "QMPExtraInputObjectMember")) + return "Unexpected extra object member"; + else if (STREQ_NULLABLE(klass, "UndefinedError")) + return "An undefined error has occurred"; + else if (STREQ_NULLABLE(klass, "Unsupported")) + return "this feature or command is not currently supported"; + else if (klass) + return klass; + else + return "unknown QEMU command error"; +} /* Ignoring OOM in this method, since we're already reporting * a more important error @@ -1043,9 +1077,8 @@ qemuAgentStringifyError(virJSONValuePtr error) if (klass) detail = virJSONValueObjectGetString(error, "desc"); - if (!detail) - detail = "unknown QEMU command error"; + detail = qemuAgentStringifyErrorClass(klass); return detail; } -- 1.7.8.5

2 3