Re: [PATCH 20/33] Don't report OOM error on xmlCopyNode failure

[Peter Krempa]

On Wed, Feb 24, 2021 at 22:25:56 -0500, Laine Stump wrote:
> On 2/24/21 11:16 AM, Peter Krempa wrote:
> > Out of memory isn't the only reason the function can fail. Add a message
> > stating that copying of a XML node failed.
> 
> Could it still fail due to OOM as well, though? And if so, is there any way
> of differentiating? (I previously looked into this for some other libxml2
> function, ended up asking DV, and I believe I was told that there isn't any
> way to tell the difference, but as usual IMBES (I May Be Experiencing
> Senility).

Yes, it's one of the options.

> At any rate, your changed message is more correct, since it's not making any
> unsubstantiated assumption about the cause of the error, while
> virRemoveOOMError() was).

I tried to formulate them such that they will be true even in OOM case
by making them as vague as the description of the error reported by the
API is.

In case of OOM though, if the OOM condition persists, the logging call
will abort() anyways right when it's called, so in all but the very
rarest cases we'd report the correct error.

Re: [PATCH 20/33] Don't report OOM error on xmlCopyNode failure

[Laine Stump]

On 2/24/21 11:16 AM, Peter Krempa wrote:

Out of memory isn't the only reason the function can fail. Add a message
stating that copying of a XML node failed.


Could it still fail due to OOM as well, though? And if so, is there any 
way of differentiating? (I previously looked into this for some other 
libxml2 function, ended up asking DV, and I believe I was told that 
there isn't any way to tell the difference, but as usual IMBES (I May Be 
Experiencing Senility).


At any rate, your changed message is more correct, since it's not making 
any unsubstantiated assumption about the cause of the error, while 
virRemoveOOMError() was).





Signed-off-by: Peter Krempa 
---
  src/conf/domain_conf.c | 3 ++-
  src/test/test_driver.c | 6 --
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c0881608af..f1fd5320a5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30460,7 +30460,8 @@ virDomainDefSetMetadata(virDomainDefPtr def,
  def->metadata = virXMLNewNode(NULL, "metadata");

  if (!(new = xmlCopyNode(doc->children, 1))) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Failed to copy XML node"));
  return -1;
  }
  }
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index bca1297d1d..71ab04aa1a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -224,7 +224,8 @@ testDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
  for (i = 0; i < n; i++) {
  xmlNodePtr newnode = xmlCopyNode(nodes[i], 1);
  if (!newnode) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Failed to copy XML node"));
  goto error;
  }

@@ -787,7 +788,8 @@ testParseXMLDocFromFile(xmlNodePtr node, const char *file, 
const char *type)

  ret = xmlCopyNode(xmlDocGetRootElement(doc), 1);
  if (!ret) {
-virReportOOMError();
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Failed to copy XML node"));
  goto error;
  }
  xmlReplaceNode(node, ret);