[libvirt] Seeking for advice on 'virsh console' problems debugging
by Roman Bogorodskiy
Hi,
I've been trying to understand why 'virsh console' doesn't work on
FreeBSD for some time and now I feel like I got stuck.
Background of the problem. Some time ago I started implementing console
support for the bhyve driver. I initially implemented it using nmdm(4)
device:
http://www.freebsd.org/cgi/man.cgi?query=nmdm&sektion=4&manpath=FreeBSD+1...
But 'virsh console' didn't work on it and exited immediately.
I though that it could be nmdm compatibility issue and decided to go
with the plan /dev/pts devices. But I got the same problem with it.
So I decided to try use console with qemu driver. And... I got the same
problem.
After some investigation, I spotted this block in tools/virsh-console.c:376
while (!con->quit) {
if (virCondWait(&con->cond, &con->lock) < 0) {
VIR_ERROR(_("unable to wait on console condition"));
goto cleanup;
}
}
The strange thing is that it doesn't look con->lock mutex before
waiting.
FreeBSD man page for pthread_cond_wait() says that it errors in that
situation:
[EPERM] The specified mutex was not locked by the
calling thread.
I replaced VIR_ERROR with virReportSystemError and it returns EPERM
indeed. So I've added virMutexLock() before calling virCondWait() and it
partially solved the problem for me -- 'virsh console' stopped exiting
immediately.
Actually, it started output for guest, but it's badly formatted: it
includes lots and lots of whitespaces. Input doesn't work properly
(cannot type in sensible stuff).
Here's a sample output, even libvirt messages are badly formatted (I
added some debug prints along the way).
http://people.freebsd.org/~novel/misc/ttyweirdness.png
So, e.g. dmesg output for the guest looks almost the same.
Looks like there's something wrong with the terminal settings, but how
could that be debugged?
PS Worth to mention that 'cu' worked all the time for me on these
devices (both created by bhyve and qemu).
Roman Bogorodskiy
11 years
[libvirt] [PATCH] Wildcard ignore tests/*test instead of listing every one
by Daniel P. Berrange
There is a forever growing list of test cases. It is just
not worth listing each one individually when a wildcard
can do the job.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
.gitignore | 73 +-------------------------------------------------------------
1 file changed, 1 insertion(+), 72 deletions(-)
diff --git a/.gitignore b/.gitignore
index cb60734..efbce50 100644
--- a/.gitignore
+++ b/.gitignore
@@ -145,85 +145,14 @@
/tests/*.log
/tests/*.pid
/tests/*.trs
-/tests/*xml2*test
/tests/commandhelper
-/tests/commandtest
-/tests/conftest
-/tests/cputest
-/tests/domainconftest
-/tests/domainsnapshotxml2xmltest
-/tests/esxutilstest
-/tests/eventtest
-/tests/fchosttest
-/tests/fdstreamtest
-/tests/hashtest
-/tests/jsontest
-/tests/libvirtdconftest
-/tests/lxcconf2xmltest
-/tests/metadatatest
-/tests/networkxml2argvtest
-/tests/nodeinfotest
-/tests/nwfilterxml2xmltest
-/tests/objecteventtest
+/tests/*test
/tests/object-locking
/tests/object-locking-files.txt
/tests/object-locking.cm[ix]
-/tests/openvzutilstest
-/tests/qemuagenttest
-/tests/qemuargv2xmltest
-/tests/qemucapabilitiestest
-/tests/qemuhelptest
-/tests/qemuhotplugtest
-/tests/qemumonitorjsontest
-/tests/qemumonitortest
-/tests/qemuxmlnstest
-/tests/qparamtest
/tests/reconnect
-/tests/secaatest
-/tests/seclabeltest
-/tests/securityselinuxlabeltest
-/tests/securityselinuxtest
-/tests/sexpr2xmltest
-/tests/shunloadtest
-/tests/sockettest
/tests/ssh
-/tests/statstest
-/tests/storagebackendsheepdogtest
-/tests/sysinfotest
/tests/test_conf
-/tests/utiltest
-/tests/viratomictest
-/tests/virauthconfigtest
-/tests/virbitmaptest
-/tests/virbuftest
-/tests/vircapstest
-/tests/vircgrouptest
-/tests/virdbustest
-/tests/virdrivermoduletest
-/tests/virendiantest
-/tests/virfiletest
-/tests/virhashtest
-/tests/viridentitytest
-/tests/virkeycodetest
-/tests/virkeyfiletest
-/tests/virkmodtest
-/tests/virlockspacetest
-/tests/virlogtest
-/tests/virnet*test
-/tests/virpcitest
-/tests/virportallocatortest
-/tests/virscsitest
-/tests/virshtest
-/tests/virstoragetest
-/tests/virstringtest
-/tests/virsystemdtest
-/tests/virtimetest
-/tests/viruritest
-/tests/virusbtest
-/tests/vmwarevertest
-/tests/vmx2xmltest
-/tests/xencapstest
-/tests/xmconfigtest
/tools/*.[18]
/tools/libvirt-guests.init
/tools/libvirt-guests.service
--
1.8.5.3
11 years
[libvirt] [PATCH] Add a mutex to serialize updates to firewall
by Daniel P. Berrange
For
https://bugzilla.redhat.com/show_bug.cgi?id=1066801
The nwfilter conf update mutex previously serialized
updates to the internal data structures for firewall
rules, and updates to the firewall itself. The latter
was recently turned into a read/write lock, and filter
instantiation allowed to proceed in parallel. It was
believed that this was ok, since each filter is created
on a seperate iptables/ebtables chain.
It turns out that there is a sutle lock ordering problem
on virNWFilterObjPtr instances. __virNWFilterInstantiateFilter
will hold a lock on the virNWFilterObjPtr it is instantiating.
This in turn invokes virNWFilterInstantiate which then invokes
virNWFilterDetermineMissingVarsRec which then invokes
virNWFilterObjFindByName. This iterates over every single
virNWFilterObjPtr in the list, locking them and checking their
name. So if 2 or more threads try to instantiate a filter in
parallel, they'll all hold 1 lock at the top level in the
__virNWFilterInstantiateFilter method which will cause the
other thread to deadlock in virNWFilterObjFindByName.
The fix is to add an exclusive mutex to serialize the
execution of __virNWFilterInstantiateFilter.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/nwfilter/nwfilter_driver.c | 6 ++++--
src/nwfilter/nwfilter_gentech_driver.c | 34 ++++++++++++++++++++++++++++++++--
src/nwfilter/nwfilter_gentech_driver.h | 2 +-
3 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 5908df7..2e89d07 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -200,7 +200,8 @@ nwfilterStateInitialize(bool privileged,
if (virNWFilterDHCPSnoopInit() < 0)
goto err_exit_learnshutdown;
- virNWFilterTechDriversInit(privileged);
+ if (virNWFilterTechDriversInit(privileged) < 0)
+ goto err_dhcpsnoop_shutdown;
if (virNWFilterConfLayerInit(virNWFilterDomainFWUpdateCB,
driverState) < 0)
@@ -251,6 +252,7 @@ error:
err_techdrivers_shutdown:
virNWFilterTechDriversShutdown();
+err_dhcpsnoop_shutdown:
virNWFilterDHCPSnoopShutdown();
err_exit_learnshutdown:
virNWFilterLearnShutdown();
@@ -327,10 +329,10 @@ nwfilterStateCleanup(void) {
if (driverState->privileged) {
virNWFilterConfLayerShutdown();
- virNWFilterTechDriversShutdown();
virNWFilterDHCPSnoopShutdown();
virNWFilterLearnShutdown();
virNWFilterIPAddrMapShutdown();
+ virNWFilterTechDriversShutdown();
nwfilterDriverLock(driverState);
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 8c5cd57..5144dce 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -55,15 +55,34 @@ static virNWFilterTechDriverPtr filter_tech_drivers[] = {
NULL
};
+/* Serializes instantiation of filters. This is neccessary
+ * to avoid lock ordering deadlocks. eg __virNWFilterInstantiateFilter
+ * will hold a lock on a virNWFilterObjPtr. This in turn invokes
+ * virNWFilterInstantiate which invokes virNWFilterDetermineMissingVarsRec
+ * which invokes virNWFilterObjFindByName. This iterates over every single
+ * virNWFilterObjPtr in the list. So if 2 threads try to instantiate a
+ * filter in parallel, they'll both hold 1 lock at the top level in
+ * __virNWFilterInstantiateFilter which will cause the other thread
+ * to dead lock in virNWFilterObjFindByName.
+ *
+ * XXX better long term solution is to make virNWFilterObjList use a
+ * hash table as is done for virDomainObjList. You can then get
+ * lockless lookup of objects by name.
+ */
+static virMutex updateMutex;
-void virNWFilterTechDriversInit(bool privileged) {
+int virNWFilterTechDriversInit(bool privileged) {
size_t i = 0;
VIR_DEBUG("Initializing NWFilter technology drivers");
+ if (virMutexInitRecursive(&updateMutex) < 0)
+ return -1;
+
while (filter_tech_drivers[i]) {
if (!(filter_tech_drivers[i]->flags & TECHDRV_FLAG_INITIALIZED))
filter_tech_drivers[i]->init(privileged);
i++;
}
+ return 0;
}
@@ -74,6 +93,7 @@ void virNWFilterTechDriversShutdown(void) {
filter_tech_drivers[i]->shutdown();
i++;
}
+ virMutexDestroy(&updateMutex);
}
@@ -935,6 +955,8 @@ _virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
int ifindex;
int rc;
+ virMutexLock(&updateMutex);
+
/* after grabbing the filter update lock check for the interface; if
it's not there anymore its filters will be or are being removed
(while holding the lock) and we don't want to build new ones */
@@ -962,6 +984,8 @@ _virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver,
foundNewFilter);
cleanup:
+ virMutexUnlock(&updateMutex);
+
return rc;
}
@@ -981,6 +1005,7 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
bool foundNewFilter = false;
virNWFilterReadLockFilterUpdates();
+ virMutexLock(&updateMutex);
rc = __virNWFilterInstantiateFilter(driver,
vmuuid,
@@ -1006,6 +1031,7 @@ virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
}
virNWFilterUnlockFilterUpdates();
+ virMutexUnlock(&updateMutex);
return rc;
}
@@ -1129,7 +1155,11 @@ _virNWFilterTeardownFilter(const char *ifname)
int
virNWFilterTeardownFilter(const virDomainNetDef *net)
{
- return _virNWFilterTeardownFilter(net->ifname);
+ int ret;
+ virMutexLock(&updateMutex);
+ ret = _virNWFilterTeardownFilter(net->ifname);
+ virMutexUnlock(&updateMutex);
+ return ret;
}
diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
index f4789e1..d72e040 100644
--- a/src/nwfilter/nwfilter_gentech_driver.h
+++ b/src/nwfilter/nwfilter_gentech_driver.h
@@ -31,7 +31,7 @@ virNWFilterTechDriverPtr virNWFilterTechDriverForName(const char *name);
int virNWFilterRuleInstAddData(virNWFilterRuleInstPtr res,
void *data);
-void virNWFilterTechDriversInit(bool privileged);
+int virNWFilterTechDriversInit(bool privileged);
void virNWFilterTechDriversShutdown(void);
enum instCase {
--
1.8.5.3
11 years
[libvirt] [PATCH] Fix locking in virsh console
by Roman Bogorodskiy
While I'm still fighting with console on FreeBSD, I decided to sumbit that fix
as it seems to be useful on its own. With that change I'm able to run virsh on
FreeBSD to connect to libvirtd on Linux and get console working properly there.
Roman Bogorodskiy (1):
Fix locking in virsh console
tools/virsh-console.c | 5 +++++
1 file changed, 5 insertions(+)
--
1.8.4.2
11 years
[libvirt] How to detect if qemu supports discard?
by Richard W.M. Jones
from src/qemu/qemu_command.c:
if (disk->discard) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DISCARD)) {
virBufferAsprintf(&opt, ",discard=%s",
virDomainDiskDiscardTypeToString(disk->discard));
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("discard is not supported by this QEMU binary"));
goto error;
}
}
However there appears to be no way to tell programmatically if the
qemu binary supports discard (ie. via libvirt). Therefore libguestfs
can never know when it is safe to use this option in the XML.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
11 years
[libvirt] [bug-report] libvirtd can't be connected by using the master branch with lots of VMs
by Wangpan
bug address: https://bugzilla.redhat.com/show_bug.cgi?id=1066801
I can reproduce it at master/v1.2.1-maint/v1.1.4-maint/v1.1.3.4 release branches everytime,
but cann't reproduce it at v1.2.1，so I believe this bug is imported by the maintain patches after v1.2.1 release.
Steps to Reproduce:
1.create many VMs(40 in my env) on the host
2.kill -9 `pid of libvirtd`
3.start libvirtd by using service libvirt-bin start
Actual results:
virsh version/list command is hung there without any responses
Expected results:
virsh version/list return the correct things
Additional info:
if there are few VMs(1 vm in my env), the virsh version command is OK
Thanks in advance!
2014-03-07
Wangpan
11 years
[libvirt] [PATCH 0/4] Add keepalive support in virsh
by Martin Kletzander
*** BLURB ***
Martin Kletzander (4):
virsh: Sort options alphabetically
virsh: Add keepalive in new vshConnect function
virsh: Prohibit virConnectOpen* functions in virsh
[DO_NOT_APPLY_UPSTREAM] virsh: add connection monitoring into
vshWatchJob
cfg.mk | 8 ++-
tests/virsh-optparse | 3 +-
tools/virsh-domain.c | 21 +++++---
tools/virsh.c | 141 +++++++++++++++++++++++++++++++++++++--------------
tools/virsh.h | 7 +++
tools/virsh.pod | 38 +++++++-------
6 files changed, 152 insertions(+), 66 deletions(-)
--
1.9.0
11 years
[libvirt] [PATCH] tests: Fix SELinux tests in VPATH build
by Jiri Denemark
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
Pushed as a build-breaker.
Notes:
abs_builddir is used in more places in tests/securityselinuxhelper.c but
since the path they are used in does not exist in abs_srcdir either, I
guess it doesn't really matter what prefix is used there.
tests/securityselinuxhelper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c
index 703381c..dbc4c29 100644
--- a/tests/securityselinuxhelper.c
+++ b/tests/securityselinuxhelper.c
@@ -278,7 +278,7 @@ const char *selinux_virtual_domain_context_path(void)
if (realis_selinux_enabled())
return realselinux_virtual_domain_context_path();
- return abs_builddir "/securityselinuxhelperdata/virtual_domain_context";
+ return abs_srcdir "/securityselinuxhelperdata/virtual_domain_context";
}
const char *selinux_virtual_image_context_path(void)
@@ -288,7 +288,7 @@ const char *selinux_virtual_image_context_path(void)
if (realis_selinux_enabled())
return realselinux_virtual_image_context_path();
- return abs_builddir "/securityselinuxhelperdata/virtual_image_context";
+ return abs_srcdir "/securityselinuxhelperdata/virtual_image_context";
}
#ifdef HAVE_SELINUX_LXC_CONTEXTS_PATH
@@ -299,7 +299,7 @@ const char *selinux_lxc_contexts_path(void)
if (realis_selinux_enabled())
return realselinux_lxc_contexts_path();
- return abs_builddir "/securityselinuxhelperdata/lxc_contexts";
+ return abs_srcdir "/securityselinuxhelperdata/lxc_contexts";
}
#endif
--
1.9.0
11 years
[libvirt] virDomainGetMaxVcpus does not work as expected
by Claudio Bley
Hi.
When calling virDomainGetMaxVcpus
(http://libvirt.org/html/libvirt-libvirt.html#virDomainGetMaxVcpus) on
an inactive domain, I receive this error:
scala> res2.getMaxVcpus()
libvirt: Domain Config error : Requested operation is not valid: domain is not running
org.libvirt.LibvirtException: Requested operation is not valid: domain is not running
at org.libvirt.ErrorHandler.processError(ErrorHandler.java:31)
at org.libvirt.ErrorHandler.processError(ErrorHandler.java:46)
at org.libvirt.Domain.getMaxVcpus(Domain.java:571)
at .<init>(<console>:13)
...
(this is from Java, but that doesn't matter)
The docs say:
> If the guest is inactive, this is basically the same as
> virConnectGetMaxVcpus(). If the guest is running this will reflect
> the maximum number of virtual CPUs the guest was booted with.
But, apparently, all the driver implementations for
virDomainGetMaxVcpus forward to
<driver>DomainGetVcpusFlags(.., VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM).
_______________________________,~~~~~~~~~~~~~~~~~~~~~~
$ git grep --show-function 'GetVcpusFlags.*AFFECT_LIVE'
src/esx/esx_driver.c=esxDomainGetMaxVcpus(virDomainPtr domain)
src/esx/esx_driver.c: return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
src/openvz/openvz_driver.c=static int openvzDomainGetMaxVcpus(virDomainPtr dom)
src/openvz/openvz_driver.c: return openvzDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
src/qemu/qemu_driver.c=qemuDomainGetMaxVcpus(virDomainPtr dom)
src/qemu/qemu_driver.c: return qemuDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
src/test/test_driver.c=testDomainGetMaxVcpus(virDomainPtr domain)
src/test/test_driver.c: return testDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
src/vbox/vbox_tmpl.c=vboxDomainGetMaxVcpus(virDomainPtr dom)
src/vbox/vbox_tmpl.c: return vboxDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE |
AFAICS, this was introduced with
commit 50c51f13e2af04afac46e181c4ed62581545a488
Author: Eric Blake <eblake(a)redhat.com>
Date: Mon Sep 27 16:37:53 2010 -0600
vcpu: make old API trivially wrap to new API
Whereas the function's contract was documented earlier by
commit b412cfadb502c76df095c2c4548c27abf7c4873f
Author: Daniel Veillard <veillard(a)redhat.com>
Date: Thu Mar 8 08:31:07 2007 +0000
To be honest, I'm not sure whether this worked as described at some
time in the past _at all_.
How to fix this? Change the documentation or the flag?
Claudio
11 years
[libvirt] [PATCH] qemu: Advertize <drivediscard/> in capabilities if 'discard' is supported.
by Richard W.M. Jones
The capabilities will have a <drivediscard/> feature if you are
allowed to use the qemu attribute:
<driver name='qemu' ... discard='unmap|ignore' />
The new capabilities XML looks like this:
<capabiltiies>
...
<guest>
<os_type>hvm</os_type>
...
<features>
<drivediscard/>
</features>
</guest>
</capabiltiies>
---
src/conf/capabilities.c | 3 ++-
src/qemu/qemu_capabilities.c | 4 ++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index c1c4ab8..ff5912a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -947,7 +947,8 @@ virCapabilitiesFormatXML(virCapsPtr caps)
STREQ(caps->guests[i]->features[j]->name, "nonpae") ||
STREQ(caps->guests[i]->features[j]->name, "ia64_be") ||
STREQ(caps->guests[i]->features[j]->name, "cpuselection") ||
- STREQ(caps->guests[i]->features[j]->name, "deviceboot")) {
+ STREQ(caps->guests[i]->features[j]->name, "deviceboot") ||
+ STREQ(caps->guests[i]->features[j]->name, "drivediscard")) {
virBufferAsprintf(&xml, " <%s/>\n",
caps->guests[i]->features[j]->name);
} else {
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index cae25e0..b755fb2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -778,6 +778,10 @@ virQEMUCapsInitGuest(virCapsPtr caps,
!virCapabilitiesAddGuestFeature(guest, "deviceboot", 1, 0))
goto error;
+ if (virQEMUCapsGet(qemubinCaps, QEMU_CAPS_DRIVE_DISCARD) &&
+ !virCapabilitiesAddGuestFeature(guest, "drivediscard", 0, 0))
+ goto error;
+
if (virCapabilitiesAddGuestDomain(guest,
"qemu",
NULL,
--
1.8.5.3
11 years
