[PATCH] qemu: don't raise error upon interface update without <frames/> for <rx/> in coalesce
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
participants (2)
-
Kristina Hanicova -
Martin Kletzander