November 2020 - Devel - libvirt List Archives

Availability of physical devices and migration (was: Re: [RFC PATCH 2/2] vfio-ccw: Connect the device request notifier)
by Cornelia Huck 20 Nov '20

20 Nov '20

On Fri, 20 Nov 2020 03:51:07 +0100 Halil Pasic <pasic(a)linux.ibm.com> wrote: > On Tue, 17 Nov 2020 04:26:05 +0100 > Eric Farman <farman(a)linux.ibm.com> wrote: > > > Now that the vfio-ccw code has a notifier interface to request that > > a device be unplugged, let's wire that together. > > I'm aware of the fact that performing an unplug is what vfio-pci does, > but I was not aware of this before, and I became curious with regards > to how is this going to mesh with migration (in the future). > > The sentence 'For this to work, QEMU has to be launched with the same > arguments the two times.' form docs/devel/migration.rst should not > be taken literally, I know, but the VM configuration changing not because > it was changed via a management interface, but because of events that > may not be under the control of whoever is managing the VM does > make thinks harder to reason about. > > I suppose, we now basically mandate that whoever manages the VM, either > a) maintains his own model of the VM and listens to the events, to > update it if such a device removal happens, or > b) inspects the VM at some appropriate point in time, to figure out how > the target VM is supposed to be started. > > I think libvirt does a). This seems like something that would be of general interest to libvirt folks, adding the list on cc:. For virtual devices, QEMU and any management software are in full control, and can simply make sure that both source and target have the device available. For physical devices, we still can make sure that source and target match when we do the setup, but device failures can happen at inconvenient times. It may suddenly be no longer possible to access state etc. Can we propagate changes like "device foobar, don't bother migrating" even when we already started migration? Should the handling be different if the target system uses a different (compatible) device? Should we fail the migration? > > I also suppose, such a device removal may not happen during device > migration. That is, form the QEMU perspective I believe taken care > by the BQL. Even if the device is not actually removed, it might still be inaccessible. > > But I'm in the dark regarding the management software/libivrt view. For > example what happens if we get a req_notification on the target while > pre-copy memory migration? At that point the destination is already > receiving pages, thus it is already constructed. > > My questions are not necessarily addressed to you Eric. Maybe the > folks working on vfio migration can help us out. I'm also adding > our libvirt guys. > > Regards, > Halil >

2 1

[PATCH 00/16] virsh: Add support for network disks to --attach-disk
by Peter Krempa 20 Nov '20

20 Nov '20

This version refactors cmdAttachDisk first so that we can use a much simpler logic to format the XML. With the simpler logic and tests the code now doesn't misformat the XML in certain situations as it was with the previous posting: virsh attach-disk --print-xml --domain ble --source "asdfsource" --target "tgt" > <source dev='asdfsource'/> <target dev='tgt'/> </disk> I've rather rewrote how we do things than trying to investigate why the original version [1] failed. The new version also adds tests so it's simpler to see that things actually work. [1] https://www.redhat.com/archives/libvir-list/2020-November/msg01071.html Peter Krempa (15): tests: virsh: Add simple testsuite for using --print-xml of various commands virsh: cmdAttachDisk: Declare one variable per line virsh: cmdAttachDisk: Use automatic memory clearing for 'xml' and 'dom' virsh: cmdAttachDisk: Refactor control flow virsh: cmdAttachDisk: Don't assume type='block' if file can't be examined virsh: cmdAttachDisk: Split out formatting of disk address virsh: Extract address formatting from cmdAttachDiskFormatAddress virsh: Unify code for <address type='drive' virsh: Refactor str2DiskAddress virsh: Rename 'struct DiskAddress' and friends virsh: virshAddress*: Move out of cmdAttachDisk block virsh: cmdAttachDisk: Use enum for disk source type virsh: cmdAttachDisk: Use virXMLFormatElement tests: virsh-output: Mark as expensive tests: virsh-checkpoint/snapshot: Mark as expensive Ryan Gahagan (1): virsh: Added attach-disk support for network disk docs/manpages/virsh.rst | 31 +- tests/meson.build | 1 + tests/virsh-checkpoint | 2 + tests/virsh-output | 29 ++ tests/virsh-output-commands | 94 +++++ tests/virsh-output.out | 496 ++++++++++++++++++++++++ tests/virsh-snapshot | 2 + tools/virsh-domain.c | 748 ++++++++++++++++++------------------ 8 files changed, 1017 insertions(+), 386 deletions(-) create mode 100755 tests/virsh-output create mode 100755 tests/virsh-output-commands create mode 100644 tests/virsh-output.out -- 2.28.0

2 18

Plans for the next release
by Jiri Denemark 20 Nov '20

20 Nov '20

We are getting close to the last release of libvirt in 2020. To aim for the release on Dec 01 I suggest entering the freeze on Wednesday Nov 25 and tagging RC2 on Friday Nov 27. I hope this works for everyone. Jirka

1 0

[PATCH v2] selinux label: restore all labels when some labels fail to set
by Jin Yan 20 Nov '20

20 Nov '20

When migration fails, qemuMigrationDstPrepareAny will call qemuProcessStop to restore labels only after all labels are successfully set. If some labels fail to set, the labels that have been set will not be restored. Signed-off-by: Jin Yan <jinyan12(a)huawei.com> --- src/qemu/qemu_security.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index 3bda96272c..0cb90c840a 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -51,16 +51,24 @@ qemuSecuritySetAllLabel(virQEMUDriverPtr driver, incomingPath, priv->chardevStdioLogd, migrated) < 0) - goto cleanup; + goto restorelabel; if (virSecurityManagerTransactionCommit(driver->securityManager, pid, priv->rememberOwner) < 0) - goto cleanup; + goto restorelabel; ret = 0; + cleanup: virSecurityManagerTransactionAbort(driver->securityManager); return ret; + + restorelabel: + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm->def, + migrated, + priv->chardevStdioLogd); + goto cleanup; } -- 2.23.0

2 6

[PATCH] apparmor: allow kvm-spice compat wrapper
by Christian Ehrhardt 19 Nov '20

19 Nov '20

'kvm-spice' is a binary name used to call 'kvm' which actually is a wrapper around qemu-system-x86_64 enabling kvm acceleration. This isn't in use for quite a while anymore, but required to work for compatibility e.g. when migrating in old guests. For years this was a symlink kvm-spice->kvm and therefore covered apparmor-wise by the existing entry: /usr/bin/kvm rmix, But due to a recent change [1] in qemu packaging this now is no symlink, but a wrapper on its own and therefore needs an own entry that allows it to be executed. [1]: https://salsa.debian.org/qemu-team/qemu/-/commit/9944836d3 Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com> --- src/security/apparmor/libvirt-qemu | 1 + 1 file changed, 1 insertion(+) diff --git a/src/security/apparmor/libvirt-qemu b/src/security/apparmor/libvirt-qemu index a03e9e2c94..85c9e61d6c 100644 --- a/src/security/apparmor/libvirt-qemu +++ b/src/security/apparmor/libvirt-qemu @@ -102,6 +102,7 @@ # the various binaries /usr/bin/kvm rmix, + /usr/bin/kvm-spice rmix, /usr/bin/qemu rmix, /usr/bin/qemu-aarch64 rmix, /usr/bin/qemu-alpha rmix, -- 2.29.2

6 8

[libvirt PATCH] sockettest: testMaskNetwork: use g_auto
by Ján Tomko 19 Nov '20

19 Nov '20

This has the added benefit of 'gotnet' only being freed after it was possibly used in the output string. ../src/internal.h:519:27: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 519 | # define fprintf(fh, ...) g_fprintf(fh, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ ../tests/sockettest.c:194:9: note: in expansion of macro ‘fprintf’ 194 | fprintf(stderr, "Expected %s, got %s\n", networkstr, gotnet); | ^~~~~~~ Signed-off-by: Ján Tomko <jtomko(a)redhat.com> Reported-by: Jaroslav Suchanek <jsuchane(a)redhat.com> Fixes: ba08c5932e556aa4f5101357127a6224c40e5ebe --- tests/sockettest.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/sockettest.c b/tests/sockettest.c index 3183ca523f..e9654fcebb 100644 --- a/tests/sockettest.c +++ b/tests/sockettest.c @@ -175,7 +175,7 @@ static int testMaskNetwork(const char *addrstr, { virSocketAddr addr; virSocketAddr network; - char *gotnet = NULL; + g_autofree char *gotnet = NULL; /* Intentionally fill with garbage */ memset(&network, 1, sizeof(network)); @@ -190,11 +190,9 @@ static int testMaskNetwork(const char *addrstr, return -1; if (STRNEQ(networkstr, gotnet)) { - VIR_FREE(gotnet); fprintf(stderr, "Expected %s, got %s\n", networkstr, gotnet); return -1; } - VIR_FREE(gotnet); return 0; } -- 2.26.2

2 1

[PATCH] util: convert char pointers to use g_autofree
by Barrett J Schonefeld 19 Nov '20

19 Nov '20

>From 82f992c7ff4ef59682f42c863fca242dd84208f9 Mon Sep 17 00:00:00 2001 From: Barrett Schonefeld <bschoney(a)utexas.edu> Date: Mon, 9 Nov 2020 16:07:25 -0600 Subject: [PATCH] util: convert char pointers to use g_autofree additional conversions to the GLib API in src/util per issue #11. Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11 Signed-off-by: Barrett Schonefeld <bschoney(a)utexas.edu> --- src/util/vircgroupv1.c | 3 +- src/util/virdnsmasq.c | 43 ++++++++------------- src/util/virfile.c | 9 ++--- src/util/virhostcpu.c | 4 +- src/util/virlockspace.c | 6 +-- src/util/virlog.c | 12 ++---- src/util/virmacmap.c | 3 +- src/util/virnetdevbandwidth.c | 48 ++++++++--------------- src/util/virresctrl.c | 25 ++++-------- src/util/virrotatingfile.c | 11 ++---- src/util/virscsihost.c | 25 +++++------- src/util/virsecret.c | 14 +++---- src/util/virstorageencryption.c | 19 +++------ src/util/virstoragefilebackend.c | 4 +- src/util/virsysinfo.c | 18 +++------ src/util/viruri.c | 7 +--- src/util/virutil.c | 66 +++++++++++--------------------- src/util/virvhba.c | 57 ++++++++++----------------- src/util/virxml.c | 7 +--- 19 files changed, 130 insertions(+), 251 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 731e9d61d4..984cd50409 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1549,7 +1549,7 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, unsigned long long *unevictable) { int ret = -1; - char *stat = NULL; + g_autofree char *stat = NULL; char *line = NULL; unsigned long long cacheVal = 0; unsigned long long activeAnonVal = 0; @@ -1614,7 +1614,6 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, ret = 0; cleanup: - VIR_FREE(stat); return ret; } diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 9030f9218a..93bc4a129f 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -164,7 +164,7 @@ addnhostsWrite(const char *path, dnsmasqAddnHost *hosts, unsigned int nhosts) { - char *tmp; + g_autofree char *tmp = NULL; FILE *f; bool istmp = true; size_t i, j; @@ -180,7 +180,7 @@ addnhostsWrite(const char *path, istmp = false; if (!(f = fopen(path, "w"))) { rc = -errno; - goto cleanup; + return rc; } } @@ -192,7 +192,7 @@ addnhostsWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } for (j = 0; j < hosts[i].nhostnames; j++) { @@ -203,7 +203,7 @@ addnhostsWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } } @@ -214,24 +214,21 @@ addnhostsWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } } if (VIR_FCLOSE(f) == EOF) { rc = -errno; - goto cleanup; + return rc; } if (istmp && rename(tmp, path) < 0) { rc = -errno; unlink(tmp); - goto cleanup; + return rc; } - cleanup: - VIR_FREE(tmp); - return rc; } @@ -364,7 +361,7 @@ hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, unsigned int nhosts) { - char *tmp; + g_autofree char *tmp = NULL; FILE *f; bool istmp = true; size_t i; @@ -380,7 +377,7 @@ hostsfileWrite(const char *path, istmp = false; if (!(f = fopen(path, "w"))) { rc = -errno; - goto cleanup; + return rc; } } @@ -392,24 +389,21 @@ hostsfileWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } } if (VIR_FCLOSE(f) == EOF) { rc = -errno; - goto cleanup; + return rc; } if (istmp && rename(tmp, path) < 0) { rc = -errno; unlink(tmp); - goto cleanup; + return rc; } - cleanup: - VIR_FREE(tmp); - return rc; } @@ -686,15 +680,13 @@ static int dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path) { int ret = -1; - char *buf = NULL; + g_autofree char *buf = NULL; if (virFileReadAll(path, 1024 * 1024, &buf) < 0) - goto cleanup; + return ret; ret = dnsmasqCapsSetFromBuffer(caps, buf); - cleanup: - VIR_FREE(buf); return ret; } @@ -704,7 +696,9 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force) int ret = -1; struct stat sb; virCommandPtr cmd = NULL; - char *help = NULL, *version = NULL, *complete = NULL; + g_autofree char *help = NULL; + g_autofree char *version = NULL; + g_autofree char *complete = NULL; if (!caps || caps->noRefresh) return 0; @@ -749,9 +743,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force) cleanup: virCommandFree(cmd); - VIR_FREE(help); - VIR_FREE(version); - VIR_FREE(complete); return ret; } diff --git a/src/util/virfile.c b/src/util/virfile.c index f57272ca2f..38207f1948 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3328,9 +3328,9 @@ virFileIsSharedFixFUSE(const char *path, FILE *f = NULL; struct mntent mb; char mntbuf[1024]; - char *mntDir = NULL; - char *mntType = NULL; - char *canonPath = NULL; + g_autofree char *mntDir = NULL; + g_autofree char *mntType = NULL; + g_autofree char *canonPath = NULL; size_t maxMatching = 0; int ret = -1; @@ -3381,9 +3381,6 @@ virFileIsSharedFixFUSE(const char *path, ret = 0; cleanup: - VIR_FREE(canonPath); - VIR_FREE(mntType); - VIR_FREE(mntDir); endmntent(f); return ret; } diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c531d65f86..4f6c3390ce 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -87,7 +87,7 @@ virHostCPUGetStatsFreeBSD(int cpuNum, int *nparams) { const char *sysctl_name; - long *cpu_times; + g_autofree long *cpu_times = NULL; struct clockinfo clkinfo; size_t i, j, cpu_times_size, clkinfo_size; int cpu_times_num, offset, hz, stathz, ret = -1; @@ -172,8 +172,6 @@ virHostCPUGetStatsFreeBSD(int cpuNum, ret = 0; cleanup: - VIR_FREE(cpu_times); - return ret; } diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index b90e13f506..71d5dfb83e 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -515,7 +515,7 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace, const char *resname) { int ret = -1; - char *respath = NULL; + g_autofree char *respath = NULL; VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname); @@ -538,7 +538,6 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace, cleanup: virMutexUnlock(&lockspace->lock); - VIR_FREE(respath); return ret; } @@ -547,7 +546,7 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace, const char *resname) { int ret = -1; - char *respath = NULL; + g_autofree char *respath = NULL; VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname); @@ -575,7 +574,6 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace, cleanup: virMutexUnlock(&lockspace->lock); - VIR_FREE(respath); return ret; } diff --git a/src/util/virlog.c b/src/util/virlog.c index 6b7a4512e9..2bf606b8c5 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -505,8 +505,8 @@ virLogVMessage(virLogSourcePtr source, va_list vargs) { static bool logInitMessageStderr = true; - char *str = NULL; - char *msg = NULL; + g_autofree char *str = NULL; + g_autofree char *msg = NULL; char timestamp[VIR_TIME_STRING_BUFLEN]; size_t i; int saved_errno = errno; @@ -528,7 +528,8 @@ virLogVMessage(virLogSourcePtr source, if (source->serial < virLogFiltersSerial) virLogSourceUpdate(source); if (priority < source->priority) - goto cleanup; + errno = saved_errno; + return; /* * serialize the error message, add level and timestamp @@ -601,11 +602,6 @@ virLogVMessage(virLogSourcePtr source, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); - - cleanup: - VIR_FREE(str); - VIR_FREE(msg); - errno = saved_errno; } diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index f9047d0fb1..e68742de10 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -129,7 +129,7 @@ static int virMacMapLoadFile(virMacMapPtr mgr, const char *file) { - char *map_str = NULL; + g_autofree char *map_str = NULL; virJSONValuePtr map = NULL; int map_str_len = 0; size_t i; @@ -189,7 +189,6 @@ virMacMapLoadFile(virMacMapPtr mgr, ret = 0; cleanup: - VIR_FREE(map_str); virJSONValueFree(map); return ret; } diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index c8eb5cfc79..2ae03e8edc 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -102,7 +102,7 @@ virNetDevBandwidthManipulateFilter(const char *ifname, bool create_new) { int ret = -1; - char *filter_id = NULL; + g_autofree char *filter_id = NULL; virCommandPtr cmd = NULL; unsigned char ifmac[VIR_MAC_BUFLEN]; char *mac[2] = {NULL, NULL}; @@ -157,7 +157,6 @@ virNetDevBandwidthManipulateFilter(const char *ifname, cleanup: VIR_FREE(mac[1]); VIR_FREE(mac[0]); - VIR_FREE(filter_id); virCommandFree(cmd); return ret; } @@ -195,9 +194,9 @@ virNetDevBandwidthSet(const char *ifname, int ret = -1; virNetDevBandwidthRatePtr rx = NULL, tx = NULL; /* From domain POV */ virCommandPtr cmd = NULL; - char *average = NULL; - char *peak = NULL; - char *burst = NULL; + g_autofree char *average = NULL; + g_autofree char *peak = NULL; + g_autofree char *burst = NULL; if (!bandwidth) { /* nothing to be enabled */ @@ -385,9 +384,6 @@ virNetDevBandwidthSet(const char *ifname, cleanup: virCommandFree(cmd); - VIR_FREE(average); - VIR_FREE(peak); - VIR_FREE(burst); return ret; } @@ -533,10 +529,10 @@ virNetDevBandwidthPlug(const char *brname, { int ret = -1; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *qdisc_id = NULL; - char *floor = NULL; - char *ceil = NULL; + g_autofree char *class_id = NULL; + g_autofree char *qdisc_id = NULL; + g_autofree char *floor = NULL; + g_autofree char *ceil = NULL; char ifmacStr[VIR_MAC_STRING_BUFLEN]; if (id <= 2) { @@ -586,10 +582,6 @@ virNetDevBandwidthPlug(const char *brname, ret = 0; cleanup: - VIR_FREE(ceil); - VIR_FREE(floor); - VIR_FREE(qdisc_id); - VIR_FREE(class_id); virCommandFree(cmd); return ret; } @@ -610,8 +602,8 @@ virNetDevBandwidthUnplug(const char *brname, int ret = -1; int cmd_ret = 0; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *qdisc_id = NULL; + g_autofree char *class_id = NULL; + g_autofree char *qdisc_id = NULL; if (id <= 2) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid class ID %d"), id); @@ -645,8 +637,6 @@ virNetDevBandwidthUnplug(const char *brname, ret = 0; cleanup: - VIR_FREE(qdisc_id); - VIR_FREE(class_id); virCommandFree(cmd); return ret; } @@ -673,9 +663,9 @@ virNetDevBandwidthUpdateRate(const char *ifname, { int ret = -1; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *rate = NULL; - char *ceil = NULL; + g_autofree char *class_id = NULL; + g_autofree char *rate = NULL; + g_autofree char *ceil = NULL; class_id = g_strdup_printf("1:%x", id); rate = g_strdup_printf("%llukbps", new_rate); @@ -696,9 +686,6 @@ virNetDevBandwidthUpdateRate(const char *ifname, cleanup: virCommandFree(cmd); - VIR_FREE(class_id); - VIR_FREE(rate); - VIR_FREE(ceil); return ret; } @@ -725,16 +712,13 @@ virNetDevBandwidthUpdateFilter(const char *ifname, unsigned int id) { int ret = -1; - char *class_id = NULL; + g_autofree char *class_id = NULL; class_id = g_strdup_printf("1:%x", id); if (virNetDevBandwidthManipulateFilter(ifname, ifmac_ptr, id, class_id, true, true) < 0) - goto cleanup; + return ret; - ret = 0; - cleanup: - VIR_FREE(class_id); - return ret; + return 0; } diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index d3087b98c1..1c2d175295 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -709,7 +709,7 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) { int ret = -1; int rv = -1; - char *featurestr = NULL; + g_autofree char *featurestr = NULL; char **features = NULL; size_t nfeatures = 0; virResctrlInfoMongrpPtr info_monitor = NULL; @@ -771,7 +771,6 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) ret = 0; cleanup: - VIR_FREE(featurestr); g_strfreev(features); VIR_FREE(info_monitor); return ret; @@ -1736,7 +1735,7 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, const char *groupname, virResctrlAllocPtr *alloc) { - char *schemata = NULL; + g_autofree char *schemata = NULL; int rv = virFileReadValueString(&schemata, SYSFS_RESCTRL_PATH "/%s/schemata", groupname); @@ -1753,11 +1752,9 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) goto error; - VIR_FREE(schemata); return 0; error: - VIR_FREE(schemata); virObjectUnref(*alloc); *alloc = NULL; return -1; @@ -2354,8 +2351,8 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, const char *machinename) { - char *schemata_path = NULL; - char *alloc_str = NULL; + g_autofree char *schemata_path = NULL; + g_autofree char *alloc_str = NULL; int ret = -1; int lockfd = -1; @@ -2403,8 +2400,6 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, ret = 0; cleanup: virResctrlUnlock(lockfd); - VIR_FREE(alloc_str); - VIR_FREE(schemata_path); return ret; } @@ -2413,8 +2408,8 @@ static int virResctrlAddPID(const char *path, pid_t pid) { - char *tasks = NULL; - char *pidstr = NULL; + g_autofree char *tasks = NULL; + g_autofree char *pidstr = NULL; int ret = 0; if (!path) { @@ -2436,8 +2431,6 @@ virResctrlAddPID(const char *path, ret = 0; cleanup: - VIR_FREE(tasks); - VIR_FREE(pidstr); return ret; } @@ -2657,8 +2650,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, size_t i = 0; unsigned long long val = 0; g_autoptr(DIR) dirp = NULL; - char *datapath = NULL; - char *filepath = NULL; + g_autofree char *datapath = NULL; + g_autofree char *filepath = NULL; struct dirent *ent = NULL; virResctrlMonitorStatsPtr stat = NULL; @@ -2737,8 +2730,6 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, ret = 0; cleanup: - VIR_FREE(datapath); - VIR_FREE(filepath); virResctrlMonitorStatsFree(stat); return ret; } diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c index 9f1ef17c3e..6fe5fa3154 100644 --- a/src/util/virrotatingfile.c +++ b/src/util/virrotatingfile.c @@ -362,8 +362,8 @@ static int virRotatingFileWriterRollover(virRotatingFileWriterPtr file) { size_t i; - char *nextpath = NULL; - char *thispath = NULL; + g_autofree char *nextpath = NULL; + g_autofree char *thispath = NULL; int ret = -1; VIR_DEBUG("Rollover %s", file->basepath); @@ -373,7 +373,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) virReportSystemError(errno, _("Unable to remove %s"), file->basepath); - goto cleanup; + return ret; } } else { nextpath = g_strdup_printf("%s.%zu", file->basepath, file->maxbackup - 1); @@ -391,7 +391,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) virReportSystemError(errno, _("Unable to rename %s to %s"), thispath, nextpath); - goto cleanup; + return ret; } VIR_FREE(nextpath); @@ -402,9 +402,6 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) VIR_DEBUG("Rollover done %s", file->basepath); ret = 0; - cleanup: - VIR_FREE(nextpath); - VIR_FREE(thispath); return ret; } diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c index 969cdd9f79..cad48b4059 100644 --- a/src/util/virscsihost.c +++ b/src/util/virscsihost.c @@ -95,12 +95,12 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *host_link = NULL; - char *host_path = NULL; + g_autofree char *host_link = NULL; + g_autofree char *host_path = NULL; char *p = NULL; char *ret = NULL; - char *buf = NULL; - char *unique_path = NULL; + g_autofree char *buf = NULL; + g_autofree char *unique_path = NULL; unsigned int read_unique_id; if (virDirOpen(&dir, prefix) < 0) @@ -113,7 +113,7 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, host_link = g_strdup_printf("%s/%s", prefix, entry->d_name); if (virFileResolveLink(host_link, &host_path) < 0) - goto cleanup; + return ret; if (!strstr(host_path, parentaddr)) { VIR_FREE(host_link); @@ -131,13 +131,13 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, } if (virFileReadAll(unique_path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((p = strchr(buf, '\n'))) *p = '\0'; if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) - goto cleanup; + return ret; VIR_FREE(buf); @@ -150,11 +150,6 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, break; } - cleanup: - VIR_FREE(unique_path); - VIR_FREE(host_link); - VIR_FREE(host_path); - VIR_FREE(buf); return ret; } @@ -226,7 +221,7 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, unsigned int unique_id) { char *name = NULL; - char *parentaddr = NULL; + g_autofree char *parentaddr = NULL; parentaddr = g_strdup_printf("%04x:%02x:%02x.%01x", domain, bus, slot, function); @@ -235,11 +230,9 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, _("Failed to find scsi_host using PCI '%s' " "and unique_id='%u'"), parentaddr, unique_id); - goto cleanup; + return name; } - cleanup: - VIR_FREE(parentaddr); return name; } diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 54d6bbcb7c..67c9b68215 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -65,8 +65,8 @@ int virSecretLookupParseSecret(xmlNodePtr secretnode, virSecretLookupTypeDefPtr def) { - char *uuid; - char *usage; + g_autofree char *uuid = NULL; + g_autofree char *usage = NULL; int ret = -1; uuid = virXMLPropString(secretnode, "uuid"); @@ -74,20 +74,20 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, if (uuid == NULL && usage == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing secret uuid or usage attribute")); - goto cleanup; + return ret; } if (uuid && usage) { virReportError(VIR_ERR_XML_ERROR, "%s", _("either secret uuid or usage expected")); - goto cleanup; + return ret; } if (uuid) { if (virUUIDParse(uuid, def->u.uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid secret uuid '%s'"), uuid); - goto cleanup; + return ret; } def->type = VIR_SECRET_LOOKUP_TYPE_UUID; } else { @@ -96,10 +96,6 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, def->type = VIR_SECRET_LOOKUP_TYPE_USAGE; } ret = 0; - - cleanup: - VIR_FREE(uuid); - VIR_FREE(usage); return ret; } diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index 399c6926bd..db886c1e76 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -142,7 +142,7 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) virStorageEncryptionSecretPtr ret; - char *type_str = NULL; + g_autofree char *type_str = NULL; ret = g_new0(virStorageEncryptionSecret, 1); @@ -164,12 +164,9 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, if (virSecretLookupParseSecret(node, &ret->seclookupdef) < 0) goto cleanup; - VIR_FREE(type_str); - return ret; cleanup: - VIR_FREE(type_str); virStorageEncryptionSecretFree(ret); return NULL; } @@ -180,12 +177,12 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virStorageEncryptionInfoDefPtr info) { int ret = -1; - char *size_str = NULL; + g_autofree char *size_str = NULL; if (!(info->cipher_name = virXMLPropString(info_node, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cipher info missing 'name' attribute")); - goto cleanup; + return ret; } if ((size_str = virXMLPropString(info_node, "size")) && @@ -193,22 +190,19 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virReportError(VIR_ERR_XML_ERROR, _("cannot parse cipher size: '%s'"), size_str); - goto cleanup; + return ret; } if (!size_str) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cipher info missing 'size' attribute")); - goto cleanup; + return ret; } info->cipher_mode = virXMLPropString(info_node, "mode"); info->cipher_hash = virXMLPropString(info_node, "hash"); ret = 0; - - cleanup: - VIR_FREE(size_str); return ret; } @@ -237,7 +231,7 @@ virStorageEncryptionParseNode(xmlNodePtr node, xmlNodePtr *nodes = NULL; virStorageEncryptionPtr encdef = NULL; virStorageEncryptionPtr ret = NULL; - char *format_str = NULL; + g_autofree char *format_str = NULL; int n; size_t i; @@ -297,7 +291,6 @@ virStorageEncryptionParseNode(xmlNodePtr node, ret = g_steal_pointer(&encdef); cleanup: - VIR_FREE(format_str); VIR_FREE(nodes); virStorageEncryptionFree(encdef); diff --git a/src/util/virstoragefilebackend.c b/src/util/virstoragefilebackend.c index 2779b5c307..55c62b0212 100644 --- a/src/util/virstoragefilebackend.c +++ b/src/util/virstoragefilebackend.c @@ -51,7 +51,7 @@ virStorageFileLoadBackendModule(const char *name, const char *regfunc, bool forceload) { - char *modfile = NULL; + g_autofree char *modfile = NULL; int ret; if (!(modfile = virFileFindResourceFull(name, @@ -64,8 +64,6 @@ virStorageFileLoadBackendModule(const char *name, ret = virModuleLoad(modfile, regfunc, forceload); - VIR_FREE(modfile); - return ret; } #endif /* WITH_STORAGE_DIR || WITH_STORAGE_FS || WITH_STORAGE_GLUSTER */ diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 217f842a37..45a950c85a 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -393,7 +393,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) const char *cur; char *eol, *tmp_base; virSysinfoProcessorDefPtr processor; - char *processor_type = NULL; + g_autofree char *processor_type = NULL; if (!(tmp_base = strstr(base, "model name")) && !(tmp_base = strstr(base, "Processor"))) @@ -411,7 +411,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) cur = strchr(base, ':') + 1; if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) - goto error; + return -1; processor = &ret->processor[ret->nprocessor - 1]; virSkipSpaces(&cur); @@ -424,12 +424,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) base = cur; } - VIR_FREE(processor_type); return 0; - - error: - VIR_FREE(processor_type); - return -1; } /* virSysinfoRead for ARMv7 @@ -532,9 +527,9 @@ static int virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) { const char *tmp_base; - char *manufacturer = NULL; - char *procline = NULL; - char *ncpu = NULL; + g_autofree char *manufacturer = NULL; + g_autofree char *procline = NULL; + g_autofree char *ncpu = NULL; int result = -1; virSysinfoProcessorDefPtr processor; @@ -593,9 +588,6 @@ virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) result = 0; error: - VIR_FREE(manufacturer); - VIR_FREE(procline); - VIR_FREE(ncpu); return result; } diff --git a/src/util/viruri.c b/src/util/viruri.c index 11753a0aef..d49821451e 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -209,7 +209,7 @@ char * virURIFormat(virURIPtr uri) { xmlURI xmluri; - char *tmpserver = NULL; + g_autofree char *tmpserver = NULL; char *ret; memset(&xmluri, 0, sizeof(xmluri)); @@ -241,12 +241,9 @@ virURIFormat(virURIPtr uri) ret = (char *)xmlSaveUri(&xmluri); if (!ret) { virReportOOMError(); - goto cleanup; + return ret; } - cleanup: - VIR_FREE(tmpserver); - return ret; } diff --git a/src/util/virutil.c b/src/util/virutil.c index a0cd0f1bcd..a7c163ab94 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -598,7 +598,7 @@ char *virGetUserRuntimeDirectory(void) static int virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bool quiet) { - char *strbuf; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -668,13 +668,12 @@ virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bo if (shell) VIR_FREE(*shell); } - VIR_FREE(strbuf); return ret; } static char *virGetGroupEnt(gid_t gid) { - char *strbuf; + g_autofree char *strbuf = NULL; char *ret; struct group grbuf; struct group *gr = NULL; @@ -717,7 +716,6 @@ static char *virGetGroupEnt(gid_t gid) } ret = g_strdup(gr->gr_name); - VIR_FREE(strbuf); return ret; } @@ -759,7 +757,7 @@ char *virGetGroupName(gid_t gid) static int virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) { - char *strbuf = NULL; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -775,7 +773,7 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; } if (!pw) { @@ -788,16 +786,13 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) } ret = 1; - goto cleanup; + return ret; } if (uid) *uid = pw->pw_uid; ret = 0; - cleanup: - VIR_FREE(strbuf); - return ret; } @@ -840,7 +835,7 @@ virGetUserID(const char *user, uid_t *uid) static int virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) { - char *strbuf = NULL; + g_autofree char *strbuf = NULL; struct group grbuf; struct group *gr = NULL; long val = sysconf(_SC_GETGR_R_SIZE_MAX); @@ -856,7 +851,7 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; } if (!gr) { @@ -869,16 +864,13 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) } ret = 1; - goto cleanup; + return ret; } if (gid) *gid = gr->gr_gid; ret = 0; - cleanup: - VIR_FREE(strbuf); - return ret; } @@ -949,7 +941,7 @@ int virGetGroupList(uid_t uid, gid_t gid, gid_t **list) { int ret = 0; - char *user = NULL; + g_autofree char *user = NULL; gid_t primary; *list = NULL; @@ -987,19 +979,17 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list) for (i = 0; i < ret; i++) { if ((*list)[i] == gid) - goto cleanup; + return ret; } if (VIR_APPEND_ELEMENT(*list, i, gid) < 0) { ret = -1; VIR_FREE(*list); - goto cleanup; + return ret; } else { ret = i; } } - cleanup: - VIR_FREE(user); return ret; } @@ -1405,8 +1395,8 @@ virSetDeviceUnprivSGIO(const char *path, const char *sysfs_dir, int unpriv_sgio) { - char *sysfs_path = NULL; - char *val = NULL; + g_autofree char *sysfs_path = NULL; + g_autofree char *val = NULL; int ret = -1; int rc; @@ -1416,20 +1406,17 @@ virSetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret; } val = g_strdup_printf("%d", unpriv_sgio); if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) { virReportSystemError(-rc, _("failed to set %s"), sysfs_path); - goto cleanup; + return ret; } ret = 0; - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(val); return ret; } @@ -1438,8 +1425,8 @@ virGetDeviceUnprivSGIO(const char *path, const char *sysfs_dir, int *unpriv_sgio) { - char *sysfs_path = NULL; - char *buf = NULL; + g_autofree char *sysfs_path = NULL; + g_autofree char *buf = NULL; char *tmp = NULL; int ret = -1; @@ -1449,11 +1436,11 @@ virGetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret; } if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -1461,13 +1448,10 @@ virGetDeviceUnprivSGIO(const char *path, if (virStrToLong_i(buf, NULL, 10, unpriv_sgio) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse value of %s"), sysfs_path); - goto cleanup; + return ret; } ret = 0; - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); return ret; } @@ -1488,7 +1472,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) int rc = -1; uid_t theuid; gid_t thegid; - char *tmp_label = NULL; + g_autofree char *tmp_label = NULL; char *sep = NULL; char *owner = NULL; char *group = NULL; @@ -1501,7 +1485,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse uid and gid from '%s'"), label); - goto cleanup; + return rc; } *sep = '\0'; owner = tmp_label; @@ -1512,7 +1496,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) */ if (virGetUserID(owner, &theuid) < 0 || virGetGroupID(group, &thegid) < 0) - goto cleanup; + return rc; if (uidPtr) *uidPtr = theuid; @@ -1520,10 +1504,6 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) *gidPtr = thegid; rc = 0; - - cleanup: - VIR_FREE(tmp_label); - return rc; } diff --git a/src/util/virvhba.c b/src/util/virvhba.c index a4e88024d1..d8642ea041 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -49,7 +49,7 @@ bool virVHBAPathExists(const char *sysfs_prefix, int host) { - char *sysfs_path = NULL; + g_autofree char *sysfs_path = NULL; bool ret = false; sysfs_path = g_strdup_printf("%s/host%d", @@ -58,7 +58,6 @@ virVHBAPathExists(const char *sysfs_prefix, if (virFileExists(sysfs_path)) ret = true; - VIR_FREE(sysfs_path); return ret; } @@ -79,8 +78,8 @@ bool virVHBAIsVportCapable(const char *sysfs_prefix, int host) { - char *scsi_host_path = NULL; - char *fc_host_path = NULL; + g_autofree char *scsi_host_path = NULL; + g_autofree char *fc_host_path = NULL; bool ret = false; fc_host_path = g_strdup_printf("%s/host%d/%s", @@ -94,8 +93,6 @@ virVHBAIsVportCapable(const char *sysfs_prefix, if (virFileExists(fc_host_path) || virFileExists(scsi_host_path)) ret = true; - VIR_FREE(fc_host_path); - VIR_FREE(scsi_host_path); return ret; } @@ -115,19 +112,19 @@ virVHBAGetConfig(const char *sysfs_prefix, int host, const char *entry) { - char *sysfs_path = NULL; + g_autofree char *sysfs_path = NULL; char *p = NULL; - char *buf = NULL; + g_autofree char *buf = NULL; char *result = NULL; sysfs_path = g_strdup_printf("%s/host%d/%s", sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, host, entry); if (!virFileExists(sysfs_path)) - goto cleanup; + return result; if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return result; if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -139,9 +136,6 @@ virVHBAGetConfig(const char *sysfs_prefix, result = g_strdup(p); - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); return result; } @@ -160,8 +154,8 @@ virVHBAFindVportHost(const char *sysfs_prefix) const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; g_autoptr(DIR) dir = NULL; struct dirent *entry = NULL; - char *max_vports = NULL; - char *vports = NULL; + g_autofree char *max_vports = NULL; + g_autofree char *vports = NULL; char *state = NULL; char *ret = NULL; @@ -212,16 +206,13 @@ virVHBAFindVportHost(const char *sysfs_prefix) ((strlen(max_vports) == strlen(vports)) && strcmp(max_vports, vports) > 0)) { ret = g_strdup(entry->d_name); - goto cleanup; + return ret; } VIR_FREE(max_vports); VIR_FREE(vports); } - cleanup: - VIR_FREE(max_vports); - VIR_FREE(vports); return ret; } @@ -241,7 +232,8 @@ virVHBAManageVport(const int parent_host, int operation) { int ret = -1; - char *operation_path = NULL, *vport_name = NULL; + g_autofree char *operation_path = NULL; + g_autofree char *vport_name = NULL; const char *operation_file = NULL; switch (operation) { @@ -254,7 +246,7 @@ virVHBAManageVport(const int parent_host, default: virReportError(VIR_ERR_OPERATION_INVALID, _("Invalid vport operation (%d)"), operation); - goto cleanup; + return ret; } operation_path = g_strdup_printf("%s/host%d/%s", SYSFS_FC_HOST_PATH, @@ -270,7 +262,7 @@ virVHBAManageVport(const int parent_host, _("vport operation '%s' is not supported " "for host%d"), operation_file, parent_host); - goto cleanup; + return ret; } } @@ -290,9 +282,6 @@ virVHBAManageVport(const int parent_host, "vport create/delete failed"), vport_name, operation_path); - cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path); return ret; } @@ -315,8 +304,8 @@ vhbaReadCompareWWN(const char *prefix, const char *f_name, const char *wwn) { - char *path; - char *buf = NULL; + g_autofree char *path = NULL; + g_autofree char *buf = NULL; char *p; int ret = -1; @@ -324,11 +313,11 @@ vhbaReadCompareWWN(const char *prefix, if (!virFileExists(path)) { ret = 0; - goto cleanup; + return ret; } if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -342,10 +331,6 @@ vhbaReadCompareWWN(const char *prefix, else ret = 1; - cleanup: - VIR_FREE(path); - VIR_FREE(buf); - return ret; } @@ -407,7 +392,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *vport_create_path = NULL; + g_autofree char *vport_create_path = NULL; char *ret = NULL; if (virDirOpen(&dir, prefix) < 0) @@ -428,7 +413,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, "fabric_name", fabric_wwn)) < 0) - goto cleanup; + return ret; if (rc == 0) continue; @@ -437,8 +422,6 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, break; } - cleanup: - VIR_FREE(vport_create_path); return ret; } diff --git a/src/util/virxml.c b/src/util/virxml.c index a3b819d85c..7df50e4b4d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -696,8 +696,8 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) unsigned int n, col; /* GCC warns if signed, because compared with sizeof() */ int domcode = VIR_FROM_XML; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *contextstr = NULL; - char *pointerstr = NULL; + g_autofree char *contextstr = NULL; + g_autofree char *pointerstr = NULL; /* conditions for error printing */ @@ -763,9 +763,6 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) contextstr, pointerstr); } - - VIR_FREE(contextstr); - VIR_FREE(pointerstr); } /** -- GitLab

2 3

[libvirt PATCH 0/3] openvz: fix NULL hostname usage
by Ján Tomko 19 Nov '20

19 Nov '20

GCC on RHEL 8 emits a warning. See the first patch. Ján Tomko (3): openvzDomainMigratePrepare3Params: correctly use hostname openvzDomainMigratePrepare3Params: remove else after goto openvzDomainMigratePrepare3Params: use g_auto src/openvz/openvz_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) -- 2.26.2

2 4

[libvirt PATCH 00/10] add support for ESP SCSI controller family
by Daniel P. Berrangé 19 Nov '20

19 Nov '20

The sparc machines types all have an NCR53C90 SCSI Controller builtin. This is from the ESP family of SCSI controllers which includs two PCI variants. Wire up support for all these SCSI controllers, so we can at least launch a sparc VM with disks attached. There are likely other gaps in CLI coverage for sparc, so I don't claim this lets you have a fully working VM yet. Daniel P. Berrangé (10): util: add ARCH_IS_MIPS64 helper macro qemu: fix default devices on sparc machines qemu: add helper method for checking if ESP SCSI is builtin conf: add support for ESP SCSI controller family qemu: add capabilities for the three ESP family SCSI controllers qemu: enable support for ESP SCSI controller family tests: add fake host CPU for sparc architecture tests: add capabilities data files for sparc emulator target tests: define QEMU driver capabilities for sparc architecture tests: add minimal XML example for sparc VM docs/formatdomain.rst | 3 +- docs/schemas/domaincommon.rng | 3 + src/conf/domain_conf.c | 8 + src/conf/domain_conf.h | 3 + src/qemu/qemu_alias.c | 6 + src/qemu/qemu_capabilities.c | 27 + src/qemu/qemu_capabilities.h | 5 + src/qemu/qemu_command.c | 15 + src/qemu/qemu_domain.c | 21 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 10 + src/qemu/qemu_validate.c | 36 + src/util/virarch.h | 3 + src/vbox/vbox_common.c | 3 + src/vmx/vmx.c | 3 + tests/domaincapsdata/qemu_5.1.0.sparc.xml | 101 + .../caps_1.5.3.x86_64.xml | 2 + .../caps_1.6.0.x86_64.xml | 2 + .../caps_1.7.0.x86_64.xml | 2 + .../caps_2.1.1.x86_64.xml | 2 + .../caps_2.10.0.aarch64.xml | 2 + .../caps_2.10.0.ppc64.xml | 2 + .../caps_2.10.0.x86_64.xml | 2 + .../caps_2.11.0.x86_64.xml | 2 + .../caps_2.12.0.aarch64.xml | 2 + .../caps_2.12.0.ppc64.xml | 2 + .../caps_2.12.0.x86_64.xml | 2 + .../caps_2.4.0.x86_64.xml | 2 + .../caps_2.5.0.x86_64.xml | 2 + .../caps_2.6.0.aarch64.xml | 2 + .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 2 + .../caps_2.6.0.x86_64.xml | 2 + .../caps_2.7.0.x86_64.xml | 2 + .../caps_2.8.0.x86_64.xml | 2 + .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 2 + .../caps_2.9.0.x86_64.xml | 2 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 2 + .../caps_3.0.0.x86_64.xml | 2 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 2 + .../caps_3.1.0.x86_64.xml | 2 + .../caps_4.0.0.aarch64.xml | 2 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 + .../caps_4.0.0.riscv32.xml | 2 + .../caps_4.0.0.riscv64.xml | 2 + .../caps_4.0.0.x86_64.xml | 2 + .../caps_4.1.0.x86_64.xml | 2 + .../caps_4.2.0.aarch64.xml | 2 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 2 + .../caps_4.2.0.x86_64.xml | 2 + .../caps_5.0.0.aarch64.xml | 2 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 2 + .../caps_5.0.0.riscv64.xml | 2 + .../caps_5.0.0.x86_64.xml | 2 + .../caps_5.1.0.sparc.replies | 17294 ++++++++++++++++ .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 133 + .../caps_5.1.0.x86_64.xml | 2 + .../caps_5.2.0.x86_64.xml | 2 + tests/qemucaps2xmloutdata/caps.sparc.xml | 25 + tests/qemuxml2argvdata/sparc-minimal.args | 34 + tests/qemuxml2argvdata/sparc-minimal.xml | 21 + tests/qemuxml2argvtest.c | 3 + tests/testutilshostcpus.h | 10 + tests/testutilsqemu.c | 62 +- 63 files changed, 17882 insertions(+), 26 deletions(-) create mode 100644 tests/domaincapsdata/qemu_5.1.0.sparc.xml create mode 100644 tests/qemucapabilitiesdata/caps_5.1.0.sparc.replies create mode 100644 tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml create mode 100644 tests/qemucaps2xmloutdata/caps.sparc.xml create mode 100644 tests/qemuxml2argvdata/sparc-minimal.args create mode 100644 tests/qemuxml2argvdata/sparc-minimal.xml -- 2.28.0

2 13

[PATCH] domain_capabilities: Assert enums fit into unsigned int bitmask
by Michal Privoznik 19 Nov '20

19 Nov '20

The way our domain capabilities work currently, is that we have virDomainCapsEnum struct which contains 'unsigned int values' member which serves as a bitmask. More complicated structs are composed from this struct, giving us whole virDomainCaps eventually. Whenever we want to report that a certain value is supported, the '1 << value' bit is set in the corresponding unsigned int member. This works as long as the resulting value after bitshift does not overflow unsigned int. There is a check inside virDomainCapsEnumSet() which ensures exactly this, but no caller really checks whether virDomainCapsEnumSet() succeeded. Also, checking at runtime is a bit too late. Fortunately, we know the largest value we want to store in each member, because each enum of ours ends with _LAST member. Therefore, we can check at build time whether an overflow can occur. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/domain_capabilities.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index f177af1744..b22d40abb2 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -36,6 +36,9 @@ struct _virDomainCapsEnum { unsigned int values; /* Bitmask of values supported in the corresponding enum */ }; +#define STATIC_ASSERT_ENUM(last) \ + G_STATIC_ASSERT(last <= sizeof(unsigned int) * CHAR_BIT) + typedef struct _virDomainCapsStringValues virDomainCapsStringValues; typedef virDomainCapsStringValues *virDomainCapsStringValuesPtr; struct _virDomainCapsStringValues { @@ -43,6 +46,8 @@ struct _virDomainCapsStringValues { size_t nvalues; /* number of strings */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_LOADER_TYPE_LAST); +STATIC_ASSERT_ENUM(VIR_TRISTATE_BOOL_LAST); typedef struct _virDomainCapsLoader virDomainCapsLoader; typedef virDomainCapsLoader *virDomainCapsLoaderPtr; struct _virDomainCapsLoader { @@ -53,6 +58,7 @@ struct _virDomainCapsLoader { virDomainCapsEnum secure; /* Info about secure:virTristateBool */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_OS_DEF_FIRMWARE_LAST); typedef struct _virDomainCapsOS virDomainCapsOS; typedef virDomainCapsOS *virDomainCapsOSPtr; struct _virDomainCapsOS { @@ -61,6 +67,9 @@ struct _virDomainCapsOS { virDomainCapsLoader loader; /* Info about virDomainLoaderDef */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_DISK_DEVICE_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_DISK_BUS_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_DISK_MODEL_LAST); typedef struct _virDomainCapsDeviceDisk virDomainCapsDeviceDisk; typedef virDomainCapsDeviceDisk *virDomainCapsDeviceDiskPtr; struct _virDomainCapsDeviceDisk { @@ -71,6 +80,7 @@ struct _virDomainCapsDeviceDisk { /* add new fields here */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_GRAPHICS_TYPE_LAST); typedef struct _virDomainCapsDeviceGraphics virDomainCapsDeviceGraphics; typedef virDomainCapsDeviceGraphics *virDomainCapsDeviceGraphicsPtr; struct _virDomainCapsDeviceGraphics { @@ -78,6 +88,7 @@ struct _virDomainCapsDeviceGraphics { virDomainCapsEnum type; /* virDomainGraphicsType */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_VIDEO_TYPE_LAST); typedef struct _virDomainCapsDeviceVideo virDomainCapsDeviceVideo; typedef virDomainCapsDeviceVideo *virDomainCapsDeviceVideoPtr; struct _virDomainCapsDeviceVideo { @@ -85,6 +96,11 @@ struct _virDomainCapsDeviceVideo { virDomainCapsEnum modelType; /* virDomainVideoType */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_HOSTDEV_MODE_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_STARTUP_POLICY_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST); typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev; typedef virDomainCapsDeviceHostdev *virDomainCapsDeviceHostdevPtr; struct _virDomainCapsDeviceHostdev { @@ -97,6 +113,8 @@ struct _virDomainCapsDeviceHostdev { /* add new fields here */ }; +STATIC_ASSERT_ENUM(VIR_DOMAIN_RNG_MODEL_LAST); +STATIC_ASSERT_ENUM(VIR_DOMAIN_RNG_BACKEND_LAST); typedef struct _virDomainCapsDeviceRNG virDomainCapsDeviceRNG; typedef virDomainCapsDeviceRNG *virDomainCapsDeviceRNGPtr; struct _virDomainCapsDeviceRNG { @@ -105,6 +123,7 @@ struct _virDomainCapsDeviceRNG { virDomainCapsEnum backendModel; /* virDomainRNGBackend */ }; +STATIC_ASSERT_ENUM(VIR_GIC_VERSION_LAST); typedef struct _virDomainCapsFeatureGIC virDomainCapsFeatureGIC; typedef virDomainCapsFeatureGIC *virDomainCapsFeatureGICPtr; struct _virDomainCapsFeatureGIC { -- 2.26.2

2 6