- Devel - libvirt List Archives

[libvirt PATCH v4 00/25] Add support for persistent mediated devices
by Jonathon Jongsma 01 Mar '21

01 Mar '21

This patch series follows the previously-merged series which added support for transient mediated devices. This series expands mdev support to include persistent device definitions. Again, it relies on mdevctl as the backend. It follows the common libvirt pattern of APIs by adding the following new APIs for node devices: - virNodeDeviceDefineXML() - defines a persistent device - virNodeDeviceUndefine() - undefines a persistent device - virNodeDeviceCreate() - starts a previously-defined device It also adds virsh commands mapping to these new APIs: nodedev-define, nodedev-undefine, and nodedev-start. Since we rely on mdevctl for the definition of mediated devices, we need a way to stay up-to-date with devices that are defined by mdevctl (outside of libvirt). The method for staying up-to-date is currently a little bit crude due to the fact that mdevctl does not emit any events when new devices are added or removed. As a workaround, we create a file monitor for the mdevctl config directory and re-query mdevctl when we detect changes within that directory. In the future, mdevctl may introduce a more elegant solution. Changes in v4: - rebase to git master - switch to throwaway thread for querying mdevctl - fixed a bug when removing devices because I was accidentally using virHashForEach() instead of virHashForeachSafe() - use DEFINED/UNDEFINED events instead of STARTED/STOPPED events - changes related to merging information about mdev devices from both udev a= nd mdevctl: - Re-used the same function to copy extra data from mdevctl regardless of whether we're processing a udev event or a mdevctl event (recommended by Erik). This results in slightly more complex handling of the object lifetimes (see patch 9), but it consolidates some code. - nodeDeviceDefCopyFromMdevctl() previously only copied the data that was unique to mdevctl and didn't exist in udev. It now copies additional data (possibly overwriting some udev). This solves a problem where a device = is defined but not active (i.e. we have not gotten any data from udev), and then changed (e.g. somebody calls 'mdevctl modify' to change the mdev type), but libvirt was not updating to the new definition. - fix a bug where we were mistakenly emitting 'update' events for devices th= at had not changed - Added the ability to specify a uuid for an mdev via device XML. - split some commits into multiple patches - updated new API version info to 7.1.0 - Fixed a bug reported by Yan Fu which hangs the client when attempting to destroy a nodedev that is in use by an active vm. See https://www.redhat.com/archives/libvir-list/2021-February/msg00116.html for solution suggested by Alex. - numerous smaller fixes from review findings changes in v3: - streamlined tests -- removed some unnecessary duplication - split out patch to factor out node device name generation function - split nodeDeviceParseMdevctlChildDevice() into a separate function - added follow-up patch to remove space-padded alignment in header - refactored the mdevctl update handling significantly: - no longer a separate persistent thread that gets signaled by a timer - now piggybacks onto the existing udev thread and signals the thread in t= he same way that the udev event does. - Daniel suggested spawning a throw-away thread to handle mdevctl updates, but that introduces the complexity of possibly serializing multiple throw-away threads (e.g. if we get an 'created' event followed immediate= ly by a 'deleted' event, two threads may be spawned and we'd need to ensure they are properly ordered) - added virNodeDeviceObjListForEach() and virNodeDeviceObjListRemoveLocked() to simplify removing devices that are removed from mdevctl. - coding style fixes - NOTE: per Erik's request, I experimented with changing the way that mdevctl commands were generated and tested (e.g. introducing something like virMdevctlGetCommand(def, MDEVCTL_COMMAND_<SUBCOMMAND>, ...)), but it was too invasive and awkward and didn't seem worthwhile Changes in v2: - rebase to latest git master Jonathon Jongsma (25): tests: remove extra trailing semicolon nodedev: introduce concept of 'active' node devices nodedev: Add ability to filter by active state nodedev: expose internal helper for naming devices nodedev: add ability to parse mdevs from mdevctl nodedev: add ability to list defined mdevs nodedev: add persistence to virNodeDeviceObj nodedev: add DEFINED/UNDEFINED lifecycle events nodedev: add mdevctl devices to node device list nodedev: add helper functions to remove node devices nodedev: handle mdevs that disappear from mdevctl nodedev: Refresh mdev devices when changes are detected nodedev: add function to generate mdevctl define command api: add virNodeDeviceDefineXML() virsh: Add --inactive, --all to nodedev-list virsh: add nodedev-define command nodedev: refactor tests to support mdev undefine api: add virNodeDeviceUndefine() virsh: Factor out function to find node device virsh: add nodedev-undefine command api: add virNodeDeviceCreate() virsh: add "nodedev-start" command nodedev: add <uuid> element to mdev caps nodedev: add ability to specify UUID for new mdevs nodedev: fix hang when destroying an mdev in use docs/schemas/nodedev.rng | 43 +- examples/c/misc/event-test.c | 4 + include/libvirt/libvirt-nodedev.h | 19 +- src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 + src/conf/node_device_conf.c | 15 + src/conf/node_device_conf.h | 8 + src/conf/virnodedeviceobj.c | 149 +++- src/conf/virnodedeviceobj.h | 25 + src/driver-nodedev.h | 14 + src/libvirt-nodedev.c | 115 +++ src/libvirt_private.syms | 6 + src/libvirt_public.syms | 7 + src/node_device/node_device_driver.c | 657 +++++++++++++++++- src/node_device/node_device_driver.h | 41 ++ src/node_device/node_device_udev.c | 214 +++++- src/remote/remote_driver.c | 3 + src/remote/remote_protocol.x | 40 +- src/remote_protocol-structs | 16 + src/rpc/gendispatch.pl | 1 + ...19_36ea_4111_8f0a_8c9a70e21366-define.argv | 2 + ...19_36ea_4111_8f0a_8c9a70e21366-define.json | 1 + ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 3 +- ...39_495e_4243_ad9f_beb3f14c23d9-define.argv | 1 + ...39_495e_4243_ad9f_beb3f14c23d9-define.json | 1 + ...16_1ca8_49ac_b176_871d16c13076-define.argv | 1 + ...16_1ca8_49ac_b176_871d16c13076-define.json | 1 + tests/nodedevmdevctldata/mdevctl-create.argv | 1 + .../mdevctl-list-defined.argv | 1 + .../mdevctl-list-multiple.json | 59 ++ .../mdevctl-list-multiple.out.xml | 43 ++ .../nodedevmdevctldata/mdevctl-undefine.argv | 1 + tests/nodedevmdevctltest.c | 222 +++++- ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 1 + tools/virsh-nodedev.c | 268 ++++++- 35 files changed, 1853 insertions(+), 138 deletions(-) create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= a70e21366-define.argv create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9= a70e21366-define.json create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= 3f14c23d9-define.argv create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb= 3f14c23d9-define.json create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= d16c13076-define.argv create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871= d16c13076-define.json create mode 100644 tests/nodedevmdevctldata/mdevctl-create.argv create mode 100644 tests/nodedevmdevctldata/mdevctl-list-defined.argv create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.json create mode 100644 tests/nodedevmdevctldata/mdevctl-list-multiple.out.xml create mode 100644 tests/nodedevmdevctldata/mdevctl-undefine.argv --=20 2.26.2

3 55

[PATCH] Use g_steal_pointer where possible
by Kristina Hanicova 01 Mar '21

01 Mar '21

Via coccinelle (not the hanbag!) spatches used: @ rule1 @ identifier a, b; symbol NULL; @@ - b = a; ... when != a - a = NULL; + b = g_steal_pointer(&a); @@ - *b = a; ... when != a - a = NULL; + *b = g_steal_pointer(&a); Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com> --- src/admin/admin_server.c | 6 ++---- src/conf/capabilities.c | 3 +-- src/conf/netdev_bandwidth_conf.c | 3 +-- src/conf/virdomainobjlist.c | 3 +-- src/conf/virnwfilterobj.c | 3 +-- src/interface/interface_backend_netcf.c | 3 +-- src/interface/interface_backend_udev.c | 3 +-- src/qemu/qemu_driver.c | 12 ++++------- src/qemu/qemu_monitor_json.c | 27 +++++++++---------------- src/remote/remote_driver.c | 12 ++++------- src/rpc/virnetserver.c | 3 +-- src/util/virfile.c | 9 +++------ src/util/virlog.c | 6 ++---- src/util/virmdev.c | 3 +-- src/util/virsysinfo.c | 21 +++++++------------ src/util/virtypedparam.c | 3 +-- src/vbox/vbox_common.c | 3 +-- src/vz/vz_driver.c | 3 +-- tests/commandhelper.c | 3 +-- tools/virsh-checkpoint.c | 3 +-- tools/virsh-domain.c | 3 +-- 21 files changed, 45 insertions(+), 90 deletions(-) diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c index ebc0cfb045..7d1a21a678 100644 --- a/src/admin/admin_server.c +++ b/src/admin/admin_server.c @@ -50,8 +50,7 @@ adminConnectListServers(virNetDaemonPtr dmn, goto cleanup; if (servers) { - *servers = srvs; - srvs = NULL; + *servers = g_steal_pointer(&srvs); } cleanup: if (ret > 0) @@ -179,8 +178,7 @@ adminServerListClients(virNetServerPtr srv, return -1; if (clients) { - *clients = clts; - clts = NULL; + *clients = g_steal_pointer(&clts); } virObjectListFreeCount(clts, ret); diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 69d9bb0e38..f610975ae5 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1501,9 +1501,8 @@ virCapabilitiesGetNUMASiblingInfo(int node, if (VIR_REALLOC_N(tmp, tmp_size) < 0) goto cleanup; - *siblings = tmp; *nsiblings = tmp_size; - tmp = NULL; + *siblings = g_steal_pointer(&tmp); tmp_size = 0; ret = 0; cleanup: diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 4fb7aa4e3d..ff25641b2c 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -205,8 +205,7 @@ virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, if (!def->in && !def->out) VIR_FREE(def); - *bandwidth = def; - def = NULL; + *bandwidth = g_steal_pointer(&def); ret = 0; cleanup: diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 2521cd56c2..d37bbc94a6 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -1083,8 +1083,7 @@ virDomainObjListExport(virDomainObjListPtr domlist, goto cleanup; } - *domains = doms; - doms = NULL; + *domains = g_steal_pointer(&doms); } ret = nvms; diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 3157522eb2..f4bf1b083d 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -464,8 +464,7 @@ virNWFilterObjListExport(virConnectPtr conn, virNWFilterObjUnlock(obj); } - *filters = tmp_filters; - tmp_filters = NULL; + *filters = g_steal_pointer(&tmp_filters); ret = nfilters; cleanup: diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index e40a4cb108..f8613fea86 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -715,8 +715,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn, if (tmp_iface_objs) { /* trim the array to the final size */ ignore_value(VIR_REALLOC_N(tmp_iface_objs, niface_objs + 1)); - *ifaces = tmp_iface_objs; - tmp_iface_objs = NULL; + *ifaces = g_steal_pointer(&tmp_iface_objs); } ret = niface_objs; diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 6a94a45044..ab95b2e3ff 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -394,8 +394,7 @@ udevConnectListAllInterfaces(virConnectPtr conn, /* Trim the array to its final size */ if (ifaces) { ignore_value(VIR_REALLOC_N(ifaces_list, count + 1)); - *ifaces = ifaces_list; - ifaces_list = NULL; + *ifaces = g_steal_pointer(&ifaces_list); } return count; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b9bbdf8d48..d1a3659774 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5097,8 +5097,7 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, bitmap = NULL; } - *info = info_ret; - info_ret = NULL; + *info = g_steal_pointer(&info_ret); ret = targetDef->niothreadids; cleanup: @@ -9133,9 +9132,8 @@ qemuDomainGetPerfEvents(virDomainPtr dom, goto endjob; } - *params = par; *nparams = npar; - par = NULL; + *params = g_steal_pointer(&par); npar = 0; ret = 0; @@ -18729,8 +18727,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, virObjectUnlock(vm); } - *retStats = tmpstats; - tmpstats = NULL; + *retStats = g_steal_pointer(&tmpstats); ret = nstats; @@ -19259,9 +19256,8 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, #undef ADD_BITMAP - *params = par; *nparams = npar; - par = NULL; + *params = g_steal_pointer(&par); ret = 0; cleanup: diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f8c78d9093..819389f5f9 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5820,8 +5820,7 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, } ret = n; - *machines = infolist; - infolist = NULL; + *machines = g_steal_pointer(&infolist); cleanup: if (infolist) { @@ -6268,8 +6267,7 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, } ret = n; - *commands = commandlist; - commandlist = NULL; + *commands = g_steal_pointer(&commandlist); cleanup: @@ -6327,8 +6325,7 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, } ret = n; - *events = eventlist; - eventlist = NULL; + *events = g_steal_pointer(&eventlist); cleanup: g_strfreev(eventlist); @@ -6472,8 +6469,7 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, } ret = n; - *types = typelist; - typelist = NULL; + *types = g_steal_pointer(&typelist); cleanup: g_strfreev(typelist); @@ -6542,8 +6538,7 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon, } ret = n; - *paths = pathlist; - pathlist = NULL; + *paths = g_steal_pointer(&pathlist); cleanup: if (pathlist) { @@ -6779,8 +6774,7 @@ qemuMonitorJSONParsePropsList(virJSONValuePtr cmd, } ret = count; - *props = proplist; - proplist = NULL; + *props = g_steal_pointer(&proplist); cleanup: g_strfreev(proplist); @@ -6962,8 +6956,7 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, } ret = n; - *capabilities = list; - list = NULL; + *capabilities = g_steal_pointer(&list); cleanup: g_strfreev(list); @@ -7091,8 +7084,7 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, } ret = n; - *capabilities = list; - list = NULL; + *capabilities = g_steal_pointer(&list); cleanup: VIR_FREE(list); @@ -8157,8 +8149,7 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, } *niothreads = n; - *iothreads = infolist; - infolist = NULL; + *iothreads = g_steal_pointer(&infolist); ret = 0; cleanup: diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a83cd866e7..31868269b1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6299,8 +6299,7 @@ remoteConnectGetCPUModelNames(virConnectPtr conn, retmodels[i] = ret.models.models_val[i]; ret.models.models_val[i] = NULL; } - *models = retmodels; - retmodels = NULL; + *models = g_steal_pointer(&retmodels); } rv = ret.ret; @@ -7464,8 +7463,7 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net, goto cleanup; } - *leases = leases_ret; - leases_ret = NULL; + *leases = g_steal_pointer(&leases_ret); } rv = ret.ret; @@ -7555,8 +7553,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, elem = NULL; } - *retStats = tmpret; - tmpret = NULL; + *retStats = g_steal_pointer(&tmpret); rv = ret.retStats.retStats_len; cleanup: @@ -7774,8 +7771,7 @@ remoteDomainInterfaceAddresses(virDomainPtr dom, } } } - *ifaces = ifaces_ret; - ifaces_ret = NULL; + *ifaces = g_steal_pointer(&ifaces_ret); rv = ret.ifaces.ifaces_len; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index f0b866a962..265400ecfa 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -1146,8 +1146,7 @@ virNetServerGetClients(virNetServerPtr srv, } } - *clts = list; - list = NULL; + *clts = g_steal_pointer(&list); ret = nclients; cleanup: diff --git a/src/util/virfile.c b/src/util/virfile.c index 5710495bbf..7cb60e66d3 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -840,8 +840,7 @@ int virFileLoopDeviceAssociate(const char *file, } VIR_DEBUG("Attached loop device %s %d to %s", file, lofd, loname); - *dev = loname; - loname = NULL; + *dev = g_steal_pointer(&loname); ret = 0; @@ -3200,8 +3199,7 @@ virFileOpenTty(int *ttyprimary, char **ttyName, int rawmode) errno = rc; goto cleanup; } - *ttyName = name; - name = NULL; + *ttyName = g_steal_pointer(&name); } ret = 0; @@ -3637,9 +3635,8 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs, tmp->deflt = tmp->size == default_hugepagesz; } - *ret_fs = fs; *ret_nfs = nfs; - fs = NULL; + *ret_fs = g_steal_pointer(&fs); nfs = 0; ret = 0; diff --git a/src/util/virlog.c b/src/util/virlog.c index cdfeba8251..00c0fcc457 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1658,8 +1658,7 @@ virLogParseOutputs(const char *src, virLogOutputPtr **outputs) } ret = noutputs; - *outputs = list; - list = NULL; + *outputs = g_steal_pointer(&list); cleanup: g_strfreev(strings); return ret; @@ -1708,8 +1707,7 @@ virLogParseFilters(const char *src, virLogFilterPtr **filters) } ret = nfilters; - *filters = list; - list = NULL; + *filters = g_steal_pointer(&list); cleanup: g_strfreev(strings); return ret; diff --git a/src/util/virmdev.c b/src/util/virmdev.c index db40fe8cc3..2b7f730bd1 100644 --- a/src/util/virmdev.c +++ b/src/util/virmdev.c @@ -95,8 +95,7 @@ virMediatedDeviceGetSysfsDeviceAPI(virMediatedDevicePtr dev, if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; - *device_api = buf; - buf = NULL; + *device_api = g_steal_pointer(&buf); return 0; } diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 995650334c..0016028254 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -258,8 +258,7 @@ virSysinfoParsePPCSystem(const char *base, virSysinfoSystemDefPtr *sysdef) def = NULL; } - *sysdef = def; - def = NULL; + *sysdef = g_steal_pointer(&def); ret = 0; virSysinfoSystemDefFree(def); return ret; @@ -381,8 +380,7 @@ virSysinfoParseARMSystem(const char *base, virSysinfoSystemDefPtr *sysdef) def = NULL; } - *sysdef = def; - def = NULL; + *sysdef = g_steal_pointer(&def); ret = 0; virSysinfoSystemDefFree(def); return ret; @@ -521,8 +519,7 @@ virSysinfoParseS390System(const char *base, virSysinfoSystemDefPtr *sysdef) def = NULL; } - *sysdef = def; - def = NULL; + *sysdef = g_steal_pointer(&def); ret = 0; cleanup: virSysinfoSystemDefFree(def); @@ -686,8 +683,7 @@ virSysinfoParseBIOS(const char *base, virSysinfoBIOSDefPtr *bios) def = NULL; } - *bios = def; - def = NULL; + *bios = g_steal_pointer(&def); ret = 0; virSysinfoBIOSDefFree(def); return ret; @@ -763,8 +759,7 @@ virSysinfoParseX86System(const char *base, virSysinfoSystemDefPtr *sysdef) def = NULL; } - *sysdef = def; - def = NULL; + *sysdef = g_steal_pointer(&def); ret = 0; virSysinfoSystemDefFree(def); return ret; @@ -845,9 +840,8 @@ virSysinfoParseX86BaseBoard(const char *base, ignore_value(VIR_REALLOC_N(boards, nboards)); } - *baseBoard = boards; *nbaseBoard = nboards; - boards = NULL; + *baseBoard = g_steal_pointer(&boards); nboards = 0; ret = 0; cleanup: @@ -915,8 +909,7 @@ virSysinfoParseX86Chassis(const char *base, def = NULL; } - *chassisdef = def; - def = NULL; + *chassisdef = g_steal_pointer(&def); ret = 0; virSysinfoChassisDefFree(def); return ret; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f9ab6f6042..c4c26a1183 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -706,9 +706,8 @@ virTypedParamsSerialize(virTypedParameterPtr params, j++; } - *remote_params_val = params_val; *remote_params_len = params_len; - params_val = NULL; + *remote_params_val = g_steal_pointer(&params_val); rv = 0; cleanup: diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 138403b034..0c4126bd38 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -7583,8 +7583,7 @@ vboxConnectListAllDomains(virConnectPtr conn, /* safe to ignore, new size will be equal or less than * previous allocation */ ignore_value(VIR_REALLOC_N(doms, count + 1)); - *domains = doms; - doms = NULL; + *domains = g_steal_pointer(&doms); } ret = count; diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 0ebcb06234..60ed286cee 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3817,8 +3817,7 @@ vzConnectGetAllDomainStats(virConnectPtr conn, tmpstats[nstats++] = tmp; } - *retStats = tmpstats; - tmpstats = NULL; + *retStats = g_steal_pointer(&tmpstats); ret = nstats; cleanup: diff --git a/tests/commandhelper.c b/tests/commandhelper.c index b3c65ab3cc..ee06339392 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -116,8 +116,7 @@ static struct Arguments *parseArguments(int argc, char** argv) } } - ret = args; - args = NULL; + ret = g_steal_pointer(&args); return ret; } diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index 6e68ba66ff..91f7c963c1 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -633,8 +633,7 @@ virshCheckpointListCollect(vshControl *ctl, qsort(checkpointlist->chks, checkpointlist->nchks, sizeof(*checkpointlist->chks), virshChkSorter); - ret = checkpointlist; - checkpointlist = NULL; + ret = g_steal_pointer(&checkpointlist); cleanup: virshCheckpointListFree(checkpointlist); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d40995f44d..3597190523 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5187,8 +5187,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, goto cleanup; ret = nparams; - *update_params = params; - params = NULL; + *update_params = g_steal_pointer(&params); cleanup: VIR_FREE(set_field); -- 2.29.2

2 2

[PATCH 0/2] backup: Don't try to update stats if VM isn't alive
by Peter Krempa 01 Mar '21

01 Mar '21

Doing so results in a crash. Peter Krempa (2): qemuBackupJobTerminate: Move cleanup of temp files earlier qemuBackupJobTerminate: Don't calculate backup job stats if VM isn't active src/qemu/qemu_backup.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) -- 2.29.2

2 3

[PATCH 0/4] Storage volume (crash) fixes
by Peter Krempa 01 Mar '21

01 Mar '21

Patch 1/4 should go into this release, the rest can wait. Peter Krempa (4): storageBackendProbeTarget: Check return value of virStorageSourceNewFromBacking storageBackendProbeTarget: Don't fail if backing store can't be parsed virStorageVolDefFormat: Extract formatting of source extents virStorageVolDefFormat: Don't format empty <source> src/conf/storage_conf.c | 59 +++++++++++-------- src/storage/storage_util.c | 5 +- .../storagevolxml2xmlout/vol-file-backing.xml | 2 - tests/storagevolxml2xmlout/vol-file-iso.xml | 2 - .../storagevolxml2xmlout/vol-file-naming.xml | 2 - tests/storagevolxml2xmlout/vol-file.xml | 2 - .../vol-gluster-dir-neg-uid.xml | 2 - .../storagevolxml2xmlout/vol-gluster-dir.xml | 2 - .../vol-logical-backing.xml | 2 - tests/storagevolxml2xmlout/vol-logical.xml | 2 - .../storagevolxml2xmlout/vol-luks-cipher.xml | 2 - tests/storagevolxml2xmlout/vol-luks.xml | 2 - tests/storagevolxml2xmlout/vol-partition.xml | 2 - .../vol-qcow2-0.10-lazy.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 2 - .../vol-qcow2-encryption.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2-luks.xml | 2 - .../vol-qcow2-nobacking.xml | 2 - .../vol-qcow2-nocapacity.xml | 2 - tests/storagevolxml2xmlout/vol-qcow2.xml | 2 - tests/storagevolxml2xmlout/vol-sheepdog.xml | 2 - 22 files changed, 35 insertions(+), 69 deletions(-) -- 2.29.2

2 6

[PATCH] build-aux: increase tests timeout
by Roman Bogorodskiy 01 Mar '21

01 Mar '21

Meson default timeout for test() is 30 seconds. This may be not enough for some tests like sc_prohibit_nonreentrant or sc_libvirt_unmarked_diagnostics, so set it to 60 seconds. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- On my system these two tests always timeout with the default value. That's what I have after increasing timeout: 157/288 libvirt:syntax-check / sc_prohibit_nonreentrant OK 52.18s 183/288 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics OK 31.48s build-aux/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/build-aux/meson.build b/build-aux/meson.build index c506feefd2..4acd5e16c1 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -44,6 +44,7 @@ if git potfiles_dep, ], suite: 'syntax-check', + timeout: 60, ) endforeach endif -- 2.30.0

3 5

Release of libvirt-7.1.0
by Jiri Denemark 01 Mar '21

01 Mar '21

The 7.1.0 release of both libvirt and libvirt-python is tagged and signed tarballs and source RPMs are available at https://libvirt.org/sources/ https://libvirt.org/sources/python/ Thanks everybody who helped with this release by sending patches, reviewing, testing, or providing any other feedback. Your work is greatly appreciated. * Portability * Implement Apple Silicon support libvirt now runs on the ARM-based Apple Silicon Macs. * New features * Introduce virtio-pmem ``<memory/>`` model The virtio-pmem is a virtio variant of NVDIMM and just like NVDIMM virtio-pmem also allows accessing host pages bypassing guest page cache. * Introduce ``<boot order/>`` for ``<filesystem>`` Booting is possible from virtiofs filesystems. Introduce an option to control the boot order, like we do for other bootable devices. * hyperv: implement new APIs The ``virDomainUndefine()``, ``virDomainUndefineFlags()``, ``virDomainDefineXML()``, ``virDomainAttachDevice()``, and ``virDomainAttachDeviceFlags()``, ``virConnectListAllNetworks()``, ``virConnectNumOfNetworks()``, ``virNetworkLookupByName()``, ``virNetworkLookupByUUID()``, ``virConnectNumOfDefinedNetworks()``, ``virConnectListDefinedNetworks()``, ``virNetworkGetAutostart()``, ``virNetworkIsActive()``, ``virNetworkIsPersistent()``, ``virNetworkGetXMLDesc()``, and ``virDomainScreenshot()``, APIs have been implemented in the Hyper-V driver. * Support <teaming> element in plain <hostdev> devices This is useful when libvirt doesn't have the privileges necessary to set the hostdev device's MAC address (which is a necessary part of the alternate <interface type='hostdev'>). * Introduce ``<disk type='vhostuser'>`` support Introduces support for QEMU vhost-user-blk device that can be used to access storage exported via the vhost-user protocol by daemons such as the ``qemu-storage-daemon``. * Bug fixes * qemu: Fix disk quiescing rollback when creating external snapshots If the qemu guest agent call to freeze filesystems failed when creating an external snapshot with ``VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE`` flag the filesystems would be unconditionally thawed. This could cause problems when the filesystems were frozen by an explicit call to ``virDomainFSFreeze`` since the guest agent then rejects any further freeze attempts once are filesystems frozen, an explicit freeze followed by a quiesced snapshot would fail and thaw filesystems. Users are also encouraged to use ``virDomainFSFreeze/Thaw`` manually instead of relying on ``VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE`` if they need finer grained control. * cgroups: Fix how we setup and configure cgroups on hosts with systemd When libvirt is running on host with systemd we register every VM with machined which creates the VM root cgroup for us as well. Before this fix we were directly modifying files in the VM root cgroup which was incorrect because all the files are managed by systemd. The implication was that any change done by libvirt to cgroup attributes supported by systemd could be removed which happens for example by running ``systemctl daemon-reload``. To fix the issue libvirt now uses DBus calls for some of the cgroup attributes that distribute the resources proportionally to the cgroup siblings and for the rest we have a new sub-cgroup that libvirt can managed directly. For more details why this is necessary see `systemd cgroup <https://systemd.io/CGROUP_DELEGATION/>`_ documentation. * qemu: Fix swtpm device with aarch64 The TPM TIS device name for x86 is ``tpm-tis``, whereas for aarch64 it is ``tpm-tis-device``. Fix the use of TPM TIS device with aarch64 by using the proper device name when building the QEMU command line. * libxl: Fix domain shutdown Commit fa30ee04a2 introduced the possibility of a race between the shutdown and death threads used to process domain shutdown and death events from libxl. On normal domain shutdown the shutdown thread handles all aspects of shutting down and cleaning up the domain. The death thread is only used to handle out-of-band domain destruction and is inhibited when domain shutdown is under libvirt's control. The race is avoided by also inhibiting the death thread when libvirt starts the shutdown thread. Enjoy. Jirka

1 0

Entering freeze for libvirt-7.1.0
by Jiri Denemark 25 Feb '21

25 Feb '21

I have just tagged v7.1.0-rc1 in the repository and pushed signed tarballs and source RPMs to https://libvirt.org/sources/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka

1 1

[libvirt PATCH] qemu_domainjob: Make copy of owner API
by Jiri Denemark 25 Feb '21

25 Feb '21

Using the job owner API name directly works fine as long as it is a static string or the owner's thread is still running. However, this is not always the case. For example, when the owner API name is filled in a job when we're reconnecting to existing domains after daemon restart, the dynamically allocated owner name will disappear with the reconnecting thread. Any follow up usage of the pointer will read random memory. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_domainjob.c | 12 ++++++------ src/qemu/qemu_process.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 3c2c6b9179..b58d6837ad 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -190,7 +190,7 @@ qemuDomainObjResetJob(qemuDomainJobObjPtr job) { job->active = QEMU_JOB_NONE; job->owner = 0; - job->ownerAPI = NULL; + g_clear_pointer(&job->ownerAPI, g_free); job->started = 0; } @@ -200,7 +200,7 @@ qemuDomainObjResetAgentJob(qemuDomainJobObjPtr job) { job->agentActive = QEMU_AGENT_JOB_NONE; job->agentOwner = 0; - job->agentOwnerAPI = NULL; + g_clear_pointer(&job->agentOwnerAPI, g_free); job->agentStarted = 0; } @@ -210,7 +210,7 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObjPtr job) { job->asyncJob = QEMU_ASYNC_JOB_NONE; job->asyncOwner = 0; - job->asyncOwnerAPI = NULL; + g_clear_pointer(&job->asyncOwnerAPI, g_free); job->asyncStarted = 0; job->phase = 0; job->mask = QEMU_JOB_DEFAULT_MASK; @@ -890,7 +890,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, obj, obj->def->name); priv->job.active = job; priv->job.owner = virThreadSelfID(); - priv->job.ownerAPI = virThreadJobGet(); + priv->job.ownerAPI = g_strdup(virThreadJobGet()); priv->job.started = now; } else { VIR_DEBUG("Started async job: %s (vm=%p name=%s)", @@ -901,7 +901,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; priv->job.asyncJob = asyncJob; priv->job.asyncOwner = virThreadSelfID(); - priv->job.asyncOwnerAPI = virThreadJobGet(); + priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet()); priv->job.asyncStarted = now; priv->job.current->started = now; } @@ -917,7 +917,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); priv->job.agentActive = agentJob; priv->job.agentOwner = virThreadSelfID(); - priv->job.agentOwnerAPI = virThreadJobGet(); + priv->job.agentOwnerAPI = g_strdup(virThreadJobGet()); priv->job.agentStarted = now; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..398f63282e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3732,7 +3732,7 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver, /* Restore the config of the async job which is not persisted */ priv->jobs_queued++; priv->job.asyncJob = QEMU_ASYNC_JOB_BACKUP; - priv->job.asyncOwnerAPI = virThreadJobGet(); + priv->job.asyncOwnerAPI = g_strdup(virThreadJobGet()); priv->job.asyncStarted = now; qemuDomainObjSetAsyncJobMask(vm, (QEMU_JOB_DEFAULT_MASK | -- 2.30.0

2 1

[libvirt PATCH] qemuProcessUpdateGuestCPU: Check cpu if check=full is set
by Tim Wiederhake 25 Feb '21

25 Feb '21

libvirt performs cpu checking if "check" is set to "partial", but skips checking the cpu if "check" is set to "full". See https://bugzilla.redhat.com/show_bug.cgi?id=1840770 Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/qemu/qemu_process.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bfa742577f..5b8c1397ef 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6149,6 +6149,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def, if (virCPUConvertLegacy(hostarch, def->cpu) < 0) return -1; + if (def->cpu->check == VIR_CPU_CHECK_FULL) { + virCPUDefPtr host = virQEMUCapsGetHostModel(qemuCaps, def->virtType, + VIR_QEMU_CAPS_HOST_CPU_FULL); + + if (virCPUCompare(hostarch, host, def->cpu, true) < 0) + return -1; + } + /* nothing to update for host-passthrough / maximum */ if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && def->cpu->mode != VIR_CPU_MODE_MAXIMUM) { -- 2.26.2

2 1

[PATCH v2] qemu: Add missing lock in qemuProcessHandleMonitorEOF
by Peng Liang 25 Feb '21

25 Feb '21

qemuMonitorUnregister will be called in multiple threads (e.g. threads in rpc worker pool and the vm event thread). In some cases, it isn't protected by the monitor lock, which may lead to call g_source_unref more than one time and a use-after-free problem eventually. Add the missing lock in qemuProcessHandleMonitorEOF (which is the only position missing lock of monitor I found). Suggested-by: Michal Privoznik <mprivozn(a)redhat.com> Signed-off-by: Peng Liang <liangpeng10(a)huawei.com> --- This patch is v2 of https://listman.redhat.com/archives/libvir-list/2021-February/msg00945.html. v1 -> v2: Locking monitor in qemuProcessHandleMonitorEOF instead of using aotmic function in qemuMonitorUnregister. src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_process.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d25c26343a7f..14e6b1fe9626 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -427,6 +427,7 @@ bool qemuMonitorWasDisposed(void); void qemuMonitorRegister(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); +/* Must be called with monitor locked. */ void qemuMonitorUnregister(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); void qemuMonitorClose(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d930ff9a74f6..bfa742577f32 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -318,7 +318,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon, /* We don't want this EOF handler to be called over and over while the * thread is waiting for a job. */ + virObjectLock(mon); qemuMonitorUnregister(mon); + virObjectUnlock(mon); /* We don't want any cleanup from EOF handler (or any other * thread) to enter qemu namespace. */ -- 2.29.2

2 3