December 2017 - Devel - libvirt List Archives

[libvirt] RFC: auto port allocations
by Nikolay Shirokovskiy 18 Dec '17

18 Dec '17

There is a problem with current port allocations in libvirt. 1. With default qemu driver conf values remote_display_port_min = 5900 and remote_websocket_port_min = 5700 one can not start more than 200 domains each of one have vnc graphics with auto allocated socket and websocket. For the 200th domain driver allocate 6100 port for both socket and websocket and qemu fail to start. 2. Different hypervisor drivers use port pools with same borders (libxl and qemu migration ports, hardcoded libxl vnc sockets and default conf qemu vnc sockets for example. However I'm not sure it is possible/practical to use these drivers simultaneously on same host). As a result there can be failures due to races when first driver check for bind successfully then second driver check for bind successfully and then both pass same port value to hypervisors. Suggestion is to make port bitmap driver/daemon global and leave only borders in port pool object. This was we can solve the first and the both issues correspondingly. Nikolay

2 1

[libvirt] [perl] Fix check of return value from virStreamRecv*
by Jim Fehlig 18 Dec '17

18 Dec '17

Commit 049baec5 introduced a small bug in the logic checking return value of virStreamRecv*. Change the logic to only croak when return value is < 0 and not equal to -2 or -3. --- Changes | 2 +- Virt.xs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index 3763b1d..96b42f8 100644 --- a/Changes +++ b/Changes @@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt 4.0.0 2018-00-00 - - XXX + - Fix check of return value from virStreamRecv* 3.9.1 2017-12-05 diff --git a/Virt.xs b/Virt.xs index 415eb8a..c47b915 100644 --- a/Virt.xs +++ b/Virt.xs @@ -8074,7 +8074,7 @@ recv(st, data, nbytes, flags=0) else RETVAL = virStreamRecv(st, rawdata, nbytes); - if (RETVAL != -2 && RETVAL != -3) { + if ((RETVAL < 0) && (RETVAL != -2 || RETVAL != -3)) { Safefree(rawdata); _croak_error(); } -- 2.15.1

2 1

[libvirt] [PATCH] deamon: TLS: use default value if ca_file, cert_file or key_file not set
by Chen Hanxiao 18 Dec '17

18 Dec '17

From: Chen Hanxiao <chenhanxiao(a)gmail.com> As the description of daemon/libvirtd.conf, setting key_file, cert_file or key_file will override the default value. But if we set any one of them, we need to set all the rest of them. This patch set default value to them as daemon/libvirtd.conf described. Signed-off-by: Chen Hanxiao <chenhanxiao(a)gmail.com> --- daemon/libvirtd.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 6d3b83355..93983f63b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -493,19 +493,28 @@ daemonSetupNetworking(virNetServerPtr srv, config->cert_file || config->key_file) { if (!config->ca_file) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No CA certificate path set to match server key/cert")); - goto cleanup; + VIR_WARN("Using default path for ca_file"); + if (VIR_STRDUP(config->ca_file, LIBVIRT_CACERT) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No CA certificate path set to match server key/cert")); + goto cleanup; + } } if (!config->cert_file) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No server certificate path set to match server key")); - goto cleanup; + VIR_WARN("Using default path for cert_file"); + if (VIR_STRDUP(config->cert_file, LIBVIRT_SERVERCERT) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No server certificate path set to match server key")); + goto cleanup; + } } if (!config->key_file) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("No server key path set to match server cert")); - goto cleanup; + VIR_WARN("Using default path for key_file"); + if (VIR_STRDUP(config->key_file, LIBVIRT_SERVERKEY) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("No server key path set to match server cert")); + goto cleanup; + } } VIR_DEBUG("Using CA='%s' cert='%s' key='%s'", config->ca_file, config->cert_file, config->key_file); -- 2.14.3

1 0

[libvirt] [PATCH] apparmor: add ptrace/mediation rules for unconfined guests
by Christian Ehrhardt 16 Dec '17

16 Dec '17

If a guest runs unconfined <seclabel type='none'>, but libvirtd is confined then the peer for signal/ptrace can only be detected as 'unconfined'. That triggers issues like: apparmor="DENIED" operation="signal" profile="/usr/sbin/libvirtd" pid=22395 comm="libvirtd" requested_mask="send" denied_mask="send" signal=term peer="unconfined" To fix this add unconfined as an allowed peer for those operations. I discussed with the apparmor folks, right now there is no better separation to be made in this case. But there might be further down the road with "policy namespaces with scope and view control + stacking" This is more a use-case addition than a fix to the following two changes: - 3b1d19e6 AppArmor: add rules needed with additional mediation features - b482925c apparmor: support ptrace checks Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- examples/apparmor/usr.sbin.libvirtd | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/apparmor/usr.sbin.libvirtd b/examples/apparmor/usr.sbin.libvirtd index 8d61d15..23e8aa3 100644 --- a/examples/apparmor/usr.sbin.libvirtd +++ b/examples/apparmor/usr.sbin.libvirtd @@ -61,6 +61,10 @@ signal (send) peer=/usr/sbin/dnsmasq, signal (read, send) peer=libvirt-*, + # required if guests run unconfined seclabel type='none' but libvirtd is confined + signal (read, send) peer=unconfined, + ptrace (trace) peer=unconfined, + # Very lenient profile for libvirtd since we want to first focus on confining # the guests. Guests will have a very restricted profile. / r, -- 2.7.4

3 2

[libvirt] [PATCH] add support of iSER transport type in qemu with libiscsi
by lichstor＠gmail.com 16 Dec '17

16 Dec '17

From: zhangshengyu <zhangshengyu(a)fusionstack.cn> --- src/conf/domain_conf.c | 13 ++++++++ src/qemu/qemu_block.c | 17 +++++++++- src/util/virstoragefile.c | 3 +- src/util/virstoragefile.h | 1 + tests/qemuxml2argvdata/disk-drive-network-iser.xml | 37 ++++++++++++++++++++++ 5 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-drive-network-iser.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66e21c4bd..bf20cfd0c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7201,6 +7201,9 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, iscsisrc->src->type = VIR_STORAGE_TYPE_NETWORK; iscsisrc->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("virDomainHostdevSubsysSCSIiSCSIDefParseXML")); + if (!(iscsisrc->src->path = virXMLPropString(sourcenode, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing iSCSI hostdev source path name")); @@ -8416,6 +8419,7 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, unsigned int flags) { char *protocol = NULL; + char *transport = NULL; char *haveTLS = NULL; char *tlsCfg = NULL; int tlsCfgVal; @@ -8427,6 +8431,10 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, goto cleanup; } + if (!(transport = virXMLPropString(node, "transport"))) { + VIR_WARN("missing network source transport type"); + } + if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown protocol type '%s'"), protocol); @@ -8495,6 +8503,9 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, if (virDomainStorageNetworkParseHosts(node, &src->hosts, &src->nhosts) < 0) goto cleanup; + if(src->hosts) + src->hosts->transport = virStorageNetHostTransportTypeFromString(transport); + virStorageSourceNetworkAssignDefaultPorts(src); ret = 0; @@ -22326,6 +22337,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, VIR_FREE(path); + virBufferEscapeString(attrBuf, " transport='%s'", "iser"); + if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT && !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && src->tlsFromConfig)) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 585f0255e..dcd7c6a5e 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -506,6 +506,20 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, goto cleanup; break; + case VIR_STORAGE_NET_HOST_TRANS_ISER: + transport = "iser"; + if (virAsprintf(&port, "%u", host->port) < 0) + goto cleanup; + + if (virJSONValueObjectCreate(&server, + "s:type", transport, + "s:host", host->name, + "s:port", port, + NULL) < 0) + goto cleanup; + + + break; case VIR_STORAGE_NET_HOST_TRANS_UNIX: if (virJSONValueObjectCreate(&server, "s:type", "unix", @@ -831,7 +845,8 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) "s:portal", portal, "s:target", target, "u:lun", lun, - "s:transport", "tcp", + "s:transport", + virStorageNetHostTransportTypeToString(src->hosts->transport), "S:user", username, "S:password-secret", objalias, NULL)); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 6594715e5..02d62a68e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -92,7 +92,8 @@ VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST, VIR_ENUM_IMPL(virStorageNetHostTransport, VIR_STORAGE_NET_HOST_TRANS_LAST, "tcp", "unix", - "rdma") + "rdma", + "iser") VIR_ENUM_IMPL(virStorageSourcePoolMode, VIR_STORAGE_SOURCE_POOL_MODE_LAST, diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 24382a0a6..4eb650186 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -147,6 +147,7 @@ typedef enum { VIR_STORAGE_NET_HOST_TRANS_TCP, VIR_STORAGE_NET_HOST_TRANS_UNIX, VIR_STORAGE_NET_HOST_TRANS_RDMA, + VIR_STORAGE_NET_HOST_TRANS_ISER, VIR_STORAGE_NET_HOST_TRANS_LAST } virStorageNetHostTransport; diff --git a/tests/qemuxml2argvdata/disk-drive-network-iser.xml b/tests/qemuxml2argvdata/disk-drive-network-iser.xml new file mode 100644 index 000000000..b3f4f9bfb --- /dev/null +++ b/tests/qemuxml2argvdata/disk-drive-network-iser.xml @@ -0,0 +1,37 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>e301d9ab-f5ad-47bf-988c-9645577a1af4</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example' transport='iser'> + <host name='example.org' port='6000'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='iscsi' name='iqn.1992-01.com.example/1' transport='iser'> + <host name='example.org' port='6000'/> + </source> + <target dev='vdb' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> -- 2.13.6 (Apple Git-96)

3 3

[libvirt] [PATCH] admin: Use the connection to determine a client is connected readonly
by Erik Skultety 15 Dec '17

15 Dec '17

Prior to this change, we relied solely on the inherited readonly attribute of a service's socket. This only worked for our UNIX sockets (and only to some degree), but doesn't work for TCP sockets which are RW by default, but such connections support RO as well. This patch forces an update on the client object once we have established a connection to reflect the nature of the connection itself rather than relying on the underlying socket's attributes. Clients connected to the admin server have always been connected as RW only. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1524399 Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- daemon/remote.c | 5 +++++ src/libvirt_remote.syms | 1 + src/rpc/virnetserverclient.c | 11 +++++++++++ src/rpc/virnetserverclient.h | 1 + 4 files changed, 18 insertions(+) diff --git a/daemon/remote.c b/daemon/remote.c index c2111ae37..8e99a4d86 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1814,6 +1814,11 @@ remoteDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, if (priv->conn == NULL) goto cleanup; + /* force update the @readonly attribute which was inherited from the + * virNetServerService object - this is important for sockets that are RW + * by default, but do accept RO flags, e.g. TCP + */ + virNetServerClientSetReadonly(client, (flags & VIR_CONNECT_RO)); rv = 0; cleanup: diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 61c20d530..1c107e1d6 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -153,6 +153,7 @@ virNetServerClientSendMessage; virNetServerClientSetAuth; virNetServerClientSetCloseHook; virNetServerClientSetDispatcher; +virNetServerClientSetReadonly; virNetServerClientStartKeepAlive; virNetServerClientWantClose; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 6e086b7b4..f4a2571f5 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -638,6 +638,17 @@ bool virNetServerClientGetReadonly(virNetServerClientPtr client) return readonly; } + +void +virNetServerClientSetReadonly(virNetServerClientPtr client, + bool readonly) +{ + virObjectLock(client); + client->readonly = readonly; + virObjectUnlock(client); +} + + unsigned long long virNetServerClientGetID(virNetServerClientPtr client) { return client->id; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index e45c78882..2569f93c3 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -81,6 +81,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, int virNetServerClientGetAuth(virNetServerClientPtr client); void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client); +void virNetServerClientSetReadonly(virNetServerClientPtr client, bool readonly); unsigned long long virNetServerClientGetID(virNetServerClientPtr client); long long virNetServerClientGetTimestamp(virNetServerClientPtr client); -- 2.13.6

2 1

[libvirt] [PATCH] storage: Add unique UUID check for virStoragePoolObjAssignDef
by John Ferlan 15 Dec '17

15 Dec '17

Commit id '4b2e0ed6e' converted to using hash tables for storing storage pool objs by name and uuid; however, neglected to add a check to virStoragePoolObjAssignDef that the pool by uuid wasn't defined. This caused issues for the virt-manager test driver which ended up using the same UUID for a newly named pool and started having failures from adding a non unique UUID. So instead of getting a "Duplicate key", let's add a more descriptive error message indicating which pool object by name already exists using the same UUID as the pool object that is attempting to be added. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- FWIW: The virt-manager test was also fixed to not use a duplicate UUID as of commit '4224b0926' in the virt-manager git repo. src/conf/virstorageobj.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 49fe24b28..e3acc817c 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -747,10 +747,18 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, return obj; } + virUUIDFormat(def->uuid, uuidstr); + if ((obj = virStoragePoolObjFindByUUIDLocked(pools, def->uuid))) { + virObjectLock(obj); + virReportError(VIR_ERR_OPERATION_FAILED, + _("storage pool '%s' already exists with uuid %s"), + obj->def->name, uuidstr); + goto error; + } + if (!(obj = virStoragePoolObjNew())) return NULL; - virUUIDFormat(def->uuid, uuidstr); if (virHashAddEntry(pools->objs, uuidstr, obj) < 0) goto error; virObjectRef(obj); -- 2.13.6

2 1

[libvirt] [PATCH 0/2] qemu: Enforce vCPU hotplug granularity constraints
by Andrea Bolognani 15 Dec '17

15 Dec '17

return -ENOBLURB; Andrea Bolognani (2): qemu: Invert condition nesting in qemuDomainDefValidate() qemu: Enforce vCPU hotplug granularity constraints src/qemu/qemu_domain.c | 56 +++++++++++++++++++--- tests/qemuxml2argvdata/cpu-hotplug-granularity.xml | 18 +++++++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/cpu-hotplug-granularity.xml -- 2.14.3

2 8

[libvirt] [RFC] 5-level paging Support
by Feng, Shaohe 15 Dec '17

15 Dec '17

Hi all, Now both qemu and kvm support 5-level paging. We can start qemu with a "cpu,+la57" to set 57-bit vitrual address space. So VM can be aware that it need to enable 5-level paging. We can also set another "cpu,phys-bits=52" to set the VM physical address space. Actually, VM can still turn on 5 level paging even without "phys-bits=52", yet this means the guest physical address width are limited, meaning less practical benefits. In to support 5-level paging, I suggest to add two attribute for the domain cpu element in libvirt. <cpu la57='yes', phys-bits='52'\> Here we need to be able to set phys-bits directly, because it is potentially migration sensitive. If la57='no', libvirt will ignore phys-bits, no matter whether hardware support la57. If la57='yes', libvirt will probe the host capability, and will throw error if hardware does support la57. If la57='yes', even phys-bits less than 52, libvirt will also pass it to qemu though less practical benefits. [info] [1] [Qemu-devel] [PATCH]x86: implement la57 paging mode <https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg02096.html> [2] [Qemu-devel] [PATCH v4 2/5] x86: Allow physical address bits to be set <https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg01950.html> BR Shaohe Feng

2 2

[libvirt] [PATCH] cfg.mk: Simplify backslash alignment check
by Andrea Bolognani 15 Dec '17

15 Dec '17

The use of [[:blank:]] was intended to cover tabs as well, but it couldn't possibly work in its current form, so the regex was tweaked in d09429abe826. With the original reason for using [[:blank:]] now gone, we can replace its usage with plain spaces. A comment about the purpose of the check is added as well. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- cfg.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index ea10ca19f..5cdeb7c65 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1037,8 +1037,10 @@ sc_prohibit_http_urls: halt='Links must use https:// protocol' \ $(_sc_search_regexp) +# Alignment is usually achieved through spaces (at least two of them) +# or tabs (at least one of them) right before the trailing backslash sc_prohibit_backslash_alignment: - @prohibit='([[:blank:]][[:blank:]]| )\\$$' \ + @prohibit='( | )\\$$' \ in_vc_files='*\.([chx]|am|mk)$$' \ halt='Do not attempt to right-align backslashes' \ $(_sc_search_regexp) -- 2.14.3

2 1