January 2012 - Devel - libvirt List Archives

[libvirt] [test-API][PATCH 1/2] Add 2 new functions in storageAPI
by Wayne Sun 01 Feb '12

01 Feb '12

* download(self, poolname, volname, stream, offset, length, flags = 0) * upload(self, poolname, volname, stream, offset, length, flags = 0) --- lib/storageAPI.py | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/lib/storageAPI.py b/lib/storageAPI.py index 6c9d286..b0733f8 100644 --- a/lib/storageAPI.py +++ b/lib/storageAPI.py @@ -466,3 +466,20 @@ class StorageAPI(object): code = e.get_error_code() raise exception.LibvirtAPI(message, code) + def download(self, poolname, volname, stream, offset, length, flags = 0): + try: + volobj = self.get_volume_obj(poolname, volname) + return volobj.download(stream, offset, length, flags) + except libvirt.libvirtError, e: + message = e.get_error_message() + code = e.get_error_code() + raise exception.LibvirtAPI(message, code) + + def upload(self, poolname, volname, stream, offset, length, flags = 0): + try: + volobj = self.get_volume_obj(poolname, volname) + return volobj.upload(stream, offset, length, flags) + except libvirt.libvirtError, e: + message = e.get_error_message() + code = e.get_error_code() + raise exception.LibvirtAPI(message, code) -- 1.7.1

3 4

[libvirt] [PATCH] tests: dynamically replace dnsmasq path
by Philipp Hahn 01 Feb '12

01 Feb '12

The path to the dnsmasq binary can be configured while in the test data the path is hard-coded to /usr/bin/. This break the test suite if a the binary is located in a different location, like /usr/local/sbin/. Replace the hard coded path in the test data by a token, which is dynamically replaced in networkxml2argvtest with the configured path after the test data has been loaded. (Another option would have been to modify configure.ac to generate the test data during configure, but I do not know of an easy way do trick configure into mass-generate those test files without listing every single one, which I consider less flexible.) Signed-off-by: Philipp Hahn <hahn(a)univention.de> --- tests/networkxml2argvdata/isolated-network.argv | 2 +- .../networkxml2argvdata/nat-network-dns-hosts.argv | 2 +- .../nat-network-dns-srv-record-minimal.argv | 2 +- .../nat-network-dns-srv-record.argv | 2 +- .../nat-network-dns-txt-record.argv | 2 +- tests/networkxml2argvdata/nat-network.argv | 2 +- tests/networkxml2argvdata/netboot-network.argv | 2 +- .../networkxml2argvdata/netboot-proxy-network.argv | 2 +- tests/networkxml2argvdata/routed-network.argv | 2 +- tests/networkxml2argvtest.c | 42 ++++++++++++++++++++ 10 files changed, 51 insertions(+), 9 deletions(-) diff --git a/tests/networkxml2argvdata/isolated-network.argv b/tests/networkxml2argvdata/isolated-network.argv index 7ea2e94..a9beb05 100644 --- a/tests/networkxml2argvdata/isolated-network.argv +++ b/tests/networkxml2argvdata/isolated-network.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ +@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \ --except-interface lo --dhcp-option=3 --no-resolv \ --listen-address 192.168.152.1 \ --dhcp-range 192.168.152.2,192.168.152.254 \ diff --git a/tests/networkxml2argvdata/nat-network-dns-hosts.argv b/tests/networkxml2argvdata/nat-network-dns-hosts.argv index 2158df8..c7e4967 100644 --- a/tests/networkxml2argvdata/nat-network-dns-hosts.argv +++ b/tests/networkxml2argvdata/nat-network-dns-hosts.argv @@ -1,3 +1,3 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com \ +@DNSMASQ@ --strict-order --bind-interfaces --domain example.com \ --conf-file= --except-interface lo --listen-address 192.168.122.1 \ --expand-hosts --addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts\ diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv index 021e8f0..ea1da6d 100644 --- a/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record-minimal.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq \ +@DNSMASQ@ \ --strict-order \ --bind-interfaces \ --conf-file= \ diff --git a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv index 85afbba..84c2d2f 100644 --- a/tests/networkxml2argvdata/nat-network-dns-srv-record.argv +++ b/tests/networkxml2argvdata/nat-network-dns-srv-record.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq \ +@DNSMASQ@ \ --strict-order \ --bind-interfaces \ --conf-file= \ diff --git a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv index be6ba4b..bed309f 100644 --- a/tests/networkxml2argvdata/nat-network-dns-txt-record.argv +++ b/tests/networkxml2argvdata/nat-network-dns-txt-record.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ +@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \ --except-interface lo --txt-record=example,example value \ --listen-address 192.168.122.1 --listen-address 192.168.123.1 \ --listen-address 2001:db8:ac10:fe01::1 \ diff --git a/tests/networkxml2argvdata/nat-network.argv b/tests/networkxml2argvdata/nat-network.argv index d7faee2..80878a8 100644 --- a/tests/networkxml2argvdata/nat-network.argv +++ b/tests/networkxml2argvdata/nat-network.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ +@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \ --except-interface lo --listen-address 192.168.122.1 \ --listen-address 192.168.123.1 --listen-address 2001:db8:ac10:fe01::1 \ --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 \ diff --git a/tests/networkxml2argvdata/netboot-network.argv b/tests/networkxml2argvdata/netboot-network.argv index 78e873c..7ae5b75 100644 --- a/tests/networkxml2argvdata/netboot-network.argv +++ b/tests/networkxml2argvdata/netboot-network.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com \ +@DNSMASQ@ --strict-order --bind-interfaces --domain example.com \ --conf-file= --except-interface lo --listen-address 192.168.122.1 \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \ diff --git a/tests/networkxml2argvdata/netboot-proxy-network.argv b/tests/networkxml2argvdata/netboot-proxy-network.argv index 5fe1b8e..2c5a0d5 100644 --- a/tests/networkxml2argvdata/netboot-proxy-network.argv +++ b/tests/networkxml2argvdata/netboot-proxy-network.argv @@ -1,4 +1,4 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --domain example.com \ +@DNSMASQ@ --strict-order --bind-interfaces --domain example.com \ --conf-file= --except-interface lo --listen-address 192.168.122.1 \ --dhcp-range 192.168.122.2,192.168.122.254 \ --dhcp-leasefile=/var/lib/libvirt/dnsmasq/netboot.leases \ diff --git a/tests/networkxml2argvdata/routed-network.argv b/tests/networkxml2argvdata/routed-network.argv index d059630..75303e6 100644 --- a/tests/networkxml2argvdata/routed-network.argv +++ b/tests/networkxml2argvdata/routed-network.argv @@ -1,2 +1,2 @@ -/usr/sbin/dnsmasq --strict-order --bind-interfaces --conf-file= \ +@DNSMASQ@ --strict-order --bind-interfaces --conf-file= \ --except-interface lo --listen-address 192.168.122.1\ diff --git a/tests/networkxml2argvtest.c b/tests/networkxml2argvtest.c index 2dd9b7f..ec2ef39 100644 --- a/tests/networkxml2argvtest.c +++ b/tests/networkxml2argvtest.c @@ -15,6 +15,45 @@ #include "memory.h" #include "network/bridge_driver.h" +/* Replace all occurrences of @token in @buf by @replacement and adjust size of + * @buf accordingly. Returns 0 on success and -1 on out-of-memory errors. */ +static int replaceTokens(char **buf, const char *token, const char *replacement) { + char *token_start, *token_end; + size_t old_len, new_len, rest_len; + void *tmp; + const size_t token_len = strlen(token); + const size_t replacement_len = strlen(replacement); + const int diff = replacement_len - token_len; + + old_len = rest_len = strlen(*buf) + 1; + token_end = *buf; + for (;;) { + token_start = strstr(token_end, token); + if (token_start == NULL) + break; + rest_len -= token_start + token_len - token_end; + token_end = token_start + token_len; + new_len = old_len + diff; + if (diff > 0) { + tmp = realloc((void *)*buf, new_len); + if (tmp == NULL) + return -1; + *buf = tmp; + } + if (diff != 0) + memmove(token_end + diff, token_end, rest_len); + memmove(token_start, replacement, replacement_len); + if (diff < 0) { + tmp = realloc((void *)*buf, new_len); + if (tmp == NULL) + return -1; + *buf = tmp; + } + old_len = new_len; + } + return 0; +} + static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { char *inXmlData = NULL; char *outArgvData = NULL; @@ -32,6 +71,9 @@ static int testCompareXMLToArgvFiles(const char *inxml, const char *outargv) { if (virtTestLoadFile(outargv, &outArgvData) < 0) goto fail; + if (replaceTokens(&outArgvData, "@DNSMASQ@", DNSMASQ)) + goto fail; + if (!(dev = virNetworkDefParseString(inXmlData))) goto fail; -- 1.7.1

2 3

[libvirt] Implement virStorageVolResize() for FS backend
by Zeeshan Ali (Khattak) 01 Feb '12

01 Feb '12

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> Currently only VIR_STORAGE_VOL_RESIZE_DELTA flag is supported. --- src/storage/storage_backend.h | 6 +++ src/storage/storage_backend_fs.c | 53 +++++++++++++++++++++++ src/storage/storage_driver.c | 88 ++++++++++++++++++++++++++++++++++++++ src/util/storage_file.c | 16 +++++++ src/util/storage_file.h | 2 + 5 files changed, 165 insertions(+), 0 deletions(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 75ed676..a37bf7c 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -44,6 +44,11 @@ typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjP typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags); +typedef int (*virStorageBackendVolumeResize)(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned long long capacity, + unsigned int flags); /* File creation/cloning functions used for cloning between backends */ int virStorageBackendCreateRaw(virConnectPtr conn, @@ -78,6 +83,7 @@ struct _virStorageBackend { virStorageBackendCreateVol createVol; virStorageBackendRefreshVol refreshVol; virStorageBackendDeleteVol deleteVol; + virStorageBackendVolumeResize resizeVol; }; virStorageBackendPtr virStorageBackendForType(int type); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d8dc29c..1af12e6 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1187,6 +1187,56 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, return 0; } +static int +virStorageBackendFilesystemResizeQemuImg(const char *path, + unsigned long long capacity) +{ + int ret = -1; + char *img_tool; + virCommandPtr cmd = NULL; + + /* KVM is usually ahead of qemu on features, so try that first */ + img_tool = virFindFileInPath("kvm-img"); + if (!img_tool) + img_tool = virFindFileInPath("qemu-img"); + + if (!img_tool) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + return -1; + } + + cmd = virCommandNew(img_tool); + virCommandAddArgList(cmd, "resize", path, NULL); + virCommandAddArgFormat(cmd, "%llu", capacity); + + ret = virCommandRun(cmd, NULL); + + VIR_FREE(img_tool); + virCommandFree(cmd); + + return ret; +} + +/** + * Resize a volume + */ +static int +virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol, + unsigned long long capacity, + unsigned int flags) +{ + virCheckFlags(0, -1); + + if (vol->target.format == VIR_STORAGE_FILE_RAW) + return virStorageFileResize(vol->target.path, capacity); + else + return virStorageBackendFilesystemResizeQemuImg(vol->target.path, + capacity); +} + virStorageBackend virStorageBackendDirectory = { .type = VIR_STORAGE_POOL_DIR, @@ -1199,6 +1249,7 @@ virStorageBackend virStorageBackendDirectory = { .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, + .resizeVol = virStorageBackendFileSystemVolResize, }; #if WITH_STORAGE_FS @@ -1216,6 +1267,7 @@ virStorageBackend virStorageBackendFileSystem = { .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, + .resizeVol = virStorageBackendFileSystemVolResize, }; virStorageBackend virStorageBackendNetFileSystem = { .type = VIR_STORAGE_POOL_NETFS, @@ -1232,5 +1284,6 @@ virStorageBackend virStorageBackendNetFileSystem = { .createVol = virStorageBackendFileSystemVolCreate, .refreshVol = virStorageBackendFileSystemVolRefresh, .deleteVol = virStorageBackendFileSystemVolDelete, + .resizeVol = virStorageBackendFileSystemVolResize, }; #endif /* WITH_STORAGE_FS */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index a332ada..e7a8a8b 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1695,7 +1695,94 @@ out: return ret; } +static int +storageVolumeResize(virStorageVolPtr obj, + long long capacity, + unsigned int flags) +{ + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStorageBackendPtr backend; + virStoragePoolObjPtr pool = NULL; + virStorageVolDefPtr vol = NULL; + unsigned long long abs_capacity; + int ret = -1; + + virCheckFlags(VIR_STORAGE_VOL_RESIZE_DELTA, -1); + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); + storageDriverUnlock(driver); + + if (!pool) { + virStorageReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching uuid")); + goto out; + } + + if (!virStoragePoolObjIsActive(pool)) { + virStorageReportError(VIR_ERR_OPERATION_INVALID, + _("storage pool is not active")); + goto out; + } + + if ((backend = virStorageBackendForType(pool->def->type)) == NULL) + goto out; + + vol = virStorageVolDefFindByName(pool, obj->name); + + if (vol == NULL) { + virStorageReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching name '%s'"), + obj->name); + goto out; + } + + if (vol->building) { + virStorageReportError(VIR_ERR_OPERATION_INVALID, + _("volume '%s' is still being allocated."), + vol->name); + goto out; + } + if (flags & VIR_STORAGE_VOL_RESIZE_DELTA) { + abs_capacity = vol->capacity + capacity; + flags &= ~VIR_STORAGE_VOL_RESIZE_DELTA; + } else { + abs_capacity = capacity; + } + + if (abs_capacity < vol->allocation) { + virStorageReportError(VIR_ERR_INVALID_ARG, + _("can't shrink capacity below " + "existing allocation")); + goto out; + } + + if (abs_capacity > vol->allocation + pool->def->available) { + virStorageReportError(VIR_ERR_INVALID_ARG, + _("Not enough space left on storage pool")); + goto out; + } + + if (!backend->resizeVol) { + virStorageReportError(VIR_ERR_NO_SUPPORT, + _("storage pool does not support changing of " + "volume capacity")); + goto out; + } + + if (backend->resizeVol(obj->conn, pool, vol, abs_capacity, flags) < 0) + goto out; + + vol->capacity = abs_capacity; + ret = 0; + +out: + if (pool) + virStoragePoolObjUnlock(pool); + + return ret; +} /* If the volume we're wiping is already a sparse file, we simply * truncate and extend it to its original size, filling it with @@ -2243,6 +2330,7 @@ static virStorageDriver storageDriver = { .volGetInfo = storageVolumeGetInfo, /* 0.4.0 */ .volGetXMLDesc = storageVolumeGetXMLDesc, /* 0.4.0 */ .volGetPath = storageVolumeGetPath, /* 0.4.0 */ + .volResize = storageVolumeResize, /* 0.9.10 */ .poolIsActive = storagePoolIsActive, /* 0.7.3 */ .poolIsPersistent = storagePoolIsPersistent, /* 0.7.3 */ diff --git a/src/util/storage_file.c b/src/util/storage_file.c index ba9cfc5..8260adb 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -931,6 +931,22 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta) VIR_FREE(meta); } +/** + * virStorageFileResize: + * + * Change the capacity of the raw storage file at 'path'. + */ +int +virStorageFileResize(const char *path, unsigned long long capacity) +{ + if (truncate(path, capacity) < 0) { + virReportSystemError(errno, _("Failed to truncate file '%s'"), path); + return -1; + } + + return 0; +} + #ifdef __linux__ # ifndef NFS_SUPER_MAGIC diff --git a/src/util/storage_file.h b/src/util/storage_file.h index b8920d0..96afb12 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -72,6 +72,8 @@ int virStorageFileGetMetadataFromFD(const char *path, void virStorageFileFreeMetadata(virStorageFileMetadata *meta); +int virStorageFileResize(const char *path, unsigned long long capacity); + enum { VIR_STORAGE_FILE_SHFS_NFS = (1 << 0), VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1), -- 1.7.7.6

3 2

[libvirt] [PATCHv3] network: Avoid memory leaks on networkBuildDnsmasqArgv
by ajia＠redhat.com 01 Feb '12

01 Feb '12

From: Alex Jia <ajia(a)redhat.com> Detected by valgrind. Leaks introduced in commit 973af236. * src/network/bridge_driver.c: fix memory leaks on failure and successful path. * How to reproduce? % make -C tests check TESTS=networkxml2argvtest % cd tests && valgrind -v --leak-check=full ./networkxml2argvtest * Actual result: ==2226== 3 bytes in 1 blocks are definitely lost in loss record 1 of 24 ==2226== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==2226== by 0x39CF0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so) ==2226== by 0x41DFF7: virVasprintf (stdio2.h:199) ==2226== by 0x41E0B7: virAsprintf (util.c:1695) ==2226== by 0x41A2D9: networkBuildDhcpDaemonCommandLine (bridge_driver.c:545) ==2226== by 0x4145C8: testCompareXMLToArgvHelper (networkxml2argvtest.c:47) ==2226== by 0x4156A1: virtTestRun (testutils.c:141) ==2226== by 0x414332: mymain (networkxml2argvtest.c:123) ==2226== by 0x414D97: virtTestMain (testutils.c:696) ==2226== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) ==2226== ==2226== 3 bytes in 1 blocks are definitely lost in loss record 2 of 24 ==2226== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==2226== by 0x39CF0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so) ==2226== by 0x41DFF7: virVasprintf (stdio2.h:199) ==2226== by 0x41E0B7: virAsprintf (util.c:1695) ==2226== by 0x41A307: networkBuildDhcpDaemonCommandLine (bridge_driver.c:551) ==2226== by 0x4145C8: testCompareXMLToArgvHelper (networkxml2argvtest.c:47) ==2226== by 0x4156A1: virtTestRun (testutils.c:141) ==2226== by 0x414332: mymain (networkxml2argvtest.c:123) ==2226== by 0x414D97: virtTestMain (testutils.c:696) ==2226== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) ==2226== ==2226== 5 bytes in 1 blocks are definitely lost in loss record 4 of 24 ==2226== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==2226== by 0x39CF0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so) ==2226== by 0x41DFF7: virVasprintf (stdio2.h:199) ==2226== by 0x41E0B7: virAsprintf (util.c:1695) ==2226== by 0x41A2AB: networkBuildDhcpDaemonCommandLine (bridge_driver.c:539) ==2226== by 0x4145C8: testCompareXMLToArgvHelper (networkxml2argvtest.c:47) ==2226== by 0x4156A1: virtTestRun (testutils.c:141) ==2226== by 0x414332: mymain (networkxml2argvtest.c:123) ==2226== by 0x414D97: virtTestMain (testutils.c:696) ==2226== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) ==2226== ==2226== LEAK SUMMARY: ==2226== definitely lost: 11 bytes in 3 blocks Signed-off-by: Alex Jia <ajia(a)redhat.com> --- src/network/bridge_driver.c | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5d0d528..c9a8ccf 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -458,7 +458,10 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, { int r, ret = -1; int nbleases = 0; - int ii; + int ii, i, j; + char *recordPort = NULL; + char *recordPriority = NULL; + char *recordWeight = NULL; virNetworkIpDefPtr tmpipdef; /* @@ -513,7 +516,6 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, if (network->def->dns != NULL) { virNetworkDNSDefPtr dns = network->def->dns; - int i; for (i = 0; i < dns->ntxtrecords; i++) { char *record = NULL; @@ -530,10 +532,6 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, for (i = 0; i < dns->nsrvrecords; i++) { char *record = NULL; - char *recordPort = NULL; - char *recordPriority = NULL; - char *recordWeight = NULL; - if (dns->srvrecords[i].service && dns->srvrecords[i].protocol) { if (dns->srvrecords[i].port) { if (virAsprintf(&recordPort, "%d", dns->srvrecords[i].port) < 0) { @@ -671,6 +669,11 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, ret = 0; cleanup: + for (j = 0; j < i; j++) { + VIR_FREE(recordPort); + VIR_FREE(recordWeight); + VIR_FREE(recordPriority); + } return ret; } -- 1.7.1

4 5

[libvirt] [PATCHv3] simplify block of codes
by Alex.Jia＠redhat.com 01 Feb '12

01 Feb '12

From: Alex Jia <ajia(a)redhat.com> Using new function 'virTypedParameterArrayClear' to simplify block of codes. * daemon/remote.c, src/remote/remote_driver.c: simplify codes. Signed-off-by: Alex Jia <ajia(a)redhat.com> --- daemon/remote.c | 6 +----- src/remote/remote_driver.c | 19 +++++-------------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index cb8423a..e7d9b2f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -848,11 +848,7 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val, cleanup: if (rv < 0) { - int j; - for (j = 0; j < i; ++j) { - if (params[j].type == VIR_TYPED_PARAM_STRING) - VIR_FREE(params[j].value.s); - } + virTypedParameterArrayClear(params, i); VIR_FREE(params); } return params; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 61b96e9..031becd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -46,6 +46,7 @@ #include "virfile.h" #include "command.h" #include "intprops.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_REMOTE @@ -1417,12 +1418,8 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val, rv = 0; cleanup: - if (rv < 0) { - int j; - for (j = 0; j < i; j++) - if (params[j].type == VIR_TYPED_PARAM_STRING) - VIR_FREE(params[j].value.s); - } + if (rv < 0) + virTypedParameterArrayClear(params, i); return rv; } @@ -2384,15 +2381,9 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, rv = ret.nparams; cleanup: - if (rv < 0) { - int max = nparams * ncpus; - int i; + if (rv < 0) + virTypedParameterArrayClear(params, nparams * ncpus); - for (i = 0; i < max; i++) { - if (params[i].type == VIR_TYPED_PARAM_STRING) - VIR_FREE(params[i].value.s); - } - } xdr_free ((xdrproc_t) xdr_remote_domain_get_cpu_stats_ret, (char *) &ret); done: -- 1.7.1

2 1

[libvirt] [PATCHv2] schema: Relax schema for domain name
by Peter Krempa 31 Jan '12

31 Jan '12

The domain schema enforced restrictions on the domain name string that the code doesn't. This patch relaxes the check, leaving the restrictions on the driver or hypervisor. The only invalid character is a newline. --- docs/schemas/domaincommon.rng | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4fa968d..ecf3484 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3030,7 +3030,7 @@ </define> <define name="domainName"> <data type="string"> - <param name="pattern">[A-Za-z0-9_\.\+\-&:/]+</param> + <param name="pattern">[^\n]+</param> </data> </define> <define name="diskSerial"> -- 1.7.3.4

2 3

[libvirt] RFC API proposal: virDomainBlockRebase
by Eric Blake 31 Jan '12

31 Jan '12

Right now, the existing virDomainBlockPull API has a tough limitation - it is an all-or-none approach. In all my examples below, I'm starting from the following relationship, where '<-' means 'is a backing file of': template <- intermediate <- current virDomainBlockPull can only convert things in a forward direction, with the merge destination being the current image, resulting in: merge template and intermediate into current, creating: current Meanwhile, qemu is adding support for a partial block pull operation, still on the current image as the merge destination, but where you can now specify an optional argument to limit the pull to just the intermediate files and altering the current image to be backed by an ancestor file, as in: merge intermediate into current, creating: template <- current For 0.9.10, I'd like to add the following API: /** * virDomainBlockRebase: * @dom: pointer to domain object * @disk: path to the block device, or device shorthand * @base: new base image, or NULL for entire block pull * @bandwidth: (optional) specify copy bandwidth limit in Mbps * @flags: extra flags; not used yet, so callers should always pass 0 * * Populate a disk image with data from its backing image chain, and * setting the new backing image to @base, where base is the absolute * path of one of the backing images in the chain. If @base is NULL, * then this operation is identical to virDomainBlockPull(). Once all * data from its backing image chain has been pulled, the disk no * longer depends on those intermediate backing images. This function * pulls data for the entire device in the background. Progress of the * operation can be checked with virDomainGetBlockJobInfo() and * the operation can be aborted with virDomainBlockJobAbort(). When * finished, an asynchronous event is raised to indicate the final * status. * * The @disk, @bandwidth, and @flags parameters are handled as in * virDomainBlockPull(). * * Returns 0 if the operation has started, -1 on failure. */ int virDomainBlockRebase(virDomainPtr dom, const char *disk, const char *base, unsigned long bandwidth, unsigned int flags); Given that Adam has a pending patch to support a VIR_DOMAIN_BLOCK_PULL_ASYNC flag, this same flag would have to be supported in virDomainBlockRebase. I've also been chatting with Federico Simoncelli about how the above operation would work for VDSM purposes in doing a live block move, while preserving a common template base file: start with: vda: template <- current1 create a disk-only snapshot, with: tmpsnap = virDomainSnapshotCreateXML(dom, "<domainsnapshot>\n" " <disks>\n" " <disk name='vda'>\n" " <source>/path/to/current2</source>\n" " </disk>\n" " <disks>\n" "</domainsnapshot>", VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) where the xml calls out the destination file name, resulting in: vda: template <- current1 <- current2 perform the block rebase, with: virDomainBlockRebase(dom, "vda", "/path/to/template", VIR_DOMAIN_BLOCK_PULL_ASYNC) as well as waiting for the event (or polling status) to wait for completion, resulting in: vda: template <- current2 delete the disk-only snapshot metadata as no longer useful, with: virDomainSnapshotDelete(tmpsnap, VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) At one point, I thought of creating a single libvirt API that performs all of those steps in one call; but right now, I'm not proposing that, because of the fact that qemu has no way to undo a snapshot. In other words, without an undo operation, if the snapshot phase succeeds but the block rebase phase fails, a single API would have to report failure even though the domain was altered, while the ideal scenario is that reporting failure means things were in the same state as before the API started. Beyond 0.9.10, there are some additional useful merge patterns that might be worth exposing. All of these operations are already possible on offline images, using qemu-img; but none of them are possible on live images using current qemu, which is why I'm thinking it is something for another day. I'm also hoping to someday enhance the set of virStorageVol APIs to make backing file manipulation of offline images easier. At any rate, the addition merge operations are: forward live merge with a non-current image as the merge destination, as in: merge template into intermediate, creating: intermediate <- current backward merge of a current image (that is, undoing a current snapshot): merge current into intermediate, creating: template <- intermediate and backward merge of a non-current image (that is, undoing an earlier snapshot, but by modifying the template rather than the current image): merge intermediate into base, creating: template <- current Backward merge of the current image seems like something easy to fit into my proposed API (add a new flag, maybe called VIR_DOMAIN_BLOCK_REBASE_BACKWARD). Manipulations of anything that does not involve the current image seems tougher, assuming qemu ever even reaches the point where it exposes those operations on live volumes - the user has to specify not one, but two backing file names. But even that could possibly be fit into my API, by adding a flag that states that the const char *backing argument is treated as an XML snippet describing the full details of the merge, with the XML listing which image is being merged to which destination, rather than as just the name of the backing file becoming the new base of the current image. Perhaps something like: virDomainBlockRebase(dom, block, "<rebase>\n" " <source>/path/to/intermediate</source>\n" " <dest>/path/to/template</dest>\n" "</rebase>", VIR_DOMAIN_BLOCK_REBASE_XML|VIR_DOMAIN_BLOCK_REBASE_BACKWARD) as a specification to take the contents of intermediate, merge those backwards into template, and as well as adjusting the rest of the backing file chain so that whatever used to be backed by intermediate is now backed by template. Or, if qemu ever gives us the ability to merge non-current images, we may decide at that time that it is worth a new API to expose those new complexities. Another thing I have been thinking about is virDomainSnapshotDelete. The above conversation talks about merging of a single disk, but a live disk snapshot operation can create backing file chains for multiple disks at once, all tracked by a snapshot. Additionally, the current code allows a snapshot delete of internal snapshots, but refuses to do anything useful with an external snapshot, because there is currently no way to specify if the snapshot is removed by merging the base into the new current, or by undoing the current and merging it backwards into the base. Alas, virDomainSnapshotDelete doesn't take any arguments for how to handle the situation, and use of a flag to make the decision would limit all disks to be handled in the same manner. So what I'm thinking is that when a snapshot is created (or redefined, using redefinition as the vehicle to add in the new XML), that the snapshot XML itself can record the preferred direction for undoing the snapshot; for example: <domainsnapshot> <disks> <disk name='/path/to/old_vda'> <source file='/path/to/new_vda'/> <on_delete merge='forward'/> </disk> <disk name='/path/to/old_vdb'> <source file='/path/to/new_vdb'/> <on_delete merge='backward'/> </disk> <disks> </domainsnapshot> then when virDomainSnapshotDelete is called on that snapshot, old_vda would be forward merged into new_vda, while new_vdb would be backward merged into old_vdb. Again, that's food for thought for post-0.9.10, and shouldn't get in the way of adding virDomainBlockRebase() now. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2 3

[libvirt] Qemu text monitor
by Shradha Shah 31 Jan '12

31 Jan '12

Hello All, I am currently working with libvirt on RHEL6.2. There are a few points I have noticed regarding the QEMU monitor. When I hotplug a device into the guest, the Qemu text monitor receives a device_add command and adds the device to its current devices list (I found this list via the qemu-monitor-command "info pci"). When I hot-unplug the device from the guest, the Qemu text monitor receives the device_del command but does not remove the device from its current devices list (qemu-monitor-command "info pci" still shows the device). The result of this is that the next hotplug of the device fails giving an error "Duplicate id". I was wondering if any of you have hit this issue before or know about any qemu-monitor bugs I may not be aware of? Many Thanks in advance, Regards, Shradha Shah

3 2

[libvirt] [PATCH v2 0/4] RFC: grant KVM guest rawio capability
by Taku Izumi 31 Jan '12

31 Jan '12

This is the new version of http://www.redhat.com/archives/libvir-list/2012-January/msg01312.html Thank you for reviewing, Danie, Paolo, and Eric. -- This patchset achieves #1 and #2 of the following tasks: > 1. rawio="yes|nmo" on the <disk> element somewhere > 2. Give the QEMU process CAP_SYS_RAWIO > 3. Use the devices cgroup to specify which individual disks > can use rawio. In short, if you specify the following XML: <disk type='block' device='lun' rawio='yes'> the domain will be granted CAP_SYS_RAWIO. # virsh start VM # cat /proc/<VM's PID>/status ... CapInh: 0000000000000000 CapPrm: fffffffc00020000 CapEff: fffffffc00020000 CapBnd: fffffffc00020000 ... *[PATCH v2 1/4] conf: add rawio attribute to disk element of domain XML *[PATCH v2 2/4] util: add functions to keep capabilities *[PATCH v2 3/4] util: extend virExecWithHook() *[PATCH v2 4/4] qemu: make qemu processes to retain rawio capability - Best regards, Taku Izumi

4 15

[libvirt] [PATCH] qemu: never send SIGKILL to qemu process unless specifically requested
by Laine Stump 31 Jan '12

31 Jan '12

When libvirt is shutting down the qemu process, it first sends SIGTERM, then waits for 1.6 seconds and, if it sees the process still there, sends a SIGKILL. There have been reports that this behavior can lead to data loss because the guest running in qemu doesn't have time to flush it's disk cache buffers before it's unceremoniously whacked. One suggestion on how to solve that problem was to remove SIGKILL from the normal virDomainDestroyFlags, but still provide the ability to kill qemu with SIGKILL by using a new flag to virDomainDestroyFlags. This patch is a quick attempt at that in order to start a conversation on the topic. So what are your opinions? Is this the right way to solve the problem? Any better ideas? --- include/libvirt/libvirt.h.in | 12 ++++++------ src/qemu/qemu_driver.c | 7 +++++-- src/qemu/qemu_process.c | 40 ++++++++++++++++++++++------------------ src/qemu/qemu_process.h | 8 +++++++- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..f9ac865 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1182,12 +1182,6 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ - -/* - * typedef enum { - * } virDomainDestroyFlagsValues; - */ - virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); @@ -1222,6 +1216,12 @@ int virDomainReset (virDomainPtr domain, unsigned int flags); int virDomainDestroy (virDomainPtr domain); + +typedef enum { + VIR_DOMAIN_DESTROY_NONE = 0, /* Default behavior */ + VIR_DOMAIN_DESTROY_IMMEDIATE = 1 << 0, /* immediately kill, could lead to data loss!! */ +} virDomainDestroyFlagsValues; + int virDomainDestroyFlags (virDomainPtr domain, unsigned int flags); int virDomainRef (virDomainPtr domain); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab69dca..0e80241 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1754,7 +1754,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_DESTROY_IMMEDIATE, -1); qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1775,7 +1775,10 @@ qemuDomainDestroyFlags(virDomainPtr dom, * can kill the process even if a job is active. Killing * it now means the job will be released */ - qemuProcessKill(vm, false); + if (flags & VIR_DOMAIN_DESTROY_IMMEDIATE) + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_IMMEDIATELY); + else + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCEFULLY); /* We need to prevent monitor EOF callback from doing our work (and sending * misleading events) while the vm is unlocked inside BeginJob API diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d22020b..efbe46c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -586,7 +586,7 @@ endjob: cleanup: if (vm) { if (ret == -1) - qemuProcessKill(vm, false); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCEFULLY); if (virDomainObjUnref(vm) > 0) virDomainObjUnlock(vm); } @@ -611,12 +611,12 @@ qemuProcessShutdownOrReboot(struct qemud_driver *driver, qemuProcessFakeReboot, vm) < 0) { VIR_ERROR(_("Failed to create reboot thread, killing domain")); - qemuProcessKill(vm, true); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_GRACEFULLY); /* Safe to ignore value since ref count was incremented above */ ignore_value(virDomainObjUnref(vm)); } } else { - qemuProcessKill(vm, true); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_GRACEFULLY); } } @@ -3524,30 +3524,34 @@ cleanup: } -void qemuProcessKill(virDomainObjPtr vm, bool gracefully) +void qemuProcessKill(virDomainObjPtr vm, unsigned int killMode) { int i; - VIR_DEBUG("vm=%s pid=%d gracefully=%d", - vm->def->name, vm->pid, gracefully); + VIR_DEBUG("vm=%s pid=%d killMode=%d", + vm->def->name, vm->pid, killMode); if (!virDomainObjIsActive(vm)) { VIR_DEBUG("VM '%s' not active", vm->def->name); return; } - /* This loop sends SIGTERM, then waits a few iterations - * (1.6 seconds) to see if it dies. If still alive then - * it does SIGKILL, and waits a few more iterations (1.6 - * seconds more) to confirm that it has really gone. + /* This loop sends SIGTERM (or SIGKILL if killMode is + * VIR_QEMU_KILL_MODE_IMMEDIATELY), then waits a few iterations + * (3.2 seconds) to see if it dies. Note that the IMMEDIATELY mode + * will very likely result in lost data in the guest, so it should + * only be used if the guest is hung and can't be destroyed in any + * other manner. */ - for (i = 0 ; i < 15 ; i++) { + for (i = 0 ; i < 16 ; i++) { int signum; - if (i == 0) - signum = SIGTERM; - else if (i == 8) - signum = SIGKILL; - else + if (i == 0) { + if (killMode == VIR_QEMU_PROCESS_KILL_IMMEDIATELY) + signum = SIGKILL; + else + signum = SIGTERM; + } else { signum = 0; /* Just check for existence */ + } if (virKillProcess(vm->pid, signum) < 0) { if (errno != ESRCH) { @@ -3558,7 +3562,7 @@ void qemuProcessKill(virDomainObjPtr vm, bool gracefully) break; } - if (i == 0 && gracefully) + if (i == 0 && (killMode == VIR_QEMU_PROCESS_KILL_GRACEFULLY)) break; usleep(200 * 1000); @@ -3651,7 +3655,7 @@ void qemuProcessStop(struct qemud_driver *driver, } /* shut it off for sure */ - qemuProcessKill(vm, false); + qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCEFULLY); /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 062d79e..755d4a6 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -68,7 +68,13 @@ int qemuProcessAttach(virConnectPtr conn, virDomainChrSourceDefPtr monConfig, bool monJSON); -void qemuProcessKill(virDomainObjPtr vm, bool gracefully); +typedef enum { + VIR_QEMU_PROCESS_KILL_GRACEFULLY = 0, + VIR_QEMU_PROCESS_KILL_FORCEFULLY = 1, + VIR_QEMU_PROCESS_KILL_IMMEDIATELY = 2, +} virQemuProcessKillMode; + +void qemuProcessKill(virDomainObjPtr vm, unsigned int killMode); int qemuProcessAutoDestroyInit(struct qemud_driver *driver); void qemuProcessAutoDestroyRun(struct qemud_driver *driver, -- 1.7.7.5

4 8