[libvirt] [PATCH] qemu: emit error when trying to update blkiotune group_name in qemuDomainChangeDiskLive
by Katerina Koukiou
All rest of blkiotune parameters are not updatable through UpdateDeviceFlags API.
https://bugzilla.redhat.com/show_bug.cgi?id=1601677
Signed-off-by: Katerina Koukiou <kkoukiou(a)redhat.com>
---
The BZ was requesting to add support for updating the group_name for
blkdeviotune here, though, all the rest of blkdeviotune elements are not
updatable though UpdateDeviceFlags API. I think emmiting error and
keeping all blkiodevtune parameters updatable in a consistent way is the
right way to go.
src/qemu/qemu_domain.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index de056272e8..0aa440e0b5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8791,6 +8791,9 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
CHECK_EQ(blkdeviotune.size_iops_sec,
"blkdeviotune size_iops_sec",
true);
+ CHECK_EQ(blkdeviotune.group_name,
+ "blkdeviotune group_name",
+ true);
if (disk->serial && STRNEQ_NULLABLE(disk->serial, orig_disk->serial)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
--
2.17.1
6 years, 3 months
[libvirt] [RFC v2 1/2] process: wait longer on kill per assigned Hostdev
by Christian Ehrhardt
It was found that in cases with host devices virProcessKillPainfully
might be able to send signal zero to the target PID for quite a while
with the process already being gone from /proc/<PID>.
That is due to cleanup and reset of devices which might include a
secondary bus reset that on top of the actions taken has a 1s delay
to let the bus settle. Due to that guests with plenty of Host devices
could easily exceed the default timeouts.
To solve that, this adds an extra delay of 2s per hostdev that is associated
to a VM.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
src/qemu/qemu_process.c | 5 +++--
src/util/virprocess.c | 18 +++++++++++++++---
src/util/virprocess.h | 1 +
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c4e33723d1..c20cc11e51 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6814,8 +6814,9 @@ qemuProcessKill(virDomainObjPtr vm, unsigned int flags)
return 0;
}
- ret = virProcessKillPainfully(vm->pid,
- !!(flags & VIR_QEMU_PROCESS_KILL_FORCE));
+ ret = virProcessKillPainfullyDelay(vm->pid,
+ !!(flags & VIR_QEMU_PROCESS_KILL_FORCE)
+ vm->def->nhostdevs * 2);
return ret;
}
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index f92b0dce37..aefe655771 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -344,15 +344,19 @@ int virProcessKill(pid_t pid, int sig)
* Returns 0 if it was killed gracefully, 1 if it
* was killed forcibly, -1 if it is still alive,
* or another error occurred.
+ *
+ * Callers can proide an extra delay to wait longer
+ * than the default.
*/
int
-virProcessKillPainfully(pid_t pid, bool force)
+virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int extradelay)
{
size_t i;
int ret = -1;
+ unsigned int delay = 75 + (extradelay*5);
const char *signame = "TERM";
- VIR_DEBUG("vpid=%lld force=%d", (long long)pid, force);
+ VIR_DEBUG("vpid=%lld force=%d delay=%u", (long long)pid, force, pid);
/* This loop sends SIGTERM, then waits a few iterations (10 seconds)
* to see if it dies. If the process still hasn't exited, and
@@ -360,9 +364,12 @@ virProcessKillPainfully(pid_t pid, bool force)
* wait up to 5 seconds more for the process to exit before
* returning.
*
+ * An extra delay can be specified for cases that are expected to clean
+ * up slower than usual.
+ *
* Note that setting @force could result in dataloss for the process.
*/
- for (i = 0; i < 75; i++) {
+ for (i = 0; i < delay; i++) {
int signum;
if (i == 0) {
signum = SIGTERM; /* kindly suggest it should exit */
@@ -405,6 +412,11 @@ virProcessKillPainfully(pid_t pid, bool force)
}
+int virProcessKillPainfully(pid_t pid, bool force)
+{
+ return virProcessKillPainfullyDelay(pid, force, 0);
+}
+
#if HAVE_SCHED_GETAFFINITY
int virProcessSetAffinity(pid_t pid, virBitmapPtr map)
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 3c5a882772..b72603ca8e 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -55,6 +55,7 @@ virProcessWait(pid_t pid, int *exitstatus, bool raw)
int virProcessKill(pid_t pid, int sig);
int virProcessKillPainfully(pid_t pid, bool force);
+int virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int);
int virProcessSetAffinity(pid_t pid, virBitmapPtr map);
--
2.17.1
6 years, 3 months
[libvirt] [RFC 0/2] Fix detection of slow guest shutdown
by Christian Ehrhardt
Hi,
I was recently looking into a case which essentially looked like this:
1. virsh shutdown guest
2. after <1 second the qemu process was gone from /proc/
3. but libvirt spun in virProcessKillPainfully because the process
was still reachable via signals
4. virProcessKillPainfully eventually fails after 15 seconds and the
guest stays in "in shutdown" state forever
This is not one of the common cases I've found for
virProcessKillPainfully to break:
- bad I/O e.g. NFS gets qemu stuck
- CPU overload stalls things to death
- qemu not being reaped (by init)
All of the above would have the process still available in /proc/<pid>
as Zombie or in uninterruptible sleep, but that is not true in my case.
It turned out that the case was dependent on the amount of hostdev resources
passed to the guest. Debugging showed that with 8 and more likely 16 GPUs
passed it took ~18 seconds from SIGTERM to "no more be reachable with signal 0".
I haven't conducted much more tests but stayed on the 16 GPU case, but
I'm rather sure more devices might make it take even longer.
Discussion with a few kernel folks revealed that the kill(2) man page
on signal 0 has to be taken very literal "check for the existence of a process
ID" - you can read this as "the PID exists, but the Process is no more".
I'm unsure why the kernel would take that much time to clean up as I
thought taking away /proc/<PID> would be almost the last step in the
cleanup of a task.
patch 2:
I happened to find that there seems to be no much better way than
signal-0 to check, but finding that this isn't reliable if the kernel
can still accept for quite some time even with the pid gone from all
other interfaces that I could find - so I wanted to suggest a fallback
in virProcessKillPainfully that considers the absence of /proc/<pid> as
a valid "the process is gone" as well on top of the ESRCH of signal-0.
We could also use the open FDs we have e.g. to the qemu monitor to see
if the remote end is dead, but that didn't seem more readable/reliable
to me and would have to cross quite some code to know about the FDs.
But maybe someone else here has the insight what exactly would take the
time in the kernel that I see and that might bring us to totally
different solutions (therefore RFC).
patch 1:
Finally after working through this code for a while I got the feeling
that if we are in a bad/non-responsive case after 10 seconds upgrading
to SIGKILL we should give it some more time to take effect. We reach
this in stressful cases only anyway and only if force is set, so then
waiting a bit more helps to resolve some of the other cases that I found
on the mailing list about virProcessKillPainfully being stuck.
If one has a personal interest in the 15 seconds we had before lets add
a VIR_WARN on 15 seconds if that would be better, but overall wait a bit
more.
P.S. Afer a short discussion with Daniel on IRC I'm also adding Alex explicitly
for passthrough experience.
P.P.S.: For now this really is only meant as an RFC to kick off the discussion.
I got taken away the system that I could trigger this case easily on before I
could complete a final verification. But the case is interesting enough
to start the discussion now.
Christian Ehrhardt (2):
process: wait longer 5->30s on hard shutdown
process: accept the lack of /proc/<pid> as valid process removal
src/util/virprocess.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
--
2.17.1
6 years, 3 months
[libvirt] [PATCH v3 0/4] iscsi-direct: first part
by clem＠lse.epita.fr
From: Clementine Hayat <clem(a)lse.epita.fr>
Hello,
This is the implementation of the iscsi-direct backend storage pool
version 3.
v1: https://www.redhat.com/archives/libvir-list/2018-July/msg00918.html
v2: https://www.redhat.com/archives/libvir-list/2018-July/msg01528.html
Best Regards,
--
Clementine Hayat
Clementine Hayat (4):
configure: Introduce libiscsi in build system
storage: Introduce iscsi_direct pool type
storage: Implement iscsi_direct pool backend
news: add storage pool iscsi-direct
configure.ac | 9 +-
docs/news.xml | 9 +
docs/schemas/storagepool.rng | 35 ++
docs/storage.html.in | 30 ++
m4/virt-libiscsi.m4 | 30 ++
m4/virt-storage-iscsi-direct.m4 | 44 ++
src/conf/domain_conf.c | 1 +
src/conf/storage_conf.c | 22 +-
src/conf/storage_conf.h | 1 +
src/conf/virstorageobj.c | 2 +
src/storage/Makefile.inc.am | 24 +
src/storage/storage_backend.c | 6 +
src/storage/storage_backend_iscsi_direct.c | 452 ++++++++++++++++++
src/storage/storage_backend_iscsi_direct.h | 6 +
src/storage/storage_driver.c | 1 +
.../pool-iscsi-direct-auth.xml | 14 +
.../pool-iscsi-direct.xml | 12 +
.../pool-iscsi-direct-auth.xml | 17 +
.../pool-iscsi-direct.xml | 14 +
tests/storagepoolxml2xmltest.c | 2 +
tools/virsh-pool.c | 3 +
21 files changed, 728 insertions(+), 6 deletions(-)
create mode 100644 m4/virt-libiscsi.m4
create mode 100644 m4/virt-storage-iscsi-direct.m4
create mode 100644 src/storage/storage_backend_iscsi_direct.c
create mode 100644 src/storage/storage_backend_iscsi_direct.h
create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-direct-auth.xml
create mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-direct.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-direct-auth.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-direct.xml
--
2.18.0
6 years, 3 months
[libvirt] [RFC] secdrivers remembering original labels
by Michal Privoznik
Dear list,
we have this very old bug [1] that I just keep pushing in front of me. I
made several attempts to fix it. However, none of them made into the
tree. I guess it's time to have discussion what to do about it. IIRC,
the algorithm that I implemented last was to keep original label in
XATTRs (among with some ref counter) and the last one to restore the
label will look there and find the original label. There was a problem
with two libvirtds fighting over a file on shared FS.
So I guess my question is can we come up with a design that would work?
Or at least work to the extent that we're satisfied with?
Personally, I like the XATTRs approach. And to resolve the NFS race we
can invent yet another lockspace to guard labelling - I also have a bug
for that [2] (although, I'm not that familiar with lockspaces). I guess
doing disk metadata locking is not going to be trivial, is it?
Michal
1: https://bugzilla.redhat.com/show_bug.cgi?id=547546
2: https://bugzilla.redhat.com/show_bug.cgi?id=1524792
6 years, 3 months
[libvirt] Release of libvirt-4.6.0
by Daniel Veillard
Okay it took a bit longer than usual but the relrase is out, it's tagged
in git and signed tarball and rpms have been pushed to the usual place, but
there was quite a few bug fixes going in after RC2 was cut.
ftp://libvirt.org/libvirt/
I also made libvirt-python 4.6.0 bindings release in a similar fashion
available at :
ftp://libvirt.org/libvirt/python/
So though the release note don't list any bug fixes we know there is a
batch of those in, otherwise this is mainly features and improvement
targetting QEmu and virsh users:
New features:
- qemu: Implement the HTM pSeries feature
Users can now decide whether HTM (Hardware Transactional Memory)
support should be available to the guest.
- qemu: Enable VNC console for mediated devices
Host devices now support a new atribute 'display' which can be used to
turn on frame buffer rendering on a vgpu mediated device instead of on
an emulated GPU, like QXL.
Improvements:
- qemu: Introduce a new video model of type 'none'
Introduce a new video model type that disables the automatic addition
of a video device to domains with 'graphics' specified in their XML.
This can be useful with GPU mediated devices which can serve as the
only rendering devices within the guest.
- virsh: Add --alias to attach-disk and attach-interface commands
Add option --alias to set customized device alias name when using
attach-disk or attach-interface commands.
- virsh: Support usb and sata address to attach-disk
Usb or sata address could be used when attach-disk with --address. For
example, use usb address as usb:<bus>.<port>, use sata address as
<controller>.<bus>.<unit>.
Thanks everybody for your help with this release, hopefully the next
one beginning of Sept will be more timely !
Enjoy,
Daniel
--
Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
6 years, 3 months
[libvirt] [PATCH] Fix include for xattr.h
by Martin Kletzander
The proper file that should be included is `sys/xattr.h` as that comes from
`glibc` and not `attr/xattr.h` which ships with the `attr` utility.
We're most probably not the only ones because `attr/xattr.h` added a #warning to
their include resulting in the following compilation errors:
In file included from securityselinuxlabeltest.c:31:0:
/usr/include/attr/xattr.h:5:2: error: #warning "Please change your <attr/xattr.h> includes to <sys/xattr.h>" [-Werror=cpp]
#warning "Please change your <attr/xattr.h> includes to <sys/xattr.h>"
^~~~~~~
In file included from securityselinuxhelper.c:37:0:
/usr/include/attr/xattr.h:5:2: error: #warning "Please change your <attr/xattr.h> includes to <sys/xattr.h>" [-Werror=cpp]
#warning "Please change your <attr/xattr.h> includes to <sys/xattr.h>"
^~~~~~~
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
Not pushing as build-breaker because we are still in freeze and I'd still like
someone with SELinux to check this patch. My VM with such system stopped
working just now =(
m4/virt-attr.m4 | 2 +-
tests/securityselinuxhelper.c | 2 +-
tests/securityselinuxlabeltest.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/m4/virt-attr.m4 b/m4/virt-attr.m4
index c55bd7dbd823..478549c17f12 100644
--- a/m4/virt-attr.m4
+++ b/m4/virt-attr.m4
@@ -5,7 +5,7 @@ AC_DEFUN([LIBVIRT_ARG_ATTR],[
])
AC_DEFUN([LIBVIRT_CHECK_ATTR],[
- LIBVIRT_CHECK_LIB([ATTR], [attr], [getxattr], [attr/xattr.h])
+ LIBVIRT_CHECK_LIB([ATTR], [attr], [getxattr], [sys/xattr.h])
])
AC_DEFUN([LIBVIRT_RESULT_ATTR],[
diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c
index 22b6709554b4..fe6f2b5fd9b0 100644
--- a/tests/securityselinuxhelper.c
+++ b/tests/securityselinuxhelper.c
@@ -34,7 +34,7 @@
#include <string.h>
#include <sys/vfs.h>
#include <unistd.h>
-#include <attr/xattr.h>
+#include <sys/xattr.h>
#ifndef NFS_SUPER_MAGIC
# define NFS_SUPER_MAGIC 0x6969
diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index c684989fd26e..48fee7cd285b 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -28,7 +28,7 @@
#include <selinux/selinux.h>
#include <selinux/context.h>
-#include <attr/xattr.h>
+#include <sys/xattr.h>
#include "internal.h"
#include "testutils.h"
--
2.18.0
6 years, 3 months
[libvirt] [PATCH] virrandom: Avoid undefined behaviour in virRandomBits
by Michal Privoznik
If nbits is 64 (or greater) then shifting 1ULL left is undefined.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virrandom.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virrandom.c b/src/util/virrandom.c
index 3c011a8615..7915f6531e 100644
--- a/src/util/virrandom.c
+++ b/src/util/virrandom.c
@@ -68,7 +68,9 @@ uint64_t virRandomBits(int nbits)
return 0;
}
- ret &= (1ULL << nbits) - 1;
+ if (nbits < 64)
+ ret &= (1ULL << nbits) - 1;
+
return ret;
}
--
2.16.4
6 years, 3 months
[libvirt] [PATCH] remote: daemon: Make sure that JSON symbols are properly loaded at startup
by Peter Krempa
Explicitly call virJSONInitialize at startup of the libvirt daemon so
that we are sure that the symbols in the compat library are properly
loaded. This will prevent any random failure from happening later on
when the daemon would want to use the JSON parser.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 4 ++++
src/remote/remote_daemon.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fc386e1699..70dfcc5e29 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2135,6 +2135,10 @@ virJSONValueObjectStealObject;
virJSONValueToString;
+# util/virjsoncompat.h
+virJSONInitialize;
+
+
# util/virkeycode.h
virKeycodeSetTypeFromString;
virKeycodeSetTypeToString;
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 9f3a5f38ad..8bbc3818bb 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -59,6 +59,7 @@
#include "virutil.h"
#include "virgettext.h"
#include "util/virnetdevopenvswitch.h"
+#include "virjsoncompat.h"
#include "driver.h"
@@ -1183,6 +1184,9 @@ int main(int argc, char **argv) {
exit(EXIT_FAILURE);
}
+ if (virJSONInitialize() < 0)
+ exit(EXIT_FAILURE);
+
daemonSetupNetDevOpenvswitch(config);
if (daemonSetupAccessManager(config) < 0) {
--
2.16.2
6 years, 3 months
[libvirt] [PATCH 0/3] conf: standardize naming
by Anya Harter
on vir*ObjListExport call stack
Anya Harter (3):
conf: rename structs used by Export function
conf: rename Export Callback functions
conf: rename Match functions
src/conf/virinterfaceobj.c | 10 +++++-----
src/conf/virnetworkobj.c | 16 ++++++++--------
src/conf/virnodedeviceobj.c | 4 ++--
src/conf/virsecretobj.c | 10 +++++-----
src/conf/virstorageobj.c | 24 ++++++++++++------------
5 files changed, 32 insertions(+), 32 deletions(-)
--
2.17.1
6 years, 3 months
