- Devel - libvirt List Archives

[libvirt] [PATCH] locking: Add io_timeout to sanlock
by Michal Privoznik 27 Oct '15

27 Oct '15

https://bugzilla.redhat.com/show_bug.cgi?id=1251190 So, if domain loses access to storage, sanlock tries to kill it after some timeout. So far, the default is 80 seconds. But for some scenarios this might not be enough. We should allow users to adjust the timeout according to their needs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/locking/libvirt_sanlock.aug | 1 + src/locking/lock_driver_sanlock.c | 8 +++++++- src/locking/sanlock.conf | 7 +++++++ src/locking/test_libvirt_sanlock.aug.in | 1 + 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/locking/libvirt_sanlock.aug b/src/locking/libvirt_sanlock.aug index a78a444..8843590 100644 --- a/src/locking/libvirt_sanlock.aug +++ b/src/locking/libvirt_sanlock.aug @@ -22,6 +22,7 @@ module Libvirt_sanlock = | int_entry "host_id" | bool_entry "require_lease_for_disks" | bool_entry "ignore_readonly_and_shared_disks" + | int_entry "io_timeout" | str_entry "user" | str_entry "group" let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index e052875..f1dbbaf 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -73,6 +73,7 @@ struct _virLockManagerSanlockDriver { int hostID; bool autoDiskLease; char *autoDiskLeasePath; + unsigned int io_timeout; /* under which permissions does sanlock run */ uid_t user; @@ -151,6 +152,10 @@ static int virLockManagerSanlockLoadConfig(const char *configFile) else driver->requireLeaseForDisks = !driver->autoDiskLease; + p = virConfGetValue(conf, "io_timeout"); + CHECK_TYPE("io_timeout", VIR_CONF_ULONG); + if (p) driver->io_timeout = p->l; + p = virConfGetValue(conf, "user"); CHECK_TYPE("user", VIR_CONF_STRING); if (p) { @@ -338,7 +343,7 @@ static int virLockManagerSanlockSetupLockspace(void) * or we can fallback to polling. */ retry: - if ((rv = sanlock_add_lockspace(&ls, 0)) < 0) { + if ((rv = sanlock_add_lockspace_timeout(&ls, 0, driver->io_timeout)) < 0) { if (-rv == EINPROGRESS && --retries) { #ifdef HAVE_SANLOCK_INQ_LOCKSPACE /* we have this function which blocks until lockspace change the @@ -404,6 +409,7 @@ static int virLockManagerSanlockInit(unsigned int version, driver->requireLeaseForDisks = true; driver->hostID = 0; driver->autoDiskLease = false; + driver->io_timeout = 0; driver->user = (uid_t) -1; driver->group = (gid_t) -1; if (VIR_STRDUP(driver->autoDiskLeasePath, LOCALSTATEDIR "/lib/libvirt/sanlock") < 0) { diff --git a/src/locking/sanlock.conf b/src/locking/sanlock.conf index e5566ef..3a1a51c 100644 --- a/src/locking/sanlock.conf +++ b/src/locking/sanlock.conf @@ -54,6 +54,13 @@ #require_lease_for_disks = 1 # +# Sanlock is able to kill qemu processes on IO timeout. By its internal +# implementation, the current default is 80 seconds. If you need to adjust +# the value change the following variable. Value of zero means use the +# default sanlock timeout. +#io_timeout = 0 + +# # The combination of user and group under which the sanlock # daemon runs. Libvirt will chown created files (like # content of disk_lease_dir) to make sure sanlock daemon can diff --git a/src/locking/test_libvirt_sanlock.aug.in b/src/locking/test_libvirt_sanlock.aug.in index ef98ea6..7f66f81 100644 --- a/src/locking/test_libvirt_sanlock.aug.in +++ b/src/locking/test_libvirt_sanlock.aug.in @@ -6,5 +6,6 @@ module Test_libvirt_sanlock = { "disk_lease_dir" = "/var/lib/libvirt/sanlock" } { "host_id" = "1" } { "require_lease_for_disks" = "1" } +{ "io_timeout" = "0" } { "user" = "root" } { "group" = "root" } -- 2.4.10

2 2

[libvirt] [PATCH 0/4] HACKING: Improve handling
by Andrea Bolognani 27 Oct '15

27 Oct '15

Generate the HACKING file at dist time like we're already doing for NEWS, AUTHORS and ChangeLog. The file shouldn't be tracked by git as it's not a source file but rather a generated one; because of this, it should also be generated in $(builddir) as opposed to $(top_srcdir). Cheers. Andrea Bolognani (4): HACKING: Generate inside the build directory HACKING: Remove generated copy HACKING: Add to ignore file HACKING: Include in EXTRA_DIST .gitignore | 1 + HACKING | 1008 ----------------------------------------------------------- Makefile.am | 3 +- cfg.mk | 3 +- 4 files changed, 4 insertions(+), 1011 deletions(-) delete mode 100644 HACKING -- 2.4.3

3 7

[libvirt] [PATCH v2 0/2] reorder usb controllers correctly
by Pavel Hrdina 27 Oct '15

27 Oct '15

new in v2: - used git format-patch -b - added a test case Pavel Hrdina (2): domain-conf: reorder usb controllers so the master is first test: add test case for usb controller order src/conf/domain_conf.c | 21 +++++++++---- .../qemuxml2argv-controller-usb-order.xml | 32 ++++++++++++++++++++ .../qemuxml2xmlout-controller-usb-order.xml | 34 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-usb-order.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-controller-usb-order.xml -- 2.6.2

2 3

[libvirt] [PATCH] domain-conf: reorder usb controllers so the master is first
by Pavel Hrdina 27 Oct '15

27 Oct '15

USB controllers can share the same 'index' which indicates, that there is some sort of master-companion relationship. Reorder the controllers in XML in to place the master controller before its companions. This is required by QEMU to not fail with error message: error: internal error: process exited while connecting to monitor: 2015-10-26T16:25:17.630265Z qemu-system-x86_64: -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6: USB bus 'usb.0' not found Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1166452 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/conf/domain_conf.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0c559d2..3f22de2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13397,6 +13397,7 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, int idx; /* Tenatively plan to insert controller at the end. */ int insertAt = -1; + virDomainControllerDefPtr current = NULL; /* Then work backwards looking for controllers of * the same type. If we find a controller with a @@ -13404,17 +13405,27 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, * that position */ for (idx = (def->ncontrollers - 1); idx >= 0; idx--) { - /* If bus matches and current controller is after - * new controller, then new controller should go here */ - if (def->controllers[idx]->type == controller->type && - def->controllers[idx]->idx > controller->idx) { - insertAt = idx; - } else if (def->controllers[idx]->type == controller->type && - insertAt == -1) { - /* Last controller with match bus is before the - * new controller, then put new controller just after - */ - insertAt = idx + 1; + current = def->controllers[idx]; + if (current->type == controller->type) { + if (current->idx > controller->idx) { + /* If bus matches and current controller is after + * new controller, then new controller should go here + * */ + insertAt = idx; + } else if (controller->info.mastertype == VIR_DOMAIN_CONTROLLER_MASTER_NONE && + current->info.mastertype != VIR_DOMAIN_CONTROLLER_MASTER_NONE && + current->idx == controller->idx) { + /* If bus matches and index matches and new controller is + * master and current isn't a master, then new controller + * should go here to be placed before its companion + */ + insertAt = idx; + } else if (insertAt == -1) { + /* Last controller with match bus is before the + * new controller, then put new controller just after + */ + insertAt = idx + 1; + } } } -- 2.6.2

2 1

[libvirt] [PATCH v2 0/5] qemu: hostdev: Unify naming
by Andrea Bolognani 27 Oct '15

27 Oct '15

Changes in v2: * make new names consistent with virHostdev (thanks John) * rename virHostdevUpdateDomainActiveDevices() as well - this was the only function in virHostdev that didn't conform to the module's naming conventions Cheers. Andrea Bolognani (5): hostdev: Rename virHostdevUpdateDomainActiveDevices() qemu: hostdev: Unify naming for qemuHostdevPrepare*Devices() qemu: hostdev: Unify naming for qemuHostdevReAttach*Devices() qemu: hostdev: Unify naming for qemuHostdevUpdateActive*Devices() qemu: hostdev: Introduce qemuHostdevUpdateActiveDomainDevices() src/libvirt_private.syms | 2 +- src/libxl/libxl_driver.c | 2 +- src/qemu/qemu_hostdev.c | 108 +++++++++++++++++++++++++++-------------------- src/qemu/qemu_hostdev.h | 69 +++++++++++++++--------------- src/qemu/qemu_hotplug.c | 20 ++++----- src/qemu/qemu_process.c | 14 ++---- src/util/virhostdev.c | 2 +- src/util/virhostdev.h | 2 +- 8 files changed, 117 insertions(+), 102 deletions(-) -- 2.4.3

2 7

[libvirt] [PATCH] tests: Fix virnetdaemontest to be skipped in non-yajl environment
by Prerna 27 Oct '15

27 Oct '15

When libvirt is compiled without yajl-devel, virnetdaemontest fails. This causes 'make check' to fail with nondescript errors, such as following: [snip]............................... PASS: read-bufsiz PASS: read-non-seekable PASS: start TEST: virsh-uriprecedence .... 4 OK PASS: virsh-uriprecedence PASS: vcpupin /home/prerna/trees/libvirt/tests/virsh-all: skipping test: This test is very expensive, so it is disabled by default. To run it anyway, rerun: make check VIR_TEST_EXPENSIVE=1 SKIP: virsh-all PASS: virsh-optparse PASS: virsh-schedinfo PASS: virsh-synopsis PASS: virsh-undefine ======================================= 1 of 108 tests failed (7 tests were not run) Please report to libvir-list(a)redhat.com ======================================= make[2]: *** [check-TESTS] Error 1 make[2]: Leaving directory `/home/prerna/build/libvirt/tests' make[1]: *** [check-am] Error 2 make[1]: Leaving directory `/home/prerna/build/libvirt/tests' make: *** [check-recursive] Error 1 ..................................... This patch skips virnetdaemontest in absence of yajl-devel, so that make check gracefully runs to completion. Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com> --- tests/virnetdaemontest.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 24cbd54..17d1f7f 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -290,6 +290,11 @@ mymain(void) { int ret = 0; +#if !WITH_YAJL + fputs("libvirt not compiled with yajl, skipping this test\n", stderr); + return EXIT_AM_SKIP; +#endif + if (virInitialize() < 0 || virEventRegisterDefaultImpl() < 0) { virDispatchError(NULL); -- 1.7.1

2 3

[libvirt] [PATCH 0/4] qemu: hostdev: Unify naming
by Andrea Bolognani 26 Oct '15

26 Oct '15

I was working on the QEMU hostdev code and the inconsistent naming used kept making things just that little harder for me to follow, so I unified it. Cheers. Andrea Bolognani (4): qemu: hostdev: Unify naming for qemuPrepareHost*Devices() qemu: hostdev: Unify naming for qemuDomainReAttachHost*Devices() qemu: hostdev: Unify naming for qemuUpdateActiveHost*Devices() qemu: hostdev: Introduce qemuUpdateActiveHostDevices() src/qemu/qemu_hostdev.c | 74 ++++++++++++++++++++++++++++++------------------- src/qemu/qemu_hostdev.h | 36 ++++++++++++------------ src/qemu/qemu_hotplug.c | 13 ++++----- src/qemu/qemu_process.c | 8 +----- 4 files changed, 72 insertions(+), 59 deletions(-) -- 2.4.3

3 8

[libvirt] [PATCH v2 0/2] tests: Add nodeinfo test data utility scripts
by Andrea Bolognani 23 Oct '15

23 Oct '15

Changes from v1: * use a better approach to copy host data (thanks Martin) * acknowledge the fact that the scripts require bash * other small cleanups and improvements Andrea Bolognani (2): tests: Add script to display nodeinfo test data tests: Add script to copy nodeinfo test data from host tests/nodeinfodata/copy-from-host.sh | 113 +++++++++++++++++++++++++++++++++++ tests/nodeinfodata/display.sh | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100755 tests/nodeinfodata/copy-from-host.sh create mode 100755 tests/nodeinfodata/display.sh -- 2.4.3

3 7

[libvirt] outreachy 2015
by mesode akwe 22 Oct '15

22 Oct '15

hello! i am mesode from cameroon and am interested in participating in the libvirt-designer enhancement project. where do i go from here? thanks.

1 0

[libvirt] [RFC] Simplifying usage of {Enter, Exit}Monitor and {Begin/End}Job
by Jiri Denemark 22 Oct '15

22 Oct '15

Hi all. Looking at the qemu driver code, we have a lot of code with the following pattern: if (doSomething() < 0) goto cleanup; if (qemuDomainObjBeginJob() < 0) goto cleanup; if (doSomethingElse() < 0) goto endjob; qemuDomainObjEnterMonitor(); if (qemuMonitorSomething() < 0) { qemuDomainObjExitMonitor(); goto endjob; } if (qemuMonitorSomethingElse() < 0) { qemuDomainObjExitMonitor(); goto endjob; } if (qemuDomainObjExitMonitor() < 0) goto endjob; ... ret = 0; endjob: qemuDomainObjEndJob(); cleanup: ... return ret; Sometimes qemuDomainObjExitMonitor is in its own label instead of being explicitly called on every single error path. Sometimes qemuDomainObjEndJob is called explicitly followed by goto cleanup. In either case, it's pretty easy to get this wrong. It's too easy to jump to a wrong label (especially when moving code around) or forget to call the appropriate exit function before jumping to a label. So what if we make the code more robust by changing who needs to track whether we are in a monitor or have a job. Now it's the caller that tracks it while I think we could teach the job/monitor APIs themselves to track the state: bool inJob = false; bool inMonitor = false; if (doSomething() < 0) goto cleanup; if (qemuDomainObjBeginJob(..., &inJob) < 0) goto cleanup; if (doSomethingElse() < 0) goto cleanup; qemuDomainObjEnterMonitor(..., &inMonitor); if (qemuMonitorSomething() < 0) goto cleanup; if (qemuMonitorSomethingElse() < 0) goto cleanup; if (qemuDomainObjExitMonitor(..., &inMonitor) < 0) goto cleanup; ... ret = 0; cleanup: if (qemuDomainObjExitMonitor(..., &inMonitor) < 0) ret = -1; qemuDomainObjEndJob(..., &inJob); ... return ret; It's not a lot shorter or longer but it saves us from jumping to different labels and it makes sure we always exit the monitor and end the job. So is it worth the effort or do you thing it's even worse than before or do you have any other ideas? Thanks, Jirka

3 4