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@xxxxxxxxxx> > --- > 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list