March 2014 - Devel - libvirt List Archives

Re: [libvirt] Adding support to limit client connections for vnc/spice display driver
by Patil, Tushar 21 Mar '14

21 Mar '14

On Wed, Mar 19, 2014 at 06:41:16PM +0000, Patil, Tushar wrote: > > >> We are using KVM hypervisor driver for running OpenStack IaaS. Couple >> of months back we have reported one security issue [1] in OS. Basically >> we want to limit on the number of vnc client connections that can be >> opened by users for a given VM. I know that there is share policy where >> you can specify "VNC_SHARE_MODE_EXCLUSIVE" share mode. But it only allows >> one client connection to the vnc console, it will disconnect all previously >> opened vnc client connections. Since vnc display driver already has the >> data of client connections, it would be possible to add the logic to limit >> on the number of client connections. >> >> What we want is the ability to specify threshold how many vnc client >> connections should be allowed at any given point of time in the domain >> xml for graphics device (especially for vnc/spice)? >> >> Example > ><graphics type="vnc" autoport="yes" keymap="en-us" listen="127.0.0.1"/, share-policy="limit-connections", connections="5"> >> >> Add support to accept new share policy "limit-connections'. >> So in the above example, when user tries to open vnc display for the 6th >>time, then the request should be rejected. >> >> I need your expert opinion in deciding whether it's a good idea to add >> this support in the vnc/spice display driver or this kind of constraint >> should be imposed outside libvirt? > Libvirt isn't at all involved in vnc/spice connections. They happen > directly between QEMU & the client app, optionally via Nova's proxy > service. If QEMU had a setting for such a limit then libvirt could > set it, but AFAIK none exists, so there's nothing libvirt can do > about this currently. So options are either todo work in QEMU to > add this config feature, then wire it into libvirt, or address it in > the Nova proxy service. Thanks for your advice. I will add a new feature request in qemu. After this feature is added to qemu, I will then add a request in libvirt to use the newly added parameters. Thanks and Best Regards, Tushar ______________________________________________________________________ Disclaimer:This email and any attachments are sent in strictest confidence for the sole use of the addressee and may contain legally privileged, confidential, and proprietary data. If you are not the intended recipient, please advise the sender by replying promptly to this email and then delete and destroy this email and any attachments without any further use, copying or forwarding

1 0

[libvirt] [PATCHv2 0/7] arbitrary qemu monitor events
by Eric Blake 21 Mar '14

21 Mar '14

v1 was here: https://www.redhat.com/archives/libvir-list/2014-February/msg00000.html Many of the patches in the series were acked back then, but the 'virsh' changes were made conditional on getting 'virsh event' working first, so it missed the last release. Changes in v2: rebase to master, s/1.2.2/1.2.3/, simplify virsh patch (2/7) to use framework for event tracking. Also, the python bindings need review (no change to v1): https://www.redhat.com/archives/libvir-list/2014-February/msg00197.html Note that Dan was hesitant about patches 6 and 7 in v1, but if we don't take those patches, then the API is over-engineered. Either we are willing to support filtering on specific event names and patterns, or I should respin a v3 of this series that simplifies the API in patch 1 to not expose that possibility. Personally, I think the ability to expose all 'spice_*' related events at once, without being drowned by other events, is worth the complexity of server-side filtering. I also note that the bulk of the complexity was isolated to the qemu-specific code with no noticeable impact to the normal event code (the changes to domain_event.c are primarily additions, and the changes to object_event.c are merely tweaks to allow qemu filtering to be server-side only in comparison to normal events that can do some of the filtering client side). Eric Blake (7): qemu: new API for tracking arbitrary monitor events qemu: virsh wrapper for qemu events qemu: create object for qemu monitor events qemu: wire up RPC for qemu monitor events qemu: enable monitor event reporting qemu: enable monitor event filtering by name qemu: allow filtering events by regex daemon/libvirtd.h | 2 + daemon/remote.c | 209 ++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-qemu.h | 46 +++++++- src/conf/domain_event.c | 234 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 23 ++++ src/conf/object_event.c | 40 ++++--- src/driver.h | 15 +++ src/libvirt-qemu.c | 124 ++++++++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_qemu.syms | 6 ++ src/qemu/qemu_driver.c | 51 +++++++++ src/qemu/qemu_monitor.c | 14 +++ src/qemu/qemu_monitor.h | 13 ++- src/qemu/qemu_monitor_json.c | 21 +++- src/qemu/qemu_process.c | 29 +++++ src/qemu_protocol-structs | 22 ++++ src/remote/qemu_protocol.x | 50 ++++++++- src/remote/remote_driver.c | 143 ++++++++++++++++++++++++- src/rpc/gendispatch.pl | 13 +-- tools/virsh-domain.c | 148 ++++++++++++++++++++++++++ tools/virsh.pod | 30 +++++- 21 files changed, 1203 insertions(+), 32 deletions(-) -- 1.8.5.3

2 9

[libvirt] Question about VIR_ALLOC vs vshMalloc
by Solly Ross 21 Mar '14

21 Mar '14

I was wondering what the reason was for seemingly near-duplicate code. VIR_ALLOC and vshMalloc (and similar methods for calloc, etc), seem to be roughly identical in function, with the only difference being that VIR_ALLOC and friends are used in libvirt proper, whereas vshMalloc and friends are used in the virsh code. Is there a reason why virsh does not simply use VIR_ALLOC and friends? Best Regards, Solly Ross

2 1

[libvirt] [PATCH] is_selinux_enabled returns -1 on error, account for this.
by Scott Sullivan 21 Mar '14

21 Mar '14

Per the documentation, is_selinux_enabled() returns -1 on error. Account for this. Previously when -1 was being returned the condition would still be true. I was noticing this because on my system that has selinux disabled I was getting this in the libvirt.log every 5 seconds: error : virIdentityGetSystem:173 : Unable to lookup SELinux process context: Invalid argument With this patch applied, I no longer get these messages every 5 seconds. I am submitting this in case its deemed useful for inclusion. Anyone have any comments on this change? This is a patch off current master. From 23e0780db43ebd3ea90710750639df901c261674 Mon Sep 17 00:00:00 2001 From: Scott Sullivan <ssullivan(a)liquidweb.com> Date: Tue, 18 Mar 2014 12:55:50 -0400 Subject: [PATCH] is_selinux_enabled returns -1 on error, account for this. --- src/security/security_selinux.c | 2 +- src/util/viridentity.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 02c7496..5f46bef 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -784,7 +784,7 @@ error: static int virSecuritySELinuxSecurityDriverProbe(const char *virtDriver) { - if (!is_selinux_enabled()) + if (is_selinux_enabled() <= 0) return SECURITY_DRIVER_DISABLE; if (virtDriver && STREQ(virtDriver, "LXC")) { diff --git a/src/util/viridentity.c b/src/util/viridentity.c index 351fdd7..05e7568 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -169,7 +169,7 @@ virIdentityPtr virIdentityGetSystem(void) goto cleanup; #if WITH_SELINUX - if (is_selinux_enabled()) { + if (is_selinux_enabled() > 0) { if (getcon(&con) < 0) { virReportSystemError(errno, "%s", _("Unable to lookup SELinux process context")); -- 1.7.10.4

2 3

[libvirt] [PATCH python] override: Return NULL on python failure in getCPUModelNames
by Cole Robinson 21 Mar '14

21 Mar '14

Eric pointed this out on the last patch, but I pushed it before noticing his message. --- libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override.c b/libvirt-override.c index 71e241e..2532400 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -2350,7 +2350,7 @@ done: error: Py_XDECREF(rv); - rv = VIR_PY_NONE; + rv = NULL; goto done; } #endif /* LIBVIR_CHECK_VERSION(1, 1, 3) */ -- 1.8.5.3

2 2

[libvirt] [PATCH python] override: GetCPUModelNames should return None on failure
by Cole Robinson 21 Mar '14

21 Mar '14

Right now, on failure, libvirt.py doesn't raise an exception and just returns -1 to the user. --- libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override.c b/libvirt-override.c index 7f746ed..9fed299 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -2326,7 +2326,7 @@ libvirt_virConnectGetCPUModelNames(PyObject *self ATTRIBUTE_UNUSED, LIBVIRT_END_ALLOW_THREADS; if (c_retval == -1) - return VIR_PY_INT_FAIL; + return VIR_PY_NONE; if ((rv = PyList_New(c_retval)) == NULL) goto error; -- 1.8.5.3

2 3

[libvirt] [PATCH 0/2] bhyve: add xml2args unittest
by Roman Bogorodskiy 21 Mar '14

21 Mar '14

Add testing for xml -> args generation. It's largely based on the similar test for qemu. As it's my first experience with unit testing in libvirt and in unit testing in C in general, I'm not sure I haven't miss some important pieces, but it works as expected for me. I'm also not sure about the license headers: some tests have them, some don't, so I decided not to add them for now. Roman Bogorodskiy (2): Move virBhyveTapGetRealDeviceName to virnetdevtap bhyve: add xml2args unittest src/bhyve/bhyve_command.c | 70 +--------- src/libvirt_private.syms | 1 + src/util/virnetdevtap.c | 78 +++++++++++ src/util/virnetdevtap.h | 3 + tests/Makefile.am | 25 ++++ .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args | 2 + tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml | 24 ++++ tests/bhyvexml2argvdata/bhyvexml2argv-base.args | 2 + tests/bhyvexml2argvdata/bhyvexml2argv-base.xml | 20 +++ .../bhyvexml2argv-disk-virtio.args | 2 + .../bhyvexml2argv-disk-virtio.xml | 20 +++ tests/bhyvexml2argvmock.c | 38 +++++ tests/bhyvexml2argvtest.c | 154 +++++++++++++++++++++ 13 files changed, 370 insertions(+), 69 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml create mode 100644 tests/bhyvexml2argvmock.c create mode 100644 tests/bhyvexml2argvtest.c -- 1.8.4.2

1 3

[libvirt] [PATCHv4] Ignore missing files on pool refresh
by Ján Tomko 21 Mar '14

21 Mar '14

If we cannot stat/open a file on pool refresh, returning -1 aborts the refresh and the pool is undefined. Don't treat missing files as fatal unless VolOpenCheckMode is called with the VIR_STORAGE_VOL_OPEN_ERROR flag. https://bugzilla.redhat.com/show_bug.cgi?id=977706 --- v1: https://www.redhat.com/archives/libvir-list/2013-July/msg00635.html v2: https://www.redhat.com/archives/libvir-list/2013-July/msg00639.html v3: https://www.redhat.com/archives/libvir-list/2013-July/msg01026.html (by Guanan Ren, also checked the 'open' call) v4: do not call open on sockets and fifos and only skip missing files if we're doing a pool refresh, otherwise 'vol-info' on a missing volume returns 'unknown error' src/storage/storage_backend.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d14e633..97ab7b8 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1212,6 +1212,10 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, char *base = last_component(path); if (lstat(path, sb) < 0) { + if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + VIR_WARN("ignoring missing file '%s'", path); + return -2; + } virReportSystemError(errno, _("cannot stat file '%s'"), path); @@ -1238,6 +1242,10 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, VIR_WARN("ignoring dangling symlink '%s'", path); return -2; } + if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + VIR_WARN("ignoring missing file '%s'", path); + return -2; + } virReportSystemError(errno, _("cannot open volume '%s'"), -- 1.8.3.2

3 5

[libvirt] [PATCH] Ignore char devices in storage pools by default
by Ján Tomko 21 Mar '14

21 Mar '14

Without this, using /dev/mapper as a directory pool fails in virStorageBackendUpdateVolTargetInfoFD: cannot seek to end of file '/dev/mapper/control': Illegal seek Skip over character devices by default. https://bugzilla.redhat.com/show_bug.cgi?id=710866 --- src/storage/storage_backend.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 5314411..f5e9440 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -130,7 +130,6 @@ enum { # define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_ERROR |\ VIR_STORAGE_VOL_OPEN_REG |\ - VIR_STORAGE_VOL_OPEN_CHAR |\ VIR_STORAGE_VOL_OPEN_BLOCK) int virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, -- 1.8.3.2

2 2

[libvirt] [PATCH] virNetClientSetTLSSession: Restore original signal mask
by Michal Privoznik 21 Mar '14

21 Mar '14

Currently, we use pthread_sigmask(SIG_BLOCK, ...) prior to calling poll(). This is okay, as we don't want poll() to be interrupted. However, then - immediately as we fall out from the poll() - we try to restore the original sigmask - again using SIG_BLOCK. But as the man page says, SIG_BLOCK adds signals to the signal mask: SIG_BLOCK The set of blocked signals is the union of the current set and the set argument. Therefore, when restoring the original mask, we need to completely overwrite the one we set earlier and hence we should be using: SIG_SETMASK The set of blocked signals is set to the argument set. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Notes: This is a very old bug. It's worth backporting onto all maint branches we have. src/rpc/virnetclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index b288b74..255997b 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -792,7 +792,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll; - ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL)); + ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); } ret = virNetTLSContextCheckCertificate(tls, client->tls); @@ -816,7 +816,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll2; - ignore_value(pthread_sigmask(SIG_BLOCK, &oldmask, NULL)); + ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); len = virNetTLSSessionRead(client->tls, buf, 1); if (len < 0 && errno != ENOMSG) { -- 1.9.0

4 5