- Devel - libvirt List Archives

[libvirt] [libvirt-php][PATCH 0/2] Fix some build issues
by Michal Privoznik 20 Apr '16

20 Apr '16

While trying to make 'distcheck' work again, this is where I got so far. Unfortunately, distcheck is still not working. Michal Privoznik (2): tools: Compile generate-api-docs into builddir configure.ac: Slightly rework configure.ac | 11 ++++++----- tools/Makefile.am | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) -- 2.7.3

2 5

[libvirt] [PATCH] virt-aa-helper: disallow VNC socket read permissions
by Cole Robinson 20 Apr '16

20 Apr '16

From: Simon Arlott <bugzilla.redhat.simon(a)arlott.org> The VM does not need read permission for its own VNC socket to create(), bind(), accept() connections or to receive(), send(), etc. on connections. https://bugzilla.redhat.com/show_bug.cgi?id=1312573 --- src/security/virt-aa-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 50d2a08..a4cb409 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1062,7 +1062,7 @@ get_files(vahControl * ctl) for (i = 0; i < ctl->def->ngraphics; i++) { if (ctl->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && ctl->def->graphics[i]->data.vnc.socket && - vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "rw")) + vah_add_file(&buf, ctl->def->graphics[i]->data.vnc.socket, "w")) goto cleanup; } -- 2.5.5

3 4

[libvirt] [PATCH] man: Clarify virsh vol-clone works within a single pool
by Cole Robinson 20 Apr '16

20 Apr '16

virsh vol-clone is expected to clone a volume within a single pool; it doesn't work for cloning across pools. Clarify the docs https://bugzilla.redhat.com/show_bug.cgi?id=1103714 --- tools/virsh-volume.c | 2 +- tools/virsh.pod | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 36dd0ed..9cc8e52 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -546,7 +546,7 @@ static const vshCmdInfo info_vol_clone[] = { .data = N_("clone a volume.") }, {.name = "desc", - .data = N_("Clone an existing volume.") + .data = N_("Clone an existing volume within the parent pool.") }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 6c9d4ec..e1c0d8e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3530,10 +3530,10 @@ only slightly higher initial disk space usage. =item B<vol-clone> [I<--pool> I<pool-or-uuid>] I<vol-name-or-key-or-path> I<name> [I<--prealloc-metadata>] [I<--reflink>] -Clone an existing volume. Less powerful, but easier to type, version of -B<vol-create-from>. +Clone an existing volume within the parent pool. Less powerful, +but easier to type, version of B<vol-create-from>. I<--pool> I<pool-or-uuid> is the name or UUID of the storage pool to create -the volume in. +that contains the source volume, and will contain the new volume. I<vol-name-or-key-or-path> is the name or key or path of the source volume. I<name> is the name of the new volume. [I<--prealloc-metadata>] preallocate metadata (for qcow2 images which don't -- 2.7.3

2 2

[libvirt] [PATCH 0/3] storage: drop support for qcow-create, kvm-img
by Cole Robinson 20 Apr '16

20 Apr '16

I don't think we need to try and support qcow-create or kvm-img binaries anymore; everywhere we care about should have a /usr/bin/qemu-img. See patches for more details Cole Robinson (3): storage: remove support for /usr/bin/qcow-create storage: remove support for /usr/bin/kvm-img storage: drop the plumbing needed for kvm-img/qcow-create src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/storage/storage_backend.c | 131 ++------------------------------------- src/storage/storage_backend.h | 9 ++- src/storage/storage_backend_fs.c | 21 ++----- src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +- 7 files changed, 18 insertions(+), 155 deletions(-) -- 2.7.3

3 10

[libvirt] [PATCH] tests: Fix enumeration value
by Andrea Bolognani 20 Apr '16

20 Apr '16

Commit 3a773c43c801 introduced the testCompareNetXML2XMLResult enumeration; however, in one instance the result variable was assigned a value from the very similar testCompareDocXML2XMLResult enumeration, leading to a build error. networkxml2xmltest.c:33:42: error: implicit conversion from enumeration type 'testCompareDomXML2XMLResult' to different enumeration type 'testCompareNetXML2XMLResult' [-Werror,-Wenum-conversion] testCompareNetXML2XMLResult result = TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS; ~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Use the proper value (TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS) instead. --- Pushed as build breaker. tests/networkxml2xmltest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index a52b350..c069f3d 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -30,7 +30,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, { char *actual = NULL; int ret; - testCompareNetXML2XMLResult result = TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS; + testCompareNetXML2XMLResult result = TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS; virNetworkDefPtr dev = NULL; if (!(dev = virNetworkDefParseFile(inxml))) { -- 2.5.5

1 0

[libvirt] [PATCH 0/7] Probe and expose GIC capabilities
by Andrea Bolognani 20 Apr '16

20 Apr '16

Changes from [RFC]: * Fix issues pointed out during review (see patches 1 and 2 for details) * Add documentation The only thing missing AFAIK is some test cases... I'm not sure whether it's possible to include QMP replies for QEMU 2.6 even though it hasn't been released yet, and I wouldn't know how to generate a .replies file anyway. Any pointers? Cheers. [RFC] https://www.redhat.com/archives/libvir-list/2016-March/msg00956.html Andrea Bolognani (7): conf: Get rid of virDomainCapsDevice qemu: Probe GIC capabilities schema: Validate GIC capabilities conf: Expose GIC capabilities qemu: Fill in GIC capabilities qemu: Cache GIC capabilities docs: Document the new XML elements docs/formatdomain.html.in | 3 +- docs/formatdomaincaps.html.in | 43 ++++- docs/schemas/domaincaps.rng | 18 ++ src/conf/domain_capabilities.c | 42 ++++- src/conf/domain_capabilities.h | 24 +-- src/qemu/qemu_capabilities.c | 194 ++++++++++++++++++++- src/qemu/qemu_monitor.c | 17 ++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 115 ++++++++++++ src/qemu/qemu_monitor_json.h | 4 + src/util/virgic.h | 13 ++ tests/domaincapsschemadata/domaincaps-basic.xml | 3 + tests/domaincapsschemadata/domaincaps-full.xml | 3 + .../domaincaps-qemu_1.6.50-1.xml | 3 + tests/domaincapstest.c | 8 +- 15 files changed, 470 insertions(+), 24 deletions(-) -- 2.5.5

2 16

[libvirt] [PATCH] network: Don't use ERR_NO_SUPPORT for invalid net-update requests
by Cole Robinson 20 Apr '16

20 Apr '16

VIR_ERR_NO_SUPPORT maps to the error string this function is not supported by the connection driver and is largely only used for when a driver doesn't have any implementation for a public API. So its usage with invalid net-update requests is a bit out of place. Instead use VIR_ERR_OPERATION_UNSUPPORTED which maps to: Operation not supported And is what qemu's hotplug routines use in similar scenarios --- src/conf/network_conf.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 4fb2e2a..eabaccd 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3376,14 +3376,14 @@ void virNetworkSetBridgeMacAddr(virNetworkDefPtr def) static void virNetworkDefUpdateNoSupport(virNetworkDefPtr def, const char *section) { - virReportError(VIR_ERR_NO_SUPPORT, + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("can't update '%s' section of network '%s'"), section, def->name); } static void virNetworkDefUpdateUnknownCommand(unsigned int command) { - virReportError(VIR_ERR_NO_SUPPORT, + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("unrecognized network update command code %d"), command); } @@ -3666,7 +3666,7 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, /* parse the xml into a virSocketAddrRange */ if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("dhcp ranges cannot be modified, " "only added or deleted")); goto cleanup; @@ -3772,7 +3772,7 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def, goto cleanup; if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("forward interface entries cannot be modified, " "only added or deleted")); goto cleanup; @@ -3973,7 +3973,7 @@ virNetworkDefUpdateDNSHost(virNetworkDefPtr def, memset(&host, 0, sizeof(host)); if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("DNS HOST records cannot be modified, " "only added or deleted")); goto cleanup; @@ -4067,7 +4067,7 @@ virNetworkDefUpdateDNSSrv(virNetworkDefPtr def, memset(&srv, 0, sizeof(srv)); if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("DNS SRV records cannot be modified, " "only added or deleted")); goto cleanup; @@ -4151,7 +4151,7 @@ virNetworkDefUpdateDNSTxt(virNetworkDefPtr def, memset(&txt, 0, sizeof(txt)); if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - virReportError(VIR_ERR_NO_SUPPORT, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("DNS TXT records cannot be modified, " "only added or deleted")); goto cleanup; @@ -4268,7 +4268,7 @@ virNetworkDefUpdateSection(virNetworkDefPtr def, ret = virNetworkDefUpdateDNSSrv(def, command, parentIndex, ctxt, flags); break; default: - virReportError(VIR_ERR_NO_SUPPORT, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("can't update unrecognized section of network")); break; } -- 2.7.3

2 1

[libvirt] [PATCH v2 1/2] Explicitly error on uri=qemu://system
by Cole Robinson 20 Apr '16

20 Apr '16

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/ht… 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

3 4

[libvirt] [PATCH] Revert "daemon: use socket activation with systemd"
by Cole Robinson 20 Apr '16

20 Apr '16

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

2 2

[libvirt] [PATCH v2] vz: handle sourceless cdroms
by Nikolay Shirokovskiy 19 Apr '16

19 Apr '16

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

2 1