8:29 a.m.
*** BLURB HERE ***
Ján Tomko (8):
Remove superfluous usage of virDomainDeviceInfoNeedsFormat
virDomainDeviceInfoFormat: delete outdated comments
Use a separate buffer for <smartcard> subelements
Use a separate buffer for <sound> subelements
Use a separate buffer for <watchdog> subelements
Use a separate buffer for <hub> subelements
conf: check for buffer errors before virBufferUse
Turn virDomainDeviceInfoFormat into void
src/conf/capabilities.c | 5 +
src/conf/cpu_conf.c | 4 +
src/conf/domain_conf.c | 237 +++++++++++++++++++++---------------------------
3 files changed, 114 insertions(+), 132 deletions(-)
--
2.13.0
8:29 a.m.
New subject: [libvirt] [PATCH 1/8] Remove superfluous usage of virDomainDeviceInfoNeedsFormat
This function returns false if virDomainDeviceInfoFormat
would not format anything.
Using it as the sole condition to decide whether to call
virDomainDeviceInfoFormat or not is pointless, since
the conditions are repeated in virDomainDeviceInfoFormat.
---
src/conf/domain_conf.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34c8f45ed..73c4b55e9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21903,8 +21903,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
virDomainControllerDriverFormat(&childBuf, def);
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
- virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
return -1;
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
@@ -23037,10 +23036,8 @@ virDomainChrDefFormat(virBufferPtr buf,
break;
}
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- }
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAsprintf(buf, "</%s>\n", elementName);
@@ -23142,10 +23139,8 @@ virDomainTPMDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</backend>\n");
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- }
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</tpm>\n");
@@ -23227,8 +23222,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
if (def->period)
virBufferAsprintf(&childrenBuf, "<stats
period='%i'/>\n", def->period);
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
- virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {
+ if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {
virBufferFreeAndReset(&childrenBuf);
return -1;
}
@@ -23267,8 +23261,7 @@ virDomainNVRAMDefFormat(virBufferPtr buf,
{
virBufferAddLit(buf, "<nvram>\n");
virBufferAdjustIndent(buf, 2);
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
- virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
return -1;
virBufferAdjustIndent(buf, -2);
@@ -23427,10 +23420,8 @@ virDomainRNGDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- }
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</rng>\n");
@@ -23548,10 +23539,8 @@ virDomainMemoryDefFormat(virBufferPtr buf,
virDomainMemoryTargetDefFormat(buf, def);
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- }
+ if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ return -1;
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</memory>\n");
--
2.13.0
3:44 p.m.
New subject: [libvirt] [PATCH 1/8] Remove superfluous usage of virDomainDeviceInfoNeedsFormat
On 07/26/2017 09:29 AM, Ján Tomko wrote:
This function returns false if virDomainDeviceInfoFormat
would not format anything.
Using it as the sole condition to decide whether to call
virDomainDeviceInfoFormat or not is pointless, since
the conditions are repeated in virDomainDeviceInfoFormat.
---
src/conf/domain_conf.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
FWIW:
minor nit... In virDomainDeviceInfoFormat, there's an "info->rombar ||"
condition that could be "info->rombar != VIR_TRISTATE_SWITCH_ABSENT ||"
8:29 a.m.
New subject: [libvirt] [PATCH 2/8] virDomainDeviceInfoFormat: delete outdated comments
This function has grown to format more than just the address.
Delete the comment completely to avoid failing to update it
in the future.
Also, the indentation is now handled by the virBuffer APIs,
so the comment about indentation no longer makes sense.
---
src/conf/domain_conf.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73c4b55e9..6c958bcf6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5349,9 +5349,6 @@ virDomainVirtioOptionsFormat(virBufferPtr buf,
}
-/* Generate a string representation of a device address
- * @info address Device address to stringify
- */
static int ATTRIBUTE_NONNULL(2)
virDomainDeviceInfoFormat(virBufferPtr buf,
virDomainDeviceInfoPtr info,
@@ -5400,7 +5397,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
return 0;
- /* We'll be in domain/devices/[device type]/ so 3 level indent */
virBufferAsprintf(buf, "<address type='%s'",
virDomainDeviceAddressTypeToString(info->type));
--
2.13.0
3:45 p.m.
New subject: [libvirt] [PATCH 2/8] virDomainDeviceInfoFormat: delete outdated comments
On 07/26/2017 09:29 AM, Ján Tomko wrote:
This function has grown to format more than just the address.
Delete the comment completely to avoid failing to update it
in the future.
Also, the indentation is now handled by the virBuffer APIs,
so the comment about indentation no longer makes sense.
---
src/conf/domain_conf.c | 4 ----
1 file changed, 4 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
8:29 a.m.
New subject: [libvirt] [PATCH 3/8] Use a separate buffer for <smartcard> subelements
Convert virDomainSmartcardDefFormat to use a separate buffer
for possible subelements, to avoid the need for duplicated
formatting logic in virDomainDeviceInfoNeedsFormat.
---
src/conf/domain_conf.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c958bcf6..ead94976d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23047,37 +23047,32 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *mode = virDomainSmartcardTypeToString(def->type);
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
size_t i;
+ virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
+
if (!mode) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected smartcard type %d"), def->type);
return -1;
}
- virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
- virBufferAdjustIndent(buf, 2);
switch (def->type) {
case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
- if (!virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "/>\n");
- return 0;
- }
- virBufferAddLit(buf, ">\n");
break;
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
- virBufferAddLit(buf, ">\n");
- for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++)
- virBufferEscapeString(buf,
"<certificate>%s</certificate>\n",
+ for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
+ virBufferEscapeString(&childBuf,
"<certificate>%s</certificate>\n",
def->data.cert.file[i]);
- virBufferEscapeString(buf, "<database>%s</database>\n",
+ }
+ virBufferEscapeString(&childBuf,
"<database>%s</database>\n",
def->data.cert.database);
break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
- if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
+ if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false,
flags) < 0)
return -1;
break;
@@ -23087,10 +23082,22 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
_("unexpected smartcard type %d"), def->type);
return -1;
}
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
+ virBufferFreeAndReset(&childBuf);
return -1;
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</smartcard>\n");
+ }
+
+ if (virBufferCheckError(&childBuf) < 0)
+ return -1;
+
+ virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
+ if (virBufferUse(&childBuf)) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAddBuffer(buf, &childBuf);
+ virBufferAddLit(buf, "</smartcard>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
return 0;
}
--
2.13.0
3:45 p.m.
New subject: [libvirt] [PATCH 3/8] Use a separate buffer for <smartcard> subelements
On 07/26/2017 09:29 AM, Ján Tomko wrote:
Convert virDomainSmartcardDefFormat to use a separate buffer
for possible subelements, to avoid the need for duplicated
formatting logic in virDomainDeviceInfoNeedsFormat.
---
src/conf/domain_conf.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
12:12 p.m.
New subject: [libvirt] [PATCH 3/8] Use a separate buffer for <smartcard> subelements
On 07/26/2017 09:29 AM, Ján Tomko wrote:
Convert virDomainSmartcardDefFormat to use a separate buffer
for possible subelements, to avoid the need for duplicated
formatting logic in virDomainDeviceInfoNeedsFormat.
---
src/conf/domain_conf.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
Looks like this patch causes a regression, currently breaking the virt-manager
test suite that danpb pointed out to me. Edit an existing VM and add
<smartcard mode='passthrough' type='spicevmc'/>
The returned XML is invalid:
<smartcard mode='passthrough'>
type='spicevmc'>
<address type='ccid' controller='0' slot='0'/>
</smartcard>
Unfortunately there aren't any xml2xml tests for smartcard bits that would
have caught this...
Thanks,
Cole
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c958bcf6..ead94976d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23047,37 +23047,32 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *mode = virDomainSmartcardTypeToString(def->type);
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
size_t i;
+ virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
+
if (!mode) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected smartcard type %d"), def->type);
return -1;
}
- virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
- virBufferAdjustIndent(buf, 2);
switch (def->type) {
case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
- if (!virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "/>\n");
- return 0;
- }
- virBufferAddLit(buf, ">\n");
break;
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
- virBufferAddLit(buf, ">\n");
- for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++)
- virBufferEscapeString(buf,
"<certificate>%s</certificate>\n",
+ for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
+ virBufferEscapeString(&childBuf,
"<certificate>%s</certificate>\n",
def->data.cert.file[i]);
- virBufferEscapeString(buf, "<database>%s</database>\n",
+ }
+ virBufferEscapeString(&childBuf,
"<database>%s</database>\n",
def->data.cert.database);
break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
- if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
+ if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false,
flags) < 0)
return -1;
break;
@@ -23087,10 +23082,22 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
_("unexpected smartcard type %d"), def->type);
return -1;
}
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
+ virBufferFreeAndReset(&childBuf);
return -1;
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</smartcard>\n");
+ }
+
+ if (virBufferCheckError(&childBuf) < 0)
+ return -1;
+
+ virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
+ if (virBufferUse(&childBuf)) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAddBuffer(buf, &childBuf);
+ virBufferAddLit(buf, "</smartcard>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
return 0;
}
7:57 a.m.
New subject: [libvirt] [PATCH 3/8] Use a separate buffer for <smartcard> subelements
On Wed, Aug 02, 2017 at 01:12:15PM -0400, Cole Robinson wrote:
On 07/26/2017 09:29 AM, Ján Tomko wrote:
> Convert virDomainSmartcardDefFormat to use a separate buffer
> for possible subelements, to avoid the need for duplicated
> formatting logic in virDomainDeviceInfoNeedsFormat.
> ---
> src/conf/domain_conf.c | 39 +++++++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
Looks like this patch causes a regression, currently breaking the virt-manager
test suite that danpb pointed out to me. Edit an existing VM and add
<smartcard mode='passthrough' type='spicevmc'/>
The returned XML is invalid:
<smartcard mode='passthrough'>
type='spicevmc'>
<address type='ccid' controller='0' slot='0'/>
</smartcard>
Unfortunately there aren't any xml2xml tests for smartcard bits that would
have caught this...
Thanks, I have sent a fix:
https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html
Jan
7:59 a.m.
New subject: [libvirt] [PATCH 3/8] Use a separate buffer for <smartcard> subelements
On 08/03/2017 08:57 AM, Ján Tomko wrote:
On Wed, Aug 02, 2017 at 01:12:15PM -0400, Cole Robinson wrote:
> On 07/26/2017 09:29 AM, Ján Tomko wrote:
>> Convert virDomainSmartcardDefFormat to use a separate buffer
>> for possible subelements, to avoid the need for duplicated
>> formatting logic in virDomainDeviceInfoNeedsFormat.
>> ---
>> src/conf/domain_conf.c | 39 +++++++++++++++++++++++----------------
>> 1 file changed, 23 insertions(+), 16 deletions(-)
>>
>
> Looks like this patch causes a regression, currently breaking the virt-manager
> test suite that danpb pointed out to me. Edit an existing VM and add
>
> <smartcard mode='passthrough' type='spicevmc'/>
>
> The returned XML is invalid:
>
> <smartcard mode='passthrough'>
> type='spicevmc'>
> <address type='ccid' controller='0' slot='0'/>
> </smartcard>
>
> Unfortunately there aren't any xml2xml tests for smartcard bits that would
> have caught this...
>
Thanks, I have sent a fix:
https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html
John sent patches too:
https://www.redhat.com/archives/libvir-list/2017-August/msg00101.html
- Cole
8:29 a.m.
New subject: [libvirt] [PATCH 4/8] Use a separate buffer for <sound> subelements
Convert virDomainSoundDefFormat to use a separate buffer
for subelements.
---
src/conf/domain_conf.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ead94976d..6c4f2f398 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23158,38 +23158,32 @@ virDomainSoundDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *model = virDomainSoundModelTypeToString(def->model);
- bool children = false;
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
size_t i;
+ virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
+
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected sound model %d"), def->model);
return -1;
}
- virBufferAsprintf(buf, "<sound model='%s'", model);
+ for (i = 0; i < def->ncodecs; i++)
+ virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
- for (i = 0; i < def->ncodecs; i++) {
- if (!children) {
- virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 2);
- children = true;
- }
- virDomainSoundCodecDefFormat(buf, def->codecs[i]);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
+ virBufferFreeAndReset(&childBuf);
+ return -1;
}
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
- if (!children) {
- virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 2);
- children = true;
- }
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- }
+ if (virBufferCheckError(&childBuf) < 0)
+ return -1;
- if (children) {
- virBufferAdjustIndent(buf, -2);
+ virBufferAsprintf(buf, "<sound model='%s'", model);
+ if (virBufferUse(&childBuf)) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAddBuffer(buf, &childBuf);
virBufferAddLit(buf, "</sound>\n");
} else {
virBufferAddLit(buf, "/>\n");
--
2.13.0
3:46 p.m.
New subject: [libvirt] [PATCH 4/8] Use a separate buffer for <sound> subelements
On 07/26/2017 09:29 AM, Ján Tomko wrote:
Convert virDomainSoundDefFormat to use a separate buffer
for subelements.
---
src/conf/domain_conf.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
8:29 a.m.
New subject: [libvirt] [PATCH 5/8] Use a separate buffer for <watchdog> subelements
Convert virDomainWatchdogDefFormat to use a separate
buffer for subelements.
---
src/conf/domain_conf.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c4f2f398..fdac0e0ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23275,6 +23275,9 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
{
const char *model = virDomainWatchdogModelTypeToString(def->model);
const char *action = virDomainWatchdogActionTypeToString(def->action);
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
if (!model) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -23288,15 +23291,18 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
return -1;
}
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ return -1;
+
+ if (virBufferCheckError(&childBuf) < 0)
+ return -1;
+
virBufferAsprintf(buf, "<watchdog model='%s'
action='%s'",
model, action);
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
+ if (virBufferUse(&childBuf)) {
virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 2);
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- virBufferAdjustIndent(buf, -2);
+ virBufferAddBuffer(buf, &childBuf);
virBufferAddLit(buf, "</watchdog>\n");
} else {
virBufferAddLit(buf, "/>\n");
--
2.13.0
3:46 p.m.
New subject: [libvirt] [PATCH 5/8] Use a separate buffer for <watchdog> subelements
On 07/26/2017 09:29 AM, Ján Tomko wrote:
Convert virDomainWatchdogDefFormat to use a separate
buffer for subelements.
---
src/conf/domain_conf.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
8:29 a.m.
New subject: [libvirt] [PATCH 6/8] Use a separate buffer for <hub> subelements
Switch virDomainHubDefFormat to use a separate buffer for subelements.
---
src/conf/domain_conf.c | 34 +++++++++++-----------------------
1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fdac0e0ba..7728321cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3529,23 +3529,6 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
return NULL;
}
-static bool
-virDomainDeviceInfoNeedsFormat(virDomainDeviceInfoPtr info, unsigned int flags)
-{
- if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
- return true;
- if (info->alias && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
- return true;
- if (info->mastertype != VIR_DOMAIN_CONTROLLER_MASTER_NONE)
- return true;
- if ((info->rombar != VIR_TRISTATE_SWITCH_ABSENT) ||
- info->romfile)
- return true;
- if (info->bootIndex)
- return true;
- return false;
-}
-
static int
virDomainDefHasDeviceAddressIterator(virDomainDefPtr def ATTRIBUTE_UNUSED,
@@ -24304,6 +24287,9 @@ virDomainHubDefFormat(virBufferPtr buf,
unsigned int flags)
{
const char *type = virDomainHubTypeToString(def->type);
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2);
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -24311,14 +24297,16 @@ virDomainHubDefFormat(virBufferPtr buf,
return -1;
}
- virBufferAsprintf(buf, "<hub type='%s'", type);
+ if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
+ return -1;
+
+ if (virBufferCheckError(&childBuf) < 0)
+ return -1;
- if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
+ virBufferAsprintf(buf, "<hub type='%s'", type);
+ if (virBufferUse(&childBuf)) {
virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 2);
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
- virBufferAdjustIndent(buf, -2);
+ virBufferAddBuffer(buf, &childBuf);
virBufferAddLit(buf, "</hub>\n");
} else {
virBufferAddLit(buf, "/>\n");
--
2.13.0
3:48 p.m.
New subject: [libvirt] [PATCH 6/8] Use a separate buffer for <hub> subelements
On 07/26/2017 09:29 AM, Ján Tomko wrote:
Switch virDomainHubDefFormat to use a separate buffer for
subelements.
Could also mention the removal of the InfoNeedsFormat
---
src/conf/domain_conf.c | 34 +++++++++++-----------------------
1 file changed, 11 insertions(+), 23 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
8:29 a.m.
New subject: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse
After an OOM error, virBuffer* APIs set buf->use to zero.
Adding a buffer to the parent buffer only if use is non-zero
would quitely drop data on error.
Check the error beforehand to make sure buf->use is zero
because we have not attempted to add anything to it.
---
src/conf/capabilities.c | 5 +++++
src/conf/cpu_conf.c | 4 ++++
src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++++--
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0f99f3096..db7efffdf 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -930,6 +930,11 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
bank->controls[j]->max_allocation);
}
+ if (virBufferCheckError(&controlBuf) < 0) {
+ VIR_FREE(cpus_str);
+ return -1;
+ }
+
if (virBufferUse(&controlBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &controlBuf);
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index da40e9ba9..065b4df99 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -646,6 +646,10 @@ virCPUDefFormatBufFull(virBufferPtr buf,
if (virDomainNumaDefCPUFormat(&childrenBuf, numa) < 0)
goto cleanup;
+ if (virBufferCheckError(&attributeBuf) < 0 ||
+ virBufferCheckError(&childrenBuf) < 0)
+ goto cleanup;
+
/* Put it all together */
if (virBufferUse(&attributeBuf) || virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, "<cpu");
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7728321cb..4dc49fdb0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21560,6 +21560,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
+ if (virBufferCheckError(&driverBuf) < 0)
+ return -1;
+
if (virBufferUse(&driverBuf)) {
virBufferAddLit(buf, "<driver");
virBufferAddBuffer(buf, &driverBuf);
@@ -21744,7 +21747,7 @@ virDomainControllerDriverFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
- if (virBufferUse(&driverBuf)) {
+ if (virBufferError(&driverBuf) != 0 || virBufferUse(&driverBuf)) {
virBufferAddLit(buf, "<driver");
virBufferAddBuffer(buf, &driverBuf);
virBufferAddLit(buf, "/>\n");
@@ -21891,6 +21894,9 @@ virDomainControllerDefFormat(virBufferPtr buf,
"pcihole64>\n",
def->opts.pciopts.pcihole64size);
}
+ if (virBufferCheckError(&childBuf) < 0)
+ return -1;
+
if (virBufferUse(&childBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &childBuf);
@@ -21962,6 +21968,9 @@ virDomainFSDefFormat(virBufferPtr buf,
virDomainVirtioOptionsFormat(&driverBuf, def->virtio);
+ if (virBufferCheckError(&driverBuf) < 0)
+ return -1;
+
if (virBufferUse(&driverBuf)) {
virBufferAddLit(buf, "<driver");
virBufferAddBuffer(buf, &driverBuf);
@@ -23223,6 +23232,9 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
}
}
+ if (virBufferCheckError(&childrenBuf) < 0)
+ return -1;
+
if (!virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, "/>\n");
} else {
@@ -23309,6 +23321,10 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
virBufferAdjustIndent(&childrenBuf, indent + 2);
if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
return -1;
+
+ if (virBufferCheckError(&childrenBuf) < 0)
+ return -1;
+
if (virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &childrenBuf);
@@ -23655,6 +23671,9 @@ virDomainInputDefFormat(virBufferPtr buf,
if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0)
return -1;
+ if (virBufferCheckError(&childbuf) < 0)
+ return -1;
+
if (!virBufferUse(&childbuf)) {
virBufferAddLit(buf, "/>\n");
} else {
@@ -24596,6 +24615,9 @@ virDomainCputuneDefFormat(virBufferPtr buf,
def->iothreadids[i]->iothread_id);
}
+ if (virBufferCheckError(&childrenBuf) < 0)
+ return -1;
+
if (virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, "<cputune>\n");
virBufferAddBuffer(buf, &childrenBuf);
@@ -24709,7 +24731,8 @@ virDomainIOMMUDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<iommu model='%s'",
virDomainIOMMUModelTypeToString(iommu->model));
- if (virBufferUse(&childBuf)) {
+
+ if (virBufferError(&childBuf) != 0 || virBufferUse(&childBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &childBuf);
virBufferAddLit(buf, "</iommu>\n");
@@ -24847,6 +24870,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAdjustIndent(&childrenBuf, -2);
virBufferAddLit(&childrenBuf, "</device>\n");
}
+
+ if (virBufferCheckError(&childrenBuf) < 0)
+ goto error;
+
if (virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, "<blkiotune>\n");
virBufferAddBuffer(buf, &childrenBuf);
--
2.13.0
4:45 p.m.
New subject: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse
On 07/26/2017 09:29 AM, Ján Tomko wrote:
After an OOM error, virBuffer* APIs set buf->use to zero.
Adding a buffer to the parent buffer only if use is non-zero
would quitely drop data on error.
s/quitely/quietly/
Check the error beforehand to make sure buf->use is zero
because we have not attempted to add anything to it.
---
src/conf/capabilities.c | 5 +++++
src/conf/cpu_conf.c | 4 ++++
src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++++--
3 files changed, 38 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
One nit (feel free to ignore) and one concern follows...
John
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0f99f3096..db7efffdf 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -930,6 +930,11 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
bank->controls[j]->max_allocation);
}
One could move the VIR_FREE(cpus_str) to right here and avoid the two
It also seems this particular function uses virBufferAdjustIndent on buf
instead of on the child buf (or in this case controlBuf)...
+ if (virBufferCheckError(&controlBuf) < 0) {
+ VIR_FREE(cpus_str);
+ return -1;
+ }
+
if (virBufferUse(&controlBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &controlBuf);
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7728321cb..4dc49fdb0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
[...]
@@ -23309,6 +23321,10 @@ static int
virDomainPanicDefFormat(virBufferPtr buf,
virBufferAdjustIndent(&childrenBuf, indent + 2);
if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
return -1;
+
+ if (virBufferCheckError(&childrenBuf) < 0)
+ return -1;
+
Seeing a virBufferFreeAndReset below this if condition first had me
thinking, well that's unnecessary; however, in actuality I think we have
a leak any time virBufferUse doesn't return a non zero value call
virBufferAddBuffer to consume the buffer.
Not necessarily something to stop this patch, but perhaps a leak for:
virDomainDiskDefFormat
virDomainControllerDriverFormat
virDomainControllerDefFormat
virDomainFSDefFormat
virDomainMemballoonDefFormat
virDomainRNGDefFormat
virDomainVideoDefFormat
virDomainInputDefFormat
virDomainIOMMUDefFormat
virDomainDiskDefFormat
if (virBufferUse(&childrenBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &childrenBuf);
@@ -23655,6 +23671,9 @@ virDomainInputDefFormat(virBufferPtr buf,
[...]
8:55 a.m.
New subject: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse
On Tue, Aug 01, 2017 at 05:45:10PM -0400, John Ferlan wrote:
[...]
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 0f99f3096..db7efffdf 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -930,6 +930,11 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> bank->controls[j]->max_allocation);
> }
>
One could move the VIR_FREE(cpus_str) to right here and avoid the two
One has pushed that as a separate patch:
https://www.redhat.com/archives/libvir-list/2017-August/msg00088.html
It also seems this particular function uses virBufferAdjustIndent on
buf
instead of on the child buf (or in this case controlBuf)...
While inconsistent, that does not concern me.
> + if (virBufferCheckError(&controlBuf) < 0) {
> + VIR_FREE(cpus_str);
> + return -1;
> + }
> +
> if (virBufferUse(&controlBuf)) {
> virBufferAddLit(buf, ">\n");
> virBufferAddBuffer(buf, &controlBuf);
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7728321cb..4dc49fdb0 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -23309,6 +23321,10 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
> virBufferAdjustIndent(&childrenBuf, indent + 2);
> if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
> return -1;
> +
> + if (virBufferCheckError(&childrenBuf) < 0)
> + return -1;
> +
Seeing a virBufferFreeAndReset below this if condition first had me
thinking, well that's unnecessary; however, in actuality I think we have
a leak any time virBufferUse doesn't return a non zero value call
virBufferAddBuffer to consume the buffer.
I do not see the leak. If we made no attempt at all to use the buffer,
nothing should have been allocated. If we tried to add something to it,
and failed on OOM, virBufferSetError should free the content and set use
to zero. The only possible leak would be when we try to extend the
buffer without actually writing any content to it - but we have no
reason to do that in an XML file, since there's always going to be
at least the element name.
Jan
Not necessarily something to stop this patch, but perhaps a leak for:
virDomainDiskDefFormat
virDomainControllerDriverFormat
virDomainControllerDefFormat
virDomainFSDefFormat
virDomainMemballoonDefFormat
virDomainRNGDefFormat
virDomainVideoDefFormat
virDomainInputDefFormat
virDomainIOMMUDefFormat
virDomainDiskDefFormat
> if (virBufferUse(&childrenBuf)) {
> virBufferAddLit(buf, ">\n");
> virBufferAddBuffer(buf, &childrenBuf);
> @@ -23655,6 +23671,9 @@ virDomainInputDefFormat(virBufferPtr buf,
[...]
9:21 a.m.
New subject: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 7728321cb..4dc49fdb0 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>
> [...]
>
>> @@ -23309,6 +23321,10 @@ static int
>> virDomainPanicDefFormat(virBufferPtr buf,
>> virBufferAdjustIndent(&childrenBuf, indent + 2);
>> if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) <
0)
>> return -1;
>> +
>> + if (virBufferCheckError(&childrenBuf) < 0)
>> + return -1;
>> +
>
> Seeing a virBufferFreeAndReset below this if condition first had me
> thinking, well that's unnecessary; however, in actuality I think we have
> a leak any time virBufferUse doesn't return a non zero value call
> virBufferAddBuffer to consume the buffer.
I do not see the leak. If we made no attempt at all to use the buffer,
nothing should have been allocated. If we tried to add something to it,
and failed on OOM, virBufferSetError should free the content and set use
to zero. The only possible leak would be when we try to extend the
buffer without actually writing any content to it - but we have no
reason to do that in an XML file, since there's always going to be
at least the element name.
Jan
Hmm.. right - I guess it's seeing the FreeAndReset in some places and
not others that got me to thinking about this and of course being
somehow convinced that there could be a leak. Perhaps those places
where FreeAndReset is called unnecessarily could be cleaned up (they're
not wrong, but they do nothing as long as the AddBuffer was used).
John
8:29 a.m.
New subject: [libvirt] [PATCH 8/8] Turn virDomainDeviceInfoFormat into void
The rombar attribute was already validated at the time of parsing
the XML.
---
src/conf/domain_conf.c | 94 ++++++++++++++++----------------------------------
1 file changed, 30 insertions(+), 64 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4dc49fdb0..460776ec6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5332,7 +5332,7 @@ virDomainVirtioOptionsFormat(virBufferPtr buf,
}
-static int ATTRIBUTE_NONNULL(2)
+static void ATTRIBUTE_NONNULL(2)
virDomainDeviceInfoFormat(virBufferPtr buf,
virDomainDeviceInfoPtr info,
unsigned int flags)
@@ -5360,16 +5360,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
virBufferAddLit(buf, "<rom");
if (info->rombar) {
-
const char *rombar = virTristateSwitchTypeToString(info->rombar);
- if (!rombar) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected rom bar value %d"),
- info->rombar);
- return -1;
- }
- virBufferAsprintf(buf, " bar='%s'", rombar);
+ if (rombar)
+ virBufferAsprintf(buf, " bar='%s'", rombar);
}
if (info->romfile)
virBufferEscapeString(buf, " file='%s'",
info->romfile);
@@ -5378,7 +5372,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
- return 0;
+ return;
virBufferAsprintf(buf, "<address type='%s'",
virDomainDeviceAddressTypeToString(info->type));
@@ -5465,7 +5459,6 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
}
virBufferAddLit(buf, "/>\n");
- return 0;
}
static int
@@ -21711,9 +21704,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->src->encryption &&
virStorageEncryptionFormat(buf, def->src->encryption) < 0)
return -1;
- if (virDomainDeviceInfoFormat(buf, &def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</disk>\n");
@@ -21885,8 +21877,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
virDomainControllerDriverFormat(&childBuf, def);
- if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
def->opts.pciopts.pcihole64) {
@@ -22018,9 +22009,7 @@ virDomainFSDefFormat(virBufferPtr buf,
if (def->readonly)
virBufferAddLit(buf, "<readonly/>\n");
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
-
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
if (def->space_hard_limit)
virBufferAsprintf(buf, "<space_hard_limit unit='bytes'>"
@@ -22786,10 +22775,9 @@ virDomainNetDefFormat(virBufferPtr buf,
virDomainNetDefCoalesceFormatXML(buf, def->coalesce);
- if (virDomainDeviceInfoFormat(buf, &def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
- | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
+ | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</interface>\n");
@@ -23024,8 +23012,7 @@ virDomainChrDefFormat(virBufferPtr buf,
break;
}
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAsprintf(buf, "</%s>\n", elementName);
@@ -23074,10 +23061,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
_("unexpected smartcard type %d"), def->type);
return -1;
}
- if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
- virBufferFreeAndReset(&childBuf);
- return -1;
- }
+ virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
return -1;
@@ -23134,8 +23118,7 @@ virDomainTPMDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</backend>\n");
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</tpm>\n");
@@ -23164,10 +23147,7 @@ virDomainSoundDefFormat(virBufferPtr buf,
for (i = 0; i < def->ncodecs; i++)
virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]);
- if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) {
- virBufferFreeAndReset(&childBuf);
- return -1;
- }
+ virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
return -1;
@@ -23211,10 +23191,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
if (def->period)
virBufferAsprintf(&childrenBuf, "<stats
period='%i'/>\n", def->period);
- if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {
- virBufferFreeAndReset(&childrenBuf);
- return -1;
- }
+ virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags);
if (def->virtio) {
virBuffer driverBuf = VIR_BUFFER_INITIALIZER;
@@ -23253,8 +23230,7 @@ virDomainNVRAMDefFormat(virBufferPtr buf,
{
virBufferAddLit(buf, "<nvram>\n");
virBufferAdjustIndent(buf, 2);
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</nvram>\n");
@@ -23286,8 +23262,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf,
return -1;
}
- if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
return -1;
@@ -23319,8 +23294,7 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
virDomainPanicModelTypeToString(def->model));
virBufferAdjustIndent(&childrenBuf, indent + 2);
- if (virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0) < 0)
- return -1;
+ virDomainDeviceInfoFormat(&childrenBuf, &def->info, 0);
if (virBufferCheckError(&childrenBuf) < 0)
return -1;
@@ -23367,8 +23341,7 @@ virDomainShmemDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</shmem>\n");
@@ -23422,8 +23395,7 @@ virDomainRNGDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</rng>\n");
@@ -23541,8 +23513,7 @@ virDomainMemoryDefFormat(virBufferPtr buf,
virDomainMemoryTargetDefFormat(buf, def);
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</memory>\n");
@@ -23618,8 +23589,7 @@ virDomainVideoDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
- if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</video>\n");
@@ -23668,8 +23638,7 @@ virDomainInputDefFormat(virBufferPtr buf,
virBufferAddLit(&childbuf, "/>\n");
}
virBufferEscapeString(&childbuf, "<source
evdev='%s'/>\n", def->source.evdev);
- if (virDomainDeviceInfoFormat(&childbuf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(&childbuf, &def->info, flags);
if (virBufferCheckError(&childbuf) < 0)
return -1;
@@ -24229,10 +24198,9 @@ virDomainHostdevDefFormat(virBufferPtr buf,
if (def->shareable)
virBufferAddLit(buf, "<shareable/>\n");
- if (virDomainDeviceInfoFormat(buf, def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
- | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
+ | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</hostdev>\n");
@@ -24253,9 +24221,8 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
virBufferAdjustIndent(buf, 2);
if (virDomainChrSourceDefFormat(buf, def->source, false, flags) < 0)
return -1;
- if (virDomainDeviceInfoFormat(buf, &def->info,
- flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0)
- return -1;
+ virDomainDeviceInfoFormat(buf, &def->info,
+ flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</redirdev>\n");
return 0;
@@ -24316,8 +24283,7 @@ virDomainHubDefFormat(virBufferPtr buf,
return -1;
}
- if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0)
- return -1;
+ virDomainDeviceInfoFormat(&childBuf, &def->info, flags);
if (virBufferCheckError(&childBuf) < 0)
return -1;
--
2.13.0
4:48 p.m.
New subject: [libvirt] [PATCH 8/8] Turn virDomainDeviceInfoFormat into void
On 07/26/2017 09:29 AM, Ján Tomko wrote:
The rombar attribute was already validated at the time of parsing
the XML.
---
src/conf/domain_conf.c | 94 ++++++++++++++++----------------------------------
1 file changed, 30 insertions(+), 64 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2836
days inactive
2844
days old
21 comments
3 participants
participants (3)
-
Cole Robinson -
John Ferlan -
Ján Tomko