Re: [libvirt] [PATCH] Add domain type checking

On 07/08/2011 02:13 AM, Matthias Bolte wrote: > The drivers were accepting domain configs without checking if those > were actually meant for them. For example the LXC driver happily > accepts configs with type QEMU. > > For convenience add an optional check for the domain type for the > virDomainDefParse* functions. It's optional because in some places > virDomainDefParse* is used to parse a config without caring about > it's type. Also the QEMU driver has to accept 4 different types and > does this checking own it's own. Can we factor the 4 QEMU types back into the common method?  Do this by treating expectedType as a bitmask: > +++ b/src/conf/domain_conf.c > @@ -1257,7 +1257,7 @@ virDomainObjSetDefTransient(virCapsPtr caps, >      if (!(xml = virDomainDefFormat(domain->def, VIR_DOMAIN_XML_WRITE_FLAGS))) >          goto out; > > -    if (!(newDef = virDomainDefParseString(caps, xml, > +    if (!(newDef = virDomainDefParseString(caps, xml, -1, >                                             VIR_DOMAIN_XML_READ_FLAGS))) -1 being the bitmask to accept all possible types, > @@ -5796,6 +5797,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, >      } >      VIR_FREE(tmp); > > +    if (expectedVirtType >= 0 && def->virtType != expectedVirtType) { here, change the logic to: if (((1 << def->virtType) & expectedVirtTypes) == 0) { > +        virDomainReportError(VIR_ERR_INTERNAL_ERROR, > +                             _("unexpected domain type %s, should be %s"), > +                             virDomainVirtTypeToString(def->virtType), > +                             virDomainVirtTypeToString(expectedVirtType)); > +        goto error; > +    } > + >      /* Extract domain name */ >      if (!(def->name = virXPathString("string(./name[1])", ctxt))) { >          virDomainReportError(VIR_ERR_NO_NAME, NULL); > @@ -6704,6 +6713,7 @@ no_memory: >  static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, >                                              xmlDocPtr xml, >                                              xmlXPathContextPtr ctxt, > +                                            int expectedVirtType, make this plural expectedVirtTypes, to reflect that it is a bitmask, > +++ b/src/esx/esx_driver.c > @@ -2855,7 +2855,8 @@ esxDomainXMLToNative(virConnectPtr conn, const char *nativeFormat, >          return NULL; >      } > > -    def = virDomainDefParseString(priv->caps, domainXml, 0); > +    def = virDomainDefParseString(priv->caps, domainXml, > +                                  VIR_DOMAIN_VIRT_VMWARE, 0); here, pass (1 << VIR_DOMAIN_VIRT_VMWARE) instead of VIR_DOMAIN_VIRT_VMWARE > +++ b/src/qemu/qemu_domain.c > @@ -700,10 +700,32 @@ void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver, >      ignore_value(virDomainObjUnref(obj)); >  } > > +virDomainDefPtr qemuDomainDefParseXML(struct qemud_driver *driver, > +                                      const char *xml, > +                                      unsigned int flags) > +{ > +    virDomainDefPtr def; > + > +    if (!(def = virDomainDefParseString(driver->caps, xml, -1, flags))) > +        return NULL; > + > +    if (def->virtType != VIR_DOMAIN_VIRT_QEMU && > +        def->virtType != VIR_DOMAIN_VIRT_KQEMU && > +        def->virtType != VIR_DOMAIN_VIRT_KVM && > +        def->virtType != VIR_DOMAIN_VIRT_XEN) { Then here, instead of having qemu parse things, you instead pass: ((1 << VIR_DOMAIN_VIRT_QEMU) |  (1 << VIR_DOMAIN_VIRT_KQEMU) |  (1 << VIR_DOMAIN_VIRT_KVM) |  (1 << VIR_DOMAIN_VIRT_XEN)) as the expectedVirtTypes.