src/conf/domain_conf.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)
With this, incomplete XML without <frames/> for <rx/> in coalesce
won't raise error as before. It will leave the coalesce parameter
empty, thanks to passing it as a parameter and return an integer
to indicate error state - previously it returned pointer (or NULL
for both error and incomplete XML).
The code went through some refactoring:
* change of a condition
* addition of a parameter
* change of order, that allowed removal of VIR_FREE
* removal of redundant labels and variables
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1535930
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
src/conf/domain_conf.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b731744f04..798594f520 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7516,11 +7516,11 @@ virDomainNetIPInfoParseXML(const char *source,
}
-static virNetDevCoalescePtr
+static int
virDomainNetDefCoalesceParseXML(xmlNodePtr node,
- xmlXPathContextPtr ctxt)
+ xmlXPathContextPtr ctxt,
+ virNetDevCoalescePtr *coalesce)
{
- virNetDevCoalescePtr ret = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
unsigned long long tmp = 0;
g_autofree char *str = NULL;
@@ -7529,15 +7529,13 @@ virDomainNetDefCoalesceParseXML(xmlNodePtr node,
str = virXPathString("string(./rx/frames/@max)", ctxt);
if (!str)
- return NULL;
-
- ret = g_new0(virNetDevCoalesce, 1);
+ return 0;
if (virStrToLong_ullp(str, NULL, 10, &tmp) < 0) {
virReportError(VIR_ERR_XML_DETAIL,
_("cannot parse value '%s' for coalesce parameter"),
str);
- goto error;
+ return -1;
}
if (tmp > UINT32_MAX) {
@@ -7545,15 +7543,13 @@ virDomainNetDefCoalesceParseXML(xmlNodePtr node,
_("value '%llu' is too big for coalesce "
"parameter, maximum is '%lu'"),
tmp, (unsigned long) UINT32_MAX);
- goto error;
+ return -1;
}
- ret->rx_max_coalesced_frames = tmp;
- return ret;
+ *coalesce = g_new0(virNetDevCoalesce, 1);
+ (*coalesce)->rx_max_coalesced_frames = tmp;
- error:
- VIR_FREE(ret);
- return NULL;
+ return 0;
}
static void
@@ -11517,8 +11513,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
node = virXPathNode("./coalesce", ctxt);
if (node) {
- def->coalesce = virDomainNetDefCoalesceParseXML(node, ctxt);
- if (!def->coalesce)
+ if (virDomainNetDefCoalesceParseXML(node, ctxt, &def->coalesce) < 0)
goto error;
}
--
2.29.2
On Thu, Mar 04, 2021 at 01:58:17PM +0100, Kristina Hanicova wrote: >With this, incomplete XML without <frames/> for <rx/> in coalesce >won't raise error as before. It will leave the coalesce parameter >empty, thanks to passing it as a parameter and return an integer >to indicate error state - previously it returned pointer (or NULL >for both error and incomplete XML). > >The code went through some refactoring: >* change of a condition >* addition of a parameter >* change of order, that allowed removal of VIR_FREE >* removal of redundant labels and variables > >Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1535930 >Signed-off-by: Kristina Hanicova <khanicov@redhat.com> >--- > src/conf/domain_conf.c | 25 ++++++++++--------------- > 1 file changed, 10 insertions(+), 15 deletions(-) > The code is good, but it could use some test(s). I guess you have couple of options here: - just show that parsing it does nothing in simple qemuxml2xmltest - make sure that this makes it possible to remove the coalesce settings in qemuhotplugtest. This might not be the case and it might result in more patches because, honestly, I am not 100% sure how to handle removal of coalesce parameters versus not touching them on update. Since this is not a critical thing to do, I'll leave that up to you to decide how to approach this ;) Thanks, Martin
© 2016 - 2024 Red Hat, Inc.