[libvirt] [PATCH 00/10] conf/caps: Modernize and clean up handling of domain caps
Peter Krempa (10): qemu: caps: Rework memory allocation in virQEMUCapsFillDomainFeatureSEVCaps qemu: caps: Make capability filler functions void conf: domaincaps: Fix broken attempt at being const-correct conf: storagecaps: Fix broken attempt at being const-correct conf: Rename virDomainCapsFeature to virDomainProcessCapsFeature conf: caps: Automaticaly free 'cpus_str' conf: domain: Split up formatting of <memtune> and <memoryBacking> conf: turn virDomainMemtuneFormat void conf: caps: Modernize virCapabilitiesFormatCaches conf: capabilities: Modernize virCapabilitiesFormatMemoryBandwidth src/conf/capabilities.c | 36 +++++---------- src/conf/domain_capabilities.c | 32 ++++++------- src/conf/domain_capabilities.h | 4 +- src/conf/domain_conf.c | 30 ++++++------ src/conf/domain_conf.h | 82 ++++++++++++++++----------------- src/conf/storage_capabilities.c | 4 +- src/conf/storage_capabilities.h | 2 +- src/libvirt_private.syms | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_container.c | 20 ++++---- src/lxc/lxc_native.c | 4 +- src/qemu/qemu_capabilities.c | 81 +++++++++++--------------------- 12 files changed, 132 insertions(+), 167 deletions(-) -- 2.23.0
Use g_new0 instead of VIR_ALLOC to avoid error cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index fc6473651c..02ac421067 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5572,21 +5572,16 @@ virQEMUCapsFillDomainFeatureSEVCaps(virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) { virSEVCapability *cap = qemuCaps->sevCapabilities; - g_autoptr(virSEVCapability) sev = NULL; if (!cap) return 0; - if (VIR_ALLOC(sev) < 0) - return -1; - - sev->pdh = g_strdup(cap->pdh); - - sev->cert_chain = g_strdup(cap->cert_chain); + domCaps->sev = g_new0(virSEVCapability, 1); - sev->cbitpos = cap->cbitpos; - sev->reduced_phys_bits = cap->reduced_phys_bits; - domCaps->sev = g_steal_pointer(&sev); + domCaps->sev->pdh = g_strdup(cap->pdh); + domCaps->sev->cert_chain = g_strdup(cap->cert_chain); + domCaps->sev->cbitpos = cap->cbitpos; + domCaps->sev->reduced_phys_bits = cap->reduced_phys_bits; return 0; } -- 2.23.0
On Tue, Nov 12, 2019 at 08:27:38AM +0100, Peter Krempa wrote:
Use g_new0 instead of VIR_ALLOC to avoid error cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Most of them don't have anything to report so we can simplify the logic. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 66 ++++++++++++------------------------ 1 file changed, 22 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 02ac421067..0229d7b5f8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5250,7 +5250,7 @@ virQEMUCapsFillDomainOSCaps(virDomainCapsOSPtr os, } -static int +static void virQEMUCapsFillDomainCPUCaps(virCapsPtr caps, virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) @@ -5287,23 +5287,19 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps, } domCaps->cpu.custom = filtered; } - - return 0; } -static int +static void virQEMUCapsFillDomainIOThreadCaps(virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) { domCaps->iothreads = virTristateBoolFromBool( virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)); - - return 0; } -static int +static void virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, const char *machine, virDomainCapsDeviceDiskPtr disk) @@ -5348,12 +5344,10 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(disk->model, VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL); } - - return 0; } -static int +static void virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceGraphicsPtr dev) { @@ -5365,12 +5359,10 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_VNC); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_SPICE); - - return 0; } -static int +static void virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceVideoPtr dev) { @@ -5389,12 +5381,10 @@ virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_VIRTIO); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY)) VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_BOCHS); - - return 0; } -static int +static void virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceHostdevPtr hostdev) { @@ -5432,12 +5422,10 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO); } - - return 0; } -static int +static void virQEMUCapsFillDomainDeviceRNGCaps(virQEMUCapsPtr qemuCaps, virDomainCapsDeviceRNGPtr rng) { @@ -5460,8 +5448,6 @@ virQEMUCapsFillDomainDeviceRNGCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(rng->backendModel, VIR_DOMAIN_RNG_BACKEND_EGD); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_RANDOM)) VIR_DOMAIN_CAPS_ENUM_SET(rng->backendModel, VIR_DOMAIN_RNG_BACKEND_RANDOM); - - return 0; } @@ -5523,10 +5509,8 @@ virQEMUCapsSupportsGICVersion(virQEMUCapsPtr qemuCaps, * and virtualization type. Moreover, a common format is used to store * information about enumerations in @domCaps, so further processing is * required. - * - * Returns: 0 on success, <0 on failure */ -static int +static void virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) { @@ -5536,7 +5520,7 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, gic->supported = VIR_TRISTATE_BOOL_NO; if (!qemuDomainMachineIsARMVirt(domCaps->machine, domCaps->arch)) - return 0; + return; for (version = VIR_GIC_VERSION_LAST - 1; version > VIR_GIC_VERSION_NONE; @@ -5551,8 +5535,6 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_CAPS_ENUM_SET(gic->version, version); } - - return 0; } @@ -5564,17 +5546,15 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, * Take the information about SEV capabilities that has been obtained * using the 'query-sev-capabilities' QMP command and stored in @qemuCaps * and convert it to a form suitable for @domCaps. - * - * Returns: 0 on success, -1 on failure */ -static int +static void virQEMUCapsFillDomainFeatureSEVCaps(virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) { virSEVCapability *cap = qemuCaps->sevCapabilities; if (!cap) - return 0; + return; domCaps->sev = g_new0(virSEVCapability, 1); @@ -5582,8 +5562,6 @@ virQEMUCapsFillDomainFeatureSEVCaps(virQEMUCapsPtr qemuCaps, domCaps->sev->cert_chain = g_strdup(cap->cert_chain); domCaps->sev->cbitpos = cap->cbitpos; domCaps->sev->reduced_phys_bits = cap->reduced_phys_bits; - - return 0; } @@ -5623,19 +5601,19 @@ virQEMUCapsFillDomainCaps(virCapsPtr caps, domCaps->machine, domCaps->arch, privileged, - firmwares, nfirmwares) < 0 || - virQEMUCapsFillDomainCPUCaps(caps, qemuCaps, domCaps) < 0 || - virQEMUCapsFillDomainIOThreadCaps(qemuCaps, domCaps) < 0 || - virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, - domCaps->machine, disk) < 0 || - virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics) < 0 || - virQEMUCapsFillDomainDeviceVideoCaps(qemuCaps, video) < 0 || - virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 || - virQEMUCapsFillDomainDeviceRNGCaps(qemuCaps, rng) < 0 || - virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0 || - virQEMUCapsFillDomainFeatureSEVCaps(qemuCaps, domCaps) < 0) + firmwares, nfirmwares) < 0) return -1; + virQEMUCapsFillDomainCPUCaps(caps, qemuCaps, domCaps); + virQEMUCapsFillDomainIOThreadCaps(qemuCaps, domCaps); + virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps->machine, disk); + virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics); + virQEMUCapsFillDomainDeviceVideoCaps(qemuCaps, video); + virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev); + virQEMUCapsFillDomainDeviceRNGCaps(qemuCaps, rng); + virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps); + virQEMUCapsFillDomainFeatureSEVCaps(qemuCaps, domCaps); + return 0; } -- 2.23.0
On Tue, Nov 12, 2019 at 08:27:39AM +0100, Peter Krempa wrote:
Most of them don't have anything to report so we can simplify the logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 66 ++++++++++++------------------------ 1 file changed, 22 insertions(+), 44 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
'virBlahPtr const blah' results into modification to the value of 'blah' triggering compilation error rather than the modification of the virBlah struct the pointer points to. All of the domain capability formatting code was broken in this regard. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_capabilities.c | 32 ++++++++++++++++---------------- src/conf/domain_capabilities.h | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 09bf047647..b9de4bfe5d 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -321,7 +321,7 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum) static int virDomainCapsEnumFormat(virBufferPtr buf, - virDomainCapsEnumPtr capsEnum, + const virDomainCapsEnum *capsEnum, const char *capsEnumName, virDomainCapsValToStr valToStr) { @@ -362,7 +362,7 @@ virDomainCapsEnumFormat(virBufferPtr buf, static void virDomainCapsStringValuesFormat(virBufferPtr buf, - virDomainCapsStringValuesPtr values) + const virDomainCapsStringValues *values) { size_t i; @@ -406,7 +406,7 @@ virDomainCapsStringValuesFormat(virBufferPtr buf, static void virDomainCapsLoaderFormat(virBufferPtr buf, - virDomainCapsLoaderPtr loader) + const virDomainCapsLoader *loader) { FORMAT_PROLOGUE(loader); @@ -420,9 +420,9 @@ virDomainCapsLoaderFormat(virBufferPtr buf, static void virDomainCapsOSFormat(virBufferPtr buf, - virDomainCapsOSPtr os) + const virDomainCapsOS *os) { - virDomainCapsLoaderPtr loader = &os->loader; + const virDomainCapsLoader *loader = &os->loader; FORMAT_PROLOGUE(os); @@ -453,7 +453,7 @@ virDomainCapsCPUCustomFormat(virBufferPtr buf, static void virDomainCapsCPUFormat(virBufferPtr buf, - virDomainCapsCPUPtr cpu) + const virDomainCapsCPU *cpu) { virBufferAddLit(buf, "<cpu>\n"); virBufferAdjustIndent(buf, 2); @@ -492,7 +492,7 @@ virDomainCapsCPUFormat(virBufferPtr buf, static void virDomainCapsDeviceDiskFormat(virBufferPtr buf, - virDomainCapsDeviceDiskPtr const disk) + const virDomainCapsDeviceDisk *disk) { FORMAT_PROLOGUE(disk); @@ -506,7 +506,7 @@ virDomainCapsDeviceDiskFormat(virBufferPtr buf, static void virDomainCapsDeviceGraphicsFormat(virBufferPtr buf, - virDomainCapsDeviceGraphicsPtr const graphics) + const virDomainCapsDeviceGraphics *graphics) { FORMAT_PROLOGUE(graphics); @@ -518,7 +518,7 @@ virDomainCapsDeviceGraphicsFormat(virBufferPtr buf, static void virDomainCapsDeviceVideoFormat(virBufferPtr buf, - virDomainCapsDeviceVideoPtr const video) + const virDomainCapsDeviceVideo *video) { FORMAT_PROLOGUE(video); @@ -530,7 +530,7 @@ virDomainCapsDeviceVideoFormat(virBufferPtr buf, static void virDomainCapsDeviceHostdevFormat(virBufferPtr buf, - virDomainCapsDeviceHostdevPtr const hostdev) + const virDomainCapsDeviceHostdev *hostdev) { FORMAT_PROLOGUE(hostdev); @@ -546,7 +546,7 @@ virDomainCapsDeviceHostdevFormat(virBufferPtr buf, static void virDomainCapsDeviceRNGFormat(virBufferPtr buf, - virDomainCapsDeviceRNGPtr const rng) + const virDomainCapsDeviceRNG *rng) { FORMAT_PROLOGUE(rng); @@ -575,7 +575,7 @@ virDomainCapsDeviceRNGFormat(virBufferPtr buf, */ static void virDomainCapsFeatureGICFormat(virBufferPtr buf, - virDomainCapsFeatureGICPtr const gic) + const virDomainCapsFeatureGIC *gic) { FORMAT_PROLOGUE(gic); @@ -586,7 +586,7 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf, static void virDomainCapsFeatureSEVFormat(virBufferPtr buf, - virSEVCapabilityPtr const sev) + const virSEVCapability *sev) { if (!sev) { virBufferAddLit(buf, "<sev supported='no'/>\n"); @@ -605,7 +605,7 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf, char * -virDomainCapsFormat(virDomainCapsPtr const caps) +virDomainCapsFormat(const virDomainCaps *caps) { virBuffer buf = VIR_BUFFER_INITIALIZER; const char *virttype_str = virDomainVirtTypeToString(caps->virttype); @@ -669,7 +669,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps) static int -virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps, +virDomainCapsDeviceRNGDefValidate(const virDomainCaps *caps, const virDomainRNGDef *dev) { if (ENUM_VALUE_MISSING(caps->rng.model, dev->model)) { @@ -683,7 +683,7 @@ virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps, int -virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, +virDomainCapsDeviceDefValidate(const virDomainCaps *caps, const virDomainDeviceDef *dev, const virDomainDef *def G_GNUC_UNUSED) { diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index a31458c653..6b27eac11f 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -226,9 +226,9 @@ int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum, unsigned int *values); void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum); -char * virDomainCapsFormat(virDomainCapsPtr const caps); +char * virDomainCapsFormat(const virDomainCaps *caps); -int virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, +int virDomainCapsDeviceDefValidate(const virDomainCaps *caps, const virDomainDeviceDef *dev, const virDomainDef *def); -- 2.23.0
On Tue, Nov 12, 2019 at 08:27:40AM +0100, Peter Krempa wrote:
'virBlahPtr const blah' results into modification to the value of 'blah' triggering compilation error rather than the modification of the virBlah struct the pointer points to.
Shall we add a syntax check rule to forbid "virFOOPtr const", and "const virFOOPtr". Indeed, I rather wish we never had the "Ptr" suffix at all, and just used a "*" normally, but that would be a too huge change.
All of the domain capability formatting code was broken in this regard.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_capabilities.c | 32 ++++++++++++++++---------------- src/conf/domain_capabilities.h | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 09bf047647..b9de4bfe5d 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -321,7 +321,7 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)
static int virDomainCapsEnumFormat(virBufferPtr buf, - virDomainCapsEnumPtr capsEnum, + const virDomainCapsEnum *capsEnum, const char *capsEnumName, virDomainCapsValToStr valToStr) { @@ -362,7 +362,7 @@ virDomainCapsEnumFormat(virBufferPtr buf,
static void virDomainCapsStringValuesFormat(virBufferPtr buf, - virDomainCapsStringValuesPtr values) + const virDomainCapsStringValues *values) { size_t i;
@@ -406,7 +406,7 @@ virDomainCapsStringValuesFormat(virBufferPtr buf,
static void virDomainCapsLoaderFormat(virBufferPtr buf, - virDomainCapsLoaderPtr loader) + const virDomainCapsLoader *loader) { FORMAT_PROLOGUE(loader);
@@ -420,9 +420,9 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
static void virDomainCapsOSFormat(virBufferPtr buf, - virDomainCapsOSPtr os) + const virDomainCapsOS *os) { - virDomainCapsLoaderPtr loader = &os->loader; + const virDomainCapsLoader *loader = &os->loader;
FORMAT_PROLOGUE(os);
@@ -453,7 +453,7 @@ virDomainCapsCPUCustomFormat(virBufferPtr buf,
static void virDomainCapsCPUFormat(virBufferPtr buf, - virDomainCapsCPUPtr cpu) + const virDomainCapsCPU *cpu) { virBufferAddLit(buf, "<cpu>\n"); virBufferAdjustIndent(buf, 2); @@ -492,7 +492,7 @@ virDomainCapsCPUFormat(virBufferPtr buf,
static void virDomainCapsDeviceDiskFormat(virBufferPtr buf, - virDomainCapsDeviceDiskPtr const disk) + const virDomainCapsDeviceDisk *disk) { FORMAT_PROLOGUE(disk);
@@ -506,7 +506,7 @@ virDomainCapsDeviceDiskFormat(virBufferPtr buf,
static void virDomainCapsDeviceGraphicsFormat(virBufferPtr buf, - virDomainCapsDeviceGraphicsPtr const graphics) + const virDomainCapsDeviceGraphics *graphics) { FORMAT_PROLOGUE(graphics);
@@ -518,7 +518,7 @@ virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
static void virDomainCapsDeviceVideoFormat(virBufferPtr buf, - virDomainCapsDeviceVideoPtr const video) + const virDomainCapsDeviceVideo *video) { FORMAT_PROLOGUE(video);
@@ -530,7 +530,7 @@ virDomainCapsDeviceVideoFormat(virBufferPtr buf,
static void virDomainCapsDeviceHostdevFormat(virBufferPtr buf, - virDomainCapsDeviceHostdevPtr const hostdev) + const virDomainCapsDeviceHostdev *hostdev) { FORMAT_PROLOGUE(hostdev);
@@ -546,7 +546,7 @@ virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
static void virDomainCapsDeviceRNGFormat(virBufferPtr buf, - virDomainCapsDeviceRNGPtr const rng) + const virDomainCapsDeviceRNG *rng) { FORMAT_PROLOGUE(rng);
@@ -575,7 +575,7 @@ virDomainCapsDeviceRNGFormat(virBufferPtr buf, */ static void virDomainCapsFeatureGICFormat(virBufferPtr buf, - virDomainCapsFeatureGICPtr const gic) + const virDomainCapsFeatureGIC *gic) { FORMAT_PROLOGUE(gic);
@@ -586,7 +586,7 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
static void virDomainCapsFeatureSEVFormat(virBufferPtr buf, - virSEVCapabilityPtr const sev) + const virSEVCapability *sev) { if (!sev) { virBufferAddLit(buf, "<sev supported='no'/>\n"); @@ -605,7 +605,7 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf,
char * -virDomainCapsFormat(virDomainCapsPtr const caps) +virDomainCapsFormat(const virDomainCaps *caps) { virBuffer buf = VIR_BUFFER_INITIALIZER; const char *virttype_str = virDomainVirtTypeToString(caps->virttype); @@ -669,7 +669,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
static int -virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps, +virDomainCapsDeviceRNGDefValidate(const virDomainCaps *caps, const virDomainRNGDef *dev) { if (ENUM_VALUE_MISSING(caps->rng.model, dev->model)) { @@ -683,7 +683,7 @@ virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps,
int -virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, +virDomainCapsDeviceDefValidate(const virDomainCaps *caps, const virDomainDeviceDef *dev, const virDomainDef *def G_GNUC_UNUSED) { diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index a31458c653..6b27eac11f 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -226,9 +226,9 @@ int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum, unsigned int *values); void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum);
-char * virDomainCapsFormat(virDomainCapsPtr const caps); +char * virDomainCapsFormat(const virDomainCaps *caps);
-int virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps, +int virDomainCapsDeviceDefValidate(const virDomainCaps *caps, const virDomainDeviceDef *dev, const virDomainDef *def);
-- 2.23.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Nov 12, 2019 at 09:30:14AM +0000, Daniel P. Berrangé wrote:
On Tue, Nov 12, 2019 at 08:27:40AM +0100, Peter Krempa wrote:
'virBlahPtr const blah' results into modification to the value of 'blah' triggering compilation error rather than the modification of the virBlah struct the pointer points to.
Shall we add a syntax check rule to forbid "virFOOPtr const", and "const virFOOPtr".
We already do have a rule against const virFOOPtr. Jano
Indeed, I rather wish we never had the "Ptr" suffix at all, and just used a "*" normally, but that would be a too huge change.
All of the domain capability formatting code was broken in this regard.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_capabilities.c | 32 ++++++++++++++++---------------- src/conf/domain_capabilities.h | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-)
The code formatting storage capabilities faithfully copied the wrong use of 'const' from domain capabilities. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_capabilities.c | 4 ++-- src/conf/storage_capabilities.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_capabilities.c b/src/conf/storage_capabilities.c index cf3ee488ac..1a3417f90b 100644 --- a/src/conf/storage_capabilities.c +++ b/src/conf/storage_capabilities.c @@ -93,7 +93,7 @@ virStoragePoolCapsIsLoaded(virCapsPtr driverCaps, static int virStoragePoolCapsFormatPool(virBufferPtr buf, int poolType, - virStoragePoolCapsPtr const caps) + const virStoragePoolCaps *caps) { bool isLoaded = virStoragePoolCapsIsLoaded(caps->driverCaps, poolType); @@ -115,7 +115,7 @@ virStoragePoolCapsFormatPool(virBufferPtr buf, char * -virStoragePoolCapsFormat(virStoragePoolCapsPtr const caps) +virStoragePoolCapsFormat(const virStoragePoolCaps *caps) { virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; diff --git a/src/conf/storage_capabilities.h b/src/conf/storage_capabilities.h index 788ea227ea..377c313aa0 100644 --- a/src/conf/storage_capabilities.h +++ b/src/conf/storage_capabilities.h @@ -37,4 +37,4 @@ virStoragePoolCapsPtr virStoragePoolCapsNew(virCapsPtr driverCaps); char * -virStoragePoolCapsFormat(virStoragePoolCapsPtr const caps); +virStoragePoolCapsFormat(const virStoragePoolCaps *caps); -- 2.23.0
On Tue, Nov 12, 2019 at 08:27:41AM +0100, Peter Krempa wrote:
The code formatting storage capabilities faithfully copied the wrong use of 'const' from domain capabilities.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_capabilities.c | 4 ++-- src/conf/storage_capabilities.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The enum name sounds too generic. It in fact describes the capabilities of the process, thus add 'Process' to the name. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 +++--- src/conf/domain_conf.h | 82 ++++++++++++++++++++-------------------- src/libvirt_private.syms | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_container.c | 20 +++++----- src/lxc/lxc_native.c | 4 +- 6 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d5aba7336f..3e81140430 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -211,8 +211,8 @@ VIR_ENUM_IMPL(virDomainMsrsUnknown, "fault", ); -VIR_ENUM_IMPL(virDomainCapsFeature, - VIR_DOMAIN_CAPS_FEATURE_LAST, +VIR_ENUM_IMPL(virDomainProcessCapsFeature, + VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST, "audit_control", "audit_write", "block_suspend", @@ -20616,7 +20616,7 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; for (i = 0; i < n; i++) { - int val = virDomainCapsFeatureTypeFromString((const char *)nodes[i]->name); + int val = virDomainProcessCapsFeatureTypeFromString((const char *)nodes[i]->name); if (val < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unexpected capability feature '%s'"), nodes[i]->name); @@ -20627,7 +20627,7 @@ virDomainDefParseXML(xmlDocPtr xml, if ((def->caps_features[val] = virTristateSwitchTypeFromString(tmp)) == -1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown state attribute '%s' of feature capability '%s'"), - tmp, virDomainCapsFeatureTypeToString(val)); + tmp, virDomainProcessCapsFeatureTypeToString(val)); goto error; } VIR_FREE(tmp); @@ -28295,10 +28295,10 @@ virDomainDefFormatFeatures(virBufferPtr buf, case VIR_DOMAIN_FEATURE_CAPABILITIES: virBufferSetChildIndent(&tmpChildBuf, &childBuf); - for (j = 0; j < VIR_DOMAIN_CAPS_FEATURE_LAST; j++) { + for (j = 0; j < VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST; j++) { if (def->caps_features[j] != VIR_TRISTATE_SWITCH_ABSENT) virBufferAsprintf(&tmpChildBuf, "<%s state='%s'/>\n", - virDomainCapsFeatureTypeToString(j), + virDomainProcessCapsFeatureTypeToString(j), virTristateSwitchTypeToString(def->caps_features[j])); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c69d1b7ef5..2f7a006711 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1817,45 +1817,45 @@ typedef enum { /* The capabilities are ordered alphabetically to help check for new ones */ typedef enum { - VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL = 0, - VIR_DOMAIN_CAPS_FEATURE_AUDIT_WRITE, - VIR_DOMAIN_CAPS_FEATURE_BLOCK_SUSPEND, - VIR_DOMAIN_CAPS_FEATURE_CHOWN, - VIR_DOMAIN_CAPS_FEATURE_DAC_OVERRIDE, - VIR_DOMAIN_CAPS_FEATURE_DAC_READ_SEARCH, - VIR_DOMAIN_CAPS_FEATURE_FOWNER, - VIR_DOMAIN_CAPS_FEATURE_FSETID, - VIR_DOMAIN_CAPS_FEATURE_IPC_LOCK, - VIR_DOMAIN_CAPS_FEATURE_IPC_OWNER, - VIR_DOMAIN_CAPS_FEATURE_KILL, - VIR_DOMAIN_CAPS_FEATURE_LEASE, - VIR_DOMAIN_CAPS_FEATURE_LINUX_IMMUTABLE, - VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN, - VIR_DOMAIN_CAPS_FEATURE_MAC_OVERRIDE, - VIR_DOMAIN_CAPS_FEATURE_MKNOD, - VIR_DOMAIN_CAPS_FEATURE_NET_ADMIN, - VIR_DOMAIN_CAPS_FEATURE_NET_BIND_SERVICE, - VIR_DOMAIN_CAPS_FEATURE_NET_BROADCAST, - VIR_DOMAIN_CAPS_FEATURE_NET_RAW, - VIR_DOMAIN_CAPS_FEATURE_SETGID, - VIR_DOMAIN_CAPS_FEATURE_SETFCAP, - VIR_DOMAIN_CAPS_FEATURE_SETPCAP, - VIR_DOMAIN_CAPS_FEATURE_SETUID, - VIR_DOMAIN_CAPS_FEATURE_SYS_ADMIN, - VIR_DOMAIN_CAPS_FEATURE_SYS_BOOT, - VIR_DOMAIN_CAPS_FEATURE_SYS_CHROOT, - VIR_DOMAIN_CAPS_FEATURE_SYS_MODULE, - VIR_DOMAIN_CAPS_FEATURE_SYS_NICE, - VIR_DOMAIN_CAPS_FEATURE_SYS_PACCT, - VIR_DOMAIN_CAPS_FEATURE_SYS_PTRACE, - VIR_DOMAIN_CAPS_FEATURE_SYS_RAWIO, - VIR_DOMAIN_CAPS_FEATURE_SYS_RESOURCE, - VIR_DOMAIN_CAPS_FEATURE_SYS_TIME, - VIR_DOMAIN_CAPS_FEATURE_SYS_TTY_CONFIG, - VIR_DOMAIN_CAPS_FEATURE_SYSLOG, - VIR_DOMAIN_CAPS_FEATURE_WAKE_ALARM, - VIR_DOMAIN_CAPS_FEATURE_LAST -} virDomainCapsFeature; + VIR_DOMAIN_PROCES_CAPS_FEATURE_AUDIT_CONTROL = 0, + VIR_DOMAIN_PROCES_CAPS_FEATURE_AUDIT_WRITE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_BLOCK_SUSPEND, + VIR_DOMAIN_PROCES_CAPS_FEATURE_CHOWN, + VIR_DOMAIN_PROCES_CAPS_FEATURE_DAC_OVERRIDE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_DAC_READ_SEARCH, + VIR_DOMAIN_PROCES_CAPS_FEATURE_FOWNER, + VIR_DOMAIN_PROCES_CAPS_FEATURE_FSETID, + VIR_DOMAIN_PROCES_CAPS_FEATURE_IPC_LOCK, + VIR_DOMAIN_PROCES_CAPS_FEATURE_IPC_OWNER, + VIR_DOMAIN_PROCES_CAPS_FEATURE_KILL, + VIR_DOMAIN_PROCES_CAPS_FEATURE_LEASE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_LINUX_IMMUTABLE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_MAC_ADMIN, + VIR_DOMAIN_PROCES_CAPS_FEATURE_MAC_OVERRIDE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_MKNOD, + VIR_DOMAIN_PROCES_CAPS_FEATURE_NET_ADMIN, + VIR_DOMAIN_PROCES_CAPS_FEATURE_NET_BIND_SERVICE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_NET_BROADCAST, + VIR_DOMAIN_PROCES_CAPS_FEATURE_NET_RAW, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SETGID, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SETFCAP, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SETPCAP, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SETUID, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_ADMIN, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_BOOT, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_CHROOT, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_MODULE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_NICE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_PACCT, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_PTRACE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_RAWIO, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_RESOURCE, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_TIME, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_TTY_CONFIG, + VIR_DOMAIN_PROCES_CAPS_FEATURE_SYSLOG, + VIR_DOMAIN_PROCES_CAPS_FEATURE_WAKE_ALARM, + VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST +} virDomainProcessCapsFeature; typedef enum { VIR_DOMAIN_LOCK_FAILURE_DEFAULT, @@ -2420,7 +2420,7 @@ struct _virDomainDef { * to handle support. A few assign specific data values to the option. * See virDomainDefFeaturesCheckABIStability() for details. */ int features[VIR_DOMAIN_FEATURE_LAST]; - int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST]; + int caps_features[VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST]; int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; int kvm_features[VIR_DOMAIN_KVM_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; @@ -3394,7 +3394,7 @@ VIR_ENUM_DECL(virDomainVirt); VIR_ENUM_DECL(virDomainBoot); VIR_ENUM_DECL(virDomainFeature); VIR_ENUM_DECL(virDomainCapabilitiesPolicy); -VIR_ENUM_DECL(virDomainCapsFeature); +VIR_ENUM_DECL(virDomainProcessCapsFeature); VIR_ENUM_DECL(virDomainLifecycle); VIR_ENUM_DECL(virDomainLifecycleAction); VIR_ENUM_DECL(virDomainDevice); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 94509d6f43..0a5431b50b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -217,7 +217,6 @@ virDomainBlockedReasonTypeToString; virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; -virDomainCapsFeatureTypeToString; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; @@ -543,6 +542,7 @@ virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; virDomainPMSuspendedReasonTypeFromString; virDomainPMSuspendedReasonTypeToString; +virDomainProcessCapsFeatureTypeToString; virDomainRedirdevBusTypeFromString; virDomainRedirdevBusTypeToString; virDomainRedirdevDefFind; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 5efb495b56..7e8259678f 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -306,7 +306,7 @@ virLXCTeardownHostUSBDeviceCgroup(virUSBDevicePtr dev G_GNUC_UNUSED, static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, virCgroupPtr cgroup) { - int capMknod = def->caps_features[VIR_DOMAIN_CAPS_FEATURE_MKNOD]; + int capMknod = def->caps_features[VIR_DOMAIN_PROCES_CAPS_FEATURE_MKNOD]; int ret = -1; size_t i; static virLXCCgroupDevicePolicy devices[] = { diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1fb9049c96..abad36c5aa 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1950,7 +1950,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, size_t i; int policy = def->features[VIR_DOMAIN_FEATURE_CAPABILITIES]; - /* Maps virDomainCapsFeature to CAPS_* */ + /* Maps virDomainProcessCapsFeature to CAPS_* */ static int capsMapping[] = {CAP_AUDIT_CONTROL, CAP_AUDIT_WRITE, CAP_BLOCK_SUSPEND, @@ -1996,7 +1996,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, capng_clear(CAPNG_SELECT_BOTH); /* Apply all single capabilities changes */ - for (i = 0; i < VIR_DOMAIN_CAPS_FEATURE_LAST; i++) { + for (i = 0; i < VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST; i++) { bool toDrop = false; int state = def->caps_features[i]; @@ -2013,21 +2013,21 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, capsMapping[i])) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to add capability %s: %d"), - virDomainCapsFeatureTypeToString(i), ret); + virDomainProcessCapsFeatureTypeToString(i), ret); return -1; } break; case VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT: switch (i) { - case VIR_DOMAIN_CAPS_FEATURE_SYS_BOOT: /* No use of reboot */ + case VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_BOOT: /* No use of reboot */ toDrop = !keepReboot && (state != VIR_TRISTATE_SWITCH_ON); break; - case VIR_DOMAIN_CAPS_FEATURE_SYS_MODULE: /* No kernel module loading */ - case VIR_DOMAIN_CAPS_FEATURE_SYS_TIME: /* No changing the clock */ - case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes */ - case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with auditing status */ - case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM config */ + case VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_MODULE: /* No kernel module loading */ + case VIR_DOMAIN_PROCES_CAPS_FEATURE_SYS_TIME: /* No changing the clock */ + case VIR_DOMAIN_PROCES_CAPS_FEATURE_MKNOD: /* No creating device nodes */ + case VIR_DOMAIN_PROCES_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with auditing status */ + case VIR_DOMAIN_PROCES_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM config */ toDrop = (state != VIR_TRISTATE_SWITCH_ON); break; default: /* User specified capabilities to drop */ @@ -2045,7 +2045,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def, capsMapping[i])) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to remove capability %s: %d"), - virDomainCapsFeatureTypeToString(i), ret); + virDomainProcessCapsFeatureTypeToString(i), ret); return -1; } break; diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index fec3b4454c..5018cf115b 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1043,8 +1043,8 @@ lxcSetCapDrop(virDomainDefPtr def, virConfPtr properties) if (virConfGetValueString(properties, "lxc.cap.drop", &value) > 0) toDrop = virStringSplit(value, " ", 0); - for (i = 0; i < VIR_DOMAIN_CAPS_FEATURE_LAST; i++) { - capString = virDomainCapsFeatureTypeToString(i); + for (i = 0; i < VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST; i++) { + capString = virDomainProcessCapsFeatureTypeToString(i); if (toDrop != NULL && virStringListHasString((const char **)toDrop, capString)) def->caps_features[i] = VIR_TRISTATE_SWITCH_OFF; -- 2.23.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 953464b09d..0a0de447b5 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -948,7 +948,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf, for (i = 0; i < cache->nbanks; i++) { virCapsHostCacheBankPtr bank = cache->banks[i]; - char *cpus_str = virBitmapFormat(bank->cpus); + g_autofree char *cpus_str = virBitmapFormat(bank->cpus); const char *unit = NULL; unsigned long long short_size = virFormatIntPretty(bank->size, &unit); @@ -965,7 +965,6 @@ virCapabilitiesFormatCaches(virBufferPtr buf, bank->id, bank->level, virCacheTypeToString(bank->type), short_size, unit, cpus_str); - VIR_FREE(cpus_str); virBufferSetChildIndent(&childrenBuf, buf); for (j = 0; j < bank->ncontrols; j++) { -- 2.23.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e81140430..0e774889a2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27985,6 +27985,17 @@ virDomainMemtuneFormat(virBufferPtr buf, virXMLFormatElement(buf, "memtune", NULL, &childBuf); + ret = 0; + return ret; +} + + +static void +virDomainMemorybackingFormat(virBufferPtr buf, + const virDomainMemtune *mem) +{ + g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + virBufferSetChildIndent(&childBuf, buf); if (mem->nhugepages) @@ -28006,9 +28017,6 @@ virDomainMemtuneFormat(virBufferPtr buf, virBufferAddLit(&childBuf, "<discard/>\n"); virXMLFormatElement(buf, "memoryBacking", NULL, &childBuf); - - ret = 0; - return ret; } @@ -28484,6 +28492,8 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, if (virDomainMemtuneFormat(buf, &def->mem) < 0) goto error; + virDomainMemorybackingFormat(buf, &def->mem); + if (virDomainCpuDefFormat(buf, def) < 0) goto error; -- 2.23.0
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0e774889a2..81ab5eb138 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -27953,12 +27953,11 @@ virDomainIOMMUDefFormat(virBufferPtr buf, } -static int +static void virDomainMemtuneFormat(virBufferPtr buf, const virDomainMemtune *mem) { g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; - int ret = -1; virBufferSetChildIndent(&childBuf, buf); @@ -27984,9 +27983,6 @@ virDomainMemtuneFormat(virBufferPtr buf, } virXMLFormatElement(buf, "memtune", NULL, &childBuf); - - ret = 0; - return ret; } @@ -28489,9 +28485,7 @@ virDomainDefFormatInternalSetRootName(virDomainDefPtr def, if (virDomainDefFormatBlkiotune(buf, def) < 0) goto error; - if (virDomainMemtuneFormat(buf, &def->mem) < 0) - goto error; - + virDomainMemtuneFormat(buf, &def->mem); virDomainMemorybackingFormat(buf, &def->mem); if (virDomainCpuDefFormat(buf, def) < 0) -- 2.23.0
Use automatic memory freeing and use virXMLFormatElement instead of open coding it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 0a0de447b5..1ab389bc65 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -938,7 +938,6 @@ virCapabilitiesFormatCaches(virBufferPtr buf, { size_t i = 0; size_t j = 0; - virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; if (!cache->nbanks) return 0; @@ -947,6 +946,8 @@ virCapabilitiesFormatCaches(virBufferPtr buf, virBufferAdjustIndent(buf, 2); for (i = 0; i < cache->nbanks; i++) { + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childrenBuf = VIR_BUFFER_INITIALIZER; virCapsHostCacheBankPtr bank = cache->banks[i]; g_autofree char *cpus_str = virBitmapFormat(bank->cpus); const char *unit = NULL; @@ -959,8 +960,8 @@ virCapabilitiesFormatCaches(virBufferPtr buf, * Let's just *hope* the size is aligned to KiBs so that it does not * bite is back in the future */ - virBufferAsprintf(buf, - "<bank id='%u' level='%u' type='%s' " + virBufferAsprintf(&attrBuf, + " id='%u' level='%u' type='%s' " "size='%llu' unit='%s' cpus='%s'", bank->id, bank->level, virCacheTypeToString(bank->type), @@ -1006,13 +1007,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf, controls->max_allocation); } - if (virBufferUse(&childrenBuf)) { - virBufferAddLit(buf, ">\n"); - virBufferAddBuffer(buf, &childrenBuf); - virBufferAddLit(buf, "</bank>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virXMLFormatElement(buf, "bank", &attrBuf, &childrenBuf); } if (virCapabilitiesFormatResctrlMonitor(buf, cache->monitor) < 0) -- 2.23.0
Use virXMLFormatElement and the automatic memory handlers to simplfy the code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1ab389bc65..f3b34b7040 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1024,7 +1024,6 @@ virCapabilitiesFormatMemoryBandwidth(virBufferPtr buf, virCapsHostMemBWPtr memBW) { size_t i = 0; - virBuffer childrenBuf = VIR_BUFFER_INITIALIZER; if (!memBW->nnodes) return 0; @@ -1033,17 +1032,18 @@ virCapabilitiesFormatMemoryBandwidth(virBufferPtr buf, virBufferAdjustIndent(buf, 2); for (i = 0; i < memBW->nnodes; i++) { + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childrenBuf = VIR_BUFFER_INITIALIZER; virCapsHostMemBWNodePtr node = memBW->nodes[i]; virResctrlInfoMemBWPerNodePtr control = &node->control; - char *cpus_str = virBitmapFormat(node->cpus); + g_autofree char *cpus_str = virBitmapFormat(node->cpus); if (!cpus_str) return -1; - virBufferAsprintf(buf, - "<node id='%u' cpus='%s'", + virBufferAsprintf(&attrBuf, + " id='%u' cpus='%s'", node->id, cpus_str); - VIR_FREE(cpus_str); virBufferSetChildIndent(&childrenBuf, buf); virBufferAsprintf(&childrenBuf, @@ -1052,13 +1052,7 @@ virCapabilitiesFormatMemoryBandwidth(virBufferPtr buf, control->granularity, control->min, control->max_allocation); - if (virBufferUse(&childrenBuf)) { - virBufferAddLit(buf, ">\n"); - virBufferAddBuffer(buf, &childrenBuf); - virBufferAddLit(buf, "</node>\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virXMLFormatElement(buf, "node", &attrBuf, &childrenBuf); } if (virCapabilitiesFormatResctrlMonitor(buf, memBW->monitor) < 0) -- 2.23.0
On Tue, Nov 12, 2019 at 08:27:47AM +0100, Peter Krempa wrote:
Use virXMLFormatElement and the automatic memory handlers to simplfy the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Nov 12, 2019 at 08:27:37AM +0100, Peter Krempa wrote:
Peter Krempa (10): qemu: caps: Rework memory allocation in virQEMUCapsFillDomainFeatureSEVCaps qemu: caps: Make capability filler functions void conf: domaincaps: Fix broken attempt at being const-correct conf: storagecaps: Fix broken attempt at being const-correct conf: Rename virDomainCapsFeature to virDomainProcessCapsFeature conf: caps: Automaticaly free 'cpus_str' conf: domain: Split up formatting of <memtune> and <memoryBacking> conf: turn virDomainMemtuneFormat void conf: caps: Modernize virCapabilitiesFormatCaches conf: capabilities: Modernize virCapabilitiesFormatMemoryBandwidth
src/conf/capabilities.c | 36 +++++---------- src/conf/domain_capabilities.c | 32 ++++++------- src/conf/domain_capabilities.h | 4 +- src/conf/domain_conf.c | 30 ++++++------ src/conf/domain_conf.h | 82 ++++++++++++++++----------------- src/conf/storage_capabilities.c | 4 +- src/conf/storage_capabilities.h | 2 +- src/libvirt_private.syms | 2 +- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_container.c | 20 ++++---- src/lxc/lxc_native.c | 4 +- src/qemu/qemu_capabilities.c | 81 +++++++++++--------------------- 12 files changed, 132 insertions(+), 167 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa