On 06/22/2016 01:37 PM, Laine Stump wrote:
> Rearrange this function to be better organized and more correct:
>
> * the error codes were changed from the incorrect INVALID_ARG to
> XML_ERROR
>
> * prefix still isn't required, but if present it must be valid or an
> error will be logged.
>
> * don't emit a debug log just because prefix is missing - this
> is valid.
>
> * group everything related to setting prefix in one place rather than
> scattered through the function.
> ---
> src/conf/domain_conf.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e57655e..c5b4815 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6115,15 +6115,9 @@ virDomainNetIPParseXML(xmlNodePtr node)
> int family = AF_UNSPEC;
> char *address = NULL;
>
> - if (!(prefixStr = virXMLPropString(node, "prefix")) ||
> - (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) {
> - // Don't shout, as some old config may not have a prefix
> - VIR_DEBUG("Missing or invalid network prefix");
> - }
> -
> if (!(address = virXMLPropString(node, "address"))) {
> - virReportError(VIR_ERR_INVALID_ARG, "%s",
> - _("Missing network address"));
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing required address in <ip>"));
> goto cleanup;
> }
>
> @@ -6139,11 +6133,22 @@ virDomainNetIPParseXML(xmlNodePtr node)
> goto cleanup;
>
> if (virSocketAddrParse(&ip->address, address, family) < 0) {
> - virReportError(VIR_ERR_INVALID_ARG,
> - _("Failed to parse IP address: '%s'"),
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid address '%s' in <ip>"),
> address);
> goto cleanup;
> }
> +
> + prefixStr = virXMLPropString(node, "prefix");
> + if (prefixStr &&
> + ((virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0) ||
> + (family == AF_INET6 && prefixValue > 128) ||
> + (family == AF_INET && prefixValue > 32))) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Invalid prefix value '%s' in <ip>"),
> + prefixStr);
> + goto cleanup;
> + }
I fear the answer to this question, but I'll ask it... Can a domain that
running today with an incorrect prefixValue?
If I'm reading things correct, it would have been assigned a value of 0,
but could still be running. On libvirtd restart with this change could
that domain 'disappear' because of this "parse" error.
ACK in principle, but you may need to move that check to the "Validate"
callback code path depending on how you answer the question.
John
> ip->prefix = prefixValue;
>
> ret = ip;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list