On 11/07/2014 03:30 PM, Michal Privoznik wrote:
As reviewing patches upstream it occurred to me, that we have two
functions doing nearly the same: virDomainParseMemory which
expects XML in the following format:
<memory unit='MiB'>1337</memory>
The other function being virDomainHugepagesParseXML expecting the
following format:
<someElement memory='1337' unit='MiB'/>
It wouldn't matter to have two functions handle two different
scenarios like this if we could only not copy code that handles
32bit arches around. So this code merges the common parts into
one by inventing new @units_xpath argument to
virDomainParseMemory which allows overriding the default location
of @unit attribute in XML. With this change both scenarios above
can be parsed with virDomainParseMemory. Sweet. Of course,
everything is commented as it should be.
There's no need to commend yourself for your comments in the commit message :)
See below for a few nit picks.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 137 +++++++++++++++++++++++++++++--------------------
1 file changed, 80 insertions(+), 57 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 21309b0..e097af7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6318,14 +6318,30 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto cleanup;
}
-
-/* Parse a value located at XPATH within CTXT, and store the
- * result into val. If REQUIRED, then the value must exist;
- * otherwise, the value is optional. The value is in bytes.
- * Return 1 on success, 0 if the value was not present and
- * is not REQUIRED, -1 on failure after issuing error. */
+/**
+ * virDomainParseScaledValue:
+ * @bytes_xpath: XPath to memory amount
+ * @units_xpath: XPath to units attribute
+ * @ctxt: XPath context
+ * @val: scaled value is stored here
+ * @scale: default scale for @val
+ * @max: maximal @val allowed
+ * @required: is the value required?
+ *
+ * Parse a value located at @bytes_xpath within @ctxt, and store
+ * the result into @val.
By default, if @units_xpath is NULL the
+ * unit attribute must be an attribute to @bytes_xpath.
The attribute doesn't have to be present.
It would also be nice to mention the scaling:
The value is scaled by units located at @units_xpath (or the 'unit' attribute
under @bytes_xpath if @units_xpath is NULL). If units are not present,
the default @scale is used.
+ * Otherwise, the unit attribute is looked for under specified
+ * path.
This sentence seems redundant.
If @required is set, then the value must exist;
+ * otherwise, the value is optional. The value is in bytes.
To distinguish it from the input value (although their existences are linked
together):
The resulting value is in bytes.
Jan