Re: [libvirt] [PATCH] Fix libvirtd restart for domains with PCI passthrough devices

On Fri, Jan 22, 2010 at 11:40:39AM -0500, Chris Lalancette wrote: > When libvirtd shuts down, it places a <state/> tag in the XML > state file it writes out for guests with PCI passthrough > devices. For devices that are attached at bootup time, the > state tag is empty. However, at libvirtd startup time, it > ignores anything with a <state/> tag in the XML, effectively > hiding the guest. > > I can think of at least 3 ways to fix this: > > 1) Don't throw an error on "unknown" tags in > virDomainHostdevSubsysPciDefParseXML(). > 2) Have virDomainLoadAllConfigs() pass the > VIR_DOMAIN_XML_INTERNAL_STATUS flag when parsing the domain > XML. > 3) Remove the check for VIR_DOMAIN_XML_INTERNAL_STATUS > when parsing the XML. > > I chose approach 3). My reasoning for this is that the > <state> tag is a legitimate part of the XML, so we should > always offer to parse it. This fixes the problem with > reconnecting to domains that have PCI passthrough devices. > > Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 74c2337..595c46c 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -2876,7 +2876,7 @@ static int > virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn, > const xmlNodePtr node, > virDomainHostdevDefPtr def, > - int flags) { > + int flags ATTRIBUTE_UNUSED) { > > int ret = -1; > xmlNodePtr cur; > @@ -2890,8 +2890,7 @@ virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn, > > if (virDomainDevicePCIAddressParseXML(conn, cur, addr) < 0) > goto out; > - } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && > - xmlStrEqual(cur->name, BAD_CAST "state")) { > + } else if (xmlStrEqual(cur->name, BAD_CAST "state")) { > /* Legacy back-compat. Don't add any more attributes here */ > char *devaddr = virXMLPropString(cur, "devaddr"); > if (devaddr && I'm not sure, it feels like 2/ is the most specific change possible to this problem, but 3/ makes sense too, I just don't know if there is any side effect. So ACK to this patch but let's keep an eye on testing in the next week,