On Thu, Feb 26, 2015 at 09:57:22AM -0500, Laine Stump wrote:
On 02/26/2015 08:53 AM, Ján Tomko wrote:Commit 6992994 started filling the listen attribute of the parent <graphics> elements from type='network' listens. When this XML is passed to UpdateDevice, parsing fails: XML error: graphics listen attribute 10.20.30.40 must match address attribute of first listen element (found none)Note that the listen attribute of <graphics> won't be filled in if you request the *inactive* xml, and so there will be no error when it is fed back to updatedevice. I can't think of any examples right now, but have a very definite memory that there are several items in the config like this - if you feed the status xml output back to update/define "you're gonna have a bad time". That's why virsh edit asks for the INACTIVE xml. Did you see this when trying to do an update-device manually, or as the result of some management application that has forgotten to add the INACTIVE flag to its request for XML? If the latter, then I guess I can live with ignoring the error in order to preserve backward compatibility with the broken application. Reluctant ACK.
I think we shouldn't fail parsing live XML when we were the ones who generated it. Take a look at device aliases for example. We parse them, but users aren't allowed to set them, so we don't parse them when we are parsing inactive XML. However, if I remember correctly, we must be parsing them when parsing e.g. status XML. So what if this patch is modified so it behaves depending on flags?
Ignore the address in the parent <graphics> attribute when no type='address' listens are found, the same we ignore the address for the <listen> subelements when parsing inactive XML. --- src/conf/domain_conf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d95dd3e..d458195 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9614,12 +9614,16 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, break; } } - if (!matched) { + if (found && !matched) { virReportError(VIR_ERR_XML_ERROR, _("graphics listen attribute %s must match address " "attribute of first listen element (found %s)"), - listenAddr, found ? found : "none"); + listenAddr, found); goto error; + } else if (!found && !matched) {You are guaranteed that if found == NULL, then match == false, so the 2nd term is redundant. (you never get into this code if listenAddr == NULL, so unless found != NULL the comparison that results in setting match = true will never be true).+ /* quietly ignore listen address if none of the listens + * are of type address */ + VIR_FREE(listenAddr); } } }-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgpHIHGKd4xG1.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list