- Devel - libvirt List Archives

[libvirt] [PATCH] Fix several memory leaks
by Ryota Ozaki 05 Sep '09

05 Sep '09

Hi, This patch fixes several memory leaks being scattered over libvirt. Thanks, ozaki-r >From 6fc282eae5192cccda208d8d4fd14c5c0676992b Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota(a)gmail.com> Date: Tue, 1 Sep 2009 01:34:40 +0900 Subject: [PATCH] Fix several memory leaks --- qemud/qemud.c | 2 +- src/domain_conf.c | 1 + src/network_conf.c | 5 +++-- src/qemu_conf.c | 13 ++++++++----- src/storage_backend_fs.c | 5 +++++ 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..17ba44a 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -1733,7 +1733,7 @@ readmore: /* Possibly need to create another receive buffer */ if ((client->nrequests < max_client_requests && - VIR_ALLOC(client->rx) < 0)) { + !client->rx && VIR_ALLOC(client->rx) < 0)) { qemudDispatchClientFailure(client); } else { if (client->rx) diff --git a/src/domain_conf.c b/src/domain_conf.c index 1d2cc7c..4b64219 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -4361,6 +4361,7 @@ int virDomainSaveXML(virConnectPtr conn, cleanup: if (fd != -1) close(fd); + VIR_FREE(configFile); return ret; } diff --git a/src/network_conf.c b/src/network_conf.c index bb649a4..58a4f32 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -820,6 +820,7 @@ int virNetworkDeleteConfig(virConnectPtr conn, { char *configFile = NULL; char *autostartLink = NULL; + int ret = -1; if ((configFile = virNetworkConfigFile(conn, configDir, net->def->name)) == NULL) goto error; @@ -836,12 +837,12 @@ int virNetworkDeleteConfig(virConnectPtr conn, goto error; } - return 0; + ret = 0; error: VIR_FREE(configFile); VIR_FREE(autostartLink); - return -1; + return ret; } char *virNetworkConfigFile(virConnectPtr conn, diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..32d6a48 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1034,7 +1034,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, virDomainNetDefPtr net, int qemuCmdFlags) { - char *brname; + char *brname = NULL; int err; int tapfd = -1; int vnet_hdr = 0; @@ -1053,7 +1053,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, if (brname == NULL) return -1; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { - brname = net->data.bridge.brname; + brname = strdup(net->data.bridge.brname); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Network type %d is not supported"), net->type); @@ -1063,7 +1063,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, if (!driver->brctl && (err = brInit(&driver->brctl))) { virReportSystemError(conn, err, "%s", _("cannot initialize bridge support")); - return -1; + goto cleanup; } if (!net->ifname || @@ -1072,7 +1072,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, VIR_FREE(net->ifname); if (!(net->ifname = strdup("vnet%d"))) { virReportOOMError(conn); - return -1; + goto cleanup; } /* avoid exposing vnet%d in dumpxml or error outputs */ template_ifname = 1; @@ -1100,9 +1100,12 @@ qemudNetworkIfaceConnect(virConnectPtr conn, } if (template_ifname) VIR_FREE(net->ifname); - return -1; + tapfd = -1; } +cleanup: + VIR_FREE(brname); + return tapfd; } diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index ca6d329..222e591 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -856,6 +856,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_FILE; vol->target.format = VIR_STORAGE_VOL_FILE_RAW; /* Real value is filled in during probe */ + if (vol->target.path) + VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) @@ -1022,6 +1024,9 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_FILE; + if (vol->target.path) + VIR_FREE(vol->target.path); + if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) { -- 1.6.0.6

4 7

[libvirt] [PATCH] storage_driver.c: remove two useless calls to virStorageBackendForType
by Jim Meyering 05 Sep '09

05 Sep '09

>From dead-store warnings: >From c925b02828083a016f7b9a7ae93eb3a7bf5f4609 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:18:29 +0200 Subject: [PATCH] storage_driver.c: remove two useless calls to virStorageBackendForType * src/storage_driver.c (storagePoolGetInfo, storagePoolDefine): The result was never used, and the function has no side effects. --- src/storage_driver.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/src/storage_driver.c b/src/storage_driver.c index e9ecb20..e8d43b7 100644 --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -519,15 +519,11 @@ storagePoolDefine(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; - virStorageBackendPtr backend; storageDriverLock(driver); if (!(def = virStoragePoolDefParseString(conn, xml))) goto cleanup; - if ((backend = virStorageBackendForType(def->type)) == NULL) - goto cleanup; - if (!(pool = virStoragePoolObjAssignDef(conn, &driver->pools, def))) goto cleanup; @@ -847,7 +843,6 @@ storagePoolGetInfo(virStoragePoolPtr obj, virStoragePoolInfoPtr info) { virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; - virStorageBackendPtr backend; int ret = -1; storageDriverLock(driver); @@ -860,9 +855,6 @@ storagePoolGetInfo(virStoragePoolPtr obj, goto cleanup; } - if ((backend = virStorageBackendForType(pool->def->type)) == NULL) - goto cleanup; - memset(info, 0, sizeof(virStoragePoolInfo)); if (pool->active) info->state = VIR_STORAGE_POOL_RUNNING; -- 1.6.4.2.409.g85dc3

2 3

[libvirt] [PATCH 6/7] VMware ESX: Add esxDomainUndefine() based on esxVI_UnregisterVM()
by Matthias Bolte 05 Sep '09

05 Sep '09

* src/esx/esx_driver.c: add esxDomainUndefine() based on esxVI_UnregisterVM() * src/esx/esx_vi_methods.[ch]: add esxVI_UnregisterVM()

2 3

[libvirt] [PATCH 5/7] VMware ESX: Add esxGetCapabilities() with basic default capabilities
by Matthias Bolte 05 Sep '09

05 Sep '09

* src/esx/esx_driver.c: add esxCapsInit() with default caps and add esxGetCapabilities()

2 3

[libvirt] [PATCH 1/7] VMware ESX: Fix VMX path parsing and URL encoding
by Matthias Bolte 05 Sep '09

05 Sep '09

* src/esx/esx_driver.c: handle spaces in VMX file path and use a virBuffer to encode spaces correctly in the resulting URL * src/esx/esx_vi.c: include the URL in the error message in case of a download error

2 3

[libvirt] [PATCH] domain_conf.c: remove dead store
by Jim Meyering 05 Sep '09

05 Sep '09

Another dead store: >From 06c5ed162cbdcfec74461e9d09c82244242cc6a2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:27:34 +0200 Subject: [PATCH] domain_conf.c: remove dead store * src/domain_conf.c (virDomainSaveXML): ...and decl of "err". --- src/domain_conf.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index 8dde5dd..4e8ad37 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -4456,12 +4456,11 @@ int virDomainSaveXML(virConnectPtr conn, char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; - int err; if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - if ((err = virFileMakePath(configDir))) { + if (virFileMakePath(configDir)) { virReportSystemError(conn, errno, _("cannot create config directory '%s'"), configDir); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] xm_internal.c: remove two ret=... dead stores
by Jim Meyering 05 Sep '09

05 Sep '09

Two more dead stores: >From 92bf654b02e5c1eb3334fc13ef52f5b0679512ea Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 17:22:19 +0200 Subject: [PATCH] xm_internal.c: remove two ret=... dead stores * src/xm_internal.c (xenXMDomainCreate): Remove dead stores. --- src/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xm_internal.c b/src/xm_internal.c index e675672..e948fa0 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1851,10 +1851,10 @@ int xenXMDomainCreate(virDomainPtr domain) { goto error; domain->id = ret; - if ((ret = xend_wait_for_devices(domain->conn, domain->name)) < 0) + if (xend_wait_for_devices(domain->conn, domain->name) < 0) goto error; - if ((ret = xenDaemonDomainResume(domain)) < 0) + if (xenDaemonDomainResume(domain) < 0) goto error; xenUnifiedUnlock(priv); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH] lxc_container.c: avoid a leak on error paths
by Jim Meyering 05 Sep '09

05 Sep '09

This started with a dead-store report: File: lxc_container.c Location: line 417, column 10 Description: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc' But there was a leak, too, upon any but the first failure. This fixes both. >From 141ba8bab2de00c911017bff6fca240c72a80633 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 16:12:35 +0200 Subject: [PATCH] lxc_container.c: avoid a leak on error paths * src/lxc_container.c (lxcContainerMountBasicFS): Don't leak upon failure. Add "cleanup:" label and change each post-allocation failure to use "goto cleanup" rather than returning immediately. --- src/lxc_container.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 2073864..f4381e7 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -1,6 +1,6 @@ /* * Copyright IBM Corp. 2008 - * Copyright Red Hat 2008 + * Copyright Red Hat 2008-2009 * * lxc_container.c: file description * @@ -384,12 +384,12 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) { "none", "/selinux", "selinuxfs" }, #endif }; - int i, rc; + int i, rc = -1; char *devpts; if (virAsprintf(&devpts, "/.oldroot%s/dev/pts", root->src) < 0) { virReportOOMError(NULL); - return -1; + return rc; } for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { @@ -397,31 +397,35 @@ static int lxcContainerMountBasicFS(virDomainFSDefPtr root) virReportSystemError(NULL, errno, _("failed to mkdir %s"), mnts[i].src); - return -1; + goto cleanup; } if (mount(mnts[i].src, mnts[i].dst, mnts[i].type, 0, NULL) < 0) { virReportSystemError(NULL, errno, _("failed to mount %s on %s"), mnts[i].type, mnts[i].type); - return -1; + goto cleanup; } } if ((rc = virFileMakePath("/dev/pts") < 0)) { virReportSystemError(NULL, rc, "%s", _("cannot create /dev/pts")); - return -1; + goto cleanup; } VIR_DEBUG("Trying to move %s to %s", devpts, "/dev/pts"); if ((rc = mount(devpts, "/dev/pts", NULL, MS_MOVE, NULL)) < 0) { virReportSystemError(NULL, errno, "%s", _("failed to mount /dev/pts in container")); - return -1; + goto cleanup; } + + rc = 0; + + cleanup: VIR_FREE(devpts); - return 0; + return rc; } static int lxcContainerPopulateDevices(void) -- 1.6.4.2.409.g85dc3

2 2

[libvirt] Dead code vs. XXX comment: remove or not?
by Jim Meyering 05 Sep '09

05 Sep '09

clang reported that this assignment to type is a dead store, since type is never used after this point. This is xm_internal.c, line 1074: /* XXX Forcing to pretend its a bridge */ if (type == -1) { type = 1; } Normally I'd just remove the whole if block, but the XXX comment makes me think someone has plans for this code. I'll wait for a second opinion.

2 3

[libvirt] address 3 more dead stores
by Jim Meyering 05 Sep '09

05 Sep '09

These changes fix two dead stores and add a comment suggesting why *not* to remove the third one. >From 41c778fdbaef9abf52bc4c3e278d3389d8198c34 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 10:55:55 +0200 Subject: [PATCH 1/3] interface_conf.c: remove a dead-store and declaration * src/interface_conf.c (virInterfaceDefParseDhcp): Remove unused "old". --- src/interface_conf.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/interface_conf.c b/src/interface_conf.c index a74d17e..96e0710 100644 --- a/src/interface_conf.c +++ b/src/interface_conf.c @@ -228,11 +228,9 @@ static int virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def, xmlNodePtr dhcp, xmlXPathContextPtr ctxt) { char *tmp; - xmlNodePtr old; int ret = 0; def->proto.dhcp = 1; - old = ctxt->node; ctxt->node = dhcp; /* Not much to do in the current version */ tmp = virXPathString(conn, "string(./@peerdns)", ctxt); -- 1.6.4.2.409.g85dc3 >From 10037ca58c8d93e427c480082704e7dd52424525 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 11:01:00 +0200 Subject: [PATCH 2/3] hash.c: remove a dead store * src/hash.c (virHashFree): Remove useless assignment to inside_table. --- src/hash.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/hash.c b/src/hash.c index 9308c0c..45c5747 100644 --- a/src/hash.c +++ b/src/hash.c @@ -231,7 +231,6 @@ virHashFree(virHashTablePtr table, virHashDeallocator f) inside_table = 0; iter = next; } - inside_table = 0; } VIR_FREE(table->table); } -- 1.6.4.2.409.g85dc3 >From 3b16e6d50d364a5307284a78a7b23d8631146663 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Fri, 4 Sep 2009 11:20:23 +0200 Subject: [PATCH 3/3] qemu_conf.c: add a comment suggesting why we leave a dead-store * src/qemu_conf.c (qemuBuildHostNetStr): Do not remove the type_sep=',' dead store, since not having it would be a problem if we ever add a new attribute=VAL option. --- src/qemu_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 2c4a37d..6ffca2f 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1269,7 +1269,7 @@ qemuBuildHostNetStr(virConnectPtr conn, if (net->hostnet_name) { virBufferVSprintf(&buf, "%cname=%s", type_sep, net->hostnet_name); - type_sep = ','; + type_sep = ','; /* dead-store, but leave it, in case... */ } if (virBufferError(&buf)) { virReportOOMError(conn); -- 1.6.4.2.409.g85dc3

2 1

[libvirt] [PATCH 7/7] VMware ESX: Check the result of virUUIDParse() in esxUtil_GetConfigUUID()
by Matthias Bolte 05 Sep '09

05 Sep '09

* src/esx/esx_util.c: let esxUtil_GetConfigUUID() report an error if virUUIDParse() fails

2 1

[libvirt] [PATCH 4/7] VMware ESX: Simplify SOAP request and response handling
by Matthias Bolte 05 Sep '09

05 Sep '09

* src/esx/esx_vi.[ch]: convert esxVI_RemoteRequest_Execute() to a simpler esxVI_Context_Execute() version, remove esxVI_RemoteRequest and convert esxVI_RemoteResponse to esxVI_Response * src/esx/esx_vi_methods.c: update and simplify callers to use esxVI_Context_Execute() instead of esxVI_RemoteRequest_Execute()

2 1

[libvirt] [PATCH 3/7] VMware ESX: Use virXPathNode*() to simplify XPath handling
by Matthias Bolte 05 Sep '09

05 Sep '09

* src/esx/esx_vi.[ch]: use virXPathNode*() in esxVI_RemoteRequest_Execute() and remove esxVI_RemoteResponse_DeserializeXPathObject*() * src/esx/esx_vi_methods.c: update callers to use the new syntax of esxVI_RemoteRequest_Execute()

2 1

[libvirt] [PATCH 2/7] VMware ESX: Make esxVI_GetVirtualMachineIdentity() more robust
by Matthias Bolte 05 Sep '09

05 Sep '09

Check the config status before requesting config subelements * src/esx/esx_driver.c: add configStatus to the requested properties to check it in esxVI_GetVirtualMachineIdentity() * src/esx/esx_vi.[ch]: add esxVI_GetManagedEntityStatus() and use it in esxVI_GetVirtualMachineIdentity() * src/esx/esx_vi_types.[ch]: add VI type esxVI_ManagedEntityStatus

2 1

[libvirt] [PATCH] Fix more OOM handling bugs
by Daniel P. Berrange 05 Sep '09

05 Sep '09

* src/qemu_conf.c: Fix leak of values upon OOM * src/xend_internal.c: Fix missing check for OOM failure * tests/qemuargv2xmltest.c, tests/qemuxml2argvtest.c: Free stateDir upon exit to avoid leak --- src/qemu_conf.c | 21 ++++++++++++++++++--- src/xend_internal.c | 6 ++++++ tests/qemuargv2xmltest.c | 1 + tests/qemuxml2argvtest.c | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..ad41104 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1722,6 +1722,8 @@ int qemudBuildCommandLine(virConnectPtr conn, continue; } + ADD_ARG_SPACE; + if (idx < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported disk type '%s'"), disk->dst); @@ -1773,7 +1775,10 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-drive"); + if ((qargv[qargc++] = strdup("-drive")) == NULL) { + VIR_FREE(optstr); + goto no_memory; + } ADD_ARG(optstr); } } else { @@ -1829,6 +1834,7 @@ int qemudBuildCommandLine(virConnectPtr conn, net->vlan = i; + ADD_ARG_SPACE; if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) && qemuAssignNetNames(def, net) < 0) goto no_memory; @@ -1836,9 +1842,14 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuBuildNicStr(conn, net, NULL, ',', net->vlan, &nic) < 0) goto error; - ADD_ARG_LIT("-net"); + if ((qargv[qargc++] = strdup("-net")) == NULL) { + VIR_FREE(nic); + goto no_memory; + } ADD_ARG(nic); + + ADD_ARG_SPACE; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); @@ -1862,7 +1873,10 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-net"); + if ((qargv[qargc++] = strdup("-net")) == NULL) { + VIR_FREE(host); + goto no_memory; + } ADD_ARG(host); VIR_FREE(tapfd_name); @@ -2230,6 +2244,7 @@ static int qemuStringToArgvEnv(const char *args, goto no_memory; for (i = 0 ; i < envend ; i++) { progenv[i] = arglist[i]; + arglist[i] = NULL; } progenv[i] = NULL; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 99847b0..20ddb89 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -5212,6 +5212,9 @@ xenDaemonFormatSxprChr(virConnectPtr conn, break; } + if (virBufferError(buf)) + return -1; + return 0; } @@ -5535,6 +5538,9 @@ xenDaemonFormatSxprSound(virConnectPtr conn, virBufferVSprintf(buf, "%s%s", i ? "," : "", str); } + if (virBufferError(buf)) + return -1; + return 0; } diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 7861520..4a92280 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -223,6 +223,7 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + free(driver.stateDir); virCapabilitiesFree(driver.caps); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6f25e7d..2f91288 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -270,6 +270,7 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + free(driver.stateDir); virCapabilitiesFree(driver.caps); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); -- 1.6.2.5

2 1

[libvirt] [PATCH] Fix logging buffer overrun read
by Daniel P. Berrange 05 Sep '09

05 Sep '09

* src/logging.c: Fix buffer offset in logging read --- src/logging.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/logging.c b/src/logging.c index e46e2be..07c2b0e 100644 --- a/src/logging.c +++ b/src/logging.c @@ -240,7 +240,7 @@ static void virLogStr(const char *str, int len) { tmp = LOG_BUFFER_SIZE - virLogEnd; memcpy(&virLogBuffer[virLogEnd], str, tmp); virLogBuffer[LOG_BUFFER_SIZE] = 0; - memcpy(&virLogBuffer[0], &str[len], len - tmp); + memcpy(&virLogBuffer[0], &str[tmp], len - tmp); virLogEnd = len - tmp; } else { memcpy(&virLogBuffer[virLogEnd], str, len); -- 1.6.2.5

3 2

[libvirt] [PATCH] uml_conf.c: don't return an uninitialized pointer
by Jim Meyering 05 Sep '09

05 Sep '09

Another "real" bug: >From 6697607bf0b023ffb692576b31d652d10719b08a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 12:05:52 +0200 Subject: [PATCH] uml_conf.c: don't return an uninitialized pointer * src/uml_conf.c (umlBuildCommandLineChr): Initialize "ret" also in the final switch cases. --- src/uml_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/uml_conf.c b/src/uml_conf.c index 838fedd..2e9c25c 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -331,6 +331,7 @@ umlBuildCommandLineChr(virConnectPtr conn, default: umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported chr device type %d"), def->type); + ret = NULL; break; } -- 1.6.4.2.395.ge3d52

3 9

[libvirt] three more clang-inspired dead-store-fixing patches
by Jim Meyering 05 Sep '09

05 Sep '09

>From a2d03c987bb724283207dbeef873178c08a6c4c5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:14:48 +0200 Subject: [PATCH 1/3] storage_backend_logical.c: appease clang: remove useless increment * src/storage_backend_logical.c (virStorageBackendLogicalBuildPool): Don't increment "n" when we won't use the result. --- src/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c index bc40dd7..43117e8 100644 --- a/src/storage_backend_logical.c +++ b/src/storage_backend_logical.c @@ -437,7 +437,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn, goto cleanup; } - vgargv[n++] = NULL; + vgargv[n] = NULL; /* Now create the volume group itself */ if (virRun(conn, vgargv, NULL) < 0) -- 1.6.4.2.395.ge3d52 >From b72ed1ba5d5e6444f000fd9be706a51a90dd2292 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:23:51 +0200 Subject: [PATCH 2/3] openvz_conf.c: Remove dead store to copy_fd * src/openvz_conf.c (openvz_copyfile): Remove unused assignment. --- src/openvz_conf.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index 41c6684..2f0471c 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -692,7 +692,6 @@ openvz_copyfile(char* from_path, char* to_path) fd = -1; if (close(copy_fd) < 0) goto error; - copy_fd = -1; return 0; -- 1.6.4.2.395.ge3d52 >From a8d2eca13635578084e3c2cc2093562d9a8fcfed Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:25:03 +0200 Subject: [PATCH 3/3] eventtest.c: detect write failure and avoid dead stores * tests/eventtest.c (mymain): Exit nonzero upon write failure. This also avoids several dead stores of the form ret = safewrite... --- tests/eventtest.c | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/eventtest.c b/tests/eventtest.c index d25381d..ef37ff1 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -248,7 +248,6 @@ resetAll(void) static int mymain(int argc, char **argv) { - int ret = 0; char *progname; int i; pthread_t eventThread; @@ -304,7 +303,8 @@ mymain(int argc, char **argv) /* First time, is easy - just try triggering one of our * registered handles */ startJob("Simple write", &test); - ret = safewrite(handles[1].pipeFD[1], &one, 1); + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(1, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -314,7 +314,8 @@ mymain(int argc, char **argv) * try triggering another handle */ virEventRemoveHandleImpl(handles[0].watch); startJob("Deleted before poll", &test); - ret = safewrite(handles[1].pipeFD[1], &one, 1); + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(1, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -346,8 +347,9 @@ mymain(int argc, char **argv) * about */ startJob("Deleted during dispatch", &test); handles[2].delete = handles[3].watch; - ret = safewrite(handles[2].pipeFD[1], &one, 1); - ret = safewrite(handles[3].pipeFD[1], &one, 1); + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1 + || safewrite(handles[3].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(2, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -356,7 +358,8 @@ mymain(int argc, char **argv) /* Extreme fun, lets delete ourselves during dispatch */ startJob("Deleted during dispatch", &test); handles[2].delete = handles[2].watch; - ret = safewrite(handles[2].pipeFD[1], &one, 1); + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(2, -1) != EXIT_SUCCESS) return EXIT_FAILURE; -- 1.6.4.2.395.ge3d52

2 2

[libvirt] [PATCH] Added Storage Volume support to VIrtualBox
by Pritesh Kothari 05 Sep '09

05 Sep '09

Hi All, I have added support for Storage Volumes in VirtualBox, the patch for the same is attached here. (needs the "little bit cleanup" patch previously posted) Regards, Pritesh

2 1

[libvirt] [PATCH] openvz_conf.c: don't use undefined local, "net"
by Jim Meyering 05 Sep '09

05 Sep '09

This looks like a real bug. >From 170af3320e68a0ac2cfe854fba28abe0e4040d2c Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 11:24:44 +0200 Subject: [PATCH] openvz_conf.c: don't use undefined local, "net" * src/openvz_conf.c (openvzReadNetworkConf): Upon openvzRead... failure, simply return -1, rather than "goto error;" where an uninitialized "net" could be dereferenced. --- src/openvz_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..b1cb31a 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -197,7 +197,7 @@ openvzReadNetworkConf(virConnectPtr conn, openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not read 'IP_ADDRESS' from config for container %d"), veid); - goto error; + return -1; } else if (ret > 0) { token = strtok_r(temp, " ", &saveptr); while (token != NULL) { -- 1.6.4.2.395.ge3d52

3 4

[libvirt] [PATCH] storage_backend.c: assure clang that inputvol can't be NULL
by Jim Meyering 05 Sep '09

05 Sep '09

clang was complaining that a NULL inputvol would be dereferenced in that "could not open..." diagnostic. Since the two sole callers of this function are careful to call it only when inputvol is non-NULL, this is a good case for giving the parameter the nonnull attribute: >From 314278acb021a1f2e63494fab352bd6e9a4a38bb Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 10:22:57 +0200 Subject: [PATCH] storage_backend.c: assure clang that inputvol can't be NULL * src/storage_backend.c (virStorageBackendCopyToFD): Declare inputvol parameter to be "nonnull". Remove test for non-NULL inputvol. Both callers ensure it's non-NULL. --- src/storage_backend.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index c818142..8d1187e 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -1,7 +1,7 @@ /* * storage_backend.c: internal storage driver backend contract * - * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2009 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -106,7 +106,7 @@ enum { static int virStorageBackendCopyToFD(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol, + virStorageVolDefPtr inputvol ATTRIBUTE_NONNULL, int fd, unsigned long long *total, int is_dest_file) @@ -119,13 +119,11 @@ virStorageBackendCopyToFD(virConnectPtr conn, char zerobuf[512]; char *buf = NULL; - if (inputvol) { - if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("could not open input path '%s'"), - inputvol->target.path); - goto cleanup; - } + if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("could not open input path '%s'"), + inputvol->target.path); + goto cleanup; } bzero(&zerobuf, sizeof(zerobuf)); -- 1.6.4.2.395.ge3d52

3 4

[libvirt] [PATCH] little bit cleanup
by Pritesh Kothari 04 Sep '09

04 Sep '09

Hi All, Just cleaned up some code to make the it more readable. The patch is attached here with. Directly applies to the HEAD as of today. Regards, Pritesh

3 2

[libvirt] [PATCH] Minor comment changes.
by Laine Stump 04 Sep '09

04 Sep '09

Fix some minor grammer (and one other) nits in comments that end up in generated API reference documentation. No functional/binary differences. --- src/libvirt.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ca8e003..ce3d6ba 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -592,9 +592,9 @@ virRegisterNetworkDriver(virNetworkDriverPtr driver) /** * virRegisterInterfaceDriver: - * @driver: pointer to a interface driver block + * @driver: pointer to an interface driver block * - * Register a interface virtualization driver + * Register an interface virtualization driver * * Returns the driver priority or -1 in case of error. */ @@ -5365,7 +5365,7 @@ error: /** * virNetworkGetXMLDesc: * @network: a network object - * @flags: and OR'ed set of extraction flags, not used yet + * @flags: an OR'ed set of extraction flags, not used yet * * Provide an XML description of the network. The description may be reused * later to relaunch the network with virNetworkCreateXML(). @@ -5545,7 +5545,7 @@ error: /** * virInterfaceGetConnect: - * @iface: pointer to a interface + * @iface: pointer to an interface * * Provides the connection pointer associated with an interface. The * reference counter on the connection is not increased by this @@ -5819,7 +5819,7 @@ error: /** * virInterfaceGetName: - * @iface: a interface object + * @iface: an interface object * * Get the public name for that interface * @@ -5842,9 +5842,9 @@ virInterfaceGetName(virInterfacePtr iface) /** * virInterfaceGetMACString: - * @iface: a interface object + * @iface: an interface object * - * Get the MAC for a interface as string. For more information about + * Get the MAC for an interface as string. For more information about * MAC see RFC4122. * * Returns a pointer to the MAC address (in null-terminated ASCII @@ -5867,11 +5867,11 @@ virInterfaceGetMACString(virInterfacePtr iface) /** * virInterfaceGetXMLDesc: - * @iface: a interface object - * @flags: and OR'ed set of extraction flags, not used yet + * @iface: an interface object + * @flags: an OR'ed set of extraction flags, not used yet * * Provide an XML description of the interface. The description may be reused - * later to recreate the interface with virInterfaceCreateXML(). + * later to redefine the interface with virInterfaceDefineXML(). * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. @@ -6100,7 +6100,7 @@ error: * This method is typically useful for applications where multiple * threads are using a connection, and it is required that the * connection remain open until all threads have finished using - * it. ie, each new thread using a interface would increment + * it. ie, each new thread using an interface would increment * the reference count. * * Returns 0 in case of success, -1 in case of failure. @@ -6121,7 +6121,7 @@ virInterfaceRef(virInterfacePtr iface) /** * virInterfaceFree: - * @iface: a interface object + * @iface: an interface object * * Free the interface object. The interface itself is unaltered. * The data structure is freed and should not be used thereafter. -- 1.6.2.5

3 2

[libvirt] [PATCH] Fix a memory leak in virsh.
by Laine Stump 04 Sep '09

04 Sep '09

I found this while looking for examples of using virNodeDeviceGetXMLDesc(). AFAIK, *all* of the *GetXMLDesc() functions return a newly allocated chunk of memory that is owned by the caller, who must free it when they're done... --- src/virsh.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/virsh.c b/src/virsh.c index 15e0cef..910d860 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -5521,6 +5521,7 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) { const char *name; virNodeDevicePtr device; + char *xml; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -5531,7 +5532,14 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) return FALSE; } - vshPrint(ctl, "%s\n", virNodeDeviceGetXMLDesc(device, 0)); + xml = virNodeDeviceGetXMLDesc(device, 0); + if (!xml) { + virNodeDeviceFree(device); + return FALSE; + } + + vshPrint(ctl, "%s\n", xml); + free(xml); virNodeDeviceFree(device); return TRUE; } -- 1.6.2.5

3 2

[libvirt] [PATCH] Fix sexpr2string() to handle empty list
by Jim Fehlig 04 Sep '09

04 Sep '09

Finally have some time to submit this small patch for a bug I'm seeing with Xen 3.3.1. If s-expression returned by xend contains an empty list, sexpr2string() failed to serialize it. E.g. sexpr containing (cpus (()())) would cause sexpr2string() to fail. I spoke with danpb on IRC about this bug and he suggested adding a XML/sexpr pair that exhibits the problem to tests/xml2sexprtest.c. I can easily produce the sexpr but not the case with XML. The empty cpus lists indicate no explicit affinity and AFAIK the only way to express this in XML is absence of cpuset attribute in vcpu element. But IMO we should not produce the (cpus (()()...)) sexpr in this case. It should only be produced when user has explicitly specified affinity. Does this sound reasonable? Regards, Jim

2 1

[libvirt] [PATCH] Fix ID field in virDomainPtr after starting Xen guest
by Daniel P. Berrange 04 Sep '09

04 Sep '09

* src/xend_internal.c: Lookup guest after starting persistent config in order to refresh the domain ID field --- src/xend_internal.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..459b6bc 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -4537,6 +4537,8 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { int xenDaemonDomainCreate(virDomainPtr domain) { xenUnifiedPrivatePtr priv; + int ret; + virDomainPtr tmp; if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG, @@ -4549,7 +4551,17 @@ int xenDaemonDomainCreate(virDomainPtr domain) if (priv->xendConfigVersion < 3) return(-1); - return xend_op(domain->conn, domain->name, "op", "start", NULL); + ret = xend_op(domain->conn, domain->name, "op", "start", NULL); + + if (ret != -1) { + /* Need to force a refresh of this object's ID */ + tmp = virDomainLookupByName(domain->conn, domain->name); + if (tmp) { + domain->id = tmp->id; + virDomainFree(tmp); + } + } + return ret; } int xenDaemonDomainUndefine(virDomainPtr domain) -- 1.6.4

2 1

[libvirt] Save & Restore Synchronization
by Erkan Unal 04 Sep '09

04 Sep '09

Hi, I am using libvirt for qemu/kvm. I have following questions in terms of save and restore (Version 0.6.2): 1) As I checked the libvirt code, restore command is asynchronous. There is a macro called VIR_EXEC_NONBLOCK but there is no macro called VIR_EXEC_BLOCK. Is it possible to execute the restore in blocking mode so that I can measure the whole restore time? 2) I tried to measure the time spent to save the VM. I am executing save command and getting a timing. However when I execute the following operations in order I got an error right after script issues the restore operation (No delay between them): "save->restore->save" ... the error is: 20:19:02.478: error : internal error Unable to open monitor path /dev/pts/3 libvir: QEMU error : internal error Unable to open monitor path /dev/pts/3 20:19:02.478: error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED libvir: QEMU error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED 20:19:02.482: error : operation failed: failed to start VM libvir: QEMU error : operation failed: failed to start VM error: Failed to restore domain from /path/to/the/save/file error: operation failed: failed to start VM (You can find the detailed error report in the attachement when LIBVIRT_DEBUG flag is 1) I think that some of the cleanup is non-blocking which are cleanup of pty device and vncserver. However, I am not sure if destroying the qemu process is asynchronous?? P.S.: If I put one second delay between first save and restore, there is no error. Save and restore work fine. Thanks, Erkan Unal 20:26:07.506: debug : virInitialize:287 : register drivers 20:26:07.506: debug : virRegisterDriver:660 : registering Test as driver 0 20:26:07.506: debug : virRegisterNetworkDriver:560 : registering Test as network driver 0 20:26:07.506: debug : virRegisterStorageDriver:591 : registering Test as storage driver 0 20:26:07.506: debug : virRegisterDeviceMonitor:622 : registering Test as device driver 0 20:26:07.506: debug : virRegisterDriver:660 : registering Xen as driver 1 20:26:07.506: debug : virRegisterDriver:660 : registering OPENVZ as driver 2 20:26:07.506: debug : virRegisterDriver:660 : registering remote as driver 3 20:26:07.506: debug : virRegisterNetworkDriver:560 : registering remote as network driver 1 20:26:07.506: debug : virRegisterStorageDriver:591 : registering remote as storage driver 1 20:26:07.506: debug : virRegisterDeviceMonitor:622 : registering remote as device driver 1 20:26:07.506: debug : virConnectOpenAuth:1089 : name=qemu:///session, auth=0x7fa21cbae660, flags=0 20:26:07.506: debug : do_open:909 : name "qemu:///session" to URI components: scheme qemu opaque (null) authority (null) server (null) user (null) port 0 path /session 20:26:07.506: debug : do_open:919 : trying driver 0 (Test) ... 20:26:07.506: debug : do_open:925 : driver 0 Test returned DECLINED 20:26:07.506: debug : do_open:919 : trying driver 1 (Xen) ... 20:26:07.506: debug : do_open:925 : driver 1 Xen returned DECLINED 20:26:07.506: debug : do_open:919 : trying driver 2 (OPENVZ) ... 20:26:07.506: debug : do_open:925 : driver 2 OPENVZ returned DECLINED 20:26:07.506: debug : do_open:919 : trying driver 3 (remote) ... 20:26:07.506: debug : remoteOpen:974 : Auto-remote UNIX socket 20:26:07.506: debug : remoteOpen:992 : Auto-spawn user daemon instance 20:26:07.506: debug : doRemoteOpen:511 : proceeding with name = qemu:///session 20:26:07.507: debug : call:6473 : Doing call 66 (nil) 20:26:07.507: debug : call:6543 : We have the buck 66 0x7fa21bdb5010 0x7fa21bdb5010 20:26:07.507: debug : processCallRecvLen:6131 : Got length, now need 36 total (32 more) 20:26:07.507: debug : processCalls:6399 : Giving up the buck 66 0x7fa21bdb5010 (nil) 20:26:07.507: debug : call:6574 : All done with our call 66 (nil) 0x7fa21bdb5010 20:26:07.507: debug : call:6473 : Doing call 1 (nil) 20:26:07.507: debug : call:6543 : We have the buck 1 0xf86d80 0xf86d80 20:26:07.509: debug : processCallRecvLen:6131 : Got length, now need 28 total (24 more) 20:26:07.509: debug : processCalls:6399 : Giving up the buck 1 0xf86d80 (nil) 20:26:07.509: debug : call:6574 : All done with our call 1 (nil) 0xf86d80 20:26:07.509: debug : doRemoteOpen:822 : Adding Handler for remote events 20:26:07.509: debug : doRemoteOpen:829 : virEventAddHandle failed: No addHandleImpl defined. continuing without events. 20:26:07.509: debug : do_open:925 : driver 3 remote returned SUCCESS 20:26:07.509: debug : do_open:945 : network driver 0 Test returned DECLINED 20:26:07.509: debug : do_open:945 : network driver 1 remote returned SUCCESS 20:26:07.509: debug : do_open:967 : storage driver 0 Test returned DECLINED 20:26:07.509: debug : do_open:967 : storage driver 1 remote returned SUCCESS 20:26:07.510: debug : do_open:988 : node driver 0 Test returned DECLINED 20:26:07.510: debug : do_open:988 : node driver 1 remote returned SUCCESS 20:26:07.510: debug : virDomainRestore:2012 : conn=0xf82430, from=/path/to/save/file 20:26:07.510: debug : call:6473 : Doing call 54 (nil) 20:26:07.510: debug : call:6543 : We have the buck 54 0xf86d80 0xf86d80 20:26:07.625: error : internal error Unable to open monitor path /dev/pts/3 libvir: QEMU error : internal error Unable to open monitor path /dev/pts/3 20:26:07.625: error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED libvir: QEMU error : internal error unable to start guest: char device redirected to /dev/pts/3 inet_listen: bind(ipv4,127.0.0.1,5912): Address already in use inet_listen: FAILED 20:26:07.629: error : operation failed: failed to start VM libvir: QEMU error : operation failed: failed to start VM 20:26:07.629: debug : processCallRecvLen:6131 : Got length, now need 160 total (156 more) 20:26:07.629: debug : processCalls:6399 : Giving up the buck 54 0xf86d80 (nil) 20:26:07.629: debug : call:6574 : All done with our call 54 (nil) 0xf86d80 error: Failed to restore domain from /path/to/save/file error: operation failed: failed to start VM 20:26:07.629: debug : virConnectClose:1107 : conn=0xf82430 20:26:07.629: debug : call:6473 : Doing call 2 (nil) 20:26:07.629: debug : call:6543 : We have the buck 2 0xf86d80 0xf86d80 20:26:07.629: debug : processCallRecvLen:6131 : Got length, now need 28 total (24 more) 20:26:07.629: debug : processCalls:6399 : Giving up the buck 2 0xf86d80 (nil) 20:26:07.629: debug : call:6574 : All done with our call 2 (nil) 0xf86d80 20:26:07.629: debug : virUnrefConnect:210 : unref connection 0xf82430 1 20:26:07.629: debug : virReleaseConnect:171 : release connection 0xf82430

2 1

[libvirt] libvirt socket closed unexpectedly / libvirt broken pipe
by Dave 04 Sep '09

04 Sep '09

Hello, I'm trying to get eucalyptus v1.5.2 working with debian lenny, the relevant versions of libvirt software installed are from the apt-get repos, which are: # dpkg -l|grep libvirt ii libvirt-bin 0.4.6-10 the programs for the libvirt library ii libvirt0 0.4.6-10 library for interfacing with different virtualization systems The the conf file looks like so: # cat libvirtd.conf|grep -v '#' unix_sock_group = "libvirt" unix_sock_rw_perms = "0770" auth_unix_ro = "none" auth_unix_rw = "none" ----- my eucalyptus nc.log has the following error, which i believe is the issue, basically any instance i start, terminates about a minute later by itself, any guidance or help is much appreciated [Sat Aug 15 03:20:41 2009][004602][EUCAERROR ] libvirt: socket closed unexpectedly (code=39) [Sat Aug 15 03:20:43 2009][004602][EUCAERROR ] libvirt: Broken pipe (code=38) thoughts? ideas?

2 1

[libvirt] [PATCH] Allow libvirtd to RPC to external libvirtd.
by Chris Lalancette 04 Sep '09

04 Sep '09

Allow the daemon itself to make RPCs to an external libvirtd, but only if the URI is fully specified. While this isn't used at the moment, it will be for the tunnelled migration support in the future. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/remote_internal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote_internal.c b/src/remote_internal.c index bc69c70..a416cf3 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -1001,7 +1001,7 @@ remoteOpen (virConnectPtr conn, int ret, rflags = 0; const char *autostart = getenv("LIBVIRT_AUTOSTART"); - if (inside_daemon) + if (inside_daemon && (!conn->uri || (conn->uri && !conn->uri->server))) return VIR_DRV_OPEN_DECLINED; if (!(priv = remoteAllocPrivateData(conn))) -- 1.6.0.6

2 3

[libvirt] On pidfile security and using -daemonize to start qemu/kvm
by Charles Duffy 04 Sep '09

04 Sep '09

I had a brief conversation with Daniel Berrange on IRC this morning regarding race conditions in libvirt's invocation of qemu domains. The preferred approach from qemu's maintainership is using -daemonize at invocation, and then waiting for the process to exit before attempting to connect to the monitor; after this occurs, qemu guarantees monitor availability, so no retry/timeout behavior is necessary. Dan pointed out an issue with this approach: To determine the PID of the qemu instance started, the -pidfile argument to qemu needs to be used with -daemonize. However, -pidfile writes the requested pidfile with qemu's permissions; if the guest were subverted, it could rewrite its pidfile and lead libvirt to kill other, arbitrary processes. Neither changing permissions with -runas rather than prior to invocation nor chown'ing the pidfile from libvirt after creation helps, as qemu keeps a file handle on it open throughout operation (to maintain a POSIX lock on the file). However, a workaround exists, and it meshes well with a security fix which needs to be implemented anyhow: At present, on initial startup or libvirtd restart, we read the PIDs associated with vCPUs from the monitor in virDetectVcpuPIDs(); doing this on libvirtd restart poses a security risk for the reason given above, so we need to have libvirtd record the PIDs on initial startup (before any untrusted code within the guest is ever run) regardless. If we also read the libvirtd pidfile at this same early time (before the guest has been started and thus had an opportunity to run untrusted code) and include its contents when writing out the vCPU PIDs, this would eliminate the issue with pidfile ownership blocking use of -daemonize. Thoughts?

2 1

[libvirt] [PATCH] xen_internal.c: remove two unused local variables
by Jim Meyering 04 Sep '09

04 Sep '09

Obviously unnecessary. Maybe someone had plans for these long ago? >From 94730ec1d99076cd18b04b950adad7c7a576071f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 17:11:14 +0200 Subject: [PATCH] xen_internal.c: remove two unused local variables * src/xen_internal.c (xenHypervisorSetSchedulerParameters): Remove set-but-never-used local variables, cap_set and weight_set. --- src/xen_internal.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..30fd8bd 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1299,9 +1299,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, TODO return(-1); case XEN_SCHEDULER_CREDIT: { - int weight_set = 0; - int cap_set = 0; - memset(&op_dom, 0, sizeof(op_dom)); op_dom.cmd = XEN_V2_OP_SCHEDULER; op_dom.domain = (domid_t) domain->id; @@ -1326,7 +1323,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return(-1); } op_dom.u.getschedinfo.u.credit.weight = val; - weight_set = 1; } else if (STREQ (params[i].field, str_cap) && params[i].type == VIR_DOMAIN_SCHED_FIELD_UINT) { val = params[i].value.ui; @@ -1336,7 +1332,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return(-1); } op_dom.u.getschedinfo.u.credit.cap = val; - cap_set = 1; } else { virXenErrorFunc (domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__, "Credit scheduler accepts 'cap' and 'weight' integer parameters", -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH] python: let libvirt_virConnectDomainEventCallback indicate success
by Jim Meyering 04 Sep '09

04 Sep '09

First dead-store-related bug fix: >From 6024fb05dfa2dd408ad79abbac88791f5048af11 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 14:09:49 +0200 Subject: [PATCH] python: let libvirt_virConnectDomainEventCallback indicate success * python/libvir.c (libvirt_virConnectDomainEventCallback): Return 0 when successful, rather than always returning -1. clang flagged this function for its dead-store of "ret=0". Once "ret" was set to 0, it was never used, and the function would always return -1. --- python/libvir.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python/libvir.c b/python/libvir.c index e2c196a..0c00b80 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -4,7 +4,7 @@ * entry points where an automatically generated stub is * unpractical * - * Copyright (C) 2005, 2007, 2008 Red Hat, Inc. + * Copyright (C) 2005, 2007-2009 Red Hat, Inc. * * Daniel Veillard <veillard(a)redhat.com> */ @@ -1812,7 +1812,7 @@ libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, cleanup: LIBVIRT_RELEASE_THREAD_STATE; - return -1; + return ret; } static PyObject * -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH 0/3] QEMU directory/monitor socket fixes/cleanup
by Daniel P. Berrange 04 Sep '09

04 Sep '09

This patch series does three things - Moves QEMU monitor sockets in /var/lib/libvirt/qemu, since this is created file and thus should be separate from libvirtd created data to avoid risk from QEMU compromise. - Sets QEMU directory ownership to match configured qemu user/group - Fixes a minor memory leak / socket unlink

2 4

[libvirt] [PATCH 2/2]: VirtualBox: Updated vboxNetworkUndefine() and vboxNetworkDestroy()
by Pritesh Kothari 04 Sep '09

04 Sep '09

Hi All, I have made some changes to the functions vboxNetworkCreateXML(), vboxNetworkDefineXML(), vboxNetworkUndefine() and vboxNetworkDestroy() to handle multiple host only interfaces as multiple host only interfaces are supported by VirtualBox 3.0 and greater. The patch's are as below: PATCH 1/2: Merged vboxNetworkCreateXML() and vboxNetworkDefineXML() and added code to handle multiple hostonly interfaces. PATCH 2/2: Merged vboxNetworkUndefine() and vboxNetworkDestroy() and added code to handle multiple hostonly interfaces. Regards, Pritesh

3 2

[libvirt] [PATCH] esx_vi: remove unreachable code
by Jim Meyering 04 Sep '09

04 Sep '09

This removes dead code and suppresses a warning from clang. However, see below: >From 2ed548cbe948aa7ef37ccd121bd084bd05019ac8 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 16:35:47 +0200 Subject: [PATCH] esx_vi: remove unreachable code * src/esx/esx_vi.c (esxVI_Enumeration_Deserialize): Remove unreachable code. --- src/esx/esx_vi.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 0da908b..c689a7a 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -857,8 +857,6 @@ esxVI_Enumeration_Deserialize(virConnectPtr conn, failure: goto cleanup; - - result = -1; } -------------------------------------------- This function does not allow the caller to distinguish via its return value whether it has failed. It always returns 0. I see that this function's return value is used in numerous places (each esxVI_.*Deserialize function), so this should be fixed. int esxVI_Enumeration_Deserialize(virConnectPtr conn, const esxVI_Enumeration *enumeration, xmlNodePtr node, int *value) { int i; int result = 0; char *name = NULL; if (value == NULL) { ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Invalid argument"); goto failure; } *value = 0; /* undefined */ if (esxVI_String_DeserializeValue(conn, node, &name) < 0) { goto failure; } for (i = 0; enumeration->values[i].name != NULL; ++i) { if (STREQ(name, enumeration->values[i].name)) { *value = enumeration->values[i].value; goto cleanup; } } ESX_VI_ERROR(conn, VIR_ERR_INTERNAL_ERROR, "Unknown value '%s' for %s", name, enumeration->type); cleanup: VIR_FREE(name); return result; failure: goto cleanup; }

2 2

[libvirt] [PATCH 0/1] Support huge pages in guests
by Daniel P. Berrange 03 Sep '09

03 Sep '09

This patch is an update of John Cooper's original work from last month http://www.redhat.com/archives/libvir-list/2009-July/msg00753.html Changes since that patch - We create a directory $MOUNTPOINT/libvirt/qemu inside the hugetlbfs mount point. This is neccessary because QEMU now runs as 'qemu:qemu' in Fedora and thus cannot create files in the root. libvirtd will automatically chown() this subdir to allow QEMU guests access - Don't automatically probe for hugetlbfs mount if runing as an unprivileged libvirtd, since user won't have access to the mount unless administrator has manually created them a subdir. - Change the XML to look like <memoryBacking> <hugepages/> </memoryBacking> This is because I fear we may have more config options for memory backing store in the future, such as whether KVM can use KSM for sharing on the guest. - Add a test case for it in the XML -> ARGV convetor - Document the new XML options - Add the new XML options to the RNG schema - Fix setup & auto-detection of hugetlbfs mount if no qemu.conf file exists at all - Add config parameter to the augeas schema - Move the code which finds a mount point into util.c since its not specific to QEMU

4 5

[libvirt] PATCH 0/0: Support setting a disk drive serial (number) v2
by Daniel P. Berrange 03 Sep '09

03 Sep '09

This is a second version of the patch posted here http://www.redhat.com/archives/libvir-list/2009-August/msg00341.html In this version, I make the QEMU driver refuse to run with any serial that doesn't contain 0-9, a-Z, -, _, since there's no way to escape special chars in QEMU command line args. Hopefully we can lift this restriction in a future version of QEMU I also add a check that serial=s is actually supported by QEMU

3 3

[libvirt] [PATCH] mdns.c: remove dead initialization
by Jim Meyering 03 Sep '09

03 Sep '09

The only following uses of group look like this: (always preceded by initialization): AVAHI_DEBUG("Client collision/connecting %p", mdns->client); group = mdns->group; while (group) { if (group->handle) avahi_entry_group_reset(group->handle); group = group->next; } So the value used in the initialization at the top is never used. >From 515e4a4542ce396b8775c618cbe17a30b78299b4 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 13:45:22 +0200 Subject: [PATCH] mdns.c: remove dead initialization * qemud/mdns.c (libvirtd_mdns_client_callback): Remove dead initialization. --- qemud/mdns.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemud/mdns.c b/qemud/mdns.c index ae6ff08..73b7d11 100644 --- a/qemud/mdns.c +++ b/qemud/mdns.c @@ -178,7 +178,7 @@ static void libvirtd_mdns_create_services(struct libvirtd_mdns_group *group) { static void libvirtd_mdns_client_callback(AvahiClient *c, AvahiClientState state, void *userdata) { struct libvirtd_mdns *mdns = (struct libvirtd_mdns *)userdata; - struct libvirtd_mdns_group *group = mdns->group; + struct libvirtd_mdns_group *group; if (!mdns->client) mdns->client = c; -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH] node_device_conf.c: remove dead initialization
by Jim Meyering 03 Sep '09

03 Sep '09

The following use of "caps" is the assignment in a for-loop, so the initialization is unnecessary. >From 7032a0b370db1a9489cf19fce0cabbb6959320ab Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 13:38:57 +0200 Subject: [PATCH] node_device_conf.c: remove dead initialization * src/node_device_conf.c (virNodeDeviceDefFormat): Remove dead initialization. --- src/node_device_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/node_device_conf.c b/src/node_device_conf.c index 56a9bb5..5b25bb7 100644 --- a/src/node_device_conf.c +++ b/src/node_device_conf.c @@ -213,7 +213,7 @@ char *virNodeDeviceDefFormat(virConnectPtr conn, const virNodeDeviceDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; - virNodeDevCapsDefPtr caps = def->caps; + virNodeDevCapsDefPtr caps; char *tmp; virBufferAddLit(&buf, "<device>\n"); -- 1.6.4.2.395.ge3d52

2 1

[libvirt] [PATCH] Fix misc OOM bugs
by Daniel P. Berrange 03 Sep '09

03 Sep '09

* tests/testutils.c: Run test function twice, once to prime it for static allocations, once to count the non-static allocations. * tests/testutilsqemu.c: Initialize variable correctl * src/capabilities.c: Don't free machines variable upon failure since caller must do that * src/xm_internal.c: Add missing check for OOM in building VIF config param --- src/capabilities.c | 17 +++++++++-------- src/xm_internal.c | 3 +++ tests/testutils.c | 12 ++++++++---- tests/testutilsqemu.c | 2 +- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/capabilities.c b/src/capabilities.c index 193a9fe..289180d 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -354,10 +354,6 @@ virCapabilitiesAddGuest(virCapsPtr caps, if (loader && (guest->arch.defaultInfo.loader = strdup(loader)) == NULL) goto no_memory; - if (nmachines) { - guest->arch.defaultInfo.nmachines = nmachines; - guest->arch.defaultInfo.machines = machines; - } if (VIR_REALLOC_N(caps->guests, caps->nguests + 1) < 0) @@ -365,6 +361,11 @@ virCapabilitiesAddGuest(virCapsPtr caps, caps->guests[caps->nguests] = guest; caps->nguests++; + if (nmachines) { + guest->arch.defaultInfo.nmachines = nmachines; + guest->arch.defaultInfo.machines = machines; + } + return guest; no_memory: @@ -407,10 +408,6 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest, if (loader && (dom->info.loader = strdup(loader)) == NULL) goto no_memory; - if (nmachines) { - dom->info.nmachines = nmachines; - dom->info.machines = machines; - } if (VIR_REALLOC_N(guest->arch.domains, guest->arch.ndomains + 1) < 0) @@ -418,6 +415,10 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest, guest->arch.domains[guest->arch.ndomains] = dom; guest->arch.ndomains++; + if (nmachines) { + dom->info.nmachines = nmachines; + dom->info.machines = machines; + } return dom; diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..66f1e1c 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -2048,6 +2048,9 @@ static int xenXMDomainConfigFormatNet(virConnectPtr conn, virBufferVSprintf(&buf, ",vifname=%s", net->ifname); + if (virBufferError(&buf)) + goto cleanup; + if (VIR_ALLOC(val) < 0) { virReportOOMError(conn); goto cleanup; diff --git a/tests/testutils.c b/tests/testutils.c index 7a1dbdc..5072fec 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -367,10 +367,7 @@ int virtTestMain(int argc, } } - if (testOOM) - virAllocTestInit(); - - /* Run once to count allocs, and ensure it passes :-) */ + /* Run once to prime any static allocations & ensure it passes */ ret = (func)(argc, argv); if (ret != EXIT_SUCCESS) goto cleanup; @@ -385,6 +382,13 @@ int virtTestMain(int argc, testOOM++; virSetErrorFunc(NULL, virtTestErrorFuncQuiet); + virAllocTestInit(); + + /* Run again to count allocs, and ensure it passes :-) */ + ret = (func)(argc, argv); + if (ret != EXIT_SUCCESS) + goto cleanup; + approxAlloc = virAllocTestCount(); testCounter++; if (testDebug) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 58707e1..6b6a185 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -9,7 +9,7 @@ virCapsPtr testQemuCapsInit(void) { struct utsname utsname; virCapsPtr caps; virCapsGuestPtr guest; - virCapsGuestMachinePtr *machines; + virCapsGuestMachinePtr *machines = NULL; int nmachines; static const char *const x86_machines[] = { "pc", "isapc" -- 1.6.2.5

2 2

[libvirt] [PATCH] remote_internal.c: don't dereference a NULL "conn"
by Jim Meyering 03 Sep '09

03 Sep '09

Here's the test just before the else-if in the patch below: if (conn && conn->driver && STREQ (conn->driver->name, "remote")) { So, in the else-branch, "conn" is guaranteed to be NULL. And dereferenced. This may be only a theoretical risk, but if so, the test of "conn" above should be changed to an assertion, and/or the parameter should get the nonnull attribute. >From a1b1d36d96f6b50ddf514539af85da20ca671bf5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 11:54:38 +0200 Subject: [PATCH] remote_internal.c: don't dereference a NULL "conn" * src/remote_internal.c (remoteDevMonOpen): Avoid NULL-dereference. --- src/remote_internal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote_internal.c b/src/remote_internal.c index ea50c11..141fef9 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -5148,7 +5148,7 @@ remoteDevMonOpen(virConnectPtr conn, conn->devMonPrivateData = priv; remoteDriverUnlock(priv); return VIR_DRV_OPEN_SUCCESS; - } else if (conn->networkDriver && + } else if (conn && conn->networkDriver && STREQ (conn->networkDriver->name, "remote")) { struct private_data *priv = conn->networkPrivateData; remoteDriverLock(priv); -- 1.6.4.2.395.ge3d52

5 9

[libvirt] [PATCH] test.c: don't use undefined local, "def"
by Jim Meyering 03 Sep '09

03 Sep '09

another real bug: >From fd41bd47252654f84684830e02ecbeb3d26e7dee Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 12:12:19 +0200 Subject: [PATCH] test.c: don't use undefined local, "def" * src/test.c (testOpenVolumesForPool): Upon early virAsprintf or virXPathNodeSet failure, "goto error" would take us to virStorageVolDefFree(def), but with "def" not defined. Initialize it to NULL. --- src/test.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/test.c b/src/test.c index 7c8f85b..895cce1 100644 --- a/src/test.c +++ b/src/test.c @@ -435,7 +435,7 @@ static int testOpenVolumesForPool(virConnectPtr conn, char *vol_xpath; int i, ret, func_ret = -1; xmlNodePtr *vols = NULL; - virStorageVolDefPtr def; + virStorageVolDefPtr def = NULL; /* Find storage volumes */ if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", poolidx) < 0) { -- 1.6.4.2.395.ge3d52

2 1

[libvirt] Support for XCI (Xen Client)
by Andreas Sommer 03 Sep '09

03 Sep '09

Hi, I just wondered whether the Xen Client project (http://www.xen.org/products/xci.html) has been tested, or is supported by libvirt at all? I'm not sure if it has a different Xen API, but it definitely has different domain configuration files - that's why I ask. Best regards

2 1

[libvirt] [PATCH] Support for defining/dumping video device in VirtualBox
by Pritesh Kothari 03 Sep '09

03 Sep '09

Hi All, I have added support for defining/dumping video device in VirtualBox. The patch for the same is attached here. Also this patch depends on the earlier patch "Support for 3d Acceleration in video tag". Regards, Pritesh

3 3

[libvirt] [PATCH] Support for 3d Acceleration in video tag
by Pritesh Kothari 03 Sep '09

03 Sep '09

Hi All, I have added support for 3d Acceleration in the video tag, the patch for the same is attached here. Regards, Pritesh

3 5

[libvirt] [PATCH] Fix bugs in virDomainMigrate v2 code.
by Chris Lalancette 03 Sep '09

03 Sep '09

Paolo Bonzini points out that in my refactoring of the code for virDomainMigrate(), I added a check for the return value from virDomainMigratePerform(). The problem is that we don't want to exit if we fail, we actually want to go on and do virDomainMigrateFinish2() with a non-0 return code to clean things up. Remove the check. While reproducing this issue, I also noticed that we wouldn't always properly propagate an error message. In particular, I found that if you blocked off the migration ports (with iptables) and then tried the migration, it would actually fail but we would get no failure output from Qemu. Therefore, we would think we succeeded, and leave a huge mess behind us. Execute the monitor command "info migrate", and look for a failure string in there as well. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/libvirt.c | 2 -- src/qemu_driver.c | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ca8e003..5c1efb6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2962,8 +2962,6 @@ virDomainMigrateVersion2 (virDomainPtr domain, */ ret = domain->conn->driver->domainMigratePerform (domain, cookie, cookielen, uri, flags, dname, bandwidth); - if (ret == -1) - goto done; /* In version 2 of the migration protocol, we pass the * status code from the sender to the destination host, diff --git a/src/qemu_driver.c b/src/qemu_driver.c index eb22940..772f2f9 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -6956,6 +6956,21 @@ qemudDomainMigratePerform (virDomainPtr dom, goto cleanup; } + /* it is also possible that the migrate didn't fail initially, but + * rather failed later on. Check the output of "info migrate" + */ + VIR_FREE(info); + if (qemudMonitorCommand(vm, "info migrate", &info) < 0) { + qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("could not get info about migration")); + goto cleanup; + } + if (strstr(info, "fail") != NULL) { + qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("migrate failed: %s"), info); + goto cleanup; + } + /* Clean up the source domain. */ qemudShutdownVMDaemon (dom->conn, driver, vm); paused = 0; -- 1.6.0.6

2 2

[libvirt] virsh vcpuinfo always returns CPU=0
by Shi Jin 03 Sep '09

03 Sep '09

Hi there, I wonder what does the CPU entry stands for in the output of vcpuinfo. I guess it is the CPU utilization rate, right? For both Ubuntu 9.04 (virsh version 0.6.1) and RHEL 5.4 (version 0.6.3), I always get the returned CPU value to be 0, no matter how busy the CPU can be and how many VCPU is used. For example, onnecting to uri: qemu:///system VCPU: 0 CPU: 0 State: running CPU Affinity: yyyyyyyyyyyyyyyy VCPU: 1 CPU: 0 State: running CPU Affinity: yyyyyyyyyyyyyyyy Is this a problem and how do I fix it? Thank you very much. -- Shi Jin, PhD

2 1

[libvirt] VMware ESX: Don't warn if a known query parameter should be ignored
by Matthias Bolte 03 Sep '09

03 Sep '09

esxUtil_ParseQuery() warns if a known query parameter should be ignored due to the corresponding char/int pointer being NULL, instead of silently ignoring it. The attached patch changes the if/else structure to fix this. Matthias

3 2

[libvirt] VMware ESX: Allow ethernet address type 'vpx'
by Matthias Bolte 03 Sep '09

03 Sep '09

The VMX entry ethernet0.addressType may be set to 'vpx' beside 'static' and 'generated'. 'vpx' indicates that the MAC address was generated by a vCenter. The attached patch adds 'vpx' to the valid values for ethernet0.addressType. Matthias

4 6

[libvirt] [PATCH] Support for getting/setting number of cpus in VirtualBox
by Pritesh Kothari 03 Sep '09

03 Sep '09

Hi All, I have just added support for getting/setting number of cpus in VirtualBox. The patch for the same is include below. Regards, Pritesh

3 2

Re: [libvirt] [PATCH 2/2] RPM spec file updated with glusterfs dependency
by Gerrit Slomma 02 Sep '09

02 Sep '09

> On Tue, Jul 07, 2009 at 05:47:56AM -0700, Harshavardhana wrote: > > Add new dependency for glusterfs rpm. > [...] > > +# For glusterfs > > +Requires: glusterfs-client >= 2.0.2 > > %endif > > why 2.0.2 ? is taht a hard requirement ? In Fedora/Rawhide we have > only 2.0.1 at the moment, > > Daniel > > -- > Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ > daniel veillard com | Rpmfind RPM search engine http://rpmfind.net/ > http://veillard.com/ | virtualization library http://libvirt.org/ Tested libvirt-0.7.0 today. And this requirement really breaks installation of builds on RHEL/CentOS/ScientificLinux. The newest available built for glusterfs is those systems is glusterfs-client-1.3.8 from freshrpms. Even --without-storage-fs does not help a bit, must be pulled in implicitly? rpm -pq --requires libvirt-0.7.0-1.x86_64.rpm (...) glusterfs-client >= 2.0.1 (...) Is this patched in upcoming 0.7.1 or how could i go around this issue?

4 3

[libvirt] UML driver struct refers to public API functions
by Matthias Bolte 02 Sep '09

02 Sep '09

Hi, The commit "Generic shared impls of all NUMA apis" (b0b968efd56f6c66bfa23eebbecd491ea993f99b) changed the UML driver struct to use the shared NUMA API. But now the UML driver struct refers to the public API functions: virNodeGetCellsFreeMemory virNodeGetFreeMemory instead of the shared NUMA API functions nodeGetCellsFreeMemory nodeGetFreeMemory This results in an infinite recursion, if someone's going to call virNodeGetCellsFreeMemory with an UML connection. Matthias

3 3

[libvirt] Re: OpenVZ : The restriction of domain name should be addressed
by Yuji NISHIDA 02 Sep '09

02 Sep '09

>> 2009/7/24 Daniel P. Berrange <berrange(a)redhat.com> >>> We should make use of this --name parameter then - I guess it didn't >>> exist when we first wrote the driver. It is useful to users to have >>> separate ID vs Name parameters - and in fact the current reusing of >>> 'ID' for the name, causes a little confusion in virsh's lookup >>> routines >>> because it can't tell whether the parameter its given is a name or >>> an >>> ID, since they are identical. >> >> There is still a question of how to specify both a name and CTID in >> XML >> description. >> By default, CTID can be obtained as openvzGimmeFirstUnusedCTID(), but >> actually >> I think that there exists a number of persons interested in giving >> CTIDs >> manually. > > Well, can <domain id=''> be used for CTID remaining <name> for > alphabetical domain name? I worte a small patch and tried with following XML setting. ### Patch ### --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -130,9 +130,6 @@ ADD_ARG_LIT(VZCTL); ADD_ARG_LIT("--quiet"); ADD_ARG_LIT("create"); - ADD_ARG_LIT(vmdef->id); - - ADD_ARG_LIT("--name"); ADD_ARG_LIT(vmdef->name); ### XML ### <domain id='100'> <name>abc</name> I found the type of id was identified as number( obj->type == XPATH_NUMBER ) in virXPathLongBase, but it is clearly string before converted. I think correct path is to go in the first if context( obj->type == XPATH_STRING ) and run strtol. Then I tried with following XML. ### XML ### <domain id=100> <name>abc</name> I got following error. AttValue: " or ' expected I'm not sure from which function should return this result. Maybe this is the first step to go forward. I am doubting openvz*LookupBy* functions be also modified, right? ----- Yuji Nishida nishidy(a)nict.go.jp

2 1

[libvirt] PATCH: Don't blindly reorder disk drives
by Daniel P. Berrange 02 Sep '09

02 Sep '09

Calling qsort() on the disks array causes disk to be unneccessarily re-ordered, potentially breaking the ability to boot if the boot disk gets moved later in the list. The new algorithm will insert a new disk as far to the end of the list as possible, while being ordered correctly wrt other disks on the same bus. * src/domain_conf.c, src/domain_conf.h: Remove disk sorting routines. Add API to insert a disk into existing list at the optimal position, without resorting disks * src/libvirt_private.syms: Export virDomainDiskInsert * src/xend_internal.c, src/xm_internal.c: Remove calls to qsort, use virDomainDiskInsert instead. * src/qemu_driver.c: Remove calls to qsort, use virDoaminDiskInsert instead. Fix reordering bugs when hotunplugging disks and networks. Fix memory leak in disk/net unplug --- src/domain_conf.c | 68 ++++++++++++++++++++++++++++++++++----------- src/domain_conf.h | 7 +++-- src/libvirt_private.syms | 3 +- src/qemu_driver.c | 30 +++++++++++--------- src/xend_internal.c | 2 - src/xm_internal.c | 7 +---- 6 files changed, 74 insertions(+), 43 deletions(-) Daniel diff --git a/src/domain_conf.c b/src/domain_conf.c index bad53f7..29a02c1 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -627,19 +627,8 @@ void virDomainRemoveInactive(virDomainObjListPtr doms, } } -#endif /* ! PROXY */ - - -int virDomainDiskCompare(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) { - if (a->bus == b->bus) - return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst); - else - return a->bus - b->bus; -} -#ifndef PROXY /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ @@ -2329,14 +2318,61 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn, } #endif -int virDomainDiskQSort(const void *a, const void *b) + +int virDomainDiskInsert(virDomainDefPtr def, + virDomainDiskDefPtr disk) { - const virDomainDiskDefPtr *da = a; - const virDomainDiskDefPtr *db = b; - return virDomainDiskCompare(*da, *db); + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + return -1; + + virDomainDiskInsertPreAlloced(def, disk); + + return 0; } +void virDomainDiskInsertPreAlloced(virDomainDefPtr def, + virDomainDiskDefPtr disk) +{ + int i; + /* Tenatively plan to insert disk at the end. */ + int insertAt = -1; + + /* Then work backwards looking for disks on + * the same bus. If we find a disk with a drive + * index greater than the new one, insert at + * that position + */ + for (i = (def->ndisks - 1) ; i >= 0 ; i--) { + /* If bus matches and current disk is after + * new disk, then new disk should go here */ + if (def->disks[i]->bus == disk->bus && + (virDiskNameToIndex(def->disks[i]->dst) > + virDiskNameToIndex(disk->dst))) { + insertAt = i; + } else if (def->disks[i]->bus == disk->bus && + insertAt == -1) { + /* Last disk with match bus is before the + * new disk, then put new disk just after + */ + insertAt = i + 1; + } + } + + /* No disks with this bus yet, so put at end of list */ + if (insertAt == -1) + insertAt = def->ndisks; + + if (insertAt < def->ndisks) + memmove(def->disks + insertAt + 1, + def->disks + insertAt, + (sizeof(def->disks[0]) * (def->ndisks-insertAt))); + + def->disks[insertAt] = disk; + def->ndisks++; +} + + #ifndef PROXY static char *virDomainDefDefaultEmulator(virConnectPtr conn, virDomainDefPtr def, @@ -2643,8 +2679,6 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, def->disks[def->ndisks++] = disk; } - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); VIR_FREE(nodes); /* analysis of the filesystems */ diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..c710986 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -683,9 +683,10 @@ char *virDomainCpuSetFormat(virConnectPtr conn, char *cpuset, int maxcpu); -int virDomainDiskQSort(const void *a, const void *b); -int virDomainDiskCompare(virDomainDiskDefPtr a, - virDomainDiskDefPtr b); +int virDomainDiskInsert(virDomainDefPtr def, + virDomainDiskDefPtr disk); +void virDomainDiskInsertPreAlloced(virDomainDefPtr def, + virDomainDiskDefPtr disk); int virDomainSaveXML(virConnectPtr conn, const char *configDir, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 23fa01b..32961ca 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -80,7 +80,8 @@ virDomainDeviceTypeToString; virDomainDiskBusTypeToString; virDomainDiskDefFree; virDomainDiskDeviceTypeToString; -virDomainDiskQSort; +virDomainDiskInsert; +virDomainDiskInsertPreAlloced; virDomainFindByID; virDomainFindByName; virDomainFindByUUID; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 4b1aeea..3ff9f9c 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -5030,9 +5030,7 @@ try_command: dev->data.disk->pci_addr.bus = bus; dev->data.disk->pci_addr.slot = slot; - vm->def->disks[vm->def->ndisks++] = dev->data.disk; - qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), - virDomainDiskQSort); + virDomainDiskInsertPreAlloced(vm->def, dev->data.disk); return 0; } @@ -5090,9 +5088,7 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn, return -1; } - vm->def->disks[vm->def->ndisks++] = dev->data.disk; - qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), - virDomainDiskQSort); + virDomainDiskInsertPreAlloced(vm->def, dev->data.disk); VIR_FREE(reply); VIR_FREE(cmd); @@ -5626,17 +5622,19 @@ try_command: } if (vm->def->ndisks > 1) { - vm->def->disks[i] = vm->def->disks[--vm->def->ndisks]; + memmove(vm->def->disks + i, + vm->def->disks + i + 1, + sizeof(*vm->def->disks) * + (vm->def->ndisks - (i + 1))); + vm->def->ndisks--; if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) { - virReportOOMError(conn); - goto cleanup; + /* ignore, harmless */ } - qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks), - virDomainDiskQSort); } else { VIR_FREE(vm->def->disks[0]); vm->def->ndisks = 0; } + virDomainDiskDefFree(detach); ret = 0; cleanup: @@ -5725,15 +5723,19 @@ qemudDomainDetachNetDevice(virConnectPtr conn, DEBUG("%s: host_net_remove reply: %s", vm->def->name, reply); if (vm->def->nnets > 1) { - vm->def->nets[i] = vm->def->nets[--vm->def->nnets]; + memmove(vm->def->nets + i, + vm->def->nets + i + 1, + sizeof(*vm->def->nets) * + (vm->def->nnets - (i + 1))); + vm->def->nnets--; if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets) < 0) { - virReportOOMError(conn); - goto cleanup; + /* ignore, harmless */ } } else { VIR_FREE(vm->def->nets[0]); vm->def->nnets = 0; } + virDomainNetDefFree(detach); ret = 0; cleanup: diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..99847b0 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -2540,8 +2540,6 @@ xenDaemonParseSxpr(virConnectPtr conn, } } } - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); /* in case of HVM we have USB device emulation */ if (hvm && diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..dd44ce5 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -990,8 +990,6 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { disk = NULL; } } - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); list = virConfGetValue(conf, "vif"); if (list && list->type == VIR_CONF_LIST) { @@ -2839,14 +2837,11 @@ xenXMDomainAttachDevice(virDomainPtr domain, const char *xml) { switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: { - if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) { + if (virDomainDiskInsert(def, dev->data.disk) < 0) { virReportOOMError(domain->conn); goto cleanup; } - def->disks[def->ndisks++] = dev->data.disk; dev->data.disk = NULL; - qsort(def->disks, def->ndisks, sizeof(*def->disks), - virDomainDiskQSort); } break; -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] [PATCH] storage_backend_fs: avoid NULL dereference on opendir failure
by Jim Meyering 02 Sep '09

02 Sep '09

I've just begun using clang's static analyzer, http://clang-analyzer.llvm.org/ It has uncovered a few problems in libvirt. Here are the first few fixes. I'll send more details later today. >From b6bb9d82effa56733fbee9013e66fed384d9ff63 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 09:42:32 +0200 Subject: [PATCH 1/4] storage_backend_fs: avoid NULL dereference on opendir failure * src/storage_backend_fs.c (virStorageBackendFileSystemRefresh): Don't call closedir on a NULL pointer. --- src/storage_backend_fs.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 65b656d..8241504 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -983,7 +983,8 @@ no_memory: /* fallthrough */ cleanup: - closedir(dir); + if (dir) + closedir(dir); virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); return -1; -- 1.6.4.2.395.ge3d52 >From eaae148291680a72d19aa9d5320f90b98f123746 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 09:58:28 +0200 Subject: [PATCH 2/4] storage_conf.c: avoid overflow upon use of "z" or "Z" (zebi) suffix * src/storage_conf.c (virStorageSize): Don't try to compute 1024^7, since it's too large for a 64-bit type. --- src/storage_conf.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/src/storage_conf.c b/src/storage_conf.c index c446069..110f0ad 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -919,12 +919,6 @@ virStorageSize(virConnectPtr conn, 1024ull; break; - case 'z': - case 'Z': - mult = 1024ull * 1024ull * 1024ull * 1024ull * 1024ull * - 1024ull * 1024ull; - break; - default: virStorageReportError(conn, VIR_ERR_XML_ERROR, _("unknown size units '%s'"), unit); -- 1.6.4.2.395.ge3d52 >From 7f453c68bc709d542e4c40a388c92c7969ad0a3a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 09:58:50 +0200 Subject: [PATCH 3/4] lxc: avoid NULL dereference when we find no mount point * src/lxc_container.c (lxcContainerUnmountOldFS): Don't pass a NULL pointer to qsort. --- src/lxc_container.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lxc_container.c b/src/lxc_container.c index 950dd50..2073864 100644 --- a/src/lxc_container.c +++ b/src/lxc_container.c @@ -546,8 +546,9 @@ static int lxcContainerUnmountOldFS(void) } endmntent(procmnt); - qsort(mounts, nmounts, sizeof(mounts[0]), - lxcContainerChildMountSort); + if (mounts) + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort); for (i = 0 ; i < nmounts ; i++) { VIR_DEBUG("Umount %s", mounts[i]); -- 1.6.4.2.395.ge3d52 >From 4e97befca175af427ed3b75f59e67cd620ee3ce2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 2 Sep 2009 10:02:49 +0200 Subject: [PATCH 4/4] lxc: don't unlink(NULL) in main * src/lxc_controller.c (main): Unlink sockpath only if it's non-NULL. --- src/lxc_controller.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..914c10a 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -803,7 +803,8 @@ cleanup: if (def) virFileDeletePid(LXC_STATE_DIR, def->name); lxcControllerCleanupInterfaces(nveths, veths); - unlink(sockpath); + if (sockpath): + unlink(sockpath); VIR_FREE(sockpath); return rc; -- 1.6.4.2.395.ge3d52

3 3

[libvirt] Add support for (qcow*) volume encryption (v4)
by Miloslav Trmač 02 Sep '09

02 Sep '09

Hello, the following patches add full support for qcow/qcow2 volume encryption, assuming a client that supports it. (Main changes since the previous version: * Add "flags" argument to virSecretDefineXML(), virSecretGetXMLDesc(), virSecretSetValue(), virSecretGetValue() * Various clean-ups and bug fixes in the local secret storage driver * Make the Python API more consistent with other objects See the specific patch change logs for more details; patches without change logs are unchanged.) New XML tags are defined to represent encryption parameters (currently format and passphrase, more can be added in the future), e.g. <encryption format='qcow'> <secret type='passphrase' uuid='724d95f2-0ed2-6ff9-84d0-0f3d1618428d'/> </encryption> The <encryption> tag can be added to a <volume> node passed to virStorageVolCreateXML() to create an encrypted volume, or to a <disk> node inside a <domain> to specify what encryption parameters to use for a domain. uuid above refers to a separately-managed secret, which was created using virSecretDefineXML() and set using virSecretSetValue(). Other properties of the secret can be managed using an XML representation. Detailed documentation of the formats and features is inside the patches.

3 42

Re: [libvirt] [PATCH 15/20] Provide missing passphrase when creating a volume.
by Miloslav Trmac 02 Sep '09

02 Sep '09

----- "Daniel P. Berrange" <berrange(a)redhat.com> wrote: > On Thu, Aug 20, 2009 at 08:18:13PM +0200, Miloslav Trma?? wrote: > > If the <encryption format='qcow'> element does not specify a secret > > during volume creation, generate a suitable secret and add it to the > > <encryption> tag. The caller can view the updated <encryption> tag > > using virStorageVolGetXMLDesc(). > > > > Similarly, when <encryption format='default'/> is specified while > > creating a qcow or qcow2-formatted volume, change the format to "qcow" > > and generate a secret as described above. > > + virBufferAddLit(&buf, "<secret ephemeral='no' private='no'>"); > > + /* <uuid/> is chosen by the secret driver */ > > + virBufferEscapeString(&buf, > > + "<description>qcow passphrase for %s</description>", > > + vol->target.path); > > + virBufferEscapeString(&buf, "<volume>%s</volume>", vol->target.path); > > + virBufferAddLit(&buf, "</secret>"); > > + if (virBufferError(&buf)) { > > + virReportOOMError(conn); > > + goto cleanup; > > + } > > + xml = virBufferContentAndReset(&buf); > > This is the first place where we should be just calling into an > internal secret_conf.h API for formatting XML from a struct, > rather than duplicating the XML formatting. OK. > > + /* A qcow passphrase is up to 16 bytes, with any data following a NUL > > + ignored. Prohibit control and non-ASCII characters to avoid possible > > + unpleasant surprises with the qemu monitor input mechanism. */ > > + fd = open("/dev/urandom", O_RDONLY); > > + if (fd < 0) { > > + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", > > + _("Cannot open /dev/urandom")); > > + goto cleanup; > > + } > > + i = 0; > > + while (i < sizeof (value)) { > > + ssize_t r; > > + > > + while ((r = read(fd, value + i, 1)) == -1 && errno == EINTR) > > + ; > > + if (r <= 0) { > > + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", > > + _("Cannot read from /dev/urandom")); > > + goto cleanup; > > + } > > + if (value[i] >= 0x20 && value[i] <= 0x7E) > > + i++; /* Got an acceptable character */ > > + } > > + close(fd); > > I reckon this snippet of code could usefully be put into the util.h file > as virFileGenerateRandomkey(), or alternatively perhaps secret_conf.h, > as virSecretGenerateRandomKey(unsigned char *buf, size_t buflen); This code, with its limitation to ASCII characters, is qcow/qemu-specific. Mirek

2 3

Re: [libvirt] [PATCH 11/20] Add volume encryption information handling.
by Miloslav Trmac 02 Sep '09

02 Sep '09

----- "Maximilian Wilhelm" <max(a)rfc2324.org> wrote: > Anno domini 2009 Miloslav Trmač scripsit: > > > diff --git a/src/storage_encryption.c b/src/storage_encryption.c > > new file mode 100644 > > index 0000000..ccb29ed > > --- /dev/null > > +++ b/src/storage_encryption.c > > @@ -0,0 +1,241 @@ > > [...] > > +#include "base64.h" > [...] > > Is there any special need for this include? Not in this file, I forgot to clean it up. > If so, where does this include file come from? gnulib. > > My buildbot refused to compile this code as it did not have any > base64.h header file. As removing this include solved the build > problem for me, I would suggest the following patch if noone objects. That change looks correct. Mirek

1 0

[libvirt] Create a Domain from windows client
by anuj rampal 01 Sep '09

01 Sep '09

Hi all, I'm trying to call the libvirt API from my Windows machine. When i try to create to a Domain using "virDomainDefineXML" function I have to pass an XML file containing the Description of the Domain. Here is the part of the XML file that im using: <domain type='qemu' id='1'> .. .. <devices> <emulator>/usr/bin/qemu</emulator> <disk type='file' device='disk'> <source file='/var/lib/libvirt/images/WinXp.img'/> <target dev='hda' bus='ide'/> </disk> <disk type='file' device='cdrom'> <source file='/home/anuj/ISOs/WinXpSp3/Windows_XP_SP3.iso'/> <target dev='hdc' bus='ide'/> <readonly/> </disk> .. .. </domain> <source file='/var/lib/libvirt/images/WinXp.img'/> This is the path which contains the img file for the Domain. I have created this file using this command: qemu-img create -f raw /var/lib/libvirt/images/WinXp.img 10G *Now: my question is:* Is there a way by which i can create this img file using some libvirt call directly and then use this for my domain creation. Or modify the XML file itself so that virDomainDefineXML call creates the img file at the path and the size that i specify. Could someone also give me some example for the same. Regards Anuj

2 1

[libvirt] libvirt and "kvm -cpu=qemu32" problem
by Scaldov M.V. 01 Sep '09

01 Sep '09

Problem is that the images of windows 2003 server, windows 2000 and some other do not start and go to BSOD due to processor mismatch if they were created under either qemu or vmware, or from bare-metal installed OS. By default libvirt starts kvm with "-cpu=qemu32" option instead of, for example, "-cpu=pentium3". There is patch for libvirt-0.6.5 to avoid this problem. Now libvirt takes into account <cpumodel> node from .xml domain config, which overrides libvirt's default -cpu option for kvm. I think, this patch is vital, because of it helps to avoid a waste of time with reinstalling and reconfiguring windows server images. <domain type='kvm'> <name>w2k3s</name> <uuid>e2725d10-7ada-4f71-eea7-fa6cc1a541ee</uuid> <memory>262144</memory> <currentMemory>262144</currentMemory> <vcpu>1</vcpu> <cpumodel>pentium3</cpumodel> <------ <os> <type arch='i686'>hvm</type> <boot dev='hd'/> </os> Sorry for not patching GIT-version, GIT is hard to compile in a small time.

2 3

[libvirt] [PATCH] support lzop save compression for qemu
by Charles Duffy 01 Sep '09

01 Sep '09

Per prior discussion -- this was, indeed, trivial. I'm a little disappointed to be breaking the ordering characteristics of the enum (as it had been ordered by increasing time requirements and decreasing output size), but breaking any save files with the old constants in the headers would of course be worse.

3 5

[libvirt] [PATCH] OpenVZ: accept NULL as type for GetMaxVCPUs.
by Chris Lalancette 01 Sep '09

01 Sep '09

All of the other drivers that support the getMaxVcpus callback also accept a NULL value for type. Make openvz also accept a NULL value. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/openvz_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/openvz_driver.c b/src/openvz_driver.c index 6b7c49d..43d54f9 100644 --- a/src/openvz_driver.c +++ b/src/openvz_driver.c @@ -1000,9 +1000,10 @@ cleanup: return ret; } -static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) { - if (STRCASEEQ(type, "openvz")) - return 1028; //OpenVZ has no limitation +static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type) +{ + if (type == NULL || STRCASEEQ(type, "openvz")) + return 1028; /* OpenVZ has no limitation */ openvzError(conn, VIR_ERR_INVALID_ARG, _("unknown type '%s'"), type); -- 1.6.0.6

2 2

[libvirt] [PATCH] Remove use of strncpy in qemudExtractMonitorPath.
by Chris Lalancette 01 Sep '09

01 Sep '09

qemudExtractMonitorPath() was doing a VIR_ALLOC_N followed by a strncpy. However, this isn't necessary; we can do the same thing using strndup, which is much safer. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 772f2f9..a40d386 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1029,12 +1029,12 @@ qemudExtractMonitorPath(virConnectPtr conn, */ while (*tmp) { if (c_isspace(*tmp)) { - if (VIR_ALLOC_N(*path, (tmp-dev)+1) < 0) { + *path = strndup(dev, tmp-dev); + if (*path == NULL) { virReportOOMError(conn); return -1; } - strncpy(*path, dev, (tmp-dev)); - (*path)[(tmp-dev)] = '\0'; + /* ... now further update offset till we get EOL */ *offset = tmp - haystack; return 0; -- 1.6.0.6

2 2

[libvirt] [PATCH]: Fix up virNodeGetCellsFreeMemory
by Chris Lalancette 01 Sep '09

01 Sep '09

The documentation for virNodeGetCellsFreeMemory claims the values returned are in kilobytes, but that's actually wrong; the value returned is actually in bytes. Fix up the documentation to be correct. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- docs/devhelp/libvirt-libvirt.html | 2 +- docs/html/libvirt-libvirt.html | 2 +- docs/libvirt-api.xml | 2 +- docs/libvirt-refs.xml | 1 - src/libvirt.c | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-)

2 2

[libvirt] [PATCH] Introduce virStrncpy.
by Chris Lalancette 01 Sep '09

01 Sep '09

Add the virStrncpy function, which takes a dst string, source string, the number of bytes to copy and the number of bytes available in the dest string. If the source string is too large to fit into the destination string, including the \0 byte, then no data is copied and the function returns NULL. Otherwise, this function copies n bytes from source into dst, including the \0, and returns a pointer to the dst string. This function is intended to replace all unsafe uses of strncpy in the code base, since strncpy does *not* guarantee that the buffer terminates with a \0. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- HACKING | 29 +++++++++ cfg.mk | 6 ++ proxy/libvirt_proxy.c | 6 ++- qemud/Makefile.am | 3 +- qemud/qemud.c | 6 +- qemud/remote.c | 8 ++- src/bridge.c | 89 ++++++++------------------- src/datatypes.c | 9 ++- src/esx/esx_driver.c | 9 ++- src/esx/esx_vmx.c | 7 ++- src/lxc_controller.c | 6 ++- src/lxc_driver.c | 12 +++- src/nodeinfo.c | 4 +- src/opennebula/one_client.c | 11 ++-- src/openvz_conf.c | 33 +++++++--- src/phyp/phyp_driver.c | 2 +- src/proxy_internal.c | 5 +- src/qemu_conf.c | 16 +++--- src/qemu_driver.c | 12 +++- src/remote_internal.c | 19 ++++-- src/test.c | 24 +++++-- src/uml_driver.c | 17 ++++-- src/util.c | 36 +++++++++-- src/util.h | 4 + src/xen_internal.c | 22 +++++-- src/xend_internal.c | 65 ++++++++++++------- src/xm_internal.c | 145 ++++++++++++++++++++++++++++--------------- 27 files changed, 392 insertions(+), 213 deletions(-) diff --git a/HACKING b/HACKING index da28e98..7a38f05 100644 --- a/HACKING +++ b/HACKING @@ -231,6 +231,35 @@ one of the following semantically named macros +String copying +============== + +Do not use the strncpy function. According to the man page, it does +*not* guarantee a NULL-terminated buffer, which makes it extremely dangerous +to use. Instead, use one of the functionally equivalent functions: + + - virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) + The first three arguments have the same meaning as for strncpy; namely the + destination, source, and number of bytes to copy, respectively. The last + argument is the number of bytes available in the destination string; if a + copy of the source string (including a \0) will not fit into the + destination, no bytes are copied and the routine returns NULL. + Otherwise, n bytes from the source are copied into the destination and a + trailing \0 is appended. + + - virStrcpy(char *dest, const char *src, size_t destbytes) + Use this variant if you know you want to copy the entire src string + into dest. This is equivalent to + virStrncpy(dest, src, strlen(src), destbytes) + + - virStrcpyStatic(char *dest, const char *src) + Use this variant if you know you want to copy the entire src string + into dest *and* you know that your destination string is a static string + (i.e. that sizeof(dest) returns something meaningful). This is + equivalent to virStrncpy(dest, src, strlen(src), sizeof(dest)). + + + Variable length string buffer ============================= diff --git a/cfg.mk b/cfg.mk index 64bd26e..2d2dd7e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -91,6 +91,12 @@ sc_prohibit_asprintf: msg='use virAsprintf, not a'sprintf \ $(_prohibit_regexp) +sc_prohibit_strncpy: + @grep -nE 'strncpy *\(' \ + $$($(VC_LIST) | grep -v 'src/util.c') && \ + { echo '$(ME): use virStrncpy, not strncpy' \ + 1>&2; exit 1; } || : + sc_prohibit_VIR_ERR_NO_MEMORY: @re='\<V''IR_ERR_NO_MEMORY\>' \ msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY \ diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index e008a7f..f058b38 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -170,7 +170,11 @@ proxyListenUnixSocket(const char *path) { memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + fprintf(stderr, "Path %s too long to fit into destination\n", path); + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 3d143da..67f1b4a 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -108,9 +108,10 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) +libvirtd_LDADD += ../src/libvirt_util.la + if WITH_DRIVER_MODULES libvirtd_LDADD += ../src/libvirt_driver.la - libvirtd_LDADD += ../src/libvirt_util.la else if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la diff --git a/qemud/qemud.c b/qemud/qemud.c index df275e6..fd6e0af 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -506,11 +506,13 @@ static int qemudListenUnix(struct qemud_server *server, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, path, sizeof(addr.sun_path)-1); + if (virStrcpyStatic(addr.sun_path, path) == NULL) { + VIR_ERROR(_("Path %s too long for unix socket"), path); + goto cleanup; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; - oldgrp = getgid(); oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask); if (server->privileged) diff --git a/qemud/remote.c b/qemud/remote.c index b29d5d8..990c15f 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -554,9 +554,11 @@ remoteDispatchDomainSetSchedulerParameters (struct qemud_server *server ATTRIBUT /* Deserialise parameters. */ for (i = 0; i < nparams; ++i) { - strncpy (params[i].field, args->params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[i].field, args->params.params_val[i].field) == NULL) { + remoteDispatchFormatError(rerr, _("Field %s too big for destination"), + args->params.params_val[i].field); + return -1; + } params[i].type = args->params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/bridge.c b/src/bridge.c index 414d87b..f9a7668 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -149,23 +149,19 @@ brHasBridge(brControl *ctl, const char *name) { struct ifreq ifr; - int len; if (!ctl || !name) { errno = EINVAL; return -1; } - if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, name) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, name, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr)) return -1; @@ -216,18 +212,14 @@ brAddDelInterface(brControl *ctl, const char *iface) { struct ifreq ifr; - int len; if (!ctl || !ctl->fd || !bridge || !iface) return EINVAL; - if ((len = strlen(bridge)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, bridge, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, bridge) == NULL) + return EINVAL; if (!(ifr.ifr_ifindex = if_nametoindex(iface))) return ENODEV; @@ -305,23 +297,19 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED, static int ifGetMtu(brControl *ctl, const char *ifname) { struct ifreq ifr; - int len; if (!ctl || !ifname) { errno = EINVAL; return -1; } - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFMTU, &ifr)) return -1; @@ -343,18 +331,14 @@ static int ifGetMtu(brControl *ctl, const char *ifname) static int ifSetMtu(brControl *ctl, const char *ifname, int mtu) { struct ifreq ifr; - int len; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; ifr.ifr_mtu = mtu; return ioctl(ctl->fd, SIOCSIFMTU, &ifr) == 0 ? 0 : errno; @@ -466,7 +450,7 @@ brAddTap(brControl *ctl, int vnet_hdr, int *tapfd) { - int fd, len; + int fd; struct ifreq ifr; if (!ctl || !ctl->fd || !bridge || !ifname) @@ -486,17 +470,14 @@ brAddTap(brControl *ctl, (void) vnet_hdr; #endif - strncpy(ifr.ifr_name, *ifname, IFNAMSIZ-1); - - if (ioctl(fd, TUNSETIFF, &ifr) < 0) - goto error; - - len = strlen(ifr.ifr_name); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { errno = EINVAL; goto error; } + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + goto error; + /* We need to set the interface MTU before adding it * to the bridge, because the bridge will have its * MTU adjusted automatically when we add the new interface. @@ -527,7 +508,6 @@ int brDeleteTap(brControl *ctl, const char *ifname) { struct ifreq try; - int len; int fd; if (!ctl || !ctl->fd || !ifname) @@ -539,15 +519,11 @@ int brDeleteTap(brControl *ctl, memset(&try, 0, sizeof(struct ifreq)); try.ifr_flags = IFF_TAP|IFF_NO_PI; - len = strlen(ifname); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpyStatic(try.ifr_name, ifname) == NULL) { errno = EINVAL; goto error; } - strncpy(try.ifr_name, ifname, len); - try.ifr_name[len] = '\0'; - if (ioctl(fd, TUNSETIFF, &try) == 0) { if ((errno = ioctl(fd, TUNSETPERSIST, 0))) goto error; @@ -576,19 +552,15 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; - int len; int flags; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -621,18 +593,14 @@ brGetInterfaceUp(brControl *ctl, int *up) { struct ifreq ifr; - int len; if (!ctl || !ifname || !up) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -654,18 +622,15 @@ brSetInetAddr(brControl *ctl, } s; struct ifreq ifr; struct in_addr inaddr; - int len, ret; + int ret; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if ((ret = inet_pton(AF_INET, addr, &inaddr)) < 0) return errno; @@ -692,18 +657,14 @@ brGetInetAddr(brControl *ctl, { struct ifreq ifr; struct in_addr *inaddr; - int len; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL) + return EINVAL; if (ioctl(ctl->fd, cmd, &ifr) < 0) return errno; diff --git a/src/datatypes.c b/src/datatypes.c index d03a679..4a8bffa 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "util.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -899,8 +900,12 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c virReportOOMError(conn); goto error; } - strncpy(ret->key, key, sizeof(ret->key)-1); - ret->key[sizeof(ret->key)-1] = '\0'; + if (virStrcpyStatic(ret->key, key) == NULL) { + virMutexUnlock(&conn->lock); + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + _("Volume key %s too large for destination"), key); + goto error; + } ret->magic = VIR_STORAGE_VOL_MAGIC; ret->conn = conn; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 0225e9a..24c4422 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -694,9 +694,12 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - strncpy (nodeinfo->model, dynamicProperty->val->string, - sizeof (nodeinfo->model) - 1); - nodeinfo->model[sizeof (nodeinfo->model) - 1] = '\0'; + if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Model %s too long for destination", + dynamicProperty->val->string); + goto failure; + } } else { VIR_WARN("Unexpected '%s' property", dynamicProperty->name); } diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 54c2594..2a58fd8 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -883,8 +883,11 @@ esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix) return NULL; } - strncpy(buffer, prefix, sizeof (buffer) - 1); - buffer[sizeof (buffer) - 1] = '\0'; + if (virStrcpyStatic(buffer, prefix) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Prefix %s too big for destination", prefix); + return NULL; + } if (idx < 26) { buffer[length] = 'a' + idx; diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..f7729ba 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -178,7 +178,11 @@ static int lxcMonitorServer(const char *sockpath) unlink(sockpath); memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too long for destination"), sockpath); + goto error; + } if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(NULL, errno, diff --git a/src/lxc_driver.c b/src/lxc_driver.c index bd0cf0e..fd49789 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -719,7 +719,11 @@ static int lxcMonitorClient(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, sockpath) == NULL) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too big for destination"), sockpath); + goto error; + } if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(conn, errno, "%s", @@ -1739,7 +1743,11 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, if (virCgroupGetCpuShares(group, &val) != 0) goto cleanup; params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); + if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { + lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too big for destination")); + goto cleanup; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; ret = 0; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0a211a7..7d26b2b 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -141,8 +141,8 @@ int nodeGetInfo(virConnectPtr conn, uname(&info); - strncpy(nodeinfo->model, info.machine, sizeof(nodeinfo->model)-1); - nodeinfo->model[sizeof(nodeinfo->model)-1] = '\0'; + if (virStrcpyStatic(nodeinfo->model, info.machine) == NULL) + return -1; #else /* !HAVE_UNAME */ diff --git a/src/opennebula/one_client.c b/src/opennebula/one_client.c index 21f303a..39bb1ea 100644 --- a/src/opennebula/one_client.c +++ b/src/opennebula/one_client.c @@ -24,6 +24,8 @@ #include <fcntl.h> #include <unistd.h> #include "one_client.h" +#include "datatypes.h" +#include "util.h" oneClient one_client; @@ -171,7 +173,7 @@ int c_oneFinalize(int vmid) return c_oneAction(vmid, (char *)"finalize"); } -int c_oneVmInfo(int vmid, char* ret_info,int length) +int c_oneVmInfo(int vmid, char* ret_info, int length) { xmlrpc_value *resultP; int return_code; @@ -186,10 +188,9 @@ int c_oneVmInfo(int vmid, char* ret_info,int length) if( return_code ) { - strncpy(ret_info, return_string, length-1); - ret_info[length-1] = '\0'; - - retval = 0; + if (virStrncpy(ret_info, return_string, length-1, length) != NULL) + /* Only set the return value to 0 if we succeeded */ + retval = 0; } xmlrpc_DECREF(resultP); diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..08951df 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -261,8 +261,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->ifname, len+1) < 0) goto no_memory; - strncpy(net->ifname, p, len); - net->ifname[len] = '\0'; + if (virStrncpy(net->ifname, p, len, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Network ifname %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "bridge=")) { p += 7; len = next - p; @@ -275,8 +278,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto no_memory; - strncpy(net->data.bridge.brname, p, len); - net->data.bridge.brname[len] = '\0'; + if (virStrncpy(net->data.bridge.brname, p, len, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge name %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "mac=")) { p += 4; len = next - p; @@ -285,8 +291,11 @@ openvzReadNetworkConf(virConnectPtr conn, "%s", _("Wrong length MAC address")); goto error; } - strncpy(cpy_temp, p, len); - cpy_temp[len] = '\0'; + if (virStrncpy(cpy_temp, p, len, sizeof(cpy_temp)) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too long for destination"), p); + goto error; + } if (virParseMacAddr(cpy_temp, net->mac) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong MAC address")); @@ -629,8 +638,10 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i if (sf[0] == '=' && sf[1] != '\0' ) { sf ++; if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL) { - strncpy(value, token, maxlen) ; - value[maxlen-1] = '\0'; + if (virStrcpy(value, token, maxlen) == NULL) { + ret = -1; + break; + } found = 1; } } @@ -812,6 +823,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) char uuidbuf[1024]; char iden[1024]; int fd, ret; + int retval = 0; if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) return -1; @@ -832,13 +844,14 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) sscanf(line, "%s %s\n", iden, uuidbuf); if(STREQ(iden, "#UUID:")) { - strncpy(uuidstr, uuidbuf, len); + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) + retval = -1; break; } } close(fd); - return 0; + return retval; } /* Do actual checking for UUID presence in conf file, diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 3100144..22c4fee 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1387,7 +1387,7 @@ escape_specialcharacters(char *src, char *dst, size_t dstlen) } temp_buffer[j] = '\0'; - if (strncpy(dst, temp_buffer, dstlen) == NULL) + if (virStrcpy(dst, temp_buffer, dstlen) == NULL) return -1; return 0; diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 5b92ad8..8609201 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -197,7 +197,10 @@ retry: memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..f47ada2 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1302,18 +1302,18 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, { switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - strncpy(buf, "null", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "null", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_VC: - strncpy(buf, "vc", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "vc", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_PTY: - strncpy(buf, "pty", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "pty", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_DEV: @@ -1335,8 +1335,8 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_STDIO: - strncpy(buf, "stdio", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "stdio", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_UDP: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index a40d386..f99d4c0 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -921,7 +921,11 @@ qemudOpenMonitorUnix(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, monitor, sizeof(addr.sun_path)); + if (virStrcpyStatic(addr.sun_path, monitor) == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Monitor path %s too big for destination"), monitor); + goto error; + } do { ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); @@ -6241,8 +6245,12 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, goto cleanup; } params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; + if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too long for destination")); + goto cleanup; + } ret = 0; diff --git a/src/remote_internal.c b/src/remote_internal.c index 26425c2..e288188 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -652,7 +652,11 @@ doRemoteOpen (virConnectPtr conn, memset (&addr, 0, sizeof addr); addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, sockname, UNIX_PATH_MAX (addr)); + if (virStrcpyStatic(addr.sun_path, sockname) == NULL) { + errorf(conn, VIR_ERR_INTERNAL_ERROR, + _("Socket %s too big for destination"), sockname); + goto failed; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; @@ -1567,8 +1571,8 @@ remoteNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info) (xdrproc_t) xdr_remote_node_get_info_ret, (char *) &ret) == -1) goto done; - strncpy (info->model, ret.model, 32); - info->model[31] = '\0'; + if (virStrcpyStatic(info->model, ret.model) == NULL) + goto done; info->memory = ret.memory; info->cpus = ret.cpus; info->mhz = ret.mhz; @@ -2999,9 +3003,12 @@ remoteDomainGetSchedulerParameters (virDomainPtr domain, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - strncpy (params[i].field, ret.params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[i].field, ret.params.params_val[i].field) == NULL) { + errorf(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Parameter %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } params[i].type = ret.params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/test.c b/src/test.c index 305f2c9..9bb8e85 100644 --- a/src/test.c +++ b/src/test.c @@ -415,10 +415,13 @@ static char *testBuildFilename(const char *relativeTo, offset = strrchr(relativeTo, '/'); if ((baseLen = (offset-relativeTo+1))) { char *absFile; - if (VIR_ALLOC_N(absFile, baseLen + strlen(filename) + 1) < 0) + int totalLen = baseLen + strlen(filename) + 1; + if (VIR_ALLOC_N(absFile, totalLen) < 0) return NULL; - strncpy(absFile, relativeTo, baseLen); - absFile[baseLen] = '\0'; + if (virStrncpy(absFile, relativeTo, baseLen, totalLen) == NULL) { + VIR_FREE(absFile); + return NULL; + } strcat(absFile, filename); return absFile; } else { @@ -571,8 +574,11 @@ static int testOpenFromFile(virConnectPtr conn, privconn->nextDomID = 1; privconn->numCells = 0; - strncpy(privconn->path, file, PATH_MAX-1); - privconn->path[PATH_MAX-1] = '\0'; + if (virStrcpyStatic(privconn->path, file) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Path %s too big for destination"), file); + goto error; + } memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); nodeInfo = &privconn->nodeInfo; @@ -628,8 +634,12 @@ static int testOpenFromFile(virConnectPtr conn, str = virXPathString(conn, "string(/node/cpu/model[1])", ctxt); if (str != NULL) { - strncpy(nodeInfo->model, str, sizeof(nodeInfo->model)-1); - nodeInfo->model[sizeof(nodeInfo->model)-1] = '\0'; + if (virStrcpyStatic(nodeInfo->model, str) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), str); + VIR_FREE(str); + goto error; + } VIR_FREE(str); } diff --git a/src/uml_driver.c b/src/uml_driver.c index a2b9495..ecc0c75 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -562,6 +562,7 @@ static int umlMonitorAddress(virConnectPtr conn, virDomainObjPtr vm, struct sockaddr_un *addr) { char *sockname; + int retval = 0; if (virAsprintf(&sockname, "%s/%s/mconsole", driver->monitorDir, vm->def->name) < 0) { @@ -571,10 +572,13 @@ static int umlMonitorAddress(virConnectPtr conn, memset(addr, 0, sizeof *addr); addr->sun_family = AF_UNIX; - strncpy(addr->sun_path, sockname, sizeof(addr->sun_path)-1); - NUL_TERMINATE(addr->sun_path); + if (virStrcpyStatic(addr->sun_path, sockname) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Unix path %s too long for destination"), sockname); + retval = -1; + } VIR_FREE(sockname); - return 0; + return retval; } static int umlOpenMonitor(virConnectPtr conn, @@ -668,8 +672,11 @@ static int umlMonitorCommand(virConnectPtr conn, cmd, req.length); return -1; } - strncpy(req.data, cmd, req.length); - req.data[req.length] = '\0'; + if (virStrcpyStatic(req.data, cmd) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Command %s too long for destination"), cmd); + return -1; + } if (sendto(vm->monitor, &req, sizeof req, 0, (struct sockaddr *)&addr, sizeof addr) != (sizeof req)) { diff --git a/src/util.c b/src/util.c index 0d4f3fa..c021953 100644 --- a/src/util.c +++ b/src/util.c @@ -1121,9 +1121,8 @@ char *virFindFileInPath(const char *file) char fullpath[PATH_MAX]; /* copy PATH env so we can tweak it */ - strncpy(pathenv, getenv("PATH"), PATH_MAX); - pathenv[PATH_MAX - 1] = '\0'; - + if (virStrcpyStatic(pathenv, getenv("PATH")) == NULL) + return NULL; /* for each path segment, append the file to search for and test for * it. return it if found. @@ -1155,8 +1154,8 @@ int virFileMakePath(const char *path) if (stat(path, &st) >= 0) return 0; - strncpy(parent, path, PATH_MAX); - parent[PATH_MAX - 1] = '\0'; + if (virStrcpyStatic(parent, path) == NULL) + return EINVAL; if (!(p = strrchr(parent, '/'))) return EINVAL; @@ -1573,6 +1572,33 @@ virAsprintf(char **strp, const char *fmt, ...) return ret; } +/** + * virStrncpy + * + * A safe version of strncpy. The last parameter is the number of bytes + * available in the destination string, *not* the number of bytes you want + * to copy. If the destination is not large enough to hold all n of the + * src string bytes plus a \0, NULL is returned and no data is copied. + * If the destination is large enough to hold the n bytes plus \0, then the + * string is copied and a pointer to the destination string is returned. + */ +char * +virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) +{ + char *ret; + + if (n > (destbytes - 1)) + return NULL; + + ret = strncpy(dest, src, n); + /* strncpy NULL terminates iff the last character is \0. Therefore + * force the last byte to be \0 + */ + dest[n] = '\0'; + + return ret; +} + /* Compare two MAC addresses, ignoring differences in case, * as well as leading zeros. */ diff --git a/src/util.h b/src/util.h index b3e628a..895e07d 100644 --- a/src/util.h +++ b/src/util.h @@ -164,6 +164,10 @@ void virSkipSpaces(const char **str); int virParseNumber(const char **str); int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3); +char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) + ATTRIBUTE_RETURN_CHECK; +#define virStrcpy(dest, src, destbytes) virStrncpy(dest, src, strlen(src), destbytes) +#define virStrcpyStatic(dest, src) virStrncpy(dest, src, strlen(src), sizeof(dest)) #define VIR_MAC_BUFLEN 6 #define VIR_MAC_PREFIX_BUFLEN 3 diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..1f4cf55 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1208,13 +1208,19 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, if (ret < 0) return(-1); - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[0].field, str_weight) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Weight %s too big for destination", str_weight); + return -1; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = op_dom.u.getschedinfo.u.credit.weight; - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[1].field, str_cap) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Cap %s too big for destination", str_cap); + return -1; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap; @@ -2431,9 +2437,11 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, while (fgets (line, sizeof line, cpuinfo)) { if (regexec (&flags_hvm_rec, line, sizeof(subs)/sizeof(regmatch_t), subs, 0) == 0 && subs[0].rm_so != -1) { - strncpy (hvm_type, - &line[subs[1].rm_so], subs[1].rm_eo-subs[1].rm_so+1); - hvm_type[subs[1].rm_eo-subs[1].rm_so] = '\0'; + if (virStrncpy(hvm_type, + &line[subs[1].rm_so], + subs[1].rm_eo-subs[1].rm_so, + sizeof(hvm_type)) == NULL) + return NULL; } else if (regexec (&flags_pae_rec, line, 0, NULL, 0) == 0) host_pae = 1; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..d39fd4d 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -763,7 +763,8 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); - strncpy(addr->sun_path, path, sizeof(addr->sun_path)); + if (virStrcpyStatic(addr->sun_path, path) == NULL) + return -1; return (0); } @@ -1433,7 +1434,7 @@ xenDaemonParseSxprChar(virConnectPtr conn, const char *value, const char *tty) { - char prefix[10]; + const char *prefix; char *tmp; virDomainChrDefPtr def; @@ -1442,18 +1443,17 @@ xenDaemonParseSxprChar(virConnectPtr conn, return NULL; } - strncpy(prefix, value, sizeof(prefix)-1); - NUL_TERMINATE(prefix); + prefix = value; if (value[0] == '/') { def->type = VIR_DOMAIN_CHR_TYPE_DEV; } else { - if ((tmp = strchr(prefix, ':')) != NULL) { + if ((tmp = strchr(value, ':')) != NULL) { *tmp = '\0'; - value += (tmp - prefix) + 1; + value = tmp + 1; } - if (STREQ(prefix, "telnet")) { + if (STRPREFIX(prefix, "telnet")) { def->type = VIR_DOMAIN_CHR_TYPE_TCP; def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET; } else { @@ -1647,8 +1647,13 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0) goto no_memory; - strncpy(disk->driverName, src, (offset-src)); - disk->driverName[offset-src] = '\0'; + if (virStrncpy(disk->driverName, src, offset-src, + (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + src); + goto error; + } src = offset + 1; @@ -1662,8 +1667,13 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverType, (offset-src)+1)< 0) goto no_memory; - strncpy(disk->driverType, src, (offset-src)); - disk->driverType[offset-src] = '\0'; + if (virStrncpy(disk->driverType, src, offset-src, + (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + src); + goto error; + } src = offset + 1; /* Its possible to use blktap driver for block devs @@ -1891,13 +1901,12 @@ xenDaemonParseSxprSound(virConnectPtr conn, len = (offset2 - offset); else len = strlen(offset); - if (len > (sizeof(model)-1)) { + if (virStrncpy(model, offset, len, sizeof(model)) == NULL) { virXendError(conn, VIR_ERR_INTERNAL_ERROR, - _("unexpected sound model %s"), offset); + _("Sound model %s too big for destination"), + offset); goto error; } - strncpy(model, offset, len); - model[len] = '\0'; if (VIR_ALLOC(sound) < 0) goto no_memory; @@ -4786,13 +4795,20 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain, goto error; } - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[0].field, str_weight) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Weight %s too big for destination"), + str_weight); + goto error; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = sexpr_int(root, "domain/cpu_weight"); - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpyStatic(params[1].field, str_cap) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Cap %s too big for destination"), str_cap); + goto error; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = sexpr_int(root, "domain/cpu_cap"); *nparams = XEN_SCHED_CRED_NPARAM; @@ -5842,6 +5858,7 @@ virDomainXMLDevID(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; + char *tmp; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { if (dev->data.disk->driverName && @@ -5859,9 +5876,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[30]; virDomainNetDefPtr def = dev->data.net; @@ -5878,9 +5896,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..83eebee 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -872,8 +872,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { } else { if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->src, head, (offset - head)); - disk->src[(offset-head)] = '\0'; + if (virStrncpy(disk->src, head, offset - head, + (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Source file %s too big for destination"), + head); + goto cleanup; + } } head = offset + 1; @@ -886,8 +891,12 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->dst, head, (offset - head)); - disk->dst[(offset-head)] = '\0'; + if (virStrncpy(disk->dst, head, offset - head, + (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Dest file %s too big for destination"), head); + goto cleanup; + } head = offset + 1; @@ -897,8 +906,14 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((tmp = strchr(disk->src, ':')) != NULL) { if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverName, disk->src, (tmp - disk->src)); - disk->driverName[tmp - disk->src] = '\0'; + if (virStrncpy(disk->driverName, disk->src, + (tmp - disk->src), + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -912,8 +927,14 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverType, disk->src, (tmp - disk->src)); - disk->driverType[tmp - disk->src] = '\0'; + if (virStrncpy(disk->driverType, disk->src, + (tmp - disk->src), + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -1026,42 +1047,52 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { data++; if (STRPREFIX(key, "mac=")) { - int len = nextkey ? (nextkey - data) : 17; - if (len > 17) - len = 17; - strncpy(mac, data, len); - mac[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(mac) - 1; + if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "bridge=")) { - int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; + int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1; type = 1; - if (len > (sizeof(bridge)-1)) - len = sizeof(bridge)-1; - strncpy(bridge, data, len); - bridge[len] = '\0'; + if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "script=")) { - int len = nextkey ? (nextkey - data) : PATH_MAX-1; - if (len > (PATH_MAX-1)) - len = PATH_MAX-1; - strncpy(script, data, len); - script[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(script) - 1; + if (virStrncpy(script, data, len, sizeof(script)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Script %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "model=")) { - int len = nextkey ? (nextkey - data) : sizeof(model)-1; - if (len > (sizeof(model)-1)) - len = sizeof(model)-1; - strncpy(model, data, len); - model[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(model) - 1; + if (virStrncpy(model, data, len, sizeof(model)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), data); + goto skipnic; + } } else if (STRPREFIX(key, "vifname=")) { - int len = nextkey ? (nextkey - data) : sizeof(vifname)-1; - if (len > (sizeof(vifname)-1)) - len = sizeof(vifname)-1; - strncpy(vifname, data, len); - vifname[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1; + if (virStrncpy(vifname, data, len, sizeof(vifname)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Vifname %s too big for destination"), + data); + goto skipnic; + } } else if (STRPREFIX(key, "ip=")) { - int len = nextkey ? (nextkey - data) : 15; - if (len > 15) - len = 15; - strncpy(ip, data, len); - ip[len] = '\0'; + int len = nextkey ? (nextkey - data) : sizeof(ip) - 1; + if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("IP %s too big for destination"), data); + goto skipnic; + } } while (nextkey && (nextkey[0] == ',' || @@ -1165,32 +1196,41 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if (!(nextkey = strchr(key, ':'))) goto skippci; - if ((nextkey - key) > (sizeof(domain)-1)) + if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Domain %s too big for destination"), key); goto skippci; - - strncpy(domain, key, sizeof(domain)); - domain[sizeof(domain)-1] = '\0'; + } key = nextkey + 1; if (!(nextkey = strchr(key, ':'))) goto skippci; - strncpy(bus, key, sizeof(bus)); - bus[sizeof(bus)-1] = '\0'; + if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bus %s too big for destination"), key); + goto skippci; + } key = nextkey + 1; if (!(nextkey = strchr(key, '.'))) goto skippci; - strncpy(slot, key, sizeof(slot)); - slot[sizeof(slot)-1] = '\0'; + if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Slot %s too big for destination"), key); + goto skippci; + } key = nextkey + 1; if (strlen(key) != 1) goto skippci; - strncpy(func, key, sizeof(func)); - func[sizeof(func)-1] = '\0'; + if (virStrncpy(func, key, 1, sizeof(func)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Function %s too big for destination"), key); + goto skippci; + } if (virStrToLong_i(domain, NULL, 16, &domainID) < 0) goto skippci; @@ -1301,8 +1341,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { list->list->str) { char vfb[MAX_VFB]; char *key = vfb; - strncpy(vfb, list->list->str, MAX_VFB-1); - vfb[MAX_VFB-1] = '\0'; + + if (virStrcpyStatic(vfb, list->list->str) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("VFB %s too big for destination"), + list->list->str); + goto cleanup; + } if (VIR_ALLOC(graphics) < 0) goto no_memory; -- 1.6.0.6

3 3

[libvirt] [PATCH] Compressed save image format for Qemu.
by Chris Lalancette 28 Aug '09

28 Aug '09

Implement a compressed save image format for qemu. While ideally we would have the choice between compressed/non-compressed available to the libvirt API, unfortunately there is no "flags" parameter to the virDomainSave() API. Therefore, implement this as a qemu.conf option. Both gzip and bzip2 are implemented, and it should be very easy to implement additional compression methods. One open question is if/how we should detect the gzip and bzip2 binaries. One way to do it is to do compile-time setting of the paths (via configure.in), but that doesn't seem like a great thing to do. Another solution (my preferred solution) is not to detect at all; when we go to run the commands that need them, if they aren't available, or aren't available in one of the standard paths, then we'll fail. Maybe somebody else has another option or opinion, though. In the future, we'll have a more robust (managed) save/restore API, at which time we can expose this functionality properly in the API. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/qemu.conf | 10 +++++++ src/qemu_conf.c | 11 ++++++++ src/qemu_conf.h | 2 + src/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 653f487..86dcc9d 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -129,3 +129,13 @@ # "/dev/ptmx", "/dev/kvm", "/dev/kqemu", # "/dev/rtc", "/dev/hpet", "/dev/net/tun", #] + +# The default format for Qemu/KVM guest save images is raw; that is, the +# memory from the domain is dumped out directly to a file. If you have +# guests with a large amount of memory, however, this can take up quite +# a bit of space. If you would like to compress the images while they +# are being saved to disk, you can also set "gzip" or "bzip2" for the +# save_image_format. Note that this means you slow down the process +# of saving a domain in order to save disk space. +# +# save_image_format = "raw" diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 7ca5a15..ed87e13 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -280,6 +280,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupDeviceACL[i] = NULL; } + p = virConfGetValue (conf, "save_image_format"); + CHECK_TYPE ("save_image_format", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->saveImageFormat); + if (!(driver->saveImageFormat = strdup(p->str))) { + virReportOOMError(NULL); + virConfFree(conf); + return -1; + } + } + virConfFree (conf); return 0; } diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 8f4ef6a..e34baab 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -111,6 +111,8 @@ struct qemud_driver { char *securityDriverName; virSecurityDriverPtr securityDriver; + + char *saveImageFormat; }; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 3c92635..f642fea 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3437,11 +3437,26 @@ static int qemudDomainSave(virDomainPtr dom, struct qemud_save_header header; int ret = -1; virDomainEventPtr event = NULL; + int internalret; memset(&header, 0, sizeof(header)); memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); header.version = QEMUD_SAVE_VERSION; + if (driver->saveImageFormat == NULL) + header.compressed = QEMUD_SAVE_FORMAT_RAW; + else if (STREQ(driver->saveImageFormat, "raw")) + header.compressed = QEMUD_SAVE_FORMAT_RAW; + else if (STREQ(driver->saveImageFormat, "gzip")) + header.compressed = QEMUD_SAVE_FORMAT_GZIP; + else if (STREQ(driver->saveImageFormat, "bzip2")) + header.compressed = QEMUD_SAVE_FORMAT_BZIP2; + else { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("Invalid save image format specified in configuration file")); + return -1; + } + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3514,11 +3529,27 @@ static int qemudDomainSave(virDomainPtr dom, virReportOOMError(dom->conn); goto cleanup; } - if (virAsprintf(&command, "migrate \"exec:" - "dd of='%s' oflag=append conv=notrunc 2>/dev/null" - "\"", safe_path) == -1) { + + if (header.compressed == QEMUD_SAVE_FORMAT_RAW) + internalret = virAsprintf(&command, "migrate \"exec:" + "dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_GZIP) + internalret = virAsprintf(&command, "migrate \"exec:" + "gzip -c | dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2) + internalret = virAsprintf(&command, "migrate \"exec:" + "bzip2 -c | dd of='%s' oflag=append conv=notrunc 2>/dev/null" + "\"", safe_path); + else { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("Invalid compress format %d"), + header.compressed); + goto cleanup; + } + if (internalret < 0) { virReportOOMError(dom->conn); - command = NULL; goto cleanup; } @@ -4039,6 +4070,9 @@ static int qemudDomainRestore(virConnectPtr conn, char *xml = NULL; struct qemud_save_header header; virDomainEventPtr event = NULL; + int intermediatefd = -1; + pid_t intermediate_pid = -1; + int childstat; qemuDriverLock(driver); /* Verify the header and read the XML */ @@ -4128,8 +4162,39 @@ static int qemudDomainRestore(virConnectPtr conn, } def = NULL; + if (header.version == 2) { + const char *intermediate_argv[3] = { NULL, "-dc", NULL }; + if (header.compressed == QEMUD_SAVE_FORMAT_GZIP) + intermediate_argv[0] = "gzip"; + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2) + intermediate_argv[0] = "bzip2"; + else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("Unknown compressed save format %d"), + header.compressed); + goto cleanup; + } + if (intermediate_argv[0] != NULL) { + intermediatefd = fd; + fd = -1; + if (virExec(conn, intermediate_argv, NULL, NULL, + &intermediate_pid, intermediatefd, &fd, NULL, 0) < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to start decompression binary %s"), + intermediate_argv[0]); + goto cleanup; + } + } + } /* Set the migration source and start it up. */ ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd); + if (intermediate_pid != -1) { + /* Wait for intermediate process to exit */ + while (waitpid(intermediate_pid, &childstat, 0) == -1 && + errno == EINTR); + } + if (intermediatefd != -1) + close(intermediatefd); close(fd); fd = -1; if (ret < 0) { -- 1.6.0.6

3 3

[libvirt] perl bindings Sys Virt compilation problems
by Daniel de Araujo 28 Aug '09

28 Aug '09

Hi, I'm trying to compile the libvirt perl bindings (Sys-Virt-0.2.1) on x86_64 RHEL 5.3 and I'm running into the following issue: /usr/bin/perl "-Iblib/arch" "-Iblib/lib" perl-Sys-Virt.spec.PL<http://perl-sys-virt.spec.pl/>perl-Sys-Virt.spec Can't load 'blib/arch/auto/Sys/Virt/Virt.so' for module Sys::Virt: libvirt.so.0: cannot open shared object file: No such file or directory at /usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/DynaLoader.pm line 230. at perl-Sys-Virt.spec.PL <http://perl-sys-virt.spec.pl/> line 9 make: *** [perl-Sys-Virt.spec] Error 9 I've specified the PKG_CONFIG_PATH as /usr/local/lib/pkgconfig; libvirt shared objects are in /usr/local/lib I'm using libvirt 0.6.5. Any help would be appreciated. -- -Dan

2 1

[libvirt] how to change the boot order of a qemu-kvm guest os
by Gopalakrishnan Subramanian 28 Aug '09

28 Aug '09

2 2

[libvirt] Schedule for next release 0.7.1
by Daniel Veillard 28 Aug '09

28 Aug '09

Hello, following a bit too much backlog on libxml2 for the last years and an happy familly event, I have been a bit sidetracked in the last weeks and will be in the near future. I know there is a tons of pending patches, updates for drives and cleanups waiting for a release. I will try to make sure everything is reviewed and pushed by end of next week, which would allow for a 0.7.1 release by Sept 11, I hope this won't generate too much disruption while the initial schedule was at the end of the month ! thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1 0

[libvirt] [PATCH] virsh vol-key segfault fix
by Pritesh Kothari 28 Aug '09

28 Aug '09

Hi, There is a bug in virsh command vol-key, it segfaults cause the fourth option to vshCommandOptVolBy() is null instead of "pool" on virsh.c:5209. The patch to fix this is attached. Regards, Pritesh

2 1

[libvirt] ANNOUNCE: perl bindings Sys-Virt release 0.2.1
by Daniel P. Berrange 26 Aug '09

26 Aug '09

FYI I have just uploaded a new release 0.2.1 of the libvirt perl bindings, Sys-Virt, to CPAN. http://search.cpan.org/dist/Sys-Virt/ Direct download link http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-0.2.1.tar.gz Changes since 0.2.0: - Add all new APIs upto libvirt 0.6.4 (network interfaces, storage volume cloning and node device create/delete) - Fix off-by-1 in UUID stringification - Set default values for optional params. - Use SvOK check for potentially undefined string params - Export all the virError related constants - Fix some memory leaks in C layer - Remove bogus module imports from test suite There will shortly be another release that adds support for 0.7.0 newly added APIs. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

1 0

[libvirt] new API to get list of *all* interfaces matching a MAC
by Laine Stump 26 Aug '09

26 Aug '09

(This probably seems like overanalysis of a simple problem. That's what I'm best at ;-) Due to oversight, the function virInterfaceLookupByMACString() can only return a single interface, but it's possible that a host may have more than one interface with the same MAC. Since the API had already been released by the time we realized this, the existing function will remain and a new one added that can return a list of interfaces. This new API will need to deal with the fact that the list length is effectively unbounded. I can see three ways of dealing with this, and want to learn which is preferred by others before spending time on the implementation. 1) The array containing the list is allocated by libvirt, freed by caller. int virInterfaceLookupAllByMACString(virConnectPtr conn, const char *mac, virInterfacePtr **matches); "matches" will point to an array of virInterfacePtr, and that array's length will be the return value. This array will be allocated by libvirt, but must be freed by the application when it's finished. Usage (ignoring errors, of course :-): virInterfacePtr *matches; int matchCt; matchCt = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", &matches); for (i = 0; i < matchCt; i++) { /* do something with an interface */ virInterfaceFree(matches[i]); } free(matches); 2) The array containing the list is allocated by the application, and its length sent to libvirt. Libvirt then returns the actual number of matches, which may be greater than the max length sent by the application, but only copies as many as the application asked for. If the application sends 0 as maxMatches and/or a NULL pointer instead of a pointer to an array, libvirt will return the actual length, but not the list itself. For example, if there are 3 interfaces matching, and maxMatches is set to 2, the function will still return 3, but only set two items in the matches array. int virInterfaceLookupAllByMACString(virConnectPtr conn, const char *mac, virInterfacePtr **matches, int maxMatches); This puts all the functionality needed in a single API function, but it will generally need to be called twice - once to learn the length, and the second (after allocating an array of appropriate size) to retrieve the list. virInterfacePtr *matches; int matchCtA, matchCtB; matchCtA = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", NULL, 0); matches = malloc (matchCt * sizeof(virInterfacePtr)); matchCtB = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", &matches, matchCt); for (i = 0; i < min(matchCtA, matchCtB); i++) { /* do something with an interface */ virInterfaceFree(matches[i]); } free(matches); 3) Again, the array containing the list is allocated by the application, but it learns the proper length to allocate by calling a different API function. The lookup function itself returns the number of interfaces delivered in the array, rather than the number of matches it *wanted* to put in the array. int virInterfaceCountAllByMACString(virConnectPtr conn, const char *mac); int virInterfaceLookupAllByMACString(virConnectPtr conn, const char *mac, virInterfacePtr **matches, int maxMatches); Usage: virInterfacePtr *matches; int matchCt; matchCt = virInterfaceCountAllByMACString(conn, "00:01:02:03:04:05"); matches = malloc (matchCt * sizeof(virInterfacePtr)); matchCt = virInterfaceLookupAllByMACString(conn, "00:01:02:03:04:05", &matches, matchCt); for (i = 0; i < matchCt; i++) { /* do something with an interface */ virInterfaceFree(matches[i]); } free(matches); ******* (3) is closest to behavior of other libvirt APIs, but requires more functions in the API, and could lead to situations where newly added interfaces aren't in the list (if a new interface is added between calling the Count function and the Lookup function) . (2) has a more compact API (one which matches the netcf API exactly), but has the same problems with potential bad counts. (1) seems like the cleanest API, but I don't know what others' opinions are on having libvirt allocate the array (since that's not how its done for other similar things, eg virConnectListInterface, virConnectListNetworks, etc), or maybe there's some limitation of the RPC I haven't considered that makes it unfeasible. Which should I implement?

3 5

[libvirt] error: failed to connect to hypervisor (openvz)
by AL.Gabteni 26 Aug '09

26 Aug '09

Hi im trying to use libvirt with openvz since 2 days but without luck ,i allways get error: failed to connect to hypervisor (openvz) ; the test connection is working , any help will be appreciated . Regards

2 1

[libvirt] Small patch for ruby-libvirt to add api wrapping for DomainAttachDevice and DomainDetachDevice
by Jacob Leaver 25 Aug '09

25 Aug '09

--- ext/libvirt/_libvirt.c.orig 2009-01-19 18:24:22.000000000 -0800 +++ ext/libvirt/_libvirt.c 2009-01-19 18:24:39.000000000 -0800 @@ -728,6 +728,21 @@ gen_call_void(virDomainSave, conn(s), domain_get(s), StringValueCStr(to)); } +/* + * Call +virDomainAttachDevice+[http://www.libvirt.org/html/libvirt-libvirt.html#virDomainAttachDevice] + */ +VALUE libvirt_dom_attach_device_xml(VALUE s, VALUE xml) { + virDomainPtr dom = domain_get(s); + return virDomainAttachDevice(dom, StringValueCStr(xml)); +} + +/* + * Call +virDomainDetachDevice+[http://www.libvirt.org/html/libvirt-libvirt.html#virDomainDetachDevice] + */ +VALUE libvirt_dom_detach_device_xml(VALUE s, VALUE xml) { + virDomainPtr dom = domain_get(s); + return virDomainDetachDevice(dom, StringValueCStr(xml)); +} /* * Call +virDomainCoreDump+[http://www.libvirt.org/html/libvirt-libvirt.html#virDomainCoreDump] @@ -1921,7 +1936,8 @@ rb_define_method(c_domain, "autostart", libvirt_dom_autostart, 0); rb_define_method(c_domain, "autostart=", libvirt_dom_autostart_set, 1); rb_define_method(c_domain, "free", libvirt_dom_free, 0); - + rb_define_method(c_domain, "attach_device", libvirt_dom_attach_device_xml, 1); + rb_define_method(c_domain, "detach_device", libvirt_dom_detach_device_xml, 1); /* * Class Libvirt::Domain::Info */

2 1

[libvirt] OS Eject & Removable Media
by Jacob Leaver 25 Aug '09

25 Aug '09

Greetings all, First, I'd like to thank everyone that's working on the libvirt project, it's certainly making my life a better place. Second, (there's always a second, eh?) I am using libvirtd 0.6.4 and KVM qemu-kvm-0.10.5, and when a guest OS ejects media, libvirtd doesn't seem to pick up the change. Looking through the API, I don't see a provision to probe for a list of block devices, so although qemu and kvm know about media changes, I don't think there's a way for libvirtd to know about them. I've scanned the list archives, and haven't really seen any discussion of this, but I'd be more than happy to be directed somewhere. Also, I have a couple of minor and hackish patches for the ruby bindings for attaching and removing devices, if someone's interested. Sincerely, Jacob Leaver

2 2

[libvirt] network not autostarting
by Mariano Absatz 24 Aug '09

24 Aug '09

Hi, I'm using libvirt-bin 0.6.1 from the standard ubuntu repositories under ubuntu 9.04 (jaunty). I have, beside the default network, another one called "isolated" using the following description file: <network> <name>isolated</name> <uuid>51cffbcc-88f5-4edc-a81c-1765c1045691</uuid> <bridge name='virbr%d' stp='on' forwardDelay='0' /> <ip address='10.3.14.1' netmask='255.255.255.0'> <dhcp> <range start='10.3.14.128' end='10.3.14.254' /> </dhcp> </ip> </network> I have configured it with "net-autostart" and the symlink in the /etc/libvirt/qemu/networks/autostart/ autostart directory is just fine... however, it isn't starting automatically anymore (and it used to start up before)... Any hints on what to look up and where? TIA. -- Mariano Absatz - "El Baby" el.baby(a)gmail.com www.clueless.com.ar -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- _ | / \ | \ / ASCII RIBBON CAMPAIGN | X AGAINST HTML MAIL | / \ www.asciiribbon.org | | | -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- * TagZilla 0.066 * http://tagzilla.mozdev.org

2 1

[libvirt] Communicating with libvirt from windows
by anuj rampal 24 Aug '09

24 Aug '09

Hi, I have KVM running on one of my machine and I can controle the geues OS using the virt-manager and Virsh tools. Now what I want to do is talk to libvirt from my windows machine. I tried to compile libvirt for windows but couldnt get any success with that. Is there a way by which i can talk to libvirt without this client..?? As in can i talk to libvirt over TCP sockets using xml (something like that)...?? Thanks a lot in advance. Regards Anuj

4 3

[libvirt] [PATCH] Ignores EOPNOTSUPP when attempting to access an image on an NFS share.
by Darryl L. Pierce 22 Aug '09

22 Aug '09

rhbz#517157 Signed-off-by: Darryl L. Pierce <dpierce(a)redhat.com> --- src/security_selinux.c | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/security_selinux.c b/src/security_selinux.c index 0db9f49..f1e3f17 100644 --- a/src/security_selinux.c +++ b/src/security_selinux.c @@ -323,6 +323,8 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) VIR_INFO("Setting SELinux context on '%s' to '%s'", path, tcon); if (setfilecon(path, tcon) < 0) { + int setfilecon_errno = errno; + if (getfilecon(path, &econ) >= 0) { if (STREQ(tcon, econ)) { freecon(econ); @@ -331,14 +333,21 @@ SELinuxSetFilecon(virConnectPtr conn, const char *path, char *tcon) } freecon(econ); } - virSecurityReportError(conn, VIR_ERR_ERROR, - _("%s: unable to set security context " - "'\%s\' on %s: %s."), __func__, - tcon, - path, - virStrerror(errno, ebuf, sizeof ebuf)); - if (security_getenforce() == 1) + + /* if the error complaint is related to an image hosted on + * an nfs mount, then ignore it. + * rhbz 517157 + */ + if (setfilecon_errno != EOPNOTSUPP) { + virSecurityReportError(conn, VIR_ERR_ERROR, + _("%s: unable to set security context " + "'\%s\' on %s: %s."), __func__, + tcon, + path, + virStrerror(errno, ebuf, sizeof ebuf)); + if (security_getenforce() == 1) return -1; + } } return 0; } -- 1.6.2.5

3 2

[libvirt] [PATCH] spec file: add URL to Source tag
by Mark McLoughlin 21 Aug '09

21 Aug '09

See https://fedoraproject.org/wiki/Packaging:SourceURL --- libvirt.spec.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index dad97ec..00c394c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -81,7 +81,7 @@ Version: @VERSION@ Release: 1%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries -Source: libvirt-%{version}.tar.gz +Source: http://libvirt.org/sources/libvirt-%{version}.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root URL: http://libvirt.org/ BuildRequires: python-devel -- 1.6.2.5

1 0

[libvirt] error: failed to connect to the hypervisor
by Dominik Klein 21 Aug '09

21 Aug '09

Hi I am running kvm-78 with libvirt 0.6.1 and see the following error from time to time when my linux-ha cluster tries to run "virsh domstate" to figure out a virtual machine's state: error: failed to connect to the hypervisor In syslog, I see: libvirtd: 23:39:11.357: error : Failed to set close-on-exec file descriptor flag Due to this error, the cluster assumes the machine is not running okay and reboots it in order to fix things. This happened twice in 2 months with 10 machines being queried like that each minute (read: not too well reproducable) What is this and how could I work around it or is this fixed in newer versions and so worth an upgrade? Regards Dominik

2 3

[libvirt] Migration fail
by Christian Hennig 21 Aug '09

21 Aug '09

hi, i am trying to migrate a VM from my local system "pc38" to a remote on "pc36": virsh # list --all Id Name State ---------------------------------- - scalaris1 shut off virsh # migrate scalaris1 qemu+ssh://csr-user@pc36/system error: monitor socket did not show up.: No such file or directory What is the monitor socket? Both system are ubuntu server 9.04 : christian@pc38:~$ virsh --version Connecting to uri: qemu:///system 0.6.1 christian@pc38:~$ Thx for help! Christian

2 1

Re: [libvirt] libvirt with libssh2?
by Emre Erenoglu 20 Aug '09

20 Aug '09

> > ---------- Forwarded message ---------- > From: Daniel Veillard <veillard(a)redhat.com> > To: Emre Erenoglu <erenoglu(a)gmail.com> > Date: Thu, 20 Aug 2009 11:14:53 +0200 > Subject: Re: [libvirt] libvirt with libssh2? > On Thu, Aug 20, 2009 at 10:50:37AM +0200, Emre Erenoglu wrote: > > Hi, > > I'm the package maintainer of libvirt for the Pardus distribution. While > > trying to update the package to the latest 0.7.0, I recognized that > > configure script does not find libssh. > > > > I checked our system and found out that we are using libssh2 instead of > > libssh. > > > > >From this website (http://www.libssh2.org/libssh2-vs-libssh.html) > libssh2 > > developers summarize their differences. > > > > My questions: > > 1) Is there a specific reason why we are using libssh? Is there a patch > to > > make libvirt use libssh2 instead? > > 2) What functionality do we loose if we don't have libssh in the system? > > (depending on this, I'll also package libssh or not) > > I guess you activated --with-phyp which currently requires libssh, > but this will be fixed in the future, the result is just that that > driver gets desactivated for now if you don't have libssh > > see http://www.mail-archive.com/libvir-list@redhat.com/msg15086.html > Thanks Daniel. I'm a bit foreign to this "Power Hypervisor" thing, but if I'm not wrong from the information that I got from this site: http://publib.boulder.ibm.com/infocenter/systems/scope/hw/index.jsp?topic=/… than it's nothing that we support in Pardus at the moment. In fact, we only support KVM in our libvirt at the moment, possibly virtualbox in the future. In this case, I'm disabling this pyhp as well as the libssh dependency. Thanks a lot for the info and let me know if my understanding is wrong :) -- Emre

1 0

[libvirt] Power Hypervisor: Fix potential segfault and memleak in phypOpen
by Matthias Bolte 20 Aug '09

20 Aug '09

Hi, I came across this line in the phypOpen function: char string[strlen(conn->uri->path)]; Here the path part of the given URI is used without checking it for NULL, this can cause a segfault as strlen expects a string != NULL. Beside that uuid_db and connection_data leak in case of an error. In this line conn->uri->path = string; the original path of the URI leaks. The patch adds a VIR_FREE call before setting the new path. The attached patch is compile-tested but I don't have a Power Hypervisor installation at hand to test it for real. Matthias

4 7

[libvirt] [PATCH] Introduce virStrcpy.
by Chris Lalancette 20 Aug '09

20 Aug '09

Add the virStrcpy function, which takes a dst string, source string, and number of bytes available in the dest string. If the source string is too large to fit into the destination string, including the \0 byte, then no data is copied and the function returns NULL. Otherwise, this function copies source into dst, including the \0, and returns a pointer to the dst string. This function is intended to replace all unsafe uses of strncpy in the code base, since strncpy does *not* guarantee that the buffer terminates with a \0. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- cfg.mk | 5 ++ proxy/libvirt_proxy.c | 7 ++- qemud/Makefile.am | 3 +- qemud/qemud.c | 7 +- qemud/remote.c | 9 ++- src/bridge.c | 89 ++++++++-------------------- src/datatypes.c | 8 ++- src/esx/esx_driver.c | 10 ++- src/esx/esx_vmx.c | 7 ++- src/lxc_controller.c | 7 ++- src/lxc_driver.c | 13 ++++- src/nodeinfo.c | 5 +- src/opennebula/one_client.c | 5 +- src/openvz_conf.c | 33 +++++++--- src/phyp/phyp_driver.c | 2 +- src/proxy_internal.c | 5 +- src/qemu_conf.c | 16 +++--- src/qemu_driver.c | 13 +++- src/remote_internal.c | 20 +++++-- src/test.c | 15 ++++- src/uml_driver.c | 17 ++++-- src/util.c | 31 ++++++++-- src/util.h | 1 + src/xen_internal.c | 22 +++++-- src/xend_internal.c | 62 ++++++++++++++------ src/xm_internal.c | 134 +++++++++++++++++++++++++++--------------- 26 files changed, 343 insertions(+), 203 deletions(-) diff --git a/cfg.mk b/cfg.mk index 64bd26e..ad96bc6 100644 --- a/cfg.mk +++ b/cfg.mk @@ -91,6 +91,11 @@ sc_prohibit_asprintf: msg='use virAsprintf, not a'sprintf \ $(_prohibit_regexp) +sc_prohibit_strncpy: + @re='strncpy *\(' \ + msg='use virStrcpy, not strncpy' \ + $(_prohibit_regexp) + sc_prohibit_VIR_ERR_NO_MEMORY: @re='\<V''IR_ERR_NO_MEMORY\>' \ msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY \ diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index e008a7f..42084c7 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -167,10 +167,13 @@ proxyListenUnixSocket(const char *path) { * Abstract socket do not hit the filesystem, way more secure and * guaranteed to be atomic */ - memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + fprintf(stderr, "Path %s too long to fit into destination\n", path); + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 5f1376e..3220ded 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -108,9 +108,10 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) +libvirtd_LDADD += ../src/libvirt_util.la + if WITH_DRIVER_MODULES libvirtd_LDADD += ../src/libvirt_driver.la - libvirtd_LDADD += ../src/libvirt_util.la else if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la diff --git a/qemud/qemud.c b/qemud/qemud.c index b2bd59d..17fc172 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -509,13 +509,14 @@ static int qemudListenUnix(struct qemud_server *server, virSetNonBlock(sock->fd) < 0) goto cleanup; - memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, path, sizeof(addr.sun_path)-1); + if (virStrcpy(addr.sun_path, path, sizeof(addr.sun_path)) == NULL) { + VIR_ERROR(_("Path %s too long for unix socket"), path); + goto cleanup; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; - oldgrp = getgid(); oldmask = umask(readonly ? ~unix_sock_ro_mask : ~unix_sock_rw_mask); if (server->privileged) diff --git a/qemud/remote.c b/qemud/remote.c index 2225d06..92cc925 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -570,9 +570,12 @@ remoteDispatchDomainSetSchedulerParameters (struct qemud_server *server ATTRIBUT /* Deserialise parameters. */ for (i = 0; i < nparams; ++i) { - strncpy (params[i].field, args->params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[i].field, args->params.params_val[i].field, + sizeof(params[i].field)) == NULL) { + remoteDispatchFormatError(rerr, _("Field %s too big for destination"), + args->params.params_val[i].field); + return -1; + } params[i].type = args->params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/bridge.c b/src/bridge.c index 414d87b..378c055 100644 --- a/src/bridge.c +++ b/src/bridge.c @@ -149,23 +149,19 @@ brHasBridge(brControl *ctl, const char *name) { struct ifreq ifr; - int len; if (!ctl || !name) { errno = EINVAL; return -1; } - if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name)) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, name, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr)) return -1; @@ -216,18 +212,14 @@ brAddDelInterface(brControl *ctl, const char *iface) { struct ifreq ifr; - int len; if (!ctl || !ctl->fd || !bridge || !iface) return EINVAL; - if ((len = strlen(bridge)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, bridge, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, bridge, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (!(ifr.ifr_ifindex = if_nametoindex(iface))) return ENODEV; @@ -305,23 +297,19 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED, static int ifGetMtu(brControl *ctl, const char *ifname) { struct ifreq ifr; - int len; if (!ctl || !ifname) { errno = EINVAL; return -1; } - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) { + memset(&ifr, 0, sizeof(struct ifreq)); + + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) { errno = EINVAL; return -1; } - memset(&ifr, 0, sizeof(struct ifreq)); - - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; - if (ioctl(ctl->fd, SIOCGIFMTU, &ifr)) return -1; @@ -343,18 +331,14 @@ static int ifGetMtu(brControl *ctl, const char *ifname) static int ifSetMtu(brControl *ctl, const char *ifname, int mtu) { struct ifreq ifr; - int len; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; ifr.ifr_mtu = mtu; return ioctl(ctl->fd, SIOCSIFMTU, &ifr) == 0 ? 0 : errno; @@ -466,7 +450,7 @@ brAddTap(brControl *ctl, int vnet_hdr, int *tapfd) { - int fd, len; + int fd; struct ifreq ifr; if (!ctl || !ctl->fd || !bridge || !ifname) @@ -486,17 +470,14 @@ brAddTap(brControl *ctl, (void) vnet_hdr; #endif - strncpy(ifr.ifr_name, *ifname, IFNAMSIZ-1); - - if (ioctl(fd, TUNSETIFF, &ifr) < 0) - goto error; - - len = strlen(ifr.ifr_name); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpy(ifr.ifr_name, *ifname, sizeof(ifr.ifr_name)) == NULL) { errno = EINVAL; goto error; } + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + goto error; + /* We need to set the interface MTU before adding it * to the bridge, because the bridge will have its * MTU adjusted automatically when we add the new interface. @@ -527,7 +508,6 @@ int brDeleteTap(brControl *ctl, const char *ifname) { struct ifreq try; - int len; int fd; if (!ctl || !ctl->fd || !ifname) @@ -539,15 +519,11 @@ int brDeleteTap(brControl *ctl, memset(&try, 0, sizeof(struct ifreq)); try.ifr_flags = IFF_TAP|IFF_NO_PI; - len = strlen(ifname); - if (len >= BR_IFNAME_MAXLEN - 1) { + if (virStrcpy(try.ifr_name, ifname, sizeof(try.ifr_name)) == NULL) { errno = EINVAL; goto error; } - strncpy(try.ifr_name, ifname, len); - try.ifr_name[len] = '\0'; - if (ioctl(fd, TUNSETIFF, &try) == 0) { if ((errno = ioctl(fd, TUNSETPERSIST, 0))) goto error; @@ -576,19 +552,15 @@ brSetInterfaceUp(brControl *ctl, int up) { struct ifreq ifr; - int len; int flags; if (!ctl || !ifname) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -621,18 +593,14 @@ brGetInterfaceUp(brControl *ctl, int *up) { struct ifreq ifr; - int len; if (!ctl || !ifname || !up) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr) < 0) return errno; @@ -654,18 +622,15 @@ brSetInetAddr(brControl *ctl, } s; struct ifreq ifr; struct in_addr inaddr; - int len, ret; + int ret; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if ((ret = inet_pton(AF_INET, addr, &inaddr)) < 0) return errno; @@ -692,18 +657,14 @@ brGetInetAddr(brControl *ctl, { struct ifreq ifr; struct in_addr *inaddr; - int len; if (!ctl || !ctl->fd || !ifname || !addr) return EINVAL; - if ((len = strlen(ifname)) >= BR_IFNAME_MAXLEN) - return EINVAL; - memset(&ifr, 0, sizeof(struct ifreq)); - strncpy(ifr.ifr_name, ifname, len); - ifr.ifr_name[len] = '\0'; + if (virStrcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) == NULL) + return EINVAL; if (ioctl(ctl->fd, cmd, &ifr) < 0) return errno; diff --git a/src/datatypes.c b/src/datatypes.c index ac61682..b10452e 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "util.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -876,8 +877,11 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c virReportOOMError(conn); goto error; } - strncpy(ret->key, key, sizeof(ret->key)-1); - ret->key[sizeof(ret->key)-1] = '\0'; + if (virStrcpy(ret->key, key, sizeof(ret->key)) == NULL) { + virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, + _("Volume key %s too large for destination"), key); + goto error; + } ret->magic = VIR_STORAGE_VOL_MAGIC; ret->conn = conn; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index dba6305..c5dbda2 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -696,9 +696,13 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) ++ptr; } - strncpy (nodeinfo->model, dynamicProperty->val->string, - sizeof (nodeinfo->model) - 1); - nodeinfo->model[sizeof (nodeinfo->model) - 1] = '\0'; + if (virStrcpy(nodeinfo->model, dynamicProperty->val->string, + sizeof(nodeinfo->model)) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Model %s too long for destination", + dynamicProperty->val->string); + goto failure; + } } else { VIR_WARN("Unexpected '%s' property", dynamicProperty->name); } diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index 91a86e2..b138d13 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -885,8 +885,11 @@ esxVMX_IndexToDiskName(virConnectPtr conn, int idx, const char *prefix) return NULL; } - strncpy(buffer, prefix, sizeof (buffer) - 1); - buffer[sizeof (buffer) - 1] = '\0'; + if (virStrcpy(buffer, prefix, sizeof(buffer)) == NULL) { + ESX_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + "Prefix %s too big for destination", prefix); + return NULL; + } if (idx < 26) { buffer[length] = 'a' + idx; diff --git a/src/lxc_controller.c b/src/lxc_controller.c index 8d11238..6bc9976 100644 --- a/src/lxc_controller.c +++ b/src/lxc_controller.c @@ -176,9 +176,12 @@ static int lxcMonitorServer(const char *sockpath) } unlink(sockpath); - memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpy(addr.sun_path, sockpath, sizeof(addr.sun_path)) == NULL) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too long for destination"), sockpath); + goto error; + } if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(NULL, errno, diff --git a/src/lxc_driver.c b/src/lxc_driver.c index c6cf935..b42f77f 100644 --- a/src/lxc_driver.c +++ b/src/lxc_driver.c @@ -719,7 +719,11 @@ static int lxcMonitorClient(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + if (virStrcpy(addr.sun_path, sockpath, sizeof(addr.sun_path)) == NULL) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("Socket path %s too big for destination"), sockpath); + goto error; + } if (connect(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { virReportSystemError(conn, errno, "%s", @@ -1737,7 +1741,12 @@ static int lxcGetSchedulerParameters(virDomainPtr domain, if (virCgroupGetCpuShares(group, &val) != 0) goto cleanup; params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); + if (virStrcpy(params[0].field, "cpu_shares", + sizeof(params[0].field)) == NULL) { + lxcError(NULL, domain, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too big for destination")); + goto cleanup; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; ret = 0; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 0a211a7..b620ced 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -141,8 +141,9 @@ int nodeGetInfo(virConnectPtr conn, uname(&info); - strncpy(nodeinfo->model, info.machine, sizeof(nodeinfo->model)-1); - nodeinfo->model[sizeof(nodeinfo->model)-1] = '\0'; + if (virStrcpy(nodeinfo->model, info.machine, + sizeof(nodeinfo->model)) == NULL) + return -1; #else /* !HAVE_UNAME */ diff --git a/src/opennebula/one_client.c b/src/opennebula/one_client.c index 21f303a..9d3a5c6 100644 --- a/src/opennebula/one_client.c +++ b/src/opennebula/one_client.c @@ -24,6 +24,8 @@ #include <fcntl.h> #include <unistd.h> #include "one_client.h" +#include "datatypes.h" +#include "util.h" oneClient one_client; @@ -186,8 +188,7 @@ int c_oneVmInfo(int vmid, char* ret_info,int length) if( return_code ) { - strncpy(ret_info, return_string, length-1); - ret_info[length-1] = '\0'; + virStrcpy(ret_info, return_string, length); retval = 0; } diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..27f0264 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -261,8 +261,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->ifname, len+1) < 0) goto no_memory; - strncpy(net->ifname, p, len); - net->ifname[len] = '\0'; + if (virStrcpy(net->ifname, p, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Network ifname %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "bridge=")) { p += 7; len = next - p; @@ -275,8 +278,11 @@ openvzReadNetworkConf(virConnectPtr conn, if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto no_memory; - strncpy(net->data.bridge.brname, p, len); - net->data.bridge.brname[len] = '\0'; + if (virStrcpy(net->data.bridge.brname, p, len+1) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge name %s too long for destination"), p); + goto error; + } } else if (STRPREFIX(p, "mac=")) { p += 4; len = next - p; @@ -285,8 +291,11 @@ openvzReadNetworkConf(virConnectPtr conn, "%s", _("Wrong length MAC address")); goto error; } - strncpy(cpy_temp, p, len); - cpy_temp[len] = '\0'; + if (virStrcpy(cpy_temp, p, sizeof(cpy_temp)) == NULL) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too long for destination"), p); + goto error; + } if (virParseMacAddr(cpy_temp, net->mac) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Wrong MAC address")); @@ -629,8 +638,10 @@ openvzReadConfigParam(const char * conf_file ,const char * param, char *value, i if (sf[0] == '=' && sf[1] != '\0' ) { sf ++; if ((token = strtok_r(sf,"\"\t\n", &saveptr)) != NULL) { - strncpy(value, token, maxlen) ; - value[maxlen-1] = '\0'; + if (virStrcpy(value, token, maxlen) == NULL) { + ret = -1; + break; + } found = 1; } } @@ -812,6 +823,7 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) char uuidbuf[1024]; char iden[1024]; int fd, ret; + int retval = 0; if (openvzLocateConfFile(vpsid, conf_file, PATH_MAX, "conf")<0) return -1; @@ -832,13 +844,14 @@ openvzGetVPSUUID(int vpsid, char *uuidstr, size_t len) sscanf(line, "%s %s\n", iden, uuidbuf); if(STREQ(iden, "#UUID:")) { - strncpy(uuidstr, uuidbuf, len); + if (virStrcpy(uuidstr, uuidbuf, len) == NULL) + retval = -1; break; } } close(fd); - return 0; + return retval; } /* Do actual checking for UUID presence in conf file, diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index f457cf4..508767e 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1369,7 +1369,7 @@ escape_specialcharacters(char *src, char *dst, size_t dstlen) } temp_buffer[j] = '\0'; - if (strncpy(dst, temp_buffer, dstlen) == NULL) + if (virStrcpy(dst, temp_buffer, dstlen) == NULL) return -1; return 0; diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 5b92ad8..8609201 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -197,7 +197,10 @@ retry: memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; addr.sun_path[0] = '\0'; - strncpy(&addr.sun_path[1], path, (sizeof(addr) - 4) - 2); + if (virStrcpy(&addr.sun_path[1], path, sizeof(addr.sun_path) - 1) == NULL) { + close(fd); + return -1; + } /* * now bind the socket to that address and listen on it diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 6b0b404..4690c69 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1282,18 +1282,18 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, { switch (dev->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - strncpy(buf, "null", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "null", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_VC: - strncpy(buf, "vc", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "vc", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_PTY: - strncpy(buf, "pty", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "pty", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_DEV: @@ -1315,8 +1315,8 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, break; case VIR_DOMAIN_CHR_TYPE_STDIO: - strncpy(buf, "stdio", buflen); - buf[buflen-1] = '\0'; + if (virStrcpy(buf, "stdio", buflen) == NULL) + return -1; break; case VIR_DOMAIN_CHR_TYPE_UDP: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 37fdec2..bcacd41 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -909,7 +909,11 @@ qemudOpenMonitorUnix(virConnectPtr conn, memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, monitor, sizeof(addr.sun_path)); + if (virStrcpy(addr.sun_path, monitor, sizeof(addr.sun_path)) == NULL) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Monitor path %s too big for destination"), monitor); + goto error; + } do { ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); @@ -5802,8 +5806,11 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, goto cleanup; } params[0].value.ul = val; - strncpy(params[0].field, "cpu_shares", sizeof(params[0].field)); - params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; + if (virStrcpy(params[0].field, "cpu_shares", sizeof(params[0].field)) == NULL) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("Field cpu_shares too long for destination")); + goto cleanup; + } ret = 0; diff --git a/src/remote_internal.c b/src/remote_internal.c index ad0cbf0..bc69c70 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -670,7 +670,11 @@ doRemoteOpen (virConnectPtr conn, memset (&addr, 0, sizeof addr); addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, sockname, UNIX_PATH_MAX (addr)); + if (virStrcpy(addr.sun_path, sockname, sizeof(addr.sun_path)) == NULL) { + errorf(conn, VIR_ERR_INTERNAL_ERROR, + _("Socket %s too big for destination"), sockname); + goto failed; + } if (addr.sun_path[0] == '@') addr.sun_path[0] = '\0'; @@ -1585,8 +1589,8 @@ remoteNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info) (xdrproc_t) xdr_remote_node_get_info_ret, (char *) &ret) == -1) goto done; - strncpy (info->model, ret.model, 32); - info->model[31] = '\0'; + if (virStrcpy(info->model, ret.model, sizeof(info->model)) == NULL) + goto done; info->memory = ret.memory; info->cpus = ret.cpus; info->mhz = ret.mhz; @@ -3017,9 +3021,13 @@ remoteDomainGetSchedulerParameters (virDomainPtr domain, /* Deserialise the result. */ for (i = 0; i < *nparams; ++i) { - strncpy (params[i].field, ret.params.params_val[i].field, - VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[i].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[i].field, ret.params.params_val[i].field, + sizeof(params[i].field)) == NULL) { + errorf(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Parameter %s too big for destination"), + ret.params.params_val[i].field); + goto cleanup; + } params[i].type = ret.params.params_val[i].value.type; switch (params[i].type) { case VIR_DOMAIN_SCHED_FIELD_INT: diff --git a/src/test.c b/src/test.c index ab6cfdf..e265d38 100644 --- a/src/test.c +++ b/src/test.c @@ -570,8 +570,11 @@ static int testOpenFromFile(virConnectPtr conn, privconn->nextDomID = 1; privconn->numCells = 0; - strncpy(privconn->path, file, PATH_MAX-1); - privconn->path[PATH_MAX-1] = '\0'; + if (virStrcpy(privconn->path, file, sizeof(privconn->path)) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Path %s too big for destination"), file); + goto error; + } memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); nodeInfo = &privconn->nodeInfo; @@ -627,8 +630,12 @@ static int testOpenFromFile(virConnectPtr conn, str = virXPathString(conn, "string(/node/cpu/model[1])", ctxt); if (str != NULL) { - strncpy(nodeInfo->model, str, sizeof(nodeInfo->model)-1); - nodeInfo->model[sizeof(nodeInfo->model)-1] = '\0'; + if (virStrcpy(nodeInfo->model, str, sizeof(nodeInfo->model)) == NULL) { + testError(NULL, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), str); + VIR_FREE(str); + goto error; + } VIR_FREE(str); } diff --git a/src/uml_driver.c b/src/uml_driver.c index 53e7e10..815a4b3 100644 --- a/src/uml_driver.c +++ b/src/uml_driver.c @@ -562,6 +562,7 @@ static int umlMonitorAddress(virConnectPtr conn, virDomainObjPtr vm, struct sockaddr_un *addr) { char *sockname; + int retval = 0; if (virAsprintf(&sockname, "%s/%s/mconsole", driver->monitorDir, vm->def->name) < 0) { @@ -571,10 +572,13 @@ static int umlMonitorAddress(virConnectPtr conn, memset(addr, 0, sizeof *addr); addr->sun_family = AF_UNIX; - strncpy(addr->sun_path, sockname, sizeof(addr->sun_path)-1); - NUL_TERMINATE(addr->sun_path); + if (virStrcpy(addr->sun_path, sockname, sizeof(addr->sun_path)) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Unix path %s too long for destination"), sockname); + retval = -1; + } VIR_FREE(sockname); - return 0; + return retval; } static int umlOpenMonitor(virConnectPtr conn, @@ -668,8 +672,11 @@ static int umlMonitorCommand(virConnectPtr conn, cmd, req.length); return -1; } - strncpy(req.data, cmd, req.length); - req.data[req.length] = '\0'; + if (virStrcpy(req.data, cmd, sizeof(req.data)) == NULL) { + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Command %s too long for destination"), cmd); + return -1; + } if (sendto(vm->monitor, &req, sizeof req, 0, (struct sockaddr *)&addr, sizeof addr) != (sizeof req)) { diff --git a/src/util.c b/src/util.c index 0d4f3fa..0d252e6 100644 --- a/src/util.c +++ b/src/util.c @@ -1121,9 +1121,8 @@ char *virFindFileInPath(const char *file) char fullpath[PATH_MAX]; /* copy PATH env so we can tweak it */ - strncpy(pathenv, getenv("PATH"), PATH_MAX); - pathenv[PATH_MAX - 1] = '\0'; - + if (virStrcpy(pathenv, getenv("PATH"), sizeof(pathenv)) == NULL) + return NULL; /* for each path segment, append the file to search for and test for * it. return it if found. @@ -1155,8 +1154,8 @@ int virFileMakePath(const char *path) if (stat(path, &st) >= 0) return 0; - strncpy(parent, path, PATH_MAX); - parent[PATH_MAX - 1] = '\0'; + if (virStrcpy(parent, path, sizeof(parent)) == NULL) + return EINVAL; if (!(p = strrchr(parent, '/'))) return EINVAL; @@ -1573,6 +1572,28 @@ virAsprintf(char **strp, const char *fmt, ...) return ret; } +/** + * virStrcpy + * + * A safe version of strcpy. The last parameter is the number of bytes + * available in the destination string, *not* the number of bytes you want + * to copy. If the destination is not large enough to hold all of the + * src string, NULL is returned and no data is copied. If the destination + * is large enough to hold all of the src, then the string is copied and + * a pointer to the destination string is returned. + */ +char * +virStrcpy(char *dest, const char *src, size_t destbytes) +{ + int len; + + len = strlen(src) + 1; + if (len > destbytes) + return NULL; + + return memcpy(dest, src, len); +} + /* Compare two MAC addresses, ignoring differences in case, * as well as leading zeros. */ diff --git a/src/util.h b/src/util.h index b3e628a..ffbf2b3 100644 --- a/src/util.h +++ b/src/util.h @@ -164,6 +164,7 @@ void virSkipSpaces(const char **str); int virParseNumber(const char **str); int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3); +char *virStrcpy(char *dest, const char *src, size_t destbytes); #define VIR_MAC_BUFLEN 6 #define VIR_MAC_PREFIX_BUFLEN 3 diff --git a/src/xen_internal.c b/src/xen_internal.c index ae78f84..fcc4977 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -1208,13 +1208,21 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, if (ret < 0) return(-1); - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[0].field, str_weight, + sizeof(params[0].field)) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Weight %s too big for destination", str_weight); + return -1; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = op_dom.u.getschedinfo.u.credit.weight; - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[1].field, str_cap, + sizeof(params[1].field)) == NULL) { + virXenError (domain->conn, VIR_ERR_INTERNAL_ERROR, + "Cap %s too big for destination", str_cap); + return -1; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap; @@ -2431,9 +2439,9 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, while (fgets (line, sizeof line, cpuinfo)) { if (regexec (&flags_hvm_rec, line, sizeof(subs)/sizeof(regmatch_t), subs, 0) == 0 && subs[0].rm_so != -1) { - strncpy (hvm_type, - &line[subs[1].rm_so], subs[1].rm_eo-subs[1].rm_so+1); - hvm_type[subs[1].rm_eo-subs[1].rm_so] = '\0'; + if (virStrcpy(hvm_type, &line[subs[1].rm_so], + sizeof(hvm_type)) == NULL) + return NULL; } else if (regexec (&flags_pae_rec, line, 0, NULL, 0) == 0) host_pae = 1; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 7bcee7d..a7167ae 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -763,7 +763,8 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) addr = (struct sockaddr_un *)&priv->addr; addr->sun_family = AF_UNIX; memset(addr->sun_path, 0, sizeof(addr->sun_path)); - strncpy(addr->sun_path, path, sizeof(addr->sun_path)); + if (virStrcpy(addr->sun_path, path, sizeof(addr->sun_path)) == NULL) + return -1; return (0); } @@ -1442,8 +1443,11 @@ xenDaemonParseSxprChar(virConnectPtr conn, return NULL; } - strncpy(prefix, value, sizeof(prefix)-1); - NUL_TERMINATE(prefix); + if (virStrcpy(prefix, value, sizeof(prefix)) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Value %s too big for prefix"), value); + goto error; + } if (value[0] == '/') { def->type = VIR_DOMAIN_CHR_TYPE_DEV; @@ -1647,8 +1651,12 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverName, (offset-src)+1) < 0) goto no_memory; - strncpy(disk->driverName, src, (offset-src)); - disk->driverName[offset-src] = '\0'; + if (virStrcpy(disk->driverName, src, (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + src); + goto error; + } src = offset + 1; @@ -1662,8 +1670,12 @@ xenDaemonParseSxprDisks(virConnectPtr conn, if (VIR_ALLOC_N(disk->driverType, (offset-src)+1)< 0) goto no_memory; - strncpy(disk->driverType, src, (offset-src)); - disk->driverType[offset-src] = '\0'; + if (virStrcpy(disk->driverType, src, (offset-src)+1) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + src); + goto error; + } src = offset + 1; /* Its possible to use blktap driver for block devs @@ -1896,8 +1908,12 @@ xenDaemonParseSxprSound(virConnectPtr conn, _("unexpected sound model %s"), offset); goto error; } - strncpy(model, offset, len); - model[len] = '\0'; + if (virStrcpy(model, offset, sizeof(model)) == NULL) { + virXendError(conn, VIR_ERR_INTERNAL_ERROR, + _("Sound model %s too big for destination"), + offset); + goto error; + } if (VIR_ALLOC(sound) < 0) goto no_memory; @@ -4786,13 +4802,22 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain, goto error; } - strncpy (params[0].field, str_weight, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[0].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[0].field, str_weight, + sizeof(params[0].field)) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Weight %s too big for destination"), + str_weight); + goto error; + } params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[0].value.ui = sexpr_int(root, "domain/cpu_weight"); - strncpy (params[1].field, str_cap, VIR_DOMAIN_SCHED_FIELD_LENGTH); - params[1].field[VIR_DOMAIN_SCHED_FIELD_LENGTH-1] = '\0'; + if (virStrcpy(params[1].field, str_cap, + sizeof(params[1].field)) == NULL) { + virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, + _("Cap %s too big for destination"), str_cap); + goto error; + } params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT; params[1].value.ui = sexpr_int(root, "domain/cpu_cap"); *nparams = XEN_SCHED_CRED_NPARAM; @@ -5842,6 +5867,7 @@ virDomainXMLDevID(virDomainPtr domain, { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; + char *tmp; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { if (dev->data.disk->driverName && @@ -5859,9 +5885,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { char mac[30]; virDomainNetDefPtr def = dev->data.net; @@ -5878,9 +5905,10 @@ virDomainXMLDevID(virDomainPtr domain, if (xref == NULL) return -1; - strncpy(ref, xref, ref_len); + tmp = virStrcpy(ref, xref, ref_len); free(xref); - ref[ref_len - 1] = '\0'; + if (tmp == NULL) + return -1; } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { diff --git a/src/xm_internal.c b/src/xm_internal.c index 71b852e..00d9e9a 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -872,8 +872,12 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { } else { if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->src, head, (offset - head)); - disk->src[(offset-head)] = '\0'; + if (virStrcpy(disk->src, head, (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Source file %s too big for destination"), + head); + goto cleanup; + } } head = offset + 1; @@ -886,8 +890,11 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->dst, (offset - head) + 1) < 0) goto no_memory; - strncpy(disk->dst, head, (offset - head)); - disk->dst[(offset-head)] = '\0'; + if (virStrcpy(disk->dst, head, (offset - head) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Dest file %s too big for destination"), head); + goto cleanup; + } head = offset + 1; @@ -897,8 +904,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((tmp = strchr(disk->src, ':')) != NULL) { if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverName, disk->src, (tmp - disk->src)); - disk->driverName[tmp - disk->src] = '\0'; + if (virStrcpy(disk->driverName, disk->src, + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver name %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -912,8 +924,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { goto skipdisk; if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) < 0) goto no_memory; - strncpy(disk->driverType, disk->src, (tmp - disk->src)); - disk->driverType[tmp - disk->src] = '\0'; + if (virStrcpy(disk->driverType, disk->src, + (tmp - disk->src) + 1) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Driver type %s too big for destination"), + disk->src); + goto cleanup; + } /* Strip the prefix we found off the source file name */ memmove(disk->src, disk->src+(tmp-disk->src)+1, @@ -1026,42 +1043,46 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { data++; if (STRPREFIX(key, "mac=")) { - int len = nextkey ? (nextkey - data) : 17; - if (len > 17) - len = 17; - strncpy(mac, data, len); - mac[len] = '\0'; + if (virStrcpy(mac, data, sizeof(mac)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("MAC address %s too big for destination"), + data); + goto cleanup; + } } else if (STRPREFIX(key, "bridge=")) { - int len = nextkey ? (nextkey - data) : sizeof(bridge)-1; + if (virStrcpy(bridge, data, sizeof(bridge)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bridge %s too big for destination"), + data); + goto cleanup; + } type = 1; - if (len > (sizeof(bridge)-1)) - len = sizeof(bridge)-1; - strncpy(bridge, data, len); - bridge[len] = '\0'; } else if (STRPREFIX(key, "script=")) { - int len = nextkey ? (nextkey - data) : PATH_MAX-1; - if (len > (PATH_MAX-1)) - len = PATH_MAX-1; - strncpy(script, data, len); - script[len] = '\0'; + if (virStrcpy(script, data, sizeof(script)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Script %s too big for destination"), + data); + goto cleanup; + } } else if (STRPREFIX(key, "model=")) { - int len = nextkey ? (nextkey - data) : sizeof(model)-1; - if (len > (sizeof(model)-1)) - len = sizeof(model)-1; - strncpy(model, data, len); - model[len] = '\0'; + if (virStrcpy(model, data, sizeof(model)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Model %s too big for destination"), data); + goto cleanup; + } } else if (STRPREFIX(key, "vifname=")) { - int len = nextkey ? (nextkey - data) : sizeof(vifname)-1; - if (len > (sizeof(vifname)-1)) - len = sizeof(vifname)-1; - strncpy(vifname, data, len); - vifname[len] = '\0'; + if (virStrcpy(vifname, data, sizeof(vifname)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Vifname %s too big for destination"), + data); + goto cleanup; + } } else if (STRPREFIX(key, "ip=")) { - int len = nextkey ? (nextkey - data) : 15; - if (len > 15) - len = 15; - strncpy(ip, data, len); - ip[len] = '\0'; + if (virStrcpy(ip, data, sizeof(ip)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("IP %s too big for destination"), data); + goto cleanup; + } } while (nextkey && (nextkey[0] == ',' || @@ -1168,29 +1189,41 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { if ((nextkey - key) > (sizeof(domain)-1)) goto skippci; - strncpy(domain, key, sizeof(domain)); - domain[sizeof(domain)-1] = '\0'; + if (virStrcpy(domain, key, sizeof(domain)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Domain %s too big for destination"), key); + goto cleanup; + } key = nextkey + 1; if (!(nextkey = strchr(key, ':'))) goto skippci; - strncpy(bus, key, sizeof(bus)); - bus[sizeof(bus)-1] = '\0'; + if (virStrcpy(bus, key, sizeof(bus)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Bus %s too big for destination"), key); + goto cleanup; + } key = nextkey + 1; if (!(nextkey = strchr(key, '.'))) goto skippci; - strncpy(slot, key, sizeof(slot)); - slot[sizeof(slot)-1] = '\0'; + if (virStrcpy(slot, key, sizeof(slot)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Slot %s too big for destination"), key); + goto cleanup; + } key = nextkey + 1; if (strlen(key) != 1) goto skippci; - strncpy(func, key, sizeof(func)); - func[sizeof(func)-1] = '\0'; + if (virStrcpy(func, key, sizeof(func)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("Function %s too big for destination"), key); + goto cleanup; + } if (virStrToLong_i(domain, NULL, 16, &domainID) < 0) goto skippci; @@ -1301,8 +1334,13 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { list->list->str) { char vfb[MAX_VFB]; char *key = vfb; - strncpy(vfb, list->list->str, MAX_VFB-1); - vfb[MAX_VFB-1] = '\0'; + + if (virStrcpy(vfb, list->list->str, sizeof(vfb)) == NULL) { + xenXMError(conn, VIR_ERR_INTERNAL_ERROR, + _("VFB %s too big for destination"), + list->list->str); + goto cleanup; + } if (VIR_ALLOC(graphics) < 0) goto no_memory; -- 1.6.0.6

2 2

[libvirt] libvirt with libssh2?
by Emre Erenoglu 20 Aug '09

20 Aug '09

Hi, I'm the package maintainer of libvirt for the Pardus distribution. While trying to update the package to the latest 0.7.0, I recognized that configure script does not find libssh. I checked our system and found out that we are using libssh2 instead of libssh. >From this website (http://www.libssh2.org/libssh2-vs-libssh.html) libssh2 developers summarize their differences. My questions: 1) Is there a specific reason why we are using libssh? Is there a patch to make libvirt use libssh2 instead? 2) What functionality do we loose if we don't have libssh in the system? (depending on this, I'll also package libssh or not) Thanks a lot for any comments, Best regards, Emre Erenoglu

2 1

[libvirt] [PATCH] Small fixes for qemu save compression.
by Chris Lalancette 20 Aug '09

20 Aug '09

Fix up a small memory leak pointed out by DanB; I was forgetting to release memory allocated to driver->saveImageFormat. Also add the "save_image_format" and "security" entries to the augeas lens. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- qemud/libvirtd_qemu.aug | 2 ++ qemud/test_libvirtd_qemu.aug | 10 +++++++++- src/qemu_driver.c | 1 + 3 files changed, 12 insertions(+), 1 deletions(-) diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug index 8b8aab2..2175e14 100644 --- a/qemud/libvirtd_qemu.aug +++ b/qemud/libvirtd_qemu.aug @@ -29,10 +29,12 @@ module Libvirtd_qemu = | str_entry "vnc_password" | bool_entry "vnc_sasl" | str_entry "vnc_sasl_dir" + | str_entry "security_driver" | str_entry "user" | str_entry "group" | str_array_entry "cgroup_controllers" | str_array_entry "cgroup_device_acl" + | str_entry "save_image_format" (* Each enty in the config is one of the following three ... *) let entry = vnc_entry diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug index 274c89d..72f2227 100644 --- a/qemud/test_libvirtd_qemu.aug +++ b/qemud/test_libvirtd_qemu.aug @@ -80,6 +80,8 @@ vnc_sasl = 1 # vnc_sasl_dir = \"/some/directory/sasl2\" +security_driver = \"selinux\" + user = \"root\" group = \"root\" @@ -87,6 +89,8 @@ group = \"root\" cgroup_controllers = [ \"cpu\", \"devices\" ] cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ] + +save_image_format = \"gzip\" " test Libvirtd_qemu.lns get conf = @@ -170,7 +174,9 @@ cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ] { "#comment" = "" } { "vnc_sasl_dir" = "/some/directory/sasl2" } { "#empty" } -{ "user"= "root" } +{ "security_driver" = "selinux" } +{ "#empty" } +{ "user" = "root" } { "#empty" } { "group" = "root" } { "#empty" } @@ -184,3 +190,5 @@ cgroup_device_acl = [ \"/dev/null\", \"/dev/full\", \"/dev/zero\" ] { "2" = "/dev/full" } { "3" = "/dev/zero" } } +{ "#empty" } +{ "save_image_format" = "gzip" } diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 86a33f8..ffe9c16 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -666,6 +666,7 @@ qemudShutdown(void) VIR_FREE(qemu_driver->vncListen); VIR_FREE(qemu_driver->vncPassword); VIR_FREE(qemu_driver->vncSASLdir); + VIR_FREE(qemu_driver->saveImageFormat); /* Free domain callback list */ virDomainEventCallbackListFree(qemu_driver->domainEventCallbacks); -- 1.6.0.6

2 2

[libvirt] [PATCH 0/1] Multipath pool support
by David Allan 20 Aug '09

20 Aug '09

The following patch implements multipath pool support. It's very basic functionality, consisting of creating a pool that contains all the multipath devices on the host. That will cover the common case of users who just want to discover all the available multipath devices and assign them to guests. It doesn't currently allow configuration of multipathing, so for now setting the multipath configuration will have to continue to be done as part of the host system build. Example XML to create the pool is: <pool type="mpath"> <name>mpath</name> <target> <path>/dev/mapper</path> </target> </pool> The target element is ignored, as it is by the disk pool, but the config code rejects the XML if it does not exist. That behavior should obviously be cleaned up, but I think that should be done in a separate patch, as it's really a bug in the config code, not related to the addition of the new pool type. Dave

4 8

[libvirt] FYI: Updated QEMU driver docs on security model
by Daniel P. Berrange 20 Aug '09

20 Aug '09

FYI, I just pushed the following patch to the repo which adds documentation to the website for all the security model related aspects of libvirt's QEMU driver. It should appear here shortly http://libvirt.org/drvqemu.html Regards, Daniel diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index e4c3d03..348eaaf 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -54,6 +54,292 @@ qemu+ssh://root@example.com/system (remote access, SSH tunnelled) </pre> + <h2><a name="security">Driver security architecture</a></h2> + + <p> + There are multiple layers to security in the QEMU driver, allowing for + flexibility in the use of QEMU based virtual machines. + </p> + + <h3><a name="securitydriver">Driver instances</a></h3> + + <p> + As explained above there are two ways to access the QEMU driver + in libvirt. The "qemu:///session" family of URIs connect to a + libvirtd instance running as the same user/group ID as the client + application. Thus the QEMU instances spawned from this driver will + share the same privileges as the client application. The intended + use case for this driver is desktop virtualization, with virtual + machines storing their disk imags in the user's home directory and + being managed from the local desktop login session. + </p> + + <p> + The "qemu:///system" family of URIs connect to a + libvirtd instance running as the privileged system account 'root'. + Thus the QEMU instances spawned from this driver may have much + higher privileges than the client application managing them. + The intended use case for this driver is server virtualization, + where the virtual machines may need to be connected to host + resources (block, PCI, USB, network devices) whose access requires + elevated privileges. + </p> + + <h3><a name="securitydac">POSIX DAC users/groups</a></h3> + + <p> + In the "session" instance, the POSIX DAC model restricts QEMU virtual + machines (and libvirtd in general) to only have access to resources + with the same user/group ID as the client application. There is no + finer level of configuration possible for the "session" instances. + </p> + + <p> + In the "system" instance, libvirt releases from 0.7.0 onwards allow + control over the user/group that the QEMU virtual machines are run + as. A build of libvirt with no configuration parameters set will + still run QEMU processes as root:root. It is possible to change + this default by using the --with-qemu-user=$USERNAME and + --with-qemu-group=$GROUPNAME arguments to 'configure' during + build. It is strongly recommended that vendors build with both + of these arguments set to 'qemu'. Regardless of this build time + default, administrators can set a per-host default setting in + the <code>/etc/libvirt/qemu.conf</code> configuration file via + the <code>user=$USERNAME</code> and <code>group=$GROUPNAME</code> + parameters. When a non-root user or group is configured, the + libvirt QEMU driver will change uid/gid to match immediately + before executing the QEMU binary for a virtual machine. + </p> + + <p> + If QEMU virtual machines from the "system" instance are being + run as non-root, there will be greater restrictions on what + host resources the QEMU process will be able to access. The + libvirtd daemon will attempt to manage permissions on resources + to minise the likelihood of unintentionale security denials, + but the administrator / application developer must be aware of + some of the consequences / restrictions. + </p> + + <ul> + <li> + <p> + The directories <code>/var/run/libvirt/qemu/</code>, + <code>/var/lib/libvirt/qemu/</code> and + <code>/var/cache/libvirt/qemu/</code> must all have their + ownership set to match the user / group ID that QEMU + guests will be run as. If the vendor has set a non-root + user/group for the QEMU driver at build time, the + permissions should be set automatically at install time. + If a host administrator customizes user/group in + <code>/etc/libvirt/qemu.conf</code>, they will need to + manually set the ownership on these directories. + </p> + </li> + <li> + <p> + When attaching PCI and USB devices to a QEMU guest, + QEMU will need to access files in <code>/dev/bus/usb</code> + and <code>/sys/bus/devices</code>. The libvirtd daemon + will automatically set the ownership on specific devices + that are assigned to a guest at start time. There should + not be any need for administrator changes in this respect. + </p> + </li> + <li> + <p> + Any files/devices used as guest disk images must be + accessible to the user/group ID that QEMU guests are + configured to run as. The libvirtd daemon will automatically + set the ownership of the file/device path to the correct + user/group ID. Applications / administrators must be aware + though that the parent directory permissions may still + deny access. The directories containing disk images + must either have their ownership set to match the user/group + configured for QEMU, or their UNIX file permissions must + have the 'execute/search' bit enabled for 'others'. + </p> + <p> + The simplest option is the latter one, of just enabling + the 'execute/search' bit. For any directory to be used + for storing disk images, this can be achived by running + the following command on the directory itself, and any + parent directories + </p> +<pre> + chmod o+x /path/to/directory +</pre> + <p> + In particular note that if using the "system" instance + and attempting to store disk images in a user home + directory, the default permissions on $HOME are typically + too restrictive to allow access. + </p> + </li> + </ul> + + <h3><a name="securitycap">Linux DAC capabilities</a></h3> + + <p> + The libvirt QEMU driver has a build time option allowing it to use + the <a href="http://people.redhat.com/sgrubb/libcap-ng/index.html">libcap-ng</a> + library to manage process capabilities. If this build option is + enabled, then the QEMU driver will use this to ensure that all + process capabilities are dropped before executing a QEMU virtual + machine. Process capabilities are what gives the 'root' account + its high power, in particular the CAP_DAC_OVERRIDE capability + is what allows a process running as 'root' to access files owned + by any user. + </p> + + <p> + If the QEMU driver is configured to run virtual machines as non-root, + then they will already loose all their process capabilities at time + of startup. The Linux capability feature is thus aimed primarily at + the scenario where the QEMU processes are running as root. In this + case, before launching a QEMU virtual machine, libvirtd will use + libcap-ng APIs to drop all process capabilities. It is important + for administrators to note that this implies the QEMU process will + <strong>only</strong> be able to access files owned by root, and + not files owned by any other user. + </p> + + <p> + Thus, if a vendor / distributor has configured their libvirt package + to run as 'qemu' by default, a number of changes will be required + before an administrator can change a host to run guests as root. + In particular it will be neccessary to change ownership on the + directories <code>/var/run/libvirt/qemu/</code>, + <code>/var/lib/libvirt/qemu/</code> and + <code>/var/cache/libvirt/qemu/</code> back to root, in addition + to changing the <code>/etc/libvirt/qemu.conf</code> settings. + </p> + + <h3><a name="securityselinux">SELinux MAC basic confinement</a></h3> + + <p> + The basic SELinux protection for QEMU virtual machines is intended to + protect the host OS from a compromised virtual machine process. There + is no protection between guests. + </p> + + <p> + In the basic model, all QEMU virtual machines run under the confined + domain <code>root:system_r:qemu_t</code>. It is required that any + disk image assigned to a QEMU virtual machine is labelled with + <code>system_u:object_r:virt_image_t</code>. In a default deployment, + package vendors/distributor will typically ensure that the directory + <code>/var/lib/libvirt/images</code> has this label, such that any + disk images created in this directory will automatically inherit the + correct labelling. If attempting to use disk images in another + location, the user/administrator must ensure the directory has be + given this requisite label. Likewise physical block devices must + be labelled <code>system_u:object_r:virt_image_t</code>. + </p> + <p> + Not all filesystems allow for labelling of individual files. In + particular NFS, VFat and NTFS have no support for labelling. In + these cases administrators must use the 'context' option when + mounting the filesystem to set the default label to + <code>system_u:object_r:virt_image_t</code>. In the case of + NFS, there is an alternative option, of enabling the <code>virt_use_nfs</code> + SELinux boolean. + </p> + + <h3><a name="securitysvirt">SELinux MAC sVirt confinement</a></h3> + + <p> + The SELinux sVirt protection for QEMU virtual machines builds to the + basic level of protection, to also allow individual guests to be + protected from each other. + </p> + + <p> + In the sVirt model, each QEMU virtual machine runs under its own + confined domain, which is based on <code>system_u:system_r:svirt_t:s0</code> + with a unique category appended, eg, <code>system_u:system_r:svirt_t:s0:c34,c44</code>. + The rules are setup such that a domain can only access files which are + labelled with the matching category level, eg + <code>system_u:object_r:svirt_image_t:s0:c34,c44</code>. This prevents one + QEMU process accessing any file resources that are prevent to another QEMU + process. + </p> + + <p> + There are two ways of assigning labels to virtual machines under sVirt. + In the default setup, if sVirt is enabled, guests will get an automatically + assigned unique label each time they are booted. The libvirtd daemon will + also automatically relabel exclusive access disk images to match this + label. Disks that are marked as <shared> will get a generic + label <code>system_u:system_r:svirt_image_t:s0</code> allowing all guests + read/write access them, while disks marked as <readonly> will + get a generic label <code>system_u:system_r:svirt_content_t:s0</code> + which allows all guests read-only access. + </p> + + <p> + With statically assigned labels, the application should include the + desired guest and file labels in the XML at time of creating the + guest with libvirt. In this scenario the application is responsible + for ensuring the disk images & similar resources are suitably + labelled to match, libvirtd will not attempt any relabelling. + </p> + + <p> + If the sVirt security model is active, then the node capabilties + XML will include its details. If a virtual machine is currently + protected by the security model, then the guest XML will include + its assigned labels. If enabled at compile time, the sVirt security + model will always be activated if SELinux is available on the host + OS. To disable sVirt, and revert to the basic level of SELinux + protection (host protection only), the <code>/etc/libvirt/qemu.conf</code> + file can be used to change the setting to <code>security_driver="none"</code> + </p> + + + <h3><a name="securityacl">Cgroups device ACLs</a></h3> + + <p> + Recent Linux kernels have a capability known as "cgroups" which is used + for resource management. It is implemented via a number of "controllers", + each controller covering a specific task/functional area. One of the + available controllers is the "devices" controller, which is able to + setup whitelists of block/character devices that a cgroup should be + allowed to access. If the "devices" controller is mounted on a host, + then libvirt will automatically create a dedicated cgroup for each + QEMU virtual machine and setup the device whitelist so that the QEMU + process can only access shared devices, and explicitly disks images + backed by block devices. + </p> + + <p> + The list of shared devices a guest is allowed access to is + </p> + + <pre> + /dev/null, /dev/full, /dev/zero, + /dev/random, /dev/urandom, + /dev/ptmx, /dev/kvm, /dev/kqemu, + /dev/rtc, /dev/hpet, /dev/net/tun + </pre> + + <p> + In the event of unanticipated needs arising, this can be customized + via the <code>/etc/libvirt/qemu.conf</code> file. + To mount the cgroups device controller, the following command + should be run as root, prior to starting libvirtd + </p> + + <pre> + mkdir /dev/cgroup + mount -t cgroup none /dev/cgroup -o devices + </pre> + + <p> + libvirt will then place each virtual machine in a cgroup at + <code>/dev/cgroup/libvirt/qemu/$VMNAME/</code> + </p> + <h2><a name="imex">Import and export of libvirt domain XML configs</a></h2> <p>The QEMU driver currently supports a single native -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

3 3

[libvirt] Add support for (qcow*) volume encryption (v3)
by Miloslav Trmač 20 Aug '09

20 Aug '09

Hello, the following patches add full support for qcow/qcow2 volume encryption, assuming a client that supports it. (Main changes since the previous version: * Significantly change the internal API, basing it on virSecretPtr instead of (virConnectPtr, secret_id); details in patch 1 * Make virsh commands more similar to the existing commands * Remove <encryption format='unencrypted'/> * Rename "secret_id" to "uuid" in the XML and API * Use "unsigned char *" for secret value See the specific patch change logs for more details; patches without change logs are unchanged.) New XML tags are defined to represent encryption parameters (currently format and passphrase, more can be added in the future), e.g. <encryption format='qcow'> <secret type='passphrase' uuid='724d95f2-0ed2-6ff9-84d0-0f3d1618428d'/> </encryption> The <encryption> tag can be added to a <volume> node passed to virStorageVolCreateXML() to create an encrypted volume, or to a <disk> node inside a <domain> to specify what encryption parameters to use for a domain. uuid above refers to a separately-managed secret, which was created using virSecretDefineXML() and set using virSecretSetValue(). Other properties of the secret can be managed using an XML representation. Detailed documentation of the formats and features is inside the patches.

4 33

Re: [libvirt] [PATCH 07/20] Secret manipulation step 7: Local driver
by Miloslav Trmac 20 Aug '09

20 Aug '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Sun, Aug 16, 2009 at 10:48:00PM +0200, Miloslav Trmač wrote: > > This implementation stores the secrets in an unencrypted text file, > > for simplicity in implementation and debugging. > > > > (Symmetric encryption, e.g. using gpgme, will not be difficult to add. > > Because the TLS private key used by libvirtd is stored unencrypted, > > encrypting the secrets file does not currently provide much additional > > security.) > > What if we change our mind in some time, would there be any obstacle > to dynamically detecting things are not encrypted and switching to > a crypted file transparently? Two simple ways: 1) use a different file name, e,g. secrets.gpg 2) check the start of the file, currently the file always starts with "id "; I can add a proper magic number if necessary. > > +static int > > +writeBase64Data(virConnectPtr conn, int fd, const char *field, > > + const void *data, size_t size) > > +{ > > + int ret = -1; > > + char *base64 = NULL; > > + > > + if (writeString(conn, fd, field) < 0 || writeString(conn, fd, " > ") < 0) > > + goto cleanup; > > + > > + base64_encode_alloc(data, size, &base64); > > where is base64_encode_alloc coming from ? gnulib - in a later patch I'm afraid, I'll reorder it. > > +static int > > +saveSecrets(virConnectPtr conn, virSecretDriverStatePtr driver) > > +{ > > + const virSecretEntry *secret; > > + char *tmp_path = NULL; > > + int fd = -1, ret = -1; > > + > > + if (virAsprintf(&tmp_path, "%sXXXXXX", driver->filename) < 0) > { > > + virReportOOMError(conn); > > + goto cleanup; > > + } > > + fd = mkstemp (tmp_path); > > Hum even if unencrypted, we should make sure of the mode of the file > beforehand... Good point. > isn't there a safer equivalent (possibly made postable by gnulib ?) Grepping of gnulib does not reveal any. > > + if (fd == -1) { > > + virReportSystemError (conn, errno, _("mkstemp(\"%s\") failed"), > > + tmp_path); > > + goto cleanup; > > + } > > + > > + for (secret = driver->secrets; secret != NULL; > > + secret = secret->next) { > > + if (!secret->ephemeral && writeSecret(conn, fd, secret) < 0) > > + goto cleanup; > > + } > > + close(fd); > > + fd = -1; > > + if (rename(tmp_path, driver->filename) < 0) { > > + virReportSystemError (conn, errno, _("rename(%s, %s) failed"), tmp_path, > > + driver->filename); > > + goto cleanup; > > + } > > Hum, the whole set smells fishy, we are creating a temp file, without > mode checked, then moving that file somewhere else. I would ratehr have > internal APIsdeling with a dump to memory and then a single open, safe > and directly to the driver->filename. "Somewhere else" is in the same directory. The mkstemp() + rename() is used to make sure the secrets file is replaced atomically, without losing any data if a save is interrupted in the middle. > > +static int > > +parseBase64String(virConnectPtr conn, const char *base64, char > **string) > > +{ > > + char *tmp; > > + size_t size; > > + > > + if (!base64_decode_alloc(base64, strlen(base64), &tmp, &size)) > { > > where is base64_decode_alloc coming from ? gnulib > > + if (stat(driver->filename, &st) < 0) { > > + if (errno == ENOENT) > > + return 0; > > + virReportSystemError (conn, errno, _("cannot stat '%s'"), > > + driver->filename); > > + goto cleanup; > > + } > > + if ((size_t)st.st_size != st.st_size || (size_t)(st.st_size + 1) == 0) { > > + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", > > + _("secrets file does not fit in memory")); > > + goto cleanup; > > + } > > + > > + fd = open(driver->filename, O_RDONLY); > > + if (fd == -1) { > > + virReportSystemError (conn, errno, _("cannot open '%s'"), > > + driver->filename); > > + goto cleanup; > > + } > > stat()/open() introduces a small race, I think open() and > then fdstat() is a bit cleaner, not a big deal though Technically there is still a race with fstat() - but fstat is a bit better. > > + if (VIR_ALLOC_N(contents, st.st_size + 1) < 0) { > > + virReportOOMError(conn); > > + goto cleanup; > > + } > > + if (saferead(fd, contents, st.st_size) != st.st_size) { > > + virReportSystemError (conn, errno, _("cannot read '%s'"), > > + driver->filename); > > + goto cleanup; > > + } > > + close(fd); > > + fd = -1; > > contents[st.st_size] = 0; > needed here. VIR_ALLOC_N automatically zeroes the memory. I'll fix all other issues you have mentioned. Thanks for the review. Mirek

1 0

Re: [libvirt] [PATCH 05/20] Secret manipulation step 5: RPC client
by Miloslav Trmac 19 Aug '09

19 Aug '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Sun, Aug 16, 2009 at 10:47:58PM +0200, Miloslav Trmač wrote: > > +static virDrvOpenStatus > > +remoteSecretOpen (virConnectPtr conn, > > + virConnectAuthPtr auth, > > + int flags) > > +{ > > + if (inside_daemon) > > + return VIR_DRV_OPEN_DECLINED; > > + > > + if (conn && > > + conn->driver && > > + STREQ (conn->driver->name, "remote")) { > > + struct private_data *priv; > > + > > + /* If we're here, the remote driver is already > > + * in use due to a) a QEMU uri, or b) a remote > > + * URI. So we can re-use existing connection > > + */ > > Hum, is that comment right ? We are certainly using a remote URI in > any case there, isn't it ? The same comment is in in all of the other "subdriver" remote...Open () routines. It's not quite clear what "remote" means where, as far as I can see "remote" means an "explicitly remote" URL, e.g. anything+tls://hostname/..., and "QEMU" means "qemu://hostname/..." (rejected by qemu_driver). Mirek

2 1

Re: [libvirt] [PATCH 01/20] Secret manipulation step 1: Public API
by Miloslav Trmac 19 Aug '09

19 Aug '09

----- "Daniel Veillard" <veillard(a)redhat.com> wrote: > On Sun, Aug 16, 2009 at 10:47:54PM +0200, Miloslav Trmač wrote: > > Rather than add explicit accessors for attributes of secrets, and > > hard-code the "secrets are related to storage volumes" association > in > > the API, the API uses XML to manipulate the association as well as > > other attributes, similarly to other areas of libvirt. > > > > The user can set attributes of the secret using XML, e.g. > > <secret ephemeral='no' private='yes'> > > <uuid>b8eecf55-798e-4db7-b2dd-025b0cf08a36</uuid> > > <volume>/var/lib/libvirt/images/mail.img</volume> > > <description>LUKS passphrase for our mail server</description> > > </secret> > > If <uuid/> is not specified, it is chosen automatically. > > Should secret always be tied to volumes. The API is generic enough > that we should make sure we can use this later to get priviledged > access to other resources, though right now I don't have a good > example in mind. The secret is not "tied" to a volume - the volume is an optional attribute. A libvirt user can ignore the volume field completely, and manage the secrets separately - but other clients, e.g. virt-manager, can use the volume attribute to locate a secret for a given volume without storing any data outside of libvirt. > > - s/secret_id/uuid/g > > Not sure I catch that, An earlier patch was calling the ID of a secret object "secret_id" everywhere, leading to constructs like <secret secret_id=.../> In an earlier patch review I was asked to change that. > > - use "unsigned char *" for secret value > > fine, assuming 0 terninated. though this may not work with everything > but since we need to import/export to XML, unless uuencoding the value > we won't be able to embed something which is not part of XML character > range (#x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | > [#x10000-#x10FFFF]) unicode code points. No, the value is an arbitrary set of bytes that is never passed through XML. Originally it was (void *value, size_t value_size), the change to (unsigned char *value, size_t value_size) was requested in an earlier review. > > diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in > > new file mode 100644 > > index 0000000..7471bf7 > > --- /dev/null > > +++ b/docs/formatsecret.html.in > > @@ -0,0 +1,52 @@ > > +<html> > > + <body> > > + <h1>Secret XML format</h1> > > + > > + <ul id="toc"></ul> > > it's good to have a document from the start :-) but this lacks a bit > of the intended use for the API, i.e. where this may be required to > use it. Those parts are documented in later patches that add references to secrets to the <volume> and <domain> XML formats. > > +virSecretPtr virSecretDefineXML (virConnectPtr conn, > > + const char *xml); > > Let's add an "unsigned int flags" to virSecretDefineXML() especially > as we don't know yet the scope of future usages. > > > +char * virSecretGetXMLDesc (virSecretPtr secret); > > And also add an "unsigned int flags" to virSecretGetXMLDesc > Will do. > > +int virSecretSetValue (virSecretPtr secret, > > + const unsigned char *value, > > + size_t value_size); > > +unsigned char * virSecretGetValue (virSecretPtr secret, > > + size_t *value_size); > > Ah, so we aren't relying on 0 termination, but in that case the value > need to be uuencoded in the XML, and that should be made clear in the > API description. Actually not having the secret completely in the clear > in the XML dump sounds like a good idea. The values are never transferred in the XML, only using the virSecret[GS]etValue functions. (Storing secrets in XML was a major objection against the first version of my patch e.g. because XML tends to be revealed in various debugging dumps.) Mirek

3 3

[libvirt] OpenVZ : The restriction of domain name should be addressed
by Yuji NISHIDA 19 Aug '09

19 Aug '09

Hi, "The current libvirt OpenVZ driver has a restriction that the domain names must match the OpenVZ container VEID" I really want to have domain name as character, not integer. It could help me very much if any patch against this problem would be released. Is there anyone tackling this problem now?( or going to? ) Otherwise, any hint would help me. Regards. ----- Yuji Nishida nishidy(a)nict.go.jp

5 5

[libvirt] Integrating MAC address based filtering into libvirt
by Gerhard Stenzel 19 Aug '09

19 Aug '09

Hello, I am currently investigating the possibility to implement MAC address based filtering in libvirt and was wondering if there is any related effort going on and what people in general would think about that. Here is a description of my current prototype implementation: I have a small setup of two guests and a network: Guest1: root@stenzel-desktop:/etc/libvirt/qemu# cat build1.xml <domain type='kvm'> <name>build1</name> ... <interface type='network'> <mac address='D0:0F:D0:0F:02:01'/> <source network='mynet'/> <model type='virtio' /> </interface> ... </domain> Guest2: root@stenzel-desktop:/etc/libvirt/qemu# cat build2.xml <domain type='kvm'> <name>build2</name> ... <interface type='network'> <mac address='D0:0F:D0:0F:02:02'/> <source network='mynet'/> <model type='virtio' /> </interface> ... </domain> and the network to which I added a new XML element "filter" with attribute "mac", which switches on the MAC address filtering: root@stenzel-desktop:/etc/libvirt/qemu# cat networks/mynet.xml <network> <name>mynet</name> <uuid>920debe0-c3ef-4395-8241-ee82d4b49c2d</uuid> <bridge name="br%d" stp="off"/> <filter mac="on"/> </network> the "filter" element is evaluated at startup of libvirtd and a generic ebtables rules is generated (all frames are dropped): stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L Bridge table: filter ... Bridge chain: FORWARD, entries: 0, policy: DROP ... When starting up guest1, ... stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh start build1 Domain build1 started ... an ebtables rule is generated to allow its mac address on the its interface: stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 1, policy: DROP -s d0:f:d0:f:2:1 -i vnet0 -j ACCEPT ... the same happens when starting up the second guest: stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh start build2 Domain build2 started stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 2, policy: DROP -s d0:f:d0:f:2:2 -i vnet1 -j ACCEPT -s d0:f:d0:f:2:1 -i vnet0 -j ACCEPT ... so the two guests are allowed to communicate. After destroying the two guests, the corresponding ebtables rules are removed: stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh destroy build2 Domain build2 destroyed stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 1, policy: DROP -s d0:f:d0:f:2:1 -i vnet0 -j ACCEPT ... stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo virsh destroy build1 Domain build1 destroyed stenzel@stenzel-desktop:~/projects/vsn/libvirt$ sudo ebtables -L ... Bridge chain: FORWARD, entries: 0, policy: DROP ... The current prototype implementation is based on the existing iptables wrapper in libvirt. I basically cloned the iptables wrapper to an ebtables wrapper and did some ebtables specific adjustments. There are currenlty four occasions when the ebtables wrapper is called: - when creating the network - when adding a guest to the network - when removing a guest from the network - when destroying the network (currently not implemented) These calls can be augmented to also do for example tagged vlan and protocol filtering. Configuring the filter rules via virsh is also an option. Comments are appreciated. -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Erich Baier Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

2 2

[libvirt] PATCH: Support setting a disk drive serial (number)
by Daniel P. Berrange 19 Aug '09

19 Aug '09

All hard disks (IDE, SCSI, USB, VirtIO) have a concept of a serial number which is intended to uniquely identify them. If you mean this off, QEMU just makes up a serial on the fly. THis is not guarenteed to be stable across guest reboots with hardware changes. It is thus desirable to be able to specify this in libvirt XML. To that end, this patch allows for a <serial> element inside a disk <disk type='file' device='disk'> <source file='/media/lacie-500-disk-2/virtual-machines/scratch5.img'/> <target dev='vda' bus='virtio'/> <serial>dan123virtio</serial> </disk> the contents is free-form. It implemenmts this for the QEMU driver for any disk using -drive style args. Unfortunately this excludes USB disk, even though internally QEMU can set a serial for these, the -usbdevice syntax does not allow it Daniel * docs/schemas/domain.rng: Add <serial> element to disks * src/domain_conf.h, src/domain_conf.c: XML parsing and formatting for disk serial numbers * src/qemu_conf.c: Set serial number when launching guests * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args, tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml: Add serial number to XML test --- docs/schemas/domain.rng | 10 ++++++++++ src/domain_conf.c | 11 +++++++++++ src/domain_conf.h | 1 + src/qemu_conf.c | 2 ++ .../qemuxml2argv-disk-drive-shared.args | 2 +- .../qemuxml2argv-disk-drive-shared.xml | 1 + 6 files changed, 26 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index f857301..16f35e4 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -336,6 +336,11 @@ <empty/> </element> </optional> + <optional> + <element name="serial"> + <ref name="diskSerial"/> + </element> + </optional> </define> <!-- A disk description can be either of type file or block @@ -1135,6 +1140,11 @@ <param name="pattern">[A-Za-z0-9_\.\+\-&:/]+</param> </data> </define> + <define name="diskSerial"> + <data type="string"> + <param name="pattern">[A-Za-z0-9_\.\+\-]+</param> + </data> + </define> <define name="genericName"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\+\-]+</param> diff --git a/src/domain_conf.c b/src/domain_conf.c index 1d2cc7c..654f753 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -284,6 +284,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) if (!def) return; + VIR_FREE(def->serial); VIR_FREE(def->src); VIR_FREE(def->dst); VIR_FREE(def->driverName); @@ -661,6 +662,7 @@ virDomainDiskDefParseXML(virConnectPtr conn, char *bus = NULL; char *cachetag = NULL; char *devaddr = NULL; + char *serial = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(conn); @@ -718,6 +720,9 @@ virDomainDiskDefParseXML(virConnectPtr conn, } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && xmlStrEqual(cur->name, BAD_CAST "state")) { devaddr = virXMLPropString(cur, "devaddr"); + } else if ((serial == NULL) && + (xmlStrEqual(cur->name, BAD_CAST "serial"))) { + serial = (char *)xmlNodeGetContent(cur); } } cur = cur->next; @@ -836,6 +841,8 @@ virDomainDiskDefParseXML(virConnectPtr conn, driverName = NULL; def->driverType = driverType; driverType = NULL; + def->serial = serial; + serial = NULL; cleanup: VIR_FREE(bus); @@ -847,6 +854,7 @@ cleanup: VIR_FREE(driverName); VIR_FREE(cachetag); VIR_FREE(devaddr); + VIR_FREE(serial); return def; @@ -3519,6 +3527,9 @@ virDomainDiskDefFormat(virConnectPtr conn, virBufferAddLit(buf, " <readonly/>\n"); if (def->shared) virBufferAddLit(buf, " <shareable/>\n"); + if (def->serial) + virBufferEscapeString(buf, " <serial>%s</serial>\n", + def->serial); if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) { virBufferAddLit(buf, " <state"); diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..7bad5e7 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -109,6 +109,7 @@ struct _virDomainDiskDef { char *dst; char *driverName; char *driverType; + char *serial; int cachemode; unsigned int readonly : 1; unsigned int shared : 1; diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 082f107..95d41e6 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1748,6 +1748,8 @@ int qemudBuildCommandLine(virConnectPtr conn, if (disk->driverType && qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) virBufferVSprintf(&opt, ",format=%s", disk->driverType); + if (disk->serial) + virBufferVSprintf(&opt, ",serial=%s", disk->serial); if (disk->cachemode) { const char *mode = diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args index 611cd33..07cbe0e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,format=qcow2,cache=off -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2,format=raw -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,format=qcow2,serial=XYZXYZXYZYXXYZYZYXYZY,cache=off -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml index b386315..c4e4734 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml @@ -19,6 +19,7 @@ <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> + <serial>XYZXYZXYZYXXYZYZYXYZY</serial> </disk> <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> -- 1.6.2.5 -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

4 7

[libvirt] [PATCH] Don't expose 'vnet%d' to the user
by Mark McLoughlin 19 Aug '09

19 Aug '09

https://bugzilla.redhat.com/517371 Matt Booth points out that if you use a non-existent bridge name when start a guest you get a weird error message: Failed to add tap interface 'vnet%d' to bridge 'virbr0' and dev='vnet%d' appears in the dumpxml output. Fix that by not including 'vnet%d' in the error message and freeing the 'vnet%d' string if adding the tap device to the bridge fails. * src/qemu_conf.c, src/uml_conf.c: fix qemudNetworkIfaceConnect() and umlConnectTapDevice() to not expose 'vnet%d' to the user --- src/qemu_conf.c | 25 +++++++++++++++++-------- src/uml_conf.c | 28 +++++++++++++++++++--------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 082f107..bcdab11 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1038,6 +1038,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn, int err; int tapfd = -1; int vnet_hdr = 0; + int template_ifname = 0; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { virNetworkPtr network = virNetworkLookupByName(conn, @@ -1059,6 +1060,14 @@ qemudNetworkIfaceConnect(virConnectPtr conn, return -1; } + char ebuf[1024]; + if (!driver->brctl && (err = brInit(&driver->brctl))) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot initialize bridge support: %s"), + virStrerror(err, ebuf, sizeof ebuf)); + return -1; + } + if (!net->ifname || STRPREFIX(net->ifname, "vnet") || strchr(net->ifname, '%')) { @@ -1067,14 +1076,8 @@ qemudNetworkIfaceConnect(virConnectPtr conn, virReportOOMError(conn); return -1; } - } - - char ebuf[1024]; - if (!driver->brctl && (err = brInit(&driver->brctl))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot initialize bridge support: %s"), - virStrerror(err, ebuf, sizeof ebuf)); - return -1; + /* avoid exposing vnet%d in dumpxml or error outputs */ + template_ifname = 1; } if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR && @@ -1088,12 +1091,18 @@ qemudNetworkIfaceConnect(virConnectPtr conn, qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to add tap interface to bridge. " "%s is not a bridge device"), brname); + } else if (template_ifname) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to add tap interface to bridge '%s' : %s"), + brname, virStrerror(err, ebuf, sizeof ebuf)); } else { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to add tap interface '%s' " "to bridge '%s' : %s"), net->ifname, brname, virStrerror(err, ebuf, sizeof ebuf)); } + if (template_ifname) + VIR_FREE(net->ifname); return -1; } diff --git a/src/uml_conf.c b/src/uml_conf.c index 4f756d4..a4c434f 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -104,17 +104,10 @@ umlConnectTapDevice(virConnectPtr conn, virDomainNetDefPtr net, const char *bridge) { + brControl *brctl = NULL; int tapfd = -1; + int template_ifname = 0; int err; - brControl *brctl = NULL; - - if (!net->ifname || - STRPREFIX(net->ifname, "vnet") || - strchr(net->ifname, '%')) { - VIR_FREE(net->ifname); - if (!(net->ifname = strdup("vnet%d"))) - goto no_memory; - } if ((err = brInit(&brctl))) { char ebuf[1024]; @@ -124,6 +117,16 @@ umlConnectTapDevice(virConnectPtr conn, goto error; } + if (!net->ifname || + STRPREFIX(net->ifname, "vnet") || + strchr(net->ifname, '%')) { + VIR_FREE(net->ifname); + if (!(net->ifname = strdup("vnet%d"))) + goto no_memory; + /* avoid exposing vnet%d in dumpxml or error outputs */ + template_ifname = 1; + } + if ((err = brAddTap(brctl, bridge, &net->ifname, BR_TAP_PERSIST, &tapfd))) { if (errno == ENOTSUP) { @@ -131,6 +134,11 @@ umlConnectTapDevice(virConnectPtr conn, umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to add tap interface to bridge. " "%s is not a bridge device"), bridge); + } else if (template_ifname) { + char ebuf[1024]; + umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to add tap interface to bridge '%s' : %s"), + bridge, virStrerror(err, ebuf, sizeof ebuf)); } else { char ebuf[1024]; umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -138,6 +146,8 @@ umlConnectTapDevice(virConnectPtr conn, "to bridge '%s' : %s"), net->ifname, bridge, virStrerror(err, ebuf, sizeof ebuf)); } + if (template_ifname) + VIR_FREE(net->ifname); goto error; } close(tapfd); -- 1.6.2.5

3 5

[libvirt] [PATCH 0/8] Various KVM PCI device assignment improvements
by Mark McLoughlin 18 Aug '09

18 Aug '09

Hi, Here's a fairly mixed set of KVM PCI device assignment improvements: - Add hotplug support - Allow PM reset on multi-function devices - Allow Secondary Bus Reset even if it causes other devices to be reset, so long as those other devices are in use by the same VM or are unused - Re-attach devices when the guest shuts down - Properly detect versions of QEMU without -pcidevice support Cheers, Mark.

4 26

[libvirt] [PATCH] Fix up connection reference counting.
by Chris Lalancette 18 Aug '09

18 Aug '09

Currently the reference counting for connections is busted. I first noticed it while trying to use virConnectRef; it would eventually cause a crash in the remote_internal driver, although that was really just a victim. Really, we should only call the close callbacks on the methods when the references drop to 0. To accomplish this, move all of the close callbacks into virUnrefConnect (since there are lots of internal users of that function), and arrange for virConnectClose to call that. V2: Make sure to drop the connection lock before we call the close callbacks, otherwise we could deadlock the daemon Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/datatypes.c | 19 +++++++++++++++++++ src/libvirt.c | 14 +------------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 9d556c8..bcb5fd4 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -237,6 +237,25 @@ virUnrefConnect(virConnectPtr conn) conn->refs--; refs = conn->refs; if (refs == 0) { + /* make sure to release the connection lock before we call the + * close() callbacks, otherwise we will deadlock if an error + * is raised by any of the callbacks + */ + virMutexUnlock(&conn->lock); + if (conn->networkDriver) + conn->networkDriver->close (conn); + if (conn->interfaceDriver) + conn->interfaceDriver->close (conn); + if (conn->storageDriver) + conn->storageDriver->close (conn); + if (conn->deviceMonitor) + conn->deviceMonitor->close (conn); + conn->driver->close (conn); + + /* reacquire the connection lock since virReleaseConnect expects + * it to already be held + */ + virMutexLock(&conn->lock); virReleaseConnect(conn); /* Already unlocked mutex */ return (0); diff --git a/src/libvirt.c b/src/libvirt.c index 5aa7f83..472c19b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1231,19 +1231,7 @@ virConnectClose(virConnectPtr conn) return (-1); } - if (conn->networkDriver) - conn->networkDriver->close (conn); - if (conn->interfaceDriver) - conn->interfaceDriver->close (conn); - if (conn->storageDriver) - conn->storageDriver->close (conn); - if (conn->deviceMonitor) - conn->deviceMonitor->close (conn); - conn->driver->close (conn); - - if (virUnrefConnect(conn) < 0) - return (-1); - return (0); + return virUnrefConnect(conn); } /** -- 1.6.0.6

2 2

[libvirt] Share storage using iscsi [EDIT as I cut off part]
by Łukasz Mierzwa 18 Aug '09

18 Aug '09

Hi, I'm trying to setup pool of machines (nodes) for virtual machines hosting and I got few question about shared storage. My main requirements are: 1. central management - I've got simple python app that stores information about all virtual machines and all nodes, this app needs to be able to manage volumes using libvirt API, so I need libvirt volume pools 2. live migration - I got shared storage with HA, I want to use it also for live migration in case one of nodes is dying or if I want to do some load balancing Right now I'm thinking about 2 machines with disks synchronized using drdb, both acting as a identical iscsi targets, iscsi HA will be provided by heartbeat. So I will end up with virtual IP pointing to working iscsi target, drdb should keep storage is sync. But: 1. I can't just use single iscsi LUN and export it as libvirt storage pool to each node, because no pool type would work that way, right? 2. http://libvirt.org/storage.html section "iSCSI volume pools" says: "Volumes must be pre-allocated on the iSCSI server, and cannot be created via the libvirt APIs." So even if I got one LUN per node and set it as iscsi volume pool I would need to create each volume on iscsi target. Libvirt can't manage volumes in such pool, it can only assign already created volumes to virtual machines, right? 3. So maybe my storage could be setup as LVM volume group and this lvm group would be managed as libvirt lvm volume pool on master (from heartbeat POV) iscsi target. I would create one logical volume per virtual machine, export this volume as a separate iscsi LUN, and use this LUN as iscsi volume for virtual machine. To create new virtual machine I would: a) create lvm volume on iscsi target using libvirt b) export this volume using iscsi c) define virtual machine with this volume But can I create iscsi volume without pool? Does it makes any sense? Are there better ways to *manage* volumes for virtual machines using iscsi? Thanks for any tips. Łukasz Mierzwa

3 5

[libvirt] [PATCH 00/08] More misc device assignment fixes
by Mark McLoughlin 18 Aug '09

18 Aug '09

Hey, I had a little change of heart about the patches committed last week to allow pciResetDevice() reset multiple devices. The callback to check whether a device is active is much to cumbersome, instead we should maintain a list of active devices. Patch 2 reverts two commits from last week and patch 7 and 8 replace them with something much better. The reset of the patches are a bunch of fairly minor fixes. Cheers, Mark.

2 15