- Devel - libvirt List Archives

[libvirt] [PATCH V2] libxl: fix dom0 balloon logic
by Jim Fehlig 31 Mar '15

31 Mar '15

Recent testing on large memory systems revealed a bug in the Xen xl tool's freemem() function. When autoballooning is enabled, freemem() is used to ensure enough memory is available to start a domain, ballooning dom0 if necessary. When ballooning large amounts of memory from dom0, freemem() would exceed its self-imposed wait time and return an error. Meanwhile, dom0 continued to balloon. Starting the domain later, after sufficient memory was ballooned from dom0, would succeed. The libvirt implementation in libxlDomainFreeMem() suffers the same bug since it is modeled after freemem(). In the end, the best place to fix the bug on the Xen side was to slightly change the behavior of libxl_wait_for_memory_target(). Instead of failing after caller-provided wait_sec, the function now blocks as long as dom0 memory ballooning is progressing. It will return failure only when more memory is needed to reach the target and wait_sec have expired with no progress being made. See xen.git commit fd3aa246. There was a dicussion on how this would affect other libxl apps like libvirt http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html If libvirt containing this patch was build against a Xen containing the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() will fail after 30 sec and domain creation will be terminated. Without this patch and with old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() does not succeed after 30 sec, but returns success anyway. Domain creation continues resulting in all sorts of fun stuff like cpu soft lockups in the guest OS. It was decided to properly fix libxl_wait_for_memory_target(), and if anything improve the default behavior of apps using the freemem reference impl in xl. xl was patched to accommodate the change in libxl_wait_for_memory_target() with xen.git commit 883b30a0. This patch does the same in the libxl driver. While at it, I changed the logic to essentially match freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner IMO and will make it easier to spot future, potentially interesting divergences. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- V2: Actually use libxl_wait_for_memory_target(), instead of libxl_wait_for_free_memory() src/libxl/libxl_domain.c | 55 +++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 407a9bd..a1739aa 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1121,38 +1121,39 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) { uint32_t needed_mem; uint32_t free_mem; - size_t i; - int ret = -1; + int ret; int tries = 3; int wait_secs = 10; - if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, - &needed_mem)) >= 0) { - for (i = 0; i < tries; ++i) { - if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) < 0) - break; + ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info, &needed_mem); + if (ret < 0) + goto error; - if (free_mem >= needed_mem) { - ret = 0; - break; - } + do { + ret = libxl_get_free_memory(priv->ctx, &free_mem); + if (ret < 0) + goto error; - if ((ret = libxl_set_memory_target(priv->ctx, 0, - free_mem - needed_mem, - /* relative */ 1, 0)) < 0) - break; + if (free_mem >= needed_mem) + return 0; - ret = libxl_wait_for_free_memory(priv->ctx, 0, needed_mem, - wait_secs); - if (ret == 0 || ret != ERROR_NOMEM) - break; + ret = libxl_set_memory_target(priv->ctx, 0, + free_mem - needed_mem, + /* relative */ 1, 0); + if (ret < 0) + goto error; - if ((ret = libxl_wait_for_memory_target(priv->ctx, 0, 1)) < 0) - break; - } - } + ret = libxl_wait_for_memory_target(priv->ctx, 0, wait_secs); + if (ret < 0) + goto error; - return ret; + tries--; + } while (tries > 0); + + error: + virReportSystemError(ret, "%s", + _("Failed to balloon domain0 memory")); + return -1; } static void @@ -1271,12 +1272,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, priv->ctx, &d_config) < 0) goto endjob; - if (cfg->autoballoon && libxlDomainFreeMem(priv, &d_config) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("libxenlight failed to get free memory for domain '%s'"), - d_config.c_info.name); + if (cfg->autoballoon && libxlDomainFreeMem(priv, &d_config) < 0) goto endjob; - } if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, VIR_HOSTDEV_SP_PCI) < 0) -- 1.8.4.5

3 4

[libvirt] [PATCH] virsh: add check for exclusive options for domstats command
by Pavel Hrdina 31 Mar '15

31 Mar '15

Option --domain cannot be combined with options --list-*. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143837 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- tools/virsh-domain-monitor.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 6951db2..e683a6d 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2110,6 +2110,15 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) const vshCmdOpt *opt = NULL; bool ret = false; + VSH_EXCLUSIVE_OPTIONS("domain", "list-active"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-inactive"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-persistent"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-transient"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-running"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-paused"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-shutoff"); + VSH_EXCLUSIVE_OPTIONS("domain", "list-other"); + if (vshCommandOptBool(cmd, "state")) stats |= VIR_DOMAIN_STATS_STATE; -- 2.0.5

2 1

[libvirt] [PATCH] virDomainShutdown*: Describe blocking behaviour
by Michal Privoznik 31 Mar '15

31 Mar '15

The description to both virDomainShutdown() and virDomainShutdownFlags() is quite misleading when it comes to blocking behaviour of these two APIs. Firstly, we support many shutdown methods, from signalizing an ACPI event, through sending a signal to guest agent assisted shutdown. Some of these methods make the API return immediately, while others block the API until domain is actually shut of. And since virDomainShutdown() is equivalent to calling virDomainShutdownFlags(0), it's up to each driver which methods to try. So the bare virDomainShutdown() may block or may return immediately at the same time. I know, it's confusing, but at least let users know. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/libvirt-domain.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index f1608dc..03b342f 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1474,9 +1474,12 @@ virDomainScreenshot(virDomainPtr domain, * 'on_poweroff' XML setting resulting in a domain that reboots instead of * shutting down. For guests that react to a shutdown request, the differences * from virDomainDestroy() are that the guests disk storage will be in a - * stable state rather than having the (virtual) power cord pulled, and - * this command returns as soon as the shutdown request is issued rather - * than blocking until the guest is no longer running. + * stable state rather than having the (virtual) power cord pulled. It's up to + * hypervisor's driver implementation what methods of + * virDomainShutdownFlagValues are tried and in which order. As described in + * virDomainShutdownFlags, this call may return immediately after the shutdown + * request is send, or it may block indefinitely long, until the domain is + * actually shut off. * * If the domain is transient and has any snapshot metadata (see * virDomainSnapshotNum()), then that metadata will automatically @@ -1540,7 +1543,9 @@ virDomainShutdown(virDomainPtr domain) * and a hypervisor is not required to support all methods. * * To use guest agent (VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) the domain XML - * must have <channel> configured. + * must have <channel> configured. Moreover, depending on underlying + * hypervisor used, passing this flag may block the API until the domain + * is shut off (which is not guaranteed to happen anyway). * * Returns 0 in case of success and -1 in case of failure. */ -- 2.0.5

2 3

[libvirt] connect: ssh: Shall we remove the dependency of netcat ?
by zhang bo 31 Mar '15

31 Mar '15

1 When we connect libvirt with URI "qemu+ssh", it uses 'nc' command to connect to libvirt-sock. # virsh -c qemu+ssh://root@9.61.1.74/system list Password: //ask users to input passwords here. Id Name State ---------------------------------------------------- 11 pxe running It in fact uses 'ssh' and 'nc' commands to connect to remote libvirt-sock, such as: ssh -l root 9.61.1.74 sh -c ''nc' -U /var/run/libvirt/libvirt-sock' The code path is : virConnectOpen->doRemoteOpen->virNetClientNewSSH->virNetSocketNewConnectSSH 2 However, netcat(nc) is considered as an insecure tool, because it's too powerful in controlling the network. It's abandoned by some organizations. 3 So, is there any good substitution for netcat to realize qemu+ssh?

2 3

[libvirt] [PATCH 0/3] For 1.2.14: Fix regression in synchronous block job ABORT/PIVOT
by Peter Krempa 31 Mar '15

31 Mar '15

When a block job is terminated via the synchronous API the backing chain would be updated in a separate thread and thus allowed applications to get outdated data. This broke live snapshot merge on oVirt. Since the commit breaking this (see patch 3/3) was not released yet I'm asking to merge this during the freeze. Peter Krempa (3): qemu: processBlockJob: Don't unlock @vm twice qemu: Extract internals of processBlockJobEvent into a helper qemu: blockjob: Synchronously update backing chain in XML on ABORT/PIVOT src/conf/domain_conf.c | 16 +++- src/conf/domain_conf.h | 6 ++ src/qemu/qemu_driver.c | 246 ++++++++++++++++++++++++------------------------ src/qemu/qemu_process.c | 38 +++++--- 4 files changed, 169 insertions(+), 137 deletions(-) -- 2.2.2

2 7

[libvirt] [PATCH] qemucaps2xmltest: fix test to successfully run without kvm support
by Pavel Hrdina 31 Mar '15

31 Mar '15

Function virQEMUCapsInitGuestFromBinary detect kvm support by testing whether /dev/kvm exists or whether we pass path to kvmbin. Provide the path we are testing via kvmbin for testing purpose instead of detecting presence of /dev/kvm to successfully run the tests on all hosts. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- tests/qemucaps2xmltest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index b3975b4..826f9ea 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -118,8 +118,8 @@ testGetCaps(char *capsData, const testQemuData *data) if (virQEMUCapsInitGuestFromBinary(caps, "/usr/bin/qemu-system-i386", qemuCaps, - NULL, - NULL, + "/usr/bin/qemu-system-i386", + qemuCaps, data->guestarch) < 0) { fprintf(stderr, "failed to create the capabilities from qemu"); goto error; -- 2.0.5

3 4

[libvirt] [PATCH] Strip control codes in virBufferEscapeString
by Ján Tomko 31 Mar '15

31 Mar '15

These cannot be represented in XML. We have been stripping them, but only if the string had characters that needed escaping: <>"'& Extend the strcspn check to include control codes, and strip them even if we don't do any escaping. https://bugzilla.redhat.com/show_bug.cgi?id=1184131 https://bugzilla.redhat.com/show_bug.cgi?id=1066564 --- src/util/virbuffer.c | 14 +++++++++++--- tests/virbuftest.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 706dbfa..3d13c90 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -438,6 +438,13 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) int len; char *escaped, *out; const char *cur; + const char forbidden_characters[] = { + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + /*\t*/ /*\n*/ 0x0B, 0x0C, /*\r*/ 0x0E, 0x0F, 0x10, + 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, + 0x19, '"', '&', '\'', '<', '>', + '\0' + }; if ((format == NULL) || (buf == NULL) || (str == NULL)) return; @@ -446,7 +453,7 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) return; len = strlen(str); - if (strcspn(str, "<>&'\"") == len) { + if (strcspn(str, forbidden_characters) == len) { virBufferAsprintf(buf, format, str); return; } @@ -490,8 +497,7 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) *out++ = 'o'; *out++ = 's'; *out++ = ';'; - } else if (((unsigned char)*cur >= 0x20) || (*cur == '\n') || (*cur == '\t') || - (*cur == '\r')) { + } else if (!strchr(forbidden_characters, *cur)) { /* * default case, just copy ! * Note that character over 0x80 are likely to give problem @@ -499,6 +505,8 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str) * it's hard to handle properly we have to assume it's UTF-8 too */ *out++ = *cur; + } else { + /* silently ignore control characters */ } cur++; } diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 21cb18b..10398d5 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -349,6 +349,39 @@ testBufAddStr(const void *opaque ATTRIBUTE_UNUSED) static int +testBufEscapeStr(const void *opaque ATTRIBUTE_UNUSED) +{ + const struct testBufAddStrData *data = opaque; + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *actual; + int ret = -1; + + virBufferAddLit(&buf, "<c>\n"); + virBufferAdjustIndent(&buf, 2); + virBufferEscapeString(&buf, "<el>%s</el>\n", data->data); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</c>"); + + if (!(actual = virBufferContentAndReset(&buf))) { + TEST_ERROR("buf is empty"); + goto cleanup; + } + + if (STRNEQ_NULLABLE(actual, data->expect)) { + TEST_ERROR("testBufEscapeStr(): Strings don't match:\n"); + virtTestDifference(stderr, data->expect, actual); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(actual); + return ret; +} + + +static int mymain(void) { int ret = 0; @@ -379,6 +412,22 @@ mymain(void) DO_TEST_ADD_STR("<a/>\n", "<c>\n <a/>\n</c>"); DO_TEST_ADD_STR("<b>\n <a/>\n</b>\n", "<c>\n <b>\n <a/>\n </b>\n</c>"); +#define DO_TEST_ESCAPE(data, expect) \ + do { \ + struct testBufAddStrData info = { data, expect }; \ + if (virtTestRun("Buf: EscapeStr", testBufEscapeStr, &info) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_ESCAPE("<td></td><td></td>", + "<c>\n <el><td></td><td></td></el>\n</c>"); + DO_TEST_ESCAPE("\007\"&&\"\x15", + "<c>\n <el>"&&"</el>\n</c>"); + DO_TEST_ESCAPE(",,'..',,", + "<c>\n <el>,,'..',,</el>\n</c>"); + DO_TEST_ESCAPE("\x01\x01\x02\x03\x05\x08", + "<c>\n <el></el>\n</c>"); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.0.5

5 7

[libvirt] [PATCH] qemu: blockCopy: Pass adjusted bandwidth when called via blockRebase
by Peter Krempa 30 Mar '15

30 Mar '15

The block copy API takes the speed in bytes/s rather than MiB/s that was the prior approach in virDomainBlockRebase. We correctly converted the speed to bytes/s in the old API but we still called the common helper virDomainBlockCopyCommon with the unadjusted variable. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207122 --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4d05221..6700fc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16811,7 +16811,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, flags &= (VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT); ret = qemuDomainBlockCopyCommon(vm, dom->conn, path, dest, - bandwidth, 0, 0, flags, true); + speed, 0, 0, flags, true); dest = NULL; cleanup: -- 2.2.2

2 2

[libvirt] [PATCH 0/6] Few NUMA fixes
by Michal Privoznik 30 Mar '15

30 Mar '15

The heart of the patchset are the last two patches. Long story short, if a domain is configured to be pinned onto a set of NUMA nodes stricly, we use both CGroups and numa_set_membind(). While we can change the former later on a user's wish, we can't change the latter. Therefore, any subsequent memory enlargement (e.g. via virsh setmem $dom) will result in memory still being allocated from old NUMA nodes and OOM killer interference is likely to occur. Michal Privoznik (6): virCgroupNewPartition: Fix comment virCgroupNew: Enhance debug message virCgroupController: Check the enum fits into 'int' qemuDomainGetNumaParameters: Check for the correct CGroup controller qemuProcessHook: Call virNuma*() iff needed virLXCControllerSetupResourceLimits: Call virNuma*() iff needed src/lxc/lxc_controller.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_process.c | 32 ++++++++++++++++++++++++++++---- src/util/vircgroup.c | 6 +++--- src/util/vircgroup.h | 5 +++++ 5 files changed, 62 insertions(+), 14 deletions(-) -- 2.0.5

2 9

[libvirt] [PATCH] interface: allow multiple IPv4 addresses in interface XML
by Laine Stump 30 Mar '15

30 Mar '15

An upcoming netcf release will support multiple ipv4 addresses, so let's loosen up libvirt's interface.rng to allow it. --- docs/schemas/interface.rng | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index bf0346c..45b40cd 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -326,12 +326,14 @@ <choice> <ref name="dhcp-element"/> <group> - <element name="ip"> - <attribute name="address"><ref name="ipv4Addr"/></attribute> - <optional> - <attribute name="prefix"><ref name="ipv4Prefix"/></attribute> - </optional> - </element> + <oneOrMore> + <element name="ip"> + <attribute name="address"><ref name="ipv4Addr"/></attribute> + <optional> + <attribute name="prefix"><ref name="ipv4Prefix"/></attribute> + </optional> + </element> + </oneOrMore> <optional> <element name="route"> <attribute name="gateway"><ref name="ipv4Addr"/></attribute> -- 2.1.0

2 1