Re: [libvirt] [PATCH] Ignore listen attribute of <graphics> for type network listens

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.

> 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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list