- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Dissolve qemuBuildVhostuserCommandLine in qemuBuildInterfaceCommandLine
by Michal Privoznik 05 Nov '18

05 Nov '18

https://bugzilla.redhat.com/show_bug.cgi?id=1524230 The qemuBuildVhostuserCommandLine builds command line for vhostuser type interfaces. It is duplicating some code of the function it is called from (qemuBuildInterfaceCommandLine) because of the way it's called. If we merge it into the caller not only we save a few lines but we also enable checks that we would have to duplicate otherwise (e.g. QoS availability). While at it, reorder some VIR_FREE() in qemuBuildInterfaceCommandLine so that it is easier to track which variables are freed and which are not. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_command.c | 113 +++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6e3ff67660..e338d3172e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8224,34 +8224,24 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, } static int -qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, +qemuInterfaceVhostuserConnect(virQEMUDriverPtr driver, virLogManagerPtr logManager, virSecurityManagerPtr secManager, virCommandPtr cmd, virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, - unsigned int bootindex) + char **chardev) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - char *chardev = NULL; - char *netdev = NULL; - unsigned int queues = net->driver.virtio.queues; - char *nic = NULL; int ret = -1; - if (!qemuDomainSupportsNicdev(def, net)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Nicdev support unavailable")); - goto cleanup; - } - switch ((virDomainChrType)net->data.vhostuser->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: - if (!(chardev = qemuBuildChrChardevStr(logManager, secManager, - cmd, cfg, def, - net->data.vhostuser, - net->info.alias, qemuCaps, 0))) + if (!(*chardev = qemuBuildChrChardevStr(logManager, secManager, + cmd, cfg, def, + net->data.vhostuser, + net->info.alias, qemuCaps, 0))) goto cleanup; break; @@ -8274,42 +8264,9 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, goto cleanup; } - if (queues > 1 && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("multi-queue is not supported for vhost-user " - "with this QEMU binary")); - goto cleanup; - } - - if (!(netdev = qemuBuildHostNetStr(net, driver, - NULL, 0, NULL, 0))) - goto cleanup; - - if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path, - &net->ifname) < 0) - goto cleanup; - - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, chardev); - - virCommandAddArg(cmd, "-netdev"); - virCommandAddArg(cmd, netdev); - - if (!(nic = qemuBuildNicDevStr(def, net, bootindex, - queues, qemuCaps))) { - goto cleanup; - } - - virCommandAddArgList(cmd, "-device", nic, NULL); - ret = 0; cleanup: virObjectUnref(cfg); - VIR_FREE(netdev); - VIR_FREE(chardev); - VIR_FREE(nic); - return ret; } @@ -8328,7 +8285,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, int **nicindexes) { int ret = -1; - char *nic = NULL, *host = NULL; + char *nic = NULL; + char *host = NULL; + char *chardev = NULL; int *tapfd = NULL; size_t tapfdSize = 0; int *vhostfd = NULL; @@ -8337,6 +8296,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, char **vhostfdName = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); virNetDevBandwidthPtr actualBandwidth; + bool requireNicdev = false; size_t i; @@ -8447,9 +8407,24 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: - ret = qemuBuildVhostuserCommandLine(driver, logManager, secManager, cmd, def, - net, qemuCaps, bootindex); - goto cleanup; + requireNicdev = true; + + if (net->driver.virtio.queues > 1 && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("multi-queue is not supported for vhost-user " + "with this QEMU binary")); + goto cleanup; + } + + if (qemuInterfaceVhostuserConnect(driver, logManager, secManager, + cmd, def, net, qemuCaps, &chardev) < 0) + goto cleanup; + + if (virNetDevOpenvswitchGetVhostuserIfname(net->data.vhostuser->data.nix.path, + &net->ifname) < 0) + goto cleanup; + break; case VIR_DOMAIN_NET_TYPE_USER: @@ -8564,6 +8539,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, goto cleanup; } + if (chardev) + virCommandAddArgList(cmd, "-chardev", chardev, NULL); + if (!(host = qemuBuildHostNetStr(net, driver, tapfdName, tapfdSize, vhostfdName, vhostfdSize))) @@ -8578,13 +8556,17 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, */ if (qemuDomainSupportsNicdev(def, net)) { if (!(nic = qemuBuildNicDevStr(def, net, bootindex, - vhostfdSize, qemuCaps))) + net->driver.virtio.queues, qemuCaps))) goto cleanup; virCommandAddArgList(cmd, "-device", nic, NULL); - } else { + } else if (!requireNicdev) { if (!(nic = qemuBuildLegacyNicStr(net))) goto cleanup; virCommandAddArgList(cmd, "-net", nic, NULL); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Nicdev support unavailable")); + goto cleanup; } ret = 0; @@ -8595,24 +8577,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, virSetError(saved_err); virFreeError(saved_err); } - for (i = 0; tapfd && i < tapfdSize && tapfd[i] >= 0; i++) { - if (ret < 0) - VIR_FORCE_CLOSE(tapfd[i]); - if (tapfdName) - VIR_FREE(tapfdName[i]); - } for (i = 0; vhostfd && i < vhostfdSize && vhostfd[i] >= 0; i++) { if (ret < 0) VIR_FORCE_CLOSE(vhostfd[i]); if (vhostfdName) VIR_FREE(vhostfdName[i]); } - VIR_FREE(tapfd); + VIR_FREE(vhostfdName); + for (i = 0; tapfd && i < tapfdSize && tapfd[i] >= 0; i++) { + if (ret < 0) + VIR_FORCE_CLOSE(tapfd[i]); + if (tapfdName) + VIR_FREE(tapfdName[i]); + } + VIR_FREE(tapfdName); VIR_FREE(vhostfd); - VIR_FREE(nic); + VIR_FREE(tapfd); + VIR_FREE(chardev); VIR_FREE(host); - VIR_FREE(tapfdName); - VIR_FREE(vhostfdName); + VIR_FREE(nic); return ret; } -- 2.18.1

2 3

[libvirt] Release of libvirt-4.9.0
by Daniel Veillard 04 Nov '18

04 Nov '18

Except an update to the news file no change was pushed to the tree since RC! so that looks like stable enough for release and so I did. It's tagged in git and signed tarball and rpms are available from the usual place: ftp://libvirt.org/libvirt/ I also tagged and pushed python bindings 4.9.0 release which can be found at ftp://libvirt.org/libvirt/python/ This release is a mix, with one important new feature, the new cgroup v2 and one important security bug for libvirt sockets, users are invited to upgrade: New features: - util: Add cgroup v2 support cgroup v2 support has been implemented in libvirt, with both "unified" (v2 only) and "hybrid" (v2 + v1) setups being usable; existing "legacy" (v1 only) setups will keep working. - qemu: Add vfio AP support The QEMU driver now has support to passthrough adjunct processors into QEMU guests on S390. Improvements: - rpc: Make 'genprotocol' output reproducible This is another step towards making libvirt builds fully reproducible. Bug fixes: - security: Fix permissions for UNIX sockets Since 4.5.0, libvirt is using FD passing to hand sockets over to QEMU, which in theory removes the need for them to be accessible by the user under which the QEMU process is running; however, other processes such as vdsm need to access the sockets as well, which means adjusting permissions is still necessary. - cpu_map: Add Icelake model definitions These CPU models will be available in the upcoming 3.1.0 QEMU release. - util: Properly parse URIs with missing trailing slash Some storage URIs were not parsed correctly, in which case libvirt ended up emitting XML that it would then refuse to parse back. Thanks everybody for your contributions to this release whatever form that took (ideas, bug reports, patches, reviews, docs ...), 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/

1 0

[libvirt] [PATCH] qemu: Process RDMA GID state change event
by Yuval Shaia 04 Nov '18

04 Nov '18

This event is emitted on the monitor when a GID table in pvrdma device is modified and the change needs to be propagate to the backend RDMA device's GID table. The control over the RDMA device's GID table is done by updating the device's Ethernet function addresses. Usually the first GID entry is determine by the MAC address, the second by the first IPv6 address and the third by the IPv4 address. Other entries can be added by adding more IP addresses. The opposite is the same, i.e. whenever an address is removed, the corresponding GID entry is removed. The process is done by the network and RDMA stacks. Whenever an address is added the ib_core driver is notified and calls the device driver's add_gid function which in turn update the device. To support this in pvrdma device we need to hook into the create_bind and destroy_bind HW commands triggered by pvrdma driver in guest. Whenever a changed is made to the pvrdma device's GID table a special QMP messages is sent to be processed by libvirt to update the address of the backend Ethernet device. Signed-off-by: Yuval Shaia <yuval.shaia(a)oracle.com> --- src/qemu/qemu_domain.c | 3 ++ src/qemu/qemu_domain.h | 15 +++++++++ src/qemu/qemu_driver.c | 40 ++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 28 +++++++++++++++++ src/qemu/qemu_monitor.h | 13 ++++++++ src/qemu/qemu_monitor_json.c | 36 ++++++++++++++++++++++ src/qemu/qemu_process.c | 59 ++++++++++++++++++++++++++++++++++++ 7 files changed, 194 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ba3fff607a..8da54c7ee9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -13479,6 +13479,9 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_GUESTPANIC: qemuMonitorEventPanicInfoFree(event->data); break; + case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED: + qemuMonitorEventRdmaGidStatusFree(event->data); + break; case QEMU_PROCESS_EVENT_WATCHDOG: case QEMU_PROCESS_EVENT_DEVICE_DELETED: case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 80bd4bde91..1b188843e3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -478,6 +478,18 @@ struct _qemuDomainVsockPrivate { }; +typedef struct _qemuDomainRdmaGidStatusChangedPrivate qemuDomainRdmaGidStatusChangedPrivate; +typedef qemuDomainRdmaGidStatusChangedPrivate *qemuDomainRdmaGidStatusChangedPrivatePtr; +struct _qemuDomainRdmaGidStatusChangedPrivate { + virObject parent; + + char *netdev; + bool gid_status; + uint64_t subnet_prefix; + uint64_t interface_id; +}; + + typedef enum { QEMU_PROCESS_EVENT_WATCHDOG = 0, QEMU_PROCESS_EVENT_GUESTPANIC, @@ -487,6 +499,7 @@ typedef enum { QEMU_PROCESS_EVENT_BLOCK_JOB, QEMU_PROCESS_EVENT_MONITOR_EOF, QEMU_PROCESS_EVENT_PR_DISCONNECT, + QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; @@ -499,6 +512,8 @@ struct qemuProcessEvent { void *data; }; +void qemuMonitorEventRdmaGidStatusFree(qemuDomainRdmaGidStatusChangedPrivatePtr info); + void qemuProcessEventFree(struct qemuProcessEvent *event); typedef struct _qemuDomainLogContext qemuDomainLogContext; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a52e2495d5..dc088d844f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4788,6 +4788,43 @@ processPRDisconnectEvent(virDomainObjPtr vm) } +static void +processRdmaGidStatusChangedEvent(virDomainObjPtr vm, + qemuDomainRdmaGidStatusChangedPrivatePtr info) +{ + unsigned int prefix_len; + virSocketAddr addr = {0}; + int rc; + + if (!virDomainObjIsActive(vm)) + return; + + VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx", + info->netdev, info->gid_status, info->subnet_prefix, + info->interface_id); + + if (info->subnet_prefix) { + prefix_len = 64; + uint32_t ipv6[4]; + memcpy(&ipv6[0], &info->subnet_prefix, sizeof(info->subnet_prefix)); + memcpy(&ipv6[2], &info->interface_id, sizeof(info->subnet_prefix)); + virSocketAddrSetIPv6AddrNetOrder(&addr, ipv6); + } else { + prefix_len = 24; + virSocketAddrSetIPv4AddrNetOrder(&addr, info->interface_id >> 32); + } + + if (info->gid_status) + rc = virNetDevIPAddrAdd(info->netdev, &addr, NULL, prefix_len); + else + rc = virNetDevIPAddrDel(info->netdev, &addr, prefix_len); + + if (rc) + VIR_ERROR(_("Fail to update address 0x%lx to %s"), info->interface_id, + info->netdev); +} + + static void qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; @@ -4828,6 +4865,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_PR_DISCONNECT: processPRDisconnectEvent(vm); break; + case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED: + processRdmaGidStatusChangedEvent(vm, processEvent->data); + break; case QEMU_PROCESS_EVENT_LAST: break; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7f7013e115..375ed7ceaf 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1686,6 +1686,23 @@ qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon, } +int +qemuMonitorEmitRdmaGidStatusChanged(qemuMonitorPtr mon, const char *netdev, + bool gid_status, uint64_t subnet_prefix, + uint64_t interface_id) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + VIR_DEBUG("netdev='%s',gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx", + netdev, gid_status, subnet_prefix, interface_id); + + QEMU_MONITOR_CALLBACK(mon, ret, domainRdmaGidStatusChanged, mon->vm, netdev, + gid_status, subnet_prefix, interface_id); + + return ret; +} + + int qemuMonitorSetCapabilities(qemuMonitorPtr mon) { @@ -4298,6 +4315,17 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info) } +void +qemuMonitorEventRdmaGidStatusFree(qemuDomainRdmaGidStatusChangedPrivatePtr info) +{ + if (!info) + return; + + VIR_FREE(info->netdev); + VIR_FREE(info); +} + + int qemuMonitorSetWatchdogAction(qemuMonitorPtr mon, const char *action) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 48b142a4f4..f5affe615a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -281,6 +281,14 @@ typedef int (*qemuMonitorDomainPRManagerStatusChangedCallback)(qemuMonitorPtr mo bool connected, void *opaque); +typedef int (*qemuMonitorDomainRdmaGidStatusChangedCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *netdev, + bool gid_status, + uint64_t subnet_prefix, + uint64_t interface_id, + void *opaque); + typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks; typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr; struct _qemuMonitorCallbacks { @@ -314,6 +322,7 @@ struct _qemuMonitorCallbacks { qemuMonitorDomainBlockThresholdCallback domainBlockThreshold; qemuMonitorDomainDumpCompletedCallback domainDumpCompleted; qemuMonitorDomainPRManagerStatusChangedCallback domainPRManagerStatusChanged; + qemuMonitorDomainRdmaGidStatusChangedCallback domainRdmaGidStatusChanged; }; char *qemuMonitorEscapeArg(const char *in); @@ -448,6 +457,10 @@ int qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon, const char *prManager, bool connected); +int qemuMonitorEmitRdmaGidStatusChanged(qemuMonitorPtr mon, const char *netdev, + bool gid_status, uint64_t subnet_prefix, + uint64_t interface_id); + int qemuMonitorStartCPUs(qemuMonitorPtr mon); int qemuMonitorStopCPUs(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3de298c9e2..8df9b426ba 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -91,6 +91,7 @@ static void qemuMonitorJSONHandleAcpiOstInfo(qemuMonitorPtr mon, virJSONValuePtr static void qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleRdmaGidStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -114,6 +115,7 @@ static qemuEventHandler eventHandlers[] = { { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, }, { "POWERDOWN", qemuMonitorJSONHandlePowerdown, }, { "PR_MANAGER_STATUS_CHANGED", qemuMonitorJSONHandlePRManagerStatusChanged, }, + { "RDMA_GID_STATUS_CHANGED", qemuMonitorJSONHandleRdmaGidStatusChanged, }, { "RESET", qemuMonitorJSONHandleReset, }, { "RESUME", qemuMonitorJSONHandleResume, }, { "RTC_CHANGE", qemuMonitorJSONHandleRTCChange, }, @@ -1351,6 +1353,40 @@ static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon, } +static void qemuMonitorJSONHandleRdmaGidStatusChanged(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + const char *netdev; + bool gid_status; + unsigned long long subnet_prefix, interface_id; + + if (!(netdev = virJSONValueObjectGetString(data, "netdev"))) { + VIR_WARN("missing netdev in GID_STATUS_CHANGED event"); + return; + } + + if (virJSONValueObjectGetBoolean(data, "gid-status", &gid_status)) { + VIR_WARN("missing gid-status in GID_STATUS_CHANGED event"); + return; + } + + if (virJSONValueObjectGetNumberUlong(data, "subnet-prefix", + &subnet_prefix)) { + VIR_WARN("missing subnet-prefix in GID_STATUS_CHANGED event"); + return; + } + + if (virJSONValueObjectGetNumberUlong(data, "interface-id", + &interface_id)) { + VIR_WARN("missing interface-id in GID_STATUS_CHANGED event"); + return; + } + + qemuMonitorEmitRdmaGidStatusChanged(mon, netdev, gid_status, subnet_prefix, + interface_id); +} + + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9cf971808c..45fc155d31 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1703,6 +1703,64 @@ qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandleRdmaGidStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, const char *netdev, + bool gid_status, uint64_t subnet_prefix, + uint64_t interface_id, void *opaque) +{ + virQEMUDriverPtr driver = opaque; + qemuDomainObjPrivatePtr priv; + struct qemuProcessEvent *processEvent = NULL; + qemuDomainRdmaGidStatusChangedPrivatePtr rdmaGitStatusChangedPriv; + int ret = -1; + + virObjectLock(vm); + + VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx", + netdev, gid_status, subnet_prefix, interface_id); + + priv = vm->privateData; + priv->prDaemonRunning = false; + + if (VIR_ALLOC(rdmaGitStatusChangedPriv) < 0) + goto out_unlock; + + if (VIR_STRDUP(rdmaGitStatusChangedPriv->netdev, netdev) < 0) + goto out_free_rdma_priv; + + rdmaGitStatusChangedPriv->gid_status = gid_status; + rdmaGitStatusChangedPriv->subnet_prefix = subnet_prefix; + rdmaGitStatusChangedPriv->interface_id = interface_id; + + if (VIR_ALLOC(processEvent) < 0) + goto out_free_rdma_priv_netdev; + + processEvent->eventType = QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED; + processEvent->vm = virObjectRef(vm); + processEvent->data = rdmaGitStatusChangedPriv; + + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + qemuProcessEventFree(processEvent); + virObjectUnref(vm); + goto out_free_rdma_priv_netdev; + } + + ret = 0; + goto out_unlock; + + out_free_rdma_priv_netdev: + VIR_FREE(rdmaGitStatusChangedPriv->netdev); + + out_free_rdma_priv: + VIR_FREE(rdmaGitStatusChangedPriv); + + out_unlock: + virObjectUnlock(vm); + return ret; +} + + static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, @@ -1732,6 +1790,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainBlockThreshold = qemuProcessHandleBlockThreshold, .domainDumpCompleted = qemuProcessHandleDumpCompleted, .domainPRManagerStatusChanged = qemuProcessHandlePRManagerStatusChanged, + .domainRdmaGidStatusChanged = qemuProcessHandleRdmaGidStatusChanged, }; static void -- 2.17.2

1 0

[libvirt] [PATCH v1 1/1] qemu_domain, qemu_process: maxCpus check to non-x86 guests
by Daniel Henrique Barboza 03 Nov '18

03 Nov '18

This patch makes two quality of life changes for non-x86 guests. The first one is a maxCpus validation at qemuDomainDefValidate. The check is made by the same function used to do that at qemuProcessStartValidateXML, qemuValidateCpuCount. This ensures that the user doesn't goes over the board with the maxCpus value when editing the XML, only to be faced with a runtime error when starting it. To do that, the following changes were made: - qemuValidateCpuCount was made public. It was renamed to qemuProcessValidateCpuCount to be compliant with the other public methods at qemu_process.h; - the method signature was slightly adapted to fit the const 'def' variable used in qemuDomainDefValidate. This change has no downside in in its original usage at qemuProcessStartValidateXML. The second QoL change is adding the maxCpus value in the error message of the now qemuProcessValidateCpuCount. This simple change allows the user to quickly edit the XML to comply with the acceptable limit without having to know QEMU internals. x86 guests, that might have been created prior to the x86 qemuDomainDefValidate maxCpus check code, will also benefit from this change. After this patch, this is the expect result running a Power 9 guest with a maxCpus of 40000: $ ./virsh start dhb error: Failed to start domain dhb error: unsupported configuration: Maximum CPUs greater than specified machine type limit 240 And this is the result when trying to edit maxCpus to an invalid value: $ ./virsh edit dhb error: unsupported configuration: Maximum CPUs greater than specified machine type limit 240 Failed. Try again? [y,n,i,f,?]: error: unsupported configuration: Maximum CPUs greater than specified machine type limit 240 Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com> --- src/qemu/qemu_domain.c | 5 +++++ src/qemu/qemu_process.c | 13 +++++++------ src/qemu/qemu_process.h | 3 +++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 939b2a3da2..5a39b11da7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4100,6 +4100,11 @@ qemuDomainDefValidate(const virDomainDef *def, } } + if (!ARCH_IS_X86(def->os.arch) && + qemuProcessValidateCpuCount(def, qemuCaps) < 0) { + goto cleanup; + } + if (def->nresctrls && def->virtType != VIR_DOMAIN_VIRT_KVM) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 29b0ba1590..0de5b503d6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3901,9 +3901,9 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, } -static int -qemuValidateCpuCount(virDomainDefPtr def, - virQEMUCapsPtr qemuCaps) +int +qemuProcessValidateCpuCount(const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { unsigned int maxCpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, def->os.machine); @@ -3914,8 +3914,9 @@ qemuValidateCpuCount(virDomainDefPtr def, } if (maxCpus > 0 && virDomainDefGetVcpusMax(def) > maxCpus) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Maximum CPUs greater than specified machine type limit")); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Maximum CPUs greater than specified machine type limit %u"), + maxCpus); return -1; } @@ -5165,7 +5166,7 @@ qemuProcessStartValidateXML(virQEMUDriverPtr driver, * If back compat isn't a concern, XML validation should probably * be done at parse time. */ - if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) + if (qemuProcessValidateCpuCount(vm->def, qemuCaps) < 0) return -1; /* checks below should not be executed when starting a qemu process for a diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index c2f7c2b5d2..1716230475 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -215,4 +215,7 @@ int qemuProcessStartManagedPRDaemon(virDomainObjPtr vm); void qemuProcessKillManagedPRDaemon(virDomainObjPtr vm); +int qemuProcessValidateCpuCount(const virDomainDef *def, + virQEMUCapsPtr qemuCaps); + #endif /* __QEMU_PROCESS_H__ */ -- 2.17.1

2 3

Re: [libvirt] [PATCH v1 2/7] pcihp: overwrite hotplug handler recursively from the start
by Igor Mammedov 02 Nov '18

02 Nov '18

On Wed, 24 Oct 2018 12:19:25 +0200 David Hildenbrand <david(a)redhat.com> wrote: > For now, the hotplug handler is not called for devices that are > being cold plugged. The hotplug handler is setup when the machine > initialization is fully done. Only bridges that were cold plugged are > considered. > > Set the hotplug handler for the root piix bus directly when realizing. > Overwrite the hotplug handler of bridges when hotplugging/coldplugging > them. > > This will now make sure that the ACPI PCI hotplug handler is also called > for cold-plugged devices (also on bridges) and for bridges that were > hotplugged. > > When trying to hotplug a device to a hotplugged bridge, we now correctly > get the error message > "Unsupported bus. Bus doesn't have property 'acpi-pcihp-bsel' set" > Insted of going via the standard PCI hotplug handler. Erroring out is probably not ok, since it can break existing setups where SHPC hotplugging to hotplugged bridge was working just fine before. Marcel/Michael what's your take on this change in behaviour? CCing libvirt in case they are doing this stuff > > Signed-off-by: David Hildenbrand <david(a)redhat.com> > --- > hw/acpi/pcihp.c | 10 ++++++++++ > hw/acpi/piix4.c | 16 +--------------- > 2 files changed, 11 insertions(+), 15 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index 5e7cef173c..647b5e8d82 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -30,6 +30,7 @@ > #include "hw/hw.h" > #include "hw/i386/pc.h" > #include "hw/pci/pci.h" > +#include "hw/pci/pci_bridge.h" > #include "hw/acpi/acpi.h" > #include "sysemu/sysemu.h" > #include "exec/address-spaces.h" > @@ -236,6 +237,15 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, > int slot = PCI_SLOT(pdev->devfn); > int bsel; > > + if (!s->legacy_piix && object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) { > + PCIBus *sec = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev)); > + > + /* Overwrite the default hotplug handler with the ACPI PCI one. */ > + qbus_set_hotplug_handler(BUS(sec), DEVICE(hotplug_dev), &error_abort); > + /* No need to do it recursively */ > + assert(QLIST_EMPTY(&sec->child)); > + } > + > /* Don't send event when device is enabled during qemu machine creation: > * it is present on boot, no hotplug event is necessary. We do send an > * event when the device is disabled later. */ > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index 167afe2cea..e611778daf 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -446,15 +446,6 @@ static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, > } > } > > -static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque) > -{ > - PIIX4PMState *s = opaque; > - > - /* pci_bus cannot outlive PIIX4PMState, because /machine keeps it alive > - * and it's not hot-unpluggable */ > - qbus_set_hotplug_handler(BUS(pci_bus), DEVICE(s), &error_abort); > -} > - > static void piix4_pm_machine_ready(Notifier *n, void *opaque) > { > PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready); > @@ -468,12 +459,6 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque) > pci_conf[0x63] = 0x60; > pci_conf[0x67] = (memory_region_present(io_as, 0x3f8) ? 0x08 : 0) | > (memory_region_present(io_as, 0x2f8) ? 0x90 : 0); > - > - if (s->use_acpi_pci_hotplug) { > - pci_for_each_bus(pci_get_bus(d), piix4_update_bus_hotplug, s); > - } else { > - piix4_update_bus_hotplug(pci_get_bus(d), s); > - } > } > > static void piix4_pm_add_propeties(PIIX4PMState *s) > @@ -547,6 +532,7 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp) > > piix4_acpi_system_hot_add_init(pci_address_space_io(dev), > pci_get_bus(dev), s); > + qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), DEVICE(s), &error_abort); > > piix4_pm_add_propeties(s); > }

3 4

[libvirt] [PATCH] news: Update for 4.9.0 release
by Andrea Bolognani 02 Nov '18

02 Nov '18

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- docs/news.xml | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index e5476a3332..b75df36ddb 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,16 @@ <libvirt> <release version="v4.9.0" date="unreleased"> <section title="New features"> + <change> + <summary> + util: Add cgroup v2 support + </summary> + <description> + cgroup v2 support has been implemented in libvirt, with both + "unified" (v2 only) and "hybrid" (v2 + v1) setups being usable; + existing "legacy" (v1 only) setups will keep working. + </description> + </change> <change> <summary> qemu: Add vfio AP support @@ -46,8 +56,47 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + rpc: Make 'genprotocol' output reproducible + </summary> + <description> + This is another step towards making libvirt builds fully + reproducible. + </description> + </change> </section> <section title="Bug fixes"> + <change> + <summary> + security: Fix permissions for UNIX sockets + </summary> + <description> + Since 4.5.0, libvirt is using FD passing to hand sockets over to + QEMU, which in theory removes the need for them to be accessible by + the user under which the QEMU process is running; however, other + processes such as vdsm need to access the sockets as well, which + means adjusting permissions is still necessary. + </description> + </change> + <change> + <summary> + cpu_map: Add Icelake model definitions + </summary> + <description> + These CPU models will be available in the upcoming 3.1.0 QEMU + release. + </description> + </change> + <change> + <summary> + util: Properly parse URIs with missing trailing slash + </summary> + <description> + Some storage URIs were not parsed correctly, in which case libvirt + ended up emitting XML that it would then refuse to parse back. + </description> + </change> </section> </release> <release version="v4.8.0" date="2018-10-01"> -- 2.19.1

2 1

[libvirt] External disk snapshot paths and the revert operation
by Povilas Kanapickas 02 Nov '18

02 Nov '18

Hey, It seems that there's an issue of how external disk snapshots currently work. The snapshot definition xml[1] that is supplied to the API has a list of disks each of which specify where the new overlay disks should be placed. This leads to ambiguities when we want to revert the domain to a snapshot with children. Consider the following situation: 0. Initial state: Domain definition: disk at disk-base.qcow2 1. We create a snapshot snap1 placing the overlay at disk-snap1.qcow2 Domain definition: disk at disk-snap1.qcow2 Disks: - disk-base.qcow2 (read-only) - disk-snap1.qcow2 (overlay, base: disk-base.qcow2) Snapshots: - snap1 (snapshot domain disk list: disk-base.qcow2, snapshot disk list: disk-snap1.qcow2) 2. We create another snapshot snap2 placing the overlay at disk-snap2.qcow2 VM definition: disk at disk-snap2.qcow2 Disks: - disk-base.qcow2 (read-only) - disk-snap1.qcow2 (overlay, base: disk-base.qcow2, read-only) - disk-snap2.qcow2 (overlay, base: disk-snap1.qcow2) Snapshots: - snap1 (snapshot domain disk list: disk-base.qcow2, snapshot disk list: disk-snap1.qcow2) - snap2 (snapshot domain disk list: disk-snap1.qcow2, snapshot disk list: disk-snap2.qcow2) Now what happens if we want to revert to snap1? We can't use any of the paths that snap1 itself references: both disk-base.qcow2 and disk-snap1.qcow2 are read-only as part of the backing chain for disks used by snap2. Possible options are these: - we can reuse disk-snap2.qcow2 which we no longer need as it contains overlay on top of snap2. This is non-optimal because now the path of the disk after revert depends on the disk path before it. - we can think of some name with the expectation that the user will change it later anyway. I think that both of the above are non-optimal because we don't actually revert to the exact state we took the snapshot of. The user is effectively no longer in control of the disk path after the revert. One of possible solutions would be add additional flag during the snapshot creation that would change the current behavior. Instead of creating a new disk for the overlay we could move the base disk to the specified path and create the overlay in its place. The advantages of this behavior would be the following: - the path to the current disk used in the domain would not change when taking snapshots. This way we could avoid the problem listed above and easily revert to any snapshot. - the confusion in naming of the overlay disks would be reduced. Currently the path specified when creating snapshot refers to the overlay image. Most of the time user does not know what will be changed in the domain until the next snapshot. So any path he chooses will likely not reflect what will end up in the overlay at the time of the next snapshot. If it was possible to specify the path where the current disk should be put instead, then the user could just name it after the snapshot and it would correctly represent the contents of that disk. The major disadvantage of the approach is that it would introduce extra complexity in the presently simple interface and also the underlying code. I've completely disregarded the case of taking snapshots while the domain is running, so there might be hidden complications there as well. Personally, I don't mind the current situation too much, but if libvirt developers think it's a thing worth fixing I would be willing to work on it. Any opinions? Regards, Povilas [1]: https://libvirt.org/formatsnapshot.html

2 3

[libvirt] [PATCH] nwfilter: intantiate filters on loading driver
by Nikolay Shirokovskiy 02 Nov '18

02 Nov '18

Before using filters binding filters instantiation was done by hypervisors drivers initialization code (qemu was the only such hypervisor). Now qemu reconnection done supposes it should be done by nwfilter driver probably. Let's add this missing step. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com> --- src/nwfilter/nwfilter_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 1ee5162..1ab906f 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -264,6 +264,9 @@ nwfilterStateInitialize(bool privileged, if (virNWFilterBindingObjListLoadAllConfigs(driver->bindings, driver->bindingDir) < 0) goto error; + if (virNWFilterBuildAll(driver, false) < 0) + goto error; + nwfilterDriverUnlock(); return 0; -- 1.8.3.1

2 10

[libvirt] [PATCH v2] nodedev: Document the udevEventHandleThread
by John Ferlan 02 Nov '18

02 Nov '18

Commit cdbe1332 neglected to document the API. So let's add some details about the algorithm and why it was used to help future readers understand the issues encountered. Based largely off a description provided by Erik Skultety <eskultet(a)redhat.com>. NB: Management of the processing udev device notification is a delicate balance between the udev process, the scheduler, and when exactly the data from/for the socket is received. The balance is particularly important for environments when multiple devices are added into the system more or less simultaneously such as is done for mdev. In these cases scheduler blocking on the udev recv() call is more noticeable. It's expected that future devices will follow similar algorithms. Even though the algorithm does present some challenges for older OS's (such as Centos 6), trying to rewrite the algorithm to fit both models would be more complex and involve pulling the monitor object out of the private data lockable object and would need to be guarded by a separate lock. Devising such an algorithm to work around issues with older OS's at the expense of more modern OS algorithms in newer event processing code may result in unexpected issues, so the choice is to encourage use of newer OS's with newer udev event processing code. Signed-off-by: John Ferlan <jferlan(a)redhat.com> --- v1: https://www.redhat.com/archives/libvir-list/2018-October/msg01053.html Changes are from code review with some minor tweaks/editing as I went along. Mistakes are my own! src/node_device/node_device_udev.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 22897591de..f2c2299d4d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1591,6 +1591,26 @@ udevEventMonitorSanityCheck(udevEventDataPtr priv, } +/** + * udevEventHandleThread + * @opaque: unused + * + * Thread to handle the udevEventHandleCallback processing when udev + * tells us there's a device change for us (add, modify, delete, etc). + * + * Once notified there is data to be processed, the actual @device + * data retrieval by libudev may be delayed due to how threads are + * scheduled. In fact, the event loop could be scheduled earlier than + * the handler thread, thus potentially emitting the very same event + * the handler thread is currently trying to process, simply because + * the data hadn't been retrieved from the socket. + * + * NB: Usage of event based socket algorithm causes some issues with + * older platforms such as CentOS 6 in which the data socket is opened + * without the NONBLOCK bit set. That can be avoided by moving the reset + * priv->dataReady to just after the udev_monitor_receive_device; however, + * scheduler issues would still come into play. + */ static void udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) { @@ -1637,6 +1657,9 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) return; } + /* Trying to move the reset of the @priv->dataReady flag to + * after the udev_monitor_receive_device wouldn't help much + * due to event mgmt and scheduler timing. */ virObjectLock(priv); priv->dataReady = false; virObjectUnlock(priv); @@ -1646,6 +1669,11 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) udevHandleOneDevice(device); udev_device_unref(device); + + /* Instead of waiting for the next event after processing @device + * data, let's keep reading from the udev monitor and only wait + * for the next event once either a EAGAIN or a EWOULDBLOCK error + * is encountered. */ } } -- 2.17.2

3 5

[libvirt] [jenkins-ci PATCH v2 0/3] Add libvirt-ocaml
by Pino Toscano 02 Nov '18

02 Nov '18

Now that the libvirt-ocaml repository is fixed, let's build it in CI. Changes from v1 to v2: - split according to Andrea's hints - removed --with-libvirt as argument for configure, no more needed after upstream changes Pino Toscano (3): guests: add mappings for OCaml components guests: pull dependencies for libvirt-ocaml Add libvirt-ocaml project guests/host_vars/libvirt-centos-7/main.yml | 1 + guests/host_vars/libvirt-debian-9/main.yml | 1 + guests/host_vars/libvirt-debian-sid/main.yml | 1 + guests/host_vars/libvirt-fedora-27/main.yml | 1 + guests/host_vars/libvirt-fedora-28/main.yml | 1 + .../host_vars/libvirt-fedora-rawhide/main.yml | 1 + guests/host_vars/libvirt-freebsd-10/main.yml | 1 + guests/host_vars/libvirt-freebsd-11/main.yml | 1 + .../libvirt-freebsd-current/main.yml | 1 + guests/host_vars/libvirt-ubuntu-16/main.yml | 1 + guests/host_vars/libvirt-ubuntu-18/main.yml | 1 + guests/playbooks/build/jobs/defaults.yml | 2 ++ .../build/projects/libvirt-ocaml.yml | 27 +++++++++++++++++++ guests/vars/mappings.yml | 6 +++++ guests/vars/projects/libvirt-ocaml.yml | 5 ++++ jobs/defaults.yaml | 2 ++ projects/libvirt-ocaml.yaml | 23 ++++++++++++++++ 17 files changed, 76 insertions(+) create mode 100644 guests/playbooks/build/projects/libvirt-ocaml.yml create mode 100644 guests/vars/projects/libvirt-ocaml.yml create mode 100644 projects/libvirt-ocaml.yaml -- 2.17.2

3 7