Re: [PATCH] conf: More clear error msg for incomplete coalesce xml

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/14/18 10:26 AM, Han Han wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1535930
> 
> Report more clear err msg instead of unknown error when coalesce
> settings is incomplete.
> 
> Signed-off-by: Han Han <hhan@xxxxxxxxxx>
> ---
>  src/conf/domain_conf.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9911d56130..e755f45d3d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7804,8 +7804,12 @@ virDomainNetDefCoalesceParseXML(xmlNodePtr node,
>      ctxt->node = node;
>  
>      str = virXPathString("string(./rx/frames/@max)", ctxt);
> -    if (!str)
> +    if (!str) {
> +        virReportError(VIR_ERR_XML_DETAIL,
> +                       "%s",

This can be put on the previous line

> +                       _("incomplete coalesce settings in interface xml"));

and specifically this could be is missing rx frames max attributes

However, according to the RNG from commit 523c9960, it seems the 'rx' is
optional as is the '@max' value.  Maybe Martin should provide a comment
on this series since he added it.

Of course that would cause the whole <coalesce> to disappear on Format.
It would also cause problems because def->coalesce would have something
that's empty.

So perhaps the best thing to do is pass the @def into here, then only if
we get beyond the initial !str comparison do we allocate and fill it in;
otherwise, we return 0 if rx/frames/@max is not there.  Prepares us for
the future.

I guess I'm not 100% clear if max frames == 0 what would happen. Maybe
Martin knows (I've CC'd him).

John

>          goto cleanup;
> +    }
>  
>      if (VIR_ALLOC(ret) < 0)
>          goto cleanup;
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux