[libvirt] [PATCH] allow memballoon type of none to desactivate it
by Daniel Veillard
The balloon device is automatically added to qemu guests if supported,
but it may be useful to desactivate it. The simplest to not change the
existing behaviour is to allow
<memballoon type="none"/>
as an extra option to desactivate it (it is automatically added if the
memballoon construct is missing for the domain).
The following simple patch just adds the extra option and does not
change the default behaviour but avoid creating a balloon device if
type="none" is used.
Daniel
P.S.: there is also a XEN type uspposedly for xen but I was unable to
find any baloon code in our current xen driver so no change there
--
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/
14 years, 3 months
[libvirt] [PATCH] nodeinfo: skip offline CPUs
by Eric Blake
https://bugzilla.redhat.com/622515 - When hot-unplugging CPUs,
libvirt failed to start a guest that had been pinned to CPUs that
were still online, because it was failing to read status from
unrelated offline CPUs.
Tested on a dual-core laptop, where I also discovered that, per
http://www.cyberciti.biz/files/linux-kernel/Documentation/cpu-hotplug.txt,
/sys/devices/system/cpu/cpu0/online does not exist on systems where it
cannot be hot-unplugged.
* src/nodeinfo.c (linuxNodeInfoCPUPopulate): Ignore CPUs that are
currently offline. Detect readdir failure.
(parse_socket): Move guts...
(get_cpu_value): ...to new function, shared with...
(cpu_online): New function.
---
src/nodeinfo.c | 109 +++++++++++++++++++++++++++++++++++++------------------
1 files changed, 73 insertions(+), 36 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5ec1bcf..6286aa2 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -23,6 +23,7 @@
#include <config.h>
+#include <stdbool.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
@@ -60,6 +61,58 @@
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
virNodeInfoPtr nodeinfo);
+/* Return the positive decimal contents of the given
+ * CPU_SYS_PATH/cpu%u/FILE, or -1 on error. If MISSING_OK and the
+ * file could not be found, return 1 instead of an error; this is
+ * because some machines cannot hot-unplug cpu0. */
+static int get_cpu_value(unsigned int cpu, const char *file, bool missing_ok)
+{
+ char *path;
+ FILE *pathfp;
+ char value_str[1024];
+ char *tmp;
+ int value = -1;
+
+ if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/%s", cpu, file) < 0) {
+ virReportOOMError();
+ return -1;
+ }
+
+ pathfp = fopen(path, "r");
+ if (pathfp == NULL) {
+ if (missing_ok && errno == ENOENT)
+ value = 1;
+ else
+ virReportSystemError(errno, _("cannot open %s"), path);
+ goto cleanup;
+ }
+
+ if (fgets(value_str, sizeof(value_str), pathfp) == NULL) {
+ virReportSystemError(errno, _("cannot read from %s"), path);
+ goto cleanup;
+ }
+ if (virStrToLong_i(value_str, &tmp, 10, &value) < 0) {
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
+ _("could not convert '%s' to an integer"),
+ value_str);
+ goto cleanup;
+ }
+
+cleanup:
+ if (pathfp)
+ fclose(pathfp);
+ VIR_FREE(path);
+
+ return value;
+}
+
+/* Check if CPU is online via CPU_SYS_PATH/cpu%u/online. Return 1 if online,
+ 0 if offline, and -1 on error. */
+static int cpu_online(unsigned int cpu)
+{
+ return get_cpu_value(cpu, "online", cpu == 0);
+}
+
static unsigned long count_thread_siblings(unsigned int cpu)
{
unsigned long ret = 0;
@@ -106,41 +159,7 @@ cleanup:
static int parse_socket(unsigned int cpu)
{
- char *path;
- FILE *pathfp;
- char socket_str[1024];
- char *tmp;
- int socket = -1;
-
- if (virAsprintf(&path, CPU_SYS_PATH "/cpu%u/topology/physical_package_id",
- cpu) < 0) {
- virReportOOMError();
- return -1;
- }
-
- pathfp = fopen(path, "r");
- if (pathfp == NULL) {
- virReportSystemError(errno, _("cannot open %s"), path);
- VIR_FREE(path);
- return -1;
- }
-
- if (fgets(socket_str, sizeof(socket_str), pathfp) == NULL) {
- virReportSystemError(errno, _("cannot read from %s"), path);
- goto cleanup;
- }
- if (virStrToLong_i(socket_str, &tmp, 10, &socket) < 0) {
- nodeReportError(VIR_ERR_INTERNAL_ERROR,
- _("could not convert '%s' to an integer"),
- socket_str);
- goto cleanup;
- }
-
-cleanup:
- fclose(pathfp);
- VIR_FREE(path);
-
- return socket;
+ return get_cpu_value(cpu, "topology/physical_package_id", false);
}
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
@@ -153,6 +172,8 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
unsigned long cur_threads;
int socket;
unsigned long long socket_mask = 0;
+ unsigned int remaining;
+ int online;
nodeinfo->cpus = 0;
nodeinfo->mhz = 0;
@@ -222,15 +243,25 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
/* OK, we've parsed what we can out of /proc/cpuinfo. Get the socket
* and thread information from /sys
*/
+ remaining = nodeinfo->cpus;
cpudir = opendir(CPU_SYS_PATH);
if (cpudir == NULL) {
virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
return -1;
}
- while ((cpudirent = readdir(cpudir))) {
+ while (errno = 0, remaining && (cpudirent = readdir(cpudir))) {
if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
continue;
+ online = cpu_online(cpu);
+ if (online < 0) {
+ closedir(cpudir);
+ return -1;
+ }
+ if (!online)
+ continue;
+ remaining--;
+
socket = parse_socket(cpu);
if (socket < 0) {
closedir(cpudir);
@@ -249,6 +280,12 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
if (cur_threads > nodeinfo->threads)
nodeinfo->threads = cur_threads;
}
+ if (errno) {
+ virReportSystemError(errno,
+ _("problem reading %s"), CPU_SYS_PATH);
+ closedir(cpudir);
+ return -1;
+ }
closedir(cpudir);
--
1.7.1
14 years, 3 months
[libvirt] [PATCH] Avoid autogen loop in VPATH builds
by Jiri Denemark
---
cfg.mk | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 7226828..e7fd63c 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -459,7 +459,7 @@ ifeq (0,$(MAKELEVEL))
# b653eda3ac4864de205419d9f41eec267cb89eeb
_submodule_hash = sed 's/^[ +-]//;s/ .*//'
_update_required := $(shell \
- test -f po/Makevars || { echo 1; exit; }; \
+ test -f $(srcdir)/po/Makevars || { echo 1; exit; }; \
cd '$(srcdir)'; \
actual=$$(git submodule status | $(_submodule_hash); \
git hash-object bootstrap.conf; \
--
1.7.2
14 years, 3 months
[libvirt] [PATCH 1/2] Fix return value usage
by Doug Goldstein
Fix the error checking to use the return value from brAddTap() instead
of checking the current errno value which might have been changed by
clean up calls inside of brAddTap().
Signed-off-by: Doug Goldstein <cardoe(a)gentoo.org>
---
src/uml/uml_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index bef8c38..025169f 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -141,7 +141,7 @@ umlConnectTapDevice(virDomainNetDefPtr net,
tapmac,
0,
&tapfd))) {
- if (errno == ENOTSUP) {
+ if (err == ENOTSUP) {
/* In this particular case, give a better diagnostic. */
umlReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to add tap interface to bridge. "
--
1.7.2
14 years, 3 months
[libvirt] [PATCH] qemu: Hack around asynchronous device_del
by Jiri Denemark
device_del command is not synchronous for PCI devices, it merely asks
the guest to release the device and returns. If the host wants to use
that device before the guest actually releases it, we are in big
trouble. To avoid this, we already added a loop which waits up to 10
seconds until the device is actually released before we do anything else
with that device. But we only added this loop for managed PCI devices
before we try reattach them back to the host.
However, we need to wait even for non-managed devices. We don't reattach
them automatically, but we still want to prevent the host from using it.
This was revealed thanks to sVirt: when we relabel sysfs files
corresponding to the PCI device before the guest finished releasing the
device, qemu is no longer allowed to access those files and if it wants
(as a result of guest's request) to write anything to them, it just
exits, which kills the guest.
This is not a proper fix and needs some further work both on libvirt and
qemu side in the future.
---
I was thinking about another possibility to implement this since the
wait loop doesn't have to be connected to the reattach action. We could
move that loop into a separate function and let qemudReattach function
do just what its name suggest. But in that case, we would need to add
the call to such function before every call to qemudReattach, which
doesn't look any better to me.
Jirka
src/qemu/qemu_driver.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 57b8271..e4f47d4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3209,16 +3209,17 @@ qemuPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED,
static void
-qemudReattachManagedDevice(pciDevice *dev, struct qemud_driver *driver)
+qemudReattachPciDevice(pciDevice *dev, struct qemud_driver *driver)
{
int retries = 100;
+ while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
+ && retries) {
+ usleep(100*1000);
+ retries--;
+ }
+
if (pciDeviceGetManaged(dev)) {
- while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
- && retries) {
- usleep(100*1000);
- retries--;
- }
if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to re-attach PCI device: %s"),
@@ -3264,7 +3265,7 @@ qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i);
- qemudReattachManagedDevice(dev, driver);
+ qemudReattachPciDevice(dev, driver);
}
pciDeviceListFree(pcidevs);
@@ -8997,7 +8998,7 @@ static int qemudDomainDetachHostPciDevice(struct qemud_driver *driver,
pciDeviceListDel(driver->activePciHostdevs, pci);
if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0)
ret = -1;
- qemudReattachManagedDevice(pci, driver);
+ qemudReattachPciDevice(pci, driver);
pciFreeDevice(pci);
}
--
1.7.2
14 years, 3 months
[libvirt] Libvirt storage api and lvm .cache issue
by Thomas Graves
Hello all,
I am using the libvirt storage api with LVM storage on rhel5 with xen and I
noticed that when using the api to create/delete lv's they get added to
/etc/lvm/cache/.cache but old deleted ones never get removed from the
.cache. This causes dmeventd to keep trying to access them but can't which
fills up the logs in /var/log.
Has anyone seen this or have a fix for it? I guess I could force a vgscan
or something to clean it up just seems like I shouldn't need to.
Thanks,
Tom
14 years, 3 months
[libvirt] [PATCH] Add "ubd" to the list of disk prefixes
by Soren Hansen
virDiskNameToIndex has a list of disk name prefixes that it uses in the
process of finding the disk's index. This list is missing "ubd" which
is the disk prefix used for UML domains.
Signed-off-by: Soren Hansen <soren(a)linux2go.dk>
---
src/util/util.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 8f2a17e..c173e49 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2367,7 +2367,7 @@ const char *virEnumToString(const char *const*types,
int virDiskNameToIndex(const char *name) {
const char *ptr = NULL;
int idx = 0;
- static char const* const drive_prefix[] = {"fd", "hd", "vd", "sd", "xvd"};
+ static char const* const drive_prefix[] = {"fd", "hd", "vd", "sd", "xvd", "ubd"};
unsigned int i;
for (i = 0; i < ARRAY_CARDINALITY(drive_prefix); i++) {
--
1.7.0.4
14 years, 3 months
[libvirt] [PATCH] build-sys: only build the test programs during the check phase.
by Diego Elio Pettenò
This avoids building the tests when testing libvirt is not the aim.
---
tests/Makefile.am | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 71c2c74..1dc7f66 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -72,42 +72,42 @@ EXTRA_DIST = \
domainsnapshotxml2xmlin \
qemuhelpdata
-noinst_PROGRAMS = virshtest conftest \
+check_PROGRAMS = virshtest conftest \
nodeinfotest statstest qparamtest
if WITH_XEN
-noinst_PROGRAMS += xml2sexprtest sexpr2xmltest \
+check_PROGRAMS += xml2sexprtest sexpr2xmltest \
reconnect xmconfigtest xencapstest
endif
if WITH_QEMU
-noinst_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuargv2xmltest qemuhelptest
+check_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuargv2xmltest qemuhelptest
endif
if WITH_ESX
-noinst_PROGRAMS += esxutilstest vmx2xmltest xml2vmxtest
+check_PROGRAMS += esxutilstest vmx2xmltest xml2vmxtest
endif
if WITH_SECDRIVER_SELINUX
-noinst_PROGRAMS += seclabeltest
+check_PROGRAMS += seclabeltest
endif
if WITH_SECDRIVER_APPARMOR
-noinst_PROGRAMS += secaatest
+check_PROGRAMS += secaatest
endif
if WITH_CIL
-noinst_PROGRAMS += object-locking
+check_PROGRAMS += object-locking
endif
-noinst_PROGRAMS += networkxml2xmltest
+check_PROGRAMS += networkxml2xmltest
-noinst_PROGRAMS += nwfilterxml2xmltest
+check_PROGRAMS += nwfilterxml2xmltest
-noinst_PROGRAMS += storagevolxml2xmltest storagepoolxml2xmltest
+check_PROGRAMS += storagevolxml2xmltest storagepoolxml2xmltest
-noinst_PROGRAMS += nodedevxml2xmltest
+check_PROGRAMS += nodedevxml2xmltest
-noinst_PROGRAMS += interfacexml2xmltest
+check_PROGRAMS += interfacexml2xmltest
test_scripts = \
capabilityschematest \
@@ -181,7 +181,7 @@ TESTS += secaatest
endif
if WITH_LIBVIRTD
-noinst_PROGRAMS += eventtest
+check_PROGRAMS += eventtest
TESTS += eventtest
endif
--
1.7.2
14 years, 3 months
[libvirt] [PATCH 1/2] util: add wrapper function to check if address string is an ip
by Justin Clift
---
src/util/network.c | 37 +++++++++++++++++++++++++++++++++++++
src/util/network.h | 2 ++
2 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/src/util/network.c b/src/util/network.c
index 6e24792..2e0cf9c 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -54,6 +54,43 @@ static int getIPv6Addr(virSocketAddrPtr addr, virIPv6AddrPtr tab) {
}
/**
+ * virIsNumericIPAddr:
+ * @address: a text string containing a host name or IPv4/IPv6 address
+ *
+ * Given a text string, determines whether it contains a valid IPv4/IPv6
+ * address.
+ *
+ * Returns 0 if the given string is an IPv4 or IPv6 address, or non-zero
+ * for any other condition.
+ */
+int
+virIsNumericIPAddr(const char *address) {
+ int returnCode;
+ struct addrinfo hints;
+ struct addrinfo *res = NULL;
+
+ /* Sanity check we've been given a text string */
+ if (address == NULL)
+ return(-1);
+
+ /* Do a name lookup on the given string, and see what we're given back */
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = AF_UNSPEC; /* Doesn't matter whether IPv4 or IPv6 */
+ hints.ai_socktype = 0; /* Socket type doesn't matter */
+ hints.ai_flags = AI_NUMERICHOST; /* Force a numeric IP address check */
+ hints.ai_protocol = 0; /* Doesn't matter which protocol */
+ returnCode = getaddrinfo (address, NULL, &hints, &res);
+ if (0 == returnCode) {
+ /* A result was returned. This means the text string we were
+ * given is a valid IP address. We now free the result(s) we
+ * were given, and pass back the return code */
+ freeaddrinfo(res);
+ }
+
+ return returnCode;
+}
+
+/**
* virSocketParseAddr:
* @val: a numeric network address IPv4 or IPv6
* @addr: where to store the return value.
diff --git a/src/util/network.h b/src/util/network.h
index 5307c8c..c2e88d8 100644
--- a/src/util/network.h
+++ b/src/util/network.h
@@ -24,6 +24,8 @@ typedef union {
} virSocketAddr;
typedef virSocketAddr *virSocketAddrPtr;
+int virIsNumericIPAddr (const char *address);
+
int virSocketParseAddr (const char *val,
virSocketAddrPtr addr,
int hint);
--
1.7.2.1
14 years, 3 months
Re: [libvirt] [PATCH] allow memballoon type of none to desactivate it
by Daniel P. Berrange
On Tue, Aug 10, 2010 at 12:09:47AM +0200, Daniel Veillard wrote:
> Mail disapeared when trying to send it... sending again ...
>
> On Mon, Aug 09, 2010 at 07:38:32PM +0100, Daniel P. Berrange wrote:
> > On Mon, Aug 09, 2010 at 08:34:23PM +0200, Daniel Veillard wrote:
> > > Grumpf ... that mean at the internal stucture level we need to add an
> > > extra field, that is detected at a completely different time in parsing
> > > too ... more complex in general, but I can understand the purity POV.
> >
> > I don't see this as a real problem. It is no different from the way that
> > we automatically add <controller> devices at the end of parsing, if we saw
> > any <disk> or <channel> devices previously.
>
> Huh ??? You can have a controller without a disk and you don't
> automagically add a controller even if nothing was suggested by the
> user, it is widely different. First you need to keep a tristate for
> def->balloonable, was that attribute defined, or not, then add the
> error case if it's not "yes" or "no", then you need to care for
> all the different cases of the tristate when you actually parse
> the devices section, how do you handle <memballon> definition when
> balloonable="no" was found, discard, error ?
We'd raise an error becasue the requested config doesn't make sense.
It doesn't actually need to be a tri-state - ballooning is either
enabled or not. If a user hasn't specified which in the XML, then
we'd pick the hypervisor default for that.
> I find this different because you provide the information in 2 places
> at different levels and you ned to cope with conflict ... while still
> maintaining the current behaviour of autoadding.
> This then breaks nearly all the qemu XML tests due to the extra
> balloonable="yes" added automatically to the output since the qemu
> driver makes that a default.
Yep, that's an unavoidable side-effect of adding more default data
to our XML output, but we've hit this many times in the past. eg
adding disk controllers, required us to change every single test
datafile.
> And now you have to explain in documentation all the various cases
> instead of a simple one liner about using "none".
>
> When data is defined at 2 different places by design, well you designed
> a mess. And in that case that's cerainly true. And after looking at the
> situation in detail, no I don't by the "purity" point of view, this
> actually make things *harder* both for user and from a implementation
> point of view.
> I firmly think that in that case the proper way to do this is to
> keep the information in one place and that place is the <memballoon>
> device, type='none' may look ugly to you, but at least the behaviour
> will be predictable, users will know where to look and that can be
> explained to the in a single line/sentence.
Neither option is keeping all the information in one place.
We already have a four-way split of memory information with
<memory> vs <currentMemory>, <memoryBacking> and now <memballoon>.
The first three are defining attributes of the VM itself, while
the latter is defining information about the device - specifically
we added it to provide info about PCI device IDs & the device alias.
Whether a guest domain supports ballooning or not is an attribute
of the domain itself. If there is no balloon, then there is no
device to provide information on, so none of the data that appears
under <memballoon> even exists, nor can you pass this device
element to the hotplug APIs, because there's no actal device
behind it. This makes the <memballoon> element non-sensical to
me.
> Current patch provided, I strongly suggest to not continue down this
> line and apply the previous patch instead.
To be fair, I don't actually like either patch, I'm just going for
what I think is the least worst option long term. If we could go
back in time, I'd never have enabled the balloon device by default
in the first place and required an explicit <memballoon>.
I'd actually prefer to just ignore this problem completely, and
declare that all KVM guest must always have a balloon and not
provide any way to disable it. The issue of limited PCI slots is
really a QEMU flaw - it should be providing a PCI bridge support
so you can have a much less limited number of PCI slots.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
14 years, 3 months
