6:21 p.m.
Fallout from merging the zPCI patches.
Andrea Bolognani (3):
conf: Fix error flow in virDomainPCIAddressEnsureAddr()
conf: Add several cleanup paths
conf: Perform error checking in virDomainDeviceInfoFormat()
src/conf/domain_addr.c | 16 ++-
src/conf/domain_conf.c | 234 +++++++++++++++++++++++++++++------------
2 files changed, 177 insertions(+), 73 deletions(-)
--
2.19.1
6:21 p.m.
New subject: [libvirt] [PATCH 1/3] conf: Fix error flow in virDomainPCIAddressEnsureAddr()
This avoids setting 'ret' multiple times, which will result
in errors being masked if the first operation fails but the
second one succeeds.
Introduced-by: f183b87fc1dbcc6446ac3c1cef9cdd345b9725fb
Spotted-by: Coverity
Reported-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_addr.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 3e1d767e4f..cc9ea82a33 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -940,15 +940,21 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
addrStr, flags, true))
goto cleanup;
- ret = virDomainPCIAddressReserveAddrInternal(addrs, &dev->addr.pci,
- flags, dev->isolationGroup,
- true);
+ if (virDomainPCIAddressReserveAddrInternal(addrs, &dev->addr.pci,
+ flags, dev->isolationGroup,
+ true) < 0) {
+ goto cleanup;
+ }
} else {
- ret = virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1);
+ if (virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1) < 0)
+ goto cleanup;
}
dev->addr.pci.extFlags = dev->pciAddrExtFlags;
- ret = virDomainPCIAddressExtensionEnsureAddr(addrs, &dev->addr.pci);
+ if (virDomainPCIAddressExtensionEnsureAddr(addrs, &dev->addr.pci) < 0)
+ goto cleanup;
+
+ ret = 0;
cleanup:
VIR_FREE(addrStr);
--
2.19.1
8:15 p.m.
New subject: [libvirt] [PATCH 1/3] conf: Fix error flow in virDomainPCIAddressEnsureAddr()
On Fri, Nov 16, 2018 at 05:21:29PM +0100, Andrea Bolognani wrote:
This avoids setting 'ret' multiple times, which will result
in errors being masked if the first operation fails but the
second one succeeds.
Introduced-by: f183b87fc1dbcc6446ac3c1cef9cdd345b9725fb
Spotted-by: Coverity
Reported-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_addr.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
6:21 p.m.
New subject: [libvirt] [PATCH 2/3] conf: Add several cleanup paths
In many cases, an early exit from a function would cause
memory allocated by local virBuffer instances not to be
released.
Provide proper cleanup paths to solve the issue.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 137 ++++++++++++++++++++++++++++++-----------
1 file changed, 100 insertions(+), 37 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c3dbba6919..1d04cac104 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -24353,13 +24353,14 @@ virDomainControllerDefFormat(virBufferPtr buf,
const char *model = NULL;
const char *modelName = NULL;
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
virBufferSetChildIndent(&childBuf, buf);
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected controller type %d"), def->type);
- return -1;
+ goto cleanup;
}
if (def->model != -1) {
@@ -24368,7 +24369,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected model type %d"), def->model);
- return -1;
+ goto cleanup;
}
}
@@ -24432,7 +24433,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected model name value %d"),
def->opts.pciopts.modelName);
- return -1;
+ goto cleanup;
}
virBufferAsprintf(&childBuf, "<model
name='%s'/>\n", modelName);
}
@@ -24483,7 +24484,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
}
if (virBufferCheckError(&childBuf) < 0)
- return -1;
+ goto cleanup;
if (virBufferUse(&childBuf)) {
virBufferAddLit(buf, ">\n");
@@ -24493,6 +24494,11 @@ virDomainControllerDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&childBuf);
+
return 0;
}
@@ -24523,17 +24529,18 @@ virDomainFSDefFormat(virBufferPtr buf,
const char *wrpolicy = virDomainFSWrpolicyTypeToString(def->wrpolicy);
const char *src = def->src->path;
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected filesystem type %d"), def->type);
- return -1;
+ goto cleanup;
}
if (!accessmode) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected accessmode %d"), def->accessmode);
- return -1;
+ goto cleanup;
}
@@ -24557,7 +24564,7 @@ virDomainFSDefFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
if (virBufferCheckError(&driverBuf) < 0)
- return -1;
+ goto cleanup;
if (virBufferUse(&driverBuf)) {
virBufferAddLit(buf, "<driver");
@@ -24617,7 +24624,13 @@ virDomainFSDefFormat(virBufferPtr buf,
}
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</filesystem>\n");
- return 0;
+
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&driverBuf);
+
+ return ret;
}
@@ -25847,13 +25860,14 @@ virDomainSoundDefFormat(virBufferPtr buf,
const char *model = virDomainSoundModelTypeToString(def->model);
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
size_t i;
+ int ret = -1;
virBufferSetChildIndent(&childBuf, buf);
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected sound model %d"), def->model);
- return -1;
+ goto cleanup;
}
for (i = 0; i < def->ncodecs; i++)
@@ -25862,7 +25876,7 @@ virDomainSoundDefFormat(virBufferPtr buf,
virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
- return -1;
+ goto cleanup;
virBufferAsprintf(buf, "<sound model='%s'", model);
if (virBufferUse(&childBuf)) {
@@ -25873,6 +25887,11 @@ virDomainSoundDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&childBuf);
+
return 0;
}
@@ -25884,11 +25903,12 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
{
const char *model = virDomainMemballoonModelTypeToString(def->model);
virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected memballoon model %d"), def->model);
- return -1;
+ goto cleanup;
}
virBufferAsprintf(buf, "<memballoon model='%s'", model);
@@ -25909,10 +25929,9 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
- if (virBufferCheckError(&driverBuf) < 0) {
- virBufferFreeAndReset(&childrenBuf);
- return -1;
- }
+ if (virBufferCheckError(&driverBuf) < 0)
+ goto cleanup;
+
if (virBufferUse(&driverBuf)) {
virBufferAddLit(&childrenBuf, "<driver");
virBufferAddBuffer(&childrenBuf, &driverBuf);
@@ -25921,7 +25940,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
}
if (virBufferCheckError(&childrenBuf) < 0)
- return -1;
+ goto cleanup;
if (!virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, "/>\n");
@@ -25931,6 +25950,11 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "</memballoon>\n");
}
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&childrenBuf);
+
return 0;
}
@@ -25958,25 +25982,26 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
const char *model = virDomainWatchdogModelTypeToString(def->model);
const char *action = virDomainWatchdogActionTypeToString(def->action);
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
virBufferSetChildIndent(&childBuf, buf);
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected watchdog model %d"), def->model);
- return -1;
+ goto cleanup;
}
if (!action) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected watchdog action %d"), def->action);
- return -1;
+ goto cleanup;
}
virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
- return -1;
+ goto cleanup;
virBufferAsprintf(buf, "<watchdog model='%s'
action='%s'",
model, action);
@@ -25989,13 +26014,19 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- return 0;
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&childBuf);
+
+ return ret;
}
static int virDomainPanicDefFormat(virBufferPtr buf,
virDomainPanicDefPtr def)
{
virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
virBufferAddLit(buf, "<panic");
@@ -26007,7 +26038,7 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0);
if (virBufferCheckError(&childrenBuf) < 0)
- return -1;
+ goto cleanup;
if (virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, ">\n");
@@ -26016,8 +26047,13 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
} else {
virBufferAddLit(buf, "/>\n");
}
+
+ ret = 0;
+
+ cleanup:
virBufferFreeAndReset(&childrenBuf);
- return 0;
+
+ return ret;
}
static int
@@ -26067,6 +26103,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
const char *model = virDomainRNGModelTypeToString(def->model);
const char *backend = virDomainRNGBackendTypeToString(def->backend);
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
virBufferAsprintf(buf, "<rng model='%s'>\n", model);
virBufferAdjustIndent(buf, 2);
@@ -26085,11 +26122,11 @@ virDomainRNGDefFormat(virBufferPtr buf,
case VIR_DOMAIN_RNG_BACKEND_EGD:
if (virDomainChrAttrsDefFormat(buf, def->source.chardev, false) < 0)
- return -1;
+ goto cleanup;
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
if (virDomainChrSourceDefFormat(buf, def->source.chardev, flags) < 0)
- return -1;
+ goto cleanup;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</backend>\n");
@@ -26099,7 +26136,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
if (virBufferCheckError(&driverBuf) < 0)
- return -1;
+ goto cleanup;
if (virBufferUse(&driverBuf)) {
virBufferAddLit(buf, "<driver");
@@ -26111,7 +26148,13 @@ virDomainRNGDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</rng>\n");
- return 0;
+
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&driverBuf);
+
+ return ret;
}
void
@@ -26258,18 +26301,19 @@ virDomainVideoDefFormat(virBufferPtr buf,
{
const char *model = virDomainVideoTypeToString(def->type);
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected video model %d"), def->type);
- return -1;
+ goto cleanup;
}
virBufferAddLit(buf, "<video>\n");
virBufferAdjustIndent(buf, 2);
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
if (virBufferCheckError(&driverBuf) < 0)
- return -1;
+ goto cleanup;
if (virBufferUse(&driverBuf) || (def->driver &&
def->driver->vgaconf)) {
virBufferAddLit(buf, "<driver");
if (virBufferUse(&driverBuf))
@@ -26308,7 +26352,13 @@ virDomainVideoDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</video>\n");
- return 0;
+
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&driverBuf);
+
+ return ret;
}
static int
@@ -26320,6 +26370,7 @@ virDomainInputDefFormat(virBufferPtr buf,
const char *bus = virDomainInputBusTypeToString(def->bus);
virBuffer childbuf = VIR_BUFFER_INITIALIZER;
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
/* don't format keyboard into migratable XML for backward compatibility */
if (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
@@ -26331,12 +26382,12 @@ virDomainInputDefFormat(virBufferPtr buf,
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected input type %d"), def->type);
- return -1;
+ goto cleanup;
}
if (!bus) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected input bus type %d"), def->bus);
- return -1;
+ goto cleanup;
}
virBufferAsprintf(buf, "<input type='%s' bus='%s'",
@@ -26345,7 +26396,7 @@ virDomainInputDefFormat(virBufferPtr buf,
virBufferSetChildIndent(&childbuf, buf);
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
if (virBufferCheckError(&driverBuf) < 0)
- return -1;
+ goto cleanup;
if (virBufferUse(&driverBuf)) {
virBufferAddLit(&childbuf, "<driver");
@@ -26356,7 +26407,7 @@ virDomainInputDefFormat(virBufferPtr buf,
virDomainDeviceInfoFormat(&childbuf, &def->info, flags);
if (virBufferCheckError(&childbuf) < 0)
- return -1;
+ goto cleanup;
if (!virBufferUse(&childbuf)) {
virBufferAddLit(buf, "/>\n");
@@ -26366,7 +26417,13 @@ virDomainInputDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "</input>\n");
}
- return 0;
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&childbuf);
+ virBufferFreeAndReset(&driverBuf);
+
+ return ret;
}
@@ -27028,19 +27085,20 @@ virDomainHubDefFormat(virBufferPtr buf,
{
const char *type = virDomainHubTypeToString(def->type);
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
virBufferSetChildIndent(&childBuf, buf);
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected hub type %d"), def->type);
- return -1;
+ goto cleanup;
}
virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
- return -1;
+ goto cleanup;
virBufferAsprintf(buf, "<hub type='%s'", type);
if (virBufferUse(&childBuf)) {
@@ -27051,7 +27109,12 @@ virDomainHubDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- return 0;
+ ret = 0;
+
+ cleanup:
+ virBufferFreeAndReset(&childBuf);
+
+ return ret;
}
--
2.19.1
8:33 p.m.
New subject: [libvirt] [PATCH 2/3] conf: Add several cleanup paths
On Fri, Nov 16, 2018 at 05:21:30PM +0100, Andrea Bolognani wrote:
In many cases, an early exit from a function would cause
memory allocated by local virBuffer instances not to be
released.
Provide proper cleanup paths to solve the issue.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 137 ++++++++++++++++++++++++++++++-----------
1 file changed, 100 insertions(+), 37 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
9:28 p.m.
New subject: [libvirt] [PATCH 2/3] conf: Add several cleanup paths
This patch breaks the error return value for:
virDomainControllerDefFormat()
virDomainSoundDefFormat()
virDomainMemballoonDefFormat()
Patch adds the "ret" variable but in error exit it use "return 0"
statement.
Actually this breaks compilation.
Was this code compiled ?
conf/domain_conf.c: In function 'virDomainControllerDefFormat':
conf/domain_conf.c:24368:9: error: variable 'ret' set but not used
[-Werror=unused-but-set-variable]
int ret = -1;
^~~
CC test/libvirt_driver_test_la-test_driver.lo
CC vmx/libvirt_vmx_la-vmx.lo
CC vmware/libvirt_driver_vmware_la-vmware_driver.lo
CC vmware/libvirt_driver_vmware_la-vmware_conf.lo
conf/domain_conf.c: In function 'virDomainSoundDefFormat':
conf/domain_conf.c:25882:9: error: variable 'ret' set but not used
[-Werror=unused-but-set-variable]
int ret = -1;
^~~
conf/domain_conf.c: In function 'virDomainMemballoonDefFormat':
conf/domain_conf.c:25926:9: error: variable 'ret' set but not used
[-Werror=unused-but-set-variable]
int ret = -1;
^~~
On Fri, 16 Nov 2018 at 19:56, Pavel Hrdina <phrdina(a)redhat.com> wrote:
On Fri, Nov 16, 2018 at 05:21:30PM +0100, Andrea Bolognani wrote:
> In many cases, an early exit from a function would cause
> memory allocated by local virBuffer instances not to be
> released.
>
> Provide proper cleanup paths to solve the issue.
>
> Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
> ---
> src/conf/domain_conf.c | 137 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 100 insertions(+), 37 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
6:21 p.m.
New subject: [libvirt] [PATCH 3/3] conf: Perform error checking in virDomainDeviceInfoFormat()
virXMLFormatElement() might fail, but we were not checking
its return value.
Fixing this requires us to change virDomainDeviceInfoFormat()
so that it can report an error back to the caller.
Introduced-by: 0d6b87335c00451b0923ecc91d617f71e4135bf8
Spotted-by: Coverity
Reported-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 97 ++++++++++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 31 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1d04cac104..8cb9b2719c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6418,13 +6418,14 @@ virDomainVirtioOptionsFormat(virBufferPtr buf,
}
-static void ATTRIBUTE_NONNULL(2)
+static int ATTRIBUTE_NONNULL(2)
virDomainDeviceInfoFormat(virBufferPtr buf,
virDomainDeviceInfoPtr info,
unsigned int flags)
{
virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) {
virBufferAsprintf(buf, "<boot order='%u'",
info->bootIndex);
@@ -6467,8 +6468,11 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
}
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
- info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
- return;
+ info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
+ /* We're done here */
+ ret = 0;
+ goto cleanup;
+ }
virBufferAsprintf(&attrBuf, " type='%s'",
virDomainDeviceAddressTypeToString(info->type));
@@ -6562,10 +6566,16 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
break;
}
- virXMLFormatElement(buf, "address", &attrBuf, &childBuf);
+ if (virXMLFormatElement(buf, "address", &attrBuf, &childBuf) <
0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
virBufferFreeAndReset(&attrBuf);
virBufferFreeAndReset(&childBuf);
+
+ return ret;
}
static int
@@ -24299,8 +24309,10 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->src->encryption && !def->src->encryptionInherited
&&
virStorageEncryptionFormat(buf, def->src->encryption) < 0)
return -1;
- virDomainDeviceInfoFormat(buf, &def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
+ if (virDomainDeviceInfoFormat(buf, &def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) {
+ return -1;
+ }
if (virDomainDiskDefFormatPrivateData(buf, def, flags, xmlopt) < 0)
return -1;
@@ -24475,7 +24487,8 @@ virDomainControllerDefFormat(virBufferPtr buf,
virDomainControllerDriverFormat(&childBuf, def);
- virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ goto cleanup;
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
def->opts.pciopts.pcihole64) {
@@ -24613,7 +24626,8 @@ virDomainFSDefFormat(virBufferPtr buf,
if (def->readonly)
virBufferAddLit(buf, "<readonly/>\n");
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ goto cleanup;
if (def->space_hard_limit)
virBufferAsprintf(buf, "<space_hard_limit unit='bytes'>"
@@ -25412,9 +25426,11 @@ virDomainNetDefFormat(virBufferPtr buf,
virDomainNetDefCoalesceFormatXML(buf, def->coalesce);
- virDomainDeviceInfoFormat(buf, &def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
- | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM);
+ if (virDomainDeviceInfoFormat(buf, &def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
+ | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) {
+ return -1;
+ }
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</interface>\n");
@@ -25723,7 +25739,8 @@ virDomainChrDefFormat(virBufferPtr buf,
if (virDomainChrTargetDefFormat(buf, def, flags) < 0)
return -1;
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAsprintf(buf, "</%s>\n", elementName);
@@ -25772,7 +25789,8 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
_("unexpected smartcard type %d"), def->type);
goto cleanup;
}
- virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ goto cleanup;
if (virBufferCheckError(&childBuf) < 0)
goto cleanup;
@@ -25843,7 +25861,8 @@ virDomainTPMDefFormat(virBufferPtr buf,
break;
}
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</tpm>\n");
@@ -25873,7 +25892,8 @@ virDomainSoundDefFormat(virBufferPtr buf,
for (i = 0; i < def->ncodecs; i++)
virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
- virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ goto cleanup;
if (virBufferCheckError(&childBuf) < 0)
goto cleanup;
@@ -25922,7 +25942,8 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
if (def->period)
virBufferAsprintf(&childrenBuf, "<stats
period='%i'/>\n", def->period);
- virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0)
+ goto cleanup;
if (def->virtio) {
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
@@ -25965,7 +25986,8 @@ virDomainNVRAMDefFormat(virBufferPtr buf,
{
virBufferAddLit(buf, "<nvram>\n");
virBufferAdjustIndent(buf, 2);
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</nvram>\n");
@@ -25998,7 +26020,8 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
goto cleanup;
}
- virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ goto cleanup;
if (virBufferCheckError(&childBuf) < 0)
goto cleanup;
@@ -26035,7 +26058,8 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
virDomainPanicModelTypeToString(def->model));
virBufferSetChildIndent(&childrenBuf, buf);
- virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0);
+ if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
+ goto cleanup;
if (virBufferCheckError(&childrenBuf) < 0)
goto cleanup;
@@ -26087,7 +26111,8 @@ virDomainShmemDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</shmem>\n");
@@ -26144,7 +26169,8 @@ virDomainRNGDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ goto cleanup;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</rng>\n");
@@ -26271,7 +26297,8 @@ virDomainMemoryDefFormat(virBufferPtr buf,
virDomainMemoryTargetDefFormat(buf, def);
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</memory>\n");
@@ -26348,7 +26375,8 @@ virDomainVideoDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- virDomainDeviceInfoFormat(buf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ goto cleanup;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</video>\n");
@@ -26404,7 +26432,8 @@ virDomainInputDefFormat(virBufferPtr buf,
virBufferAddLit(&childbuf, "/>\n");
}
virBufferEscapeString(&childbuf, "<source
evdev='%s'/>\n", def->source.evdev);
- virDomainDeviceInfoFormat(&childbuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0)
+ goto cleanup;
if (virBufferCheckError(&childbuf) < 0)
goto cleanup;
@@ -27004,9 +27033,11 @@ virDomainHostdevDefFormat(virBufferPtr buf,
if (def->shareable)
virBufferAddLit(buf, "<shareable/>\n");
- virDomainDeviceInfoFormat(buf, def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
- | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM);
+ if (virDomainDeviceInfoFormat(buf, def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
+ | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0) {
+ return -1;
+ }
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</hostdev>\n");
@@ -27032,8 +27063,10 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
if (virDomainChrSourceDefFormat(buf, def->source, flags) < 0)
return -1;
- virDomainDeviceInfoFormat(buf, &def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
+ if (virDomainDeviceInfoFormat(buf, &def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) {
+ return -1;
+ }
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</redirdev>\n");
return 0;
@@ -27095,7 +27128,8 @@ virDomainHubDefFormat(virBufferPtr buf,
goto cleanup;
}
- virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ goto cleanup;
if (virBufferCheckError(&childBuf) < 0)
goto cleanup;
@@ -27816,7 +27850,8 @@ virDomainVsockDefFormat(virBufferPtr buf,
if (virXMLFormatElement(&childBuf, "cid", &cidAttrBuf, NULL) <
0)
goto cleanup;
- virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0);
+ if (virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0) < 0)
+ goto cleanup;
if (virXMLFormatElement(buf, "vsock", &attrBuf, &childBuf) < 0)
goto cleanup;
--
2.19.1
8:38 p.m.
New subject: [libvirt] [PATCH 3/3] conf: Perform error checking in virDomainDeviceInfoFormat()
On Fri, Nov 16, 2018 at 05:21:31PM +0100, Andrea Bolognani wrote:
virXMLFormatElement() might fail, but we were not checking
its return value.
Fixing this requires us to change virDomainDeviceInfoFormat()
so that it can report an error back to the caller.
Introduced-by: 0d6b87335c00451b0923ecc91d617f71e4135bf8
Spotted-by: Coverity
Reported-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/domain_conf.c | 97 ++++++++++++++++++++++++++++--------------
1 file changed, 66 insertions(+), 31 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
2266
days inactive
2267
days old
7 comments
3 participants
participants (3)
-
Andrea Bolognani -
Pavel Hrdina -
Radoslaw Biernacki