[libvirt] [PATCH v2 1/2] Explicitly error on uri=qemu://system
by Cole Robinson
It's a fairly common error that a user tries to connect to a URI
like qemu://system or qemu://session (missing a slash). This errors
like:
$ virsh --connect qemu://session
error: failed to connect to the hypervisor
error: Unable to resolve address 'session' service '16514': No address associated with hostname
If you already know that the standard qemu URI has 3 slashes, that
error will make it obvious enough. But new user's may not get it.
There's even a RHEL support page explicitly mentioning it!:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/...
Catch this error early in libvirt.c virConnectOpen for qemu (and vbox
which has similar rules
https://bugzilla.redhat.com/show_bug.cgi?id=1038304
---
v2:
Use conventional function naming
Improve a comment
Handle 'vz' driver
src/libvirt.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index a21d00e..919c9cb 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -928,6 +928,35 @@ virConnectGetDefaultURI(virConfPtr conf,
}
+/*
+ * Check to see if an invalid URI like qemu://system (missing /) was passed,
+ * offer the suggested fix.
+ */
+static int
+virConnectCheckURIMissingSlash(const char *uristr, virURIPtr uri)
+{
+ /* To avoid false positives, only check drivers that mandate
+ a path component in the URI, like /system or /session */
+ if (STRNEQ(uri->scheme, "qemu") &&
+ STRNEQ(uri->scheme, "vbox") &&
+ STRNEQ(uri->scheme, "vz"))
+ return 0;
+
+ if (uri->path != NULL)
+ return 0;
+
+ if (STREQ(uri->server, "session") ||
+ STREQ(uri->server, "system")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid URI %s (maybe you want %s:///%s)"),
+ uristr, uri->scheme, uri->server);
+ return -1;
+ }
+
+ return 0;
+}
+
+
static virConnectPtr
do_open(const char *name,
virConnectAuthPtr auth,
@@ -995,6 +1024,12 @@ do_open(const char *name,
NULLSTR(ret->uri->user), ret->uri->port,
NULLSTR(ret->uri->path));
+ if (virConnectCheckURIMissingSlash(alias ? alias : name,
+ ret->uri) < 0) {
+ VIR_FREE(alias);
+ goto failed;
+ }
+
VIR_FREE(alias);
} else {
VIR_DEBUG("no name, allowing driver auto-select");
--
2.7.3
8 years, 7 months
[libvirt] [PATCH] Revert "daemon: use socket activation with systemd"
by Cole Robinson
This reverts commit 1e9808d3a1e00a7121bae8b163d9c42d441d2ca8.
We shouldn't advertise libvirtd.socket activation, since currently
it means VM/network/... autostart won't work as expected.
We tried to find a middle ground by installing the config file without
an [Install] section, since systemd won't allow .socket to be enabled
without one... or at least it did do that; presently on f24 it allows
activating the socket quite happily. This also caused user confusion[1]
Just remove the socket file. I've filed a new RFE to track coming up
with a solution to the autostart problem[2], we can point users at that
if there's more confusion:
[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1279348
[2]: https://bugzilla.redhat.com/show_bug.cgi?id=1326136
---
.gitignore | 1 -
daemon/Makefile.am | 14 ++------------
daemon/libvirtd.conf | 5 -----
daemon/libvirtd.service.in | 5 +++++
daemon/libvirtd.socket.in | 11 -----------
libvirt.spec.in | 7 ++-----
6 files changed, 9 insertions(+), 34 deletions(-)
delete mode 100644 daemon/libvirtd.socket.in
diff --git a/.gitignore b/.gitignore
index 0d12c5c..381db69 100644
--- a/.gitignore
+++ b/.gitignore
@@ -63,7 +63,6 @@
/daemon/libvirtd.pod
/daemon/libvirtd.policy
/daemon/libvirtd.service
-/daemon/libvirtd.socket
/daemon/test_libvirtd.aug
/docs/aclperms.htmlinc
/docs/apibuild.py.stamp
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 2dbe81b..fc6fd95 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -59,7 +59,6 @@ EXTRA_DIST = \
libvirt.rules \
libvirtd.sasl \
libvirtd.service.in \
- libvirtd.socket.in \
libvirtd.sysconf \
libvirtd.sysctl \
libvirtd.aug \
@@ -446,18 +445,15 @@ endif ! LIBVIRT_INIT_SCRIPT_UPSTART
if LIBVIRT_INIT_SCRIPT_SYSTEMD
SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system
-BUILT_SOURCES += libvirtd.service libvirtd.socket
+BUILT_SOURCES += libvirtd.service
-install-init-systemd: install-sysconfig libvirtd.service libvirtd.socket
+install-init-systemd: install-sysconfig libvirtd.service
$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
$(INSTALL_DATA) libvirtd.service \
$(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
- $(INSTALL_DATA) libvirtd.socket \
- $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
uninstall-init-systemd: uninstall-sysconfig
rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.service
- rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/libvirtd.socket
rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :
else ! LIBVIRT_INIT_SCRIPT_SYSTEMD
install-init-systemd:
@@ -481,12 +477,6 @@ libvirtd.service: libvirtd.service.in $(top_builddir)/config.status
< $< > $@-t && \
mv $@-t $@
-libvirtd.socket: libvirtd.socket.in $(top_builddir)/config.status
- $(AM_V_GEN)sed \
- -e 's|[@]runstatedir[@]|$(runstatedir)|g' \
- < $< > $@-t && \
- mv $@-t $@
-
check-local: check-augeas
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 5485f98..d2c439c 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -77,11 +77,6 @@
# UNIX socket access controls
#
-# Beware that if you are changing *any* of these options, and you use
-# socket activation with systemd, you need to adjust the settings in
-# the libvirtd.socket file as well since it could impose a security
-# risk if you rely on file permission checking only.
-
# Set the UNIX domain socket group ownership. This can be used to
# allow a 'trusted' set of users access to management capabilities
# without becoming root.
diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 608221c..1616e7a 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -1,3 +1,8 @@
+# NB we don't use socket activation. When libvirtd starts it will
+# spawn any virtual machines registered for autostart. We want this
+# to occur on every boot, regardless of whether any client connects
+# to a socket. Thus socket activation doesn't have any benefit
+
[Unit]
Description=Virtualization daemon
Before=libvirt-guests.service
diff --git a/daemon/libvirtd.socket.in b/daemon/libvirtd.socket.in
deleted file mode 100644
index 0915bb3..0000000
--- a/daemon/libvirtd.socket.in
+++ /dev/null
@@ -1,11 +0,0 @@
-[Socket]
-ListenStream=@runstatedir@/libvirt/libvirt-sock
-ListenStream=@runstatedir@/libvirt/libvirt-sock-ro
-
-; The following settings must match libvirtd.conf file in order to
-; work as expected because libvirtd can't change them later.
-; SocketMode=0777 is safe only if authentication on the socket is set
-; up. For further information, please see the libvirtd.conf file.
-SocketMode=0777
-SocketUser=root
-SocketGroup=root
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8036fa3..c3bfea3 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1710,7 +1710,7 @@ exit 0
%if %{with_systemd}
%if %{with_systemd_macros}
- %systemd_post virtlockd.socket virtlogd.socket libvirtd.service libvirtd.socket
+ %systemd_post virtlockd.socket virtlogd.socket libvirtd.service
%else
if [ $1 -eq 1 ] ; then
# Initial installation
@@ -1739,19 +1739,17 @@ fi
%preun daemon
%if %{with_systemd}
%if %{with_systemd_macros}
- %systemd_preun libvirtd.socket libvirtd.service virtlogd.socket virtlogd.service virtlockd.socket virtlockd.service
+ %systemd_preun libvirtd.service virtlogd.socket virtlogd.service virtlockd.socket virtlockd.service
%else
if [ $1 -eq 0 ] ; then
# Package removal, not upgrade
/bin/systemctl --no-reload disable \
- libvirtd.socket \
libvirtd.service \
virtlogd.socket \
virtlogd.service \
virtlockd.socket \
virtlockd.service > /dev/null 2>&1 || :
/bin/systemctl stop \
- libvirtd.socket \
libvirtd.service \
virtlogd.socket \
virtlogd.service \
@@ -1966,7 +1964,6 @@ exit 0
%if %{with_systemd}
%{_unitdir}/libvirtd.service
-%{_unitdir}/libvirtd.socket
%{_unitdir}/virtlogd.service
%{_unitdir}/virtlogd.socket
%{_unitdir}/virtlockd.service
--
2.7.3
8 years, 7 months
[libvirt] [PATCH v2] vz: handle sourceless cdroms
by Nikolay Shirokovskiy
From: Mikhail Feoktistov <mfeoktistov(a)virtuozzo.com>
libvirt handles empty source as NULL, while vz sdk as
"" thus we need a bit of conversion.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
---
src/vz/vz_sdk.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
changes from v1:
================
1. simplify empty string check in prlsdkGetDiskInfo
2. call setting "" explicitly in prlsdkAddDisk.
point 2 is the actual reason for releasing new version. I need
this semantics on my future work of updating disks.
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 2e9544b..9b783af 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -565,7 +565,7 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
if (!(buf = prlsdkGetStringParamVar(PrlVmDev_GetFriendlyName, prldisk)))
goto cleanup;
- if (virDomainDiskSetSource(disk, buf) < 0)
+ if (*buf != '\0' && virDomainDiskSetSource(disk, buf) < 0)
goto cleanup;
if (prlsdkGetDiskId(prldisk, isCt, &disk->bus, &disk->dst) < 0)
@@ -3183,6 +3183,7 @@ static int prlsdkAddDisk(vzDriverPtr driver,
PRL_DEVICE_TYPE devType;
PRL_CLUSTERED_DEVICE_SUBTYPE scsiModel;
char *dst = NULL;
+ const char *path = disk->src->path ? : "";
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
devType = PDE_HARD_DISK;
@@ -3206,10 +3207,10 @@ static int prlsdkAddDisk(vzDriverPtr driver,
pret = PrlVmDev_SetEmulatedType(sdkdisk, emutype);
prlsdkCheckRetGoto(pret, cleanup);
- pret = PrlVmDev_SetSysName(sdkdisk, disk->src->path);
+ pret = PrlVmDev_SetSysName(sdkdisk, path);
prlsdkCheckRetGoto(pret, cleanup);
- pret = PrlVmDev_SetFriendlyName(sdkdisk, disk->src->path);
+ pret = PrlVmDev_SetFriendlyName(sdkdisk, path);
prlsdkCheckRetGoto(pret, cleanup);
drive = &disk->info.addr.drive;
--
1.8.3.1
8 years, 7 months
[libvirt] [PATCH REBASE 0/4] vz: use disk bus and target name as disk id
by Nikolay Shirokovskiy
This series is a preparation to implement device update functionality.
In case of disk updates disk bus and disk target name pair is considered
disk id. We already have places in vz driver code where we use disk source
as id. These places are either not critical (see boot patch) or
correct (see detach patch) so we need not to fix them immediately to use more
rubust id. But as this id will be introduced anyway for disk updates
let's move to new id in existing code. This way we have single
id for disks.
Nikolay Shirokovskiy (4):
vz: introduce vzsdk disk id function
vz: fix detach disk to use new disk id
vz: fix boot check to use new disk id
vz: cleanup: remove trivial function
src/vz/vz_sdk.c | 251 ++++++++++++++++++++++++++++----------------------------
1 file changed, 126 insertions(+), 125 deletions(-)
--
1.8.3.1
8 years, 7 months
[libvirt] [PATCH 00/10] Convert secret driver to use hashed object
by John Ferlan
This series replaces the usage of linked list in secret driver with a
hashed object list as was suggested in the review in the last patch of
the previous secret driver changes series:
http://www.redhat.com/archives/libvir-list/2016-February/msg01274.html
Patch 1 - starts the ball rolling
Patch 2 - is yet another code optimization (as found in bridge_driver)
Patches 3-8 - Add the new API's "slowly" (for review purposes)
Patch 9 - replaces the usage of linked lists with the hashed object
Patch 10 - moves the secretLoadAllConfigs to secret_conf
The changes are loosely modeled after both virdomainobj and network_conf
functionality. The real meat and potato(e)s is found in patches 5 and 9.
Other functions should be relatively straightforward.
I've done testing of various virsh secret-* commands (both valid and invalid
options) as well as performing driver start, stop, and reload testing.
Each patch was built using 'check' and 'syntax-check'.
The end result is much more code in secret_conf and much less in secret_driver.
John Ferlan (10):
secret: Move virSecretObj to secret_conf.h
Add secretObjFromSecret
secret: Add hashed virSecretObj and virSecretObjList
secret: Introduce virSecretObjListFindBy{UUID|Usage} support
secret: Introduce virSecretObjListAdd* and virSecretObjListRemove
secret: Introduce virSecretObjListNumOfSecrets
secret: Introduce virSecretObjListExport
secret: Introduce virSecretObjListGetUUIDs
secret: Use the hashed virSecretObjList
secret: Move and rename secretLoadAllConfigs
src/conf/secret_conf.c | 819 ++++++++++++++++++++++++++++++++++++++++++++-
src/conf/secret_conf.h | 76 ++++-
src/libvirt_private.syms | 11 +
src/secret/secret_driver.c | 647 +++--------------------------------
4 files changed, 957 insertions(+), 596 deletions(-)
--
2.5.0
8 years, 7 months
[libvirt] [PATCH 0/3] qemu: cleanup duplicate calls to DomainObjIsActive
by Cole Robinson
Some API entry points have duplicate calls to DomainObjIsActive. Remove
the redundant ones, and document the valid ones that confused me :)
Cole Robinson (3):
qemu: Remove redundant DomainObjIsActive calls
qemu: Clarify usage of DomainObjIsActive for PMSuspended
qemu: Clarify usage of DomainObjIsActive for SetTime
src/qemu/qemu_driver.c | 43 ++++++++++++++++---------------------------
1 file changed, 16 insertions(+), 27 deletions(-)
--
2.7.3
8 years, 7 months
[libvirt] [PATCH] Explicitly error on uri=qemu://system
by Cole Robinson
It's a fairly common error that a user tries to connect to a URI
like qemu://system or qemu://session (missing a slash). This errors
like:
$ virsh --connect qemu://session
error: failed to connect to the hypervisor
error: Unable to resolve address 'session' service '16514': No address associated with hostname
If you already know that the standard qemu URI has 3 slashes, that
error will make it obvious enough. But new user's may not get it.
There's even a RHEL support page explicitly mentioning it!:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/...
Catch this error early in libvirt.c virConnectOpen for qemu (and vbox
which has similar rules
https://bugzilla.redhat.com/show_bug.cgi?id=1038304
---
src/libvirt.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index a21d00e..7607ae3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -928,6 +928,33 @@ virConnectGetDefaultURI(virConfPtr conf,
}
+/*
+ * Check to see if an invalid URI like qemu://system (missing /) was passed,
+ * offer the suggested fix.
+ */
+static int
+check_uri_missing_slash(const char *uristr, virURIPtr uri)
+{
+ /* These drivers _only_ accepts URIs with a 'path' element */
+ if (STRNEQ(uri->scheme, "qemu") &&
+ STRNEQ(uri->scheme, "vbox"))
+ return 0;
+
+ if (uri->path != NULL)
+ return 0;
+
+ if (STREQ(uri->server, "session") ||
+ STREQ(uri->server, "system")) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid URI %s (maybe you want %s:///%s)"),
+ uristr, uri->scheme, uri->server);
+ return -1;
+ }
+
+ return 0;
+}
+
+
static virConnectPtr
do_open(const char *name,
virConnectAuthPtr auth,
@@ -995,6 +1022,11 @@ do_open(const char *name,
NULLSTR(ret->uri->user), ret->uri->port,
NULLSTR(ret->uri->path));
+ if (check_uri_missing_slash(alias ? alias : name, ret->uri) < 0) {
+ VIR_FREE(alias);
+ goto failed;
+ }
+
VIR_FREE(alias);
} else {
VIR_DEBUG("no name, allowing driver auto-select");
--
2.7.3
8 years, 7 months
[libvirt] [libvirt-php][PATCH 0/3] Couple of small fixes
by Michal Privoznik
While playing with our php bindings I've noticed couple of memory
leaks mostly. And one bug in our examples.
Michal Privoznik (3):
examples: Check properly if connected
Fix some leaks related to get_string_from_xpath
Revisit some VIRT_RETURN_STRING
examples/index.php | 4 +-
src/libvirt-php.c | 121 ++++++++++++++++++++++++++++++-----------------------
2 files changed, 72 insertions(+), 53 deletions(-)
--
2.7.3
8 years, 7 months
[libvirt] RFC: Drop unmaintained hv drivers? (phyp, xenapi, hyperv)
by Cole Robinson
Hi all,
There's a few old hypervisor drivers in the tree that haven't been actively
maintained for a long time. I'm curious if anyone knows of these drivers being
actively used. If not I think we should consider dropping them
src/phyp/ : for power VM hypervisor. Added in July 2009. The last commit that
looks like it wasn't either internal API conversion, or caught by code
analysis, is:
commit 41461ff7f7d6ee6679aae2a8004e305c5830c9e8
Author: Eduardo Otubo <otubo(a)linux.vnet.ibm.com>
Date: Tue Apr 19 12:34:08 2011 -0300
PHYP: Adding reboot domain function
Adding reboot <domain> function for pHyp driver.
Nearly 5 years ago. Eduardo is the primary driver author too (CCd at his email
from github).
Searching the upstream bug tracker for all bugs with 'phyp', the only one
that's actually about the phyp driver is a report from 2 years ago that it
crashes trying to open a connection:
https://bugzilla.redhat.com/show_bug.cgi?id=1093094
src/xenapi/: Connecting to a xen api server. Added in March 2010. Largely
appears to be a code drop, the original author/committer has never had another
commit. The last xenapi specific commit seems to be:
commit 484460ec4678a264c5e7355495c2f0da72cb42bd
Author: Matthias Bolte <matthias.bolte(a)googlemail.com>
Date: Thu Jul 21 15:16:11 2011 +0200
xenapi: Improve error reporting in xenapiOpen once again
Nearly 5 years ago. The only upstream bug that was filed about xenapi is:
https://bugzilla.redhat.com/show_bug.cgi?id=711372
Which was about a connection failure that was eventually fixed upstream, and
dovetailed into the above referenced commit. Current xen guys, you know of
anyone using this?
src/hyperv/: Added in July 2011. This was largely a code drop as well;
committed and patched a few times by Matthias but it was a university project
by someone else. Last hyperv targeted patch was:
commit 9e9ea3ead9825bd1dc2c17cea4abc8c4165591d0
Author: Matthias Bolte <matthias.bolte(a)googlemail.com>
Date: Sun Sep 9 17:39:40 2012 +0200
hyperv: Fix and improve hypervListAllDomains
The driver is fairly minimal as well: it can only list existing VMs and
perform lifecycle operations. It can't create new VMs, and doesn't list VM
device config AFAICT.
Also, in general, I've never heard about anyone _actually_ using any of those
drivers in the wild. There's reports here and there but it mostly sounds like
people trying them out. Just an anecdote so take it with a grain of salt
- Cole
8 years, 7 months
