[libvirt] [PATCHv4 1/9] conf: Add post XML parse callbacks and prepare for cleaning of virCaps

Laine Stump laine at laine.org
Mon Mar 25 20:25:54 UTC 2013


On 03/15/2013 11:26 AM, Peter Krempa wrote:
> This patch adds instrumentation that will allow hypervisor drivers to
> fill and validate domain and device definitions after parsed by the XML
> parser.
>
> With this patch, after the XML is parsed, a callback to the driver is
> issued requesing to fill and validate driver specific details of the
> configuration. This allows to use sensible defaults and checks on a per
> driver basis at the time the XML is parsed.
>
> Two callback pointers are stored in the new virDomainXMLConf object:
> * virDomainDeviceDefPostParseCallback (devicesConfCallback)
>   - called for a single device parsed and for every single device in a
>     domain config. A virDomainDeviceDefPtr is passed along with the
>     domain definition and virCaps.
>
> * virDomainDefPostParseCallback, (domainConfCallback)
>   - A callback that is meant to process the domain config after it's
>   parsed.  A virDomainDefPtr is passed along with virCaps.
>
> Both types of callbacks support arbitrary opaque data passed for the
> callback functions.
>
> Errors may be reported in those callbacks resulting in a XML parsing
> failure.
> ---
>
> Notes:
>     Version 4:
>     - added support for opaque data for the callback
>     - removed post-devices domain config callback until it's needed
>     - renamed the structure holding the data as it will also contain some defaults as values
>     - squashed patch adding the new argument to the contstructor
>
>  src/conf/domain_conf.c           | 101 +++++++++++++++++++++++++++++++++++++--
>  src/conf/domain_conf.h           |  27 +++++++++--
>  src/esx/esx_driver.c             |   2 +-
>  src/libxl/libxl_driver.c         |   9 ++--
>  src/lxc/lxc_conf.c               |   4 +-
>  src/lxc/lxc_driver.c             |   6 ++-
>  src/openvz/openvz_conf.c         |   1 +
>  src/openvz/openvz_driver.c       |   6 +--
>  src/parallels/parallels_driver.c |   2 +-
>  src/phyp/phyp_driver.c           |   6 +--
>  src/qemu/qemu_conf.c             |   3 +-
>  src/qemu/qemu_driver.c           |  11 +++--
>  src/security/virt-aa-helper.c    |   2 +-
>  src/test/test_driver.c           |   2 +-
>  src/uml/uml_driver.c             |   7 ++-
>  src/vbox/vbox_tmpl.c             |  10 ++--
>  src/vmware/vmware_driver.c       |   2 +-
>  src/xen/xen_driver.c             |   2 +-
>  src/xen/xend_internal.c          |   6 +--
>  src/xen/xm_internal.c            |   2 +
>  src/xenapi/xenapi_driver.c       |   2 +-
>  tests/testutilsxen.c             |   2 +-
>  tests/xml2vmxtest.c              |   2 +-
>  23 files changed, 173 insertions(+), 44 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3278e9c..a1b634b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -73,6 +73,9 @@ struct _virDomainObjList {
>  struct _virDomainXMLConf {
>      virObject parent;
>
> +    /* XML parser callbacks and defaults */
> +    virDomainDefParserConfig config;
> +
>      /* domain private data management callbacks */
>      virDomainXMLPrivateDataCallbacks privateData;
>
> @@ -732,6 +735,7 @@ static virClassPtr virDomainObjListClass;
>  static virClassPtr virDomainXMLConfClass;
>  static void virDomainObjDispose(void *obj);
>  static void virDomainObjListDispose(void *obj);
> +static void virDomainXMLConfClassDispose(void *obj);
>
>  static int virDomainObjOnceInit(void)
>  {
> @@ -750,7 +754,7 @@ static int virDomainObjOnceInit(void)
>      if (!(virDomainXMLConfClass = virClassNew(virClassForObject(),
>                                                "virDomainXMLConf",
>                                                sizeof(virDomainXMLConf),
> -                                              NULL)))
> +                                              virDomainXMLConfClassDispose)))
>          return -1;
>
>      return 0;
> @@ -759,13 +763,24 @@ static int virDomainObjOnceInit(void)
>  VIR_ONCE_GLOBAL_INIT(virDomainObj)
>
>
> +static void
> +virDomainXMLConfClassDispose(void *obj)
> +{
> +    virDomainXMLConfPtr xmlconf = obj;
> +
> +    if (xmlconf->config.privFree)
> +        (xmlconf->config.privFree)(xmlconf->config.priv);
> +}
> +
> +
>  /**
>   * virDomainXMLConfNew:
>   *
>   * Allocate a new domain XML configuration
>   */
>  virDomainXMLConfPtr
> -virDomainXMLConfNew(virDomainXMLPrivateDataCallbacksPtr priv,
> +virDomainXMLConfNew(virDomainDefParserConfigPtr config,
> +                    virDomainXMLPrivateDataCallbacksPtr priv,
>                      virDomainXMLNamespacePtr xmlns)
>  {
>      virDomainXMLConfPtr xmlconf;
> @@ -779,6 +794,9 @@ virDomainXMLConfNew(virDomainXMLPrivateDataCallbacksPtr priv,
>      if (priv)
>          xmlconf->privateData = *priv;
>
> +    if (config)
> +        xmlconf->config = *config;
> +
>      if (xmlns)
>          xmlconf->ns = *xmlns;
>
> @@ -2469,6 +2487,73 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
>  }
>
>
> +static int
> +virDomainDeviceDefPostParse(virDomainXMLConfPtr xmlconf,
> +                            virDomainDeviceDefPtr dev,
> +                            virDomainDefPtr def,
> +                            virCapsPtr caps)
> +{
> +    int ret;
> +
> +    if (xmlconf && xmlconf->config.devicesConfigCallback) {
> +        ret = xmlconf->config.devicesConfigCallback(dev, def, caps,
> +                                                    xmlconf->config.priv);

The implementation of these functions in the hypervisor drivers has been
changed to xxxPostParsexxx, but you're still calling it
devicesConfigCallback in the struct that's setup. I think this struct
should have the members called xxxPostParsexxx as well. This is
especially confusing because the parser is parsing a domain config (so
we have the "config for the parser of the config" - it makes it a bit
difficult to follow whether we're talking about the domain config or
parser config sometimes.).

> +        if (ret < 0)
> +            return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +struct virDomainDefPostParseDeviceIteratorData {
> +    virCapsPtr caps;
> +    virDomainDefPtr def;
> +    virDomainXMLConfPtr xmlconf;
> +};
> +
> +
> +static int
> +virDomainDefPostParseDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED,
> +                                    virDomainDeviceDefPtr dev,
> +                                    virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED,
> +                                    void *opaque)
> +{
> +    struct virDomainDefPostParseDeviceIteratorData *data = opaque;
> +    return virDomainDeviceDefPostParse(data->xmlconf, dev, data->def, data->caps);
> +}
> +
> +
> +static int
> +virDomainDefPostParse(virDomainXMLConfPtr xmlconf,
> +                      virDomainDefPtr def,
> +                      virCapsPtr caps)
> +{
> +    int ret;
> +    struct virDomainDefPostParseDeviceIteratorData data = {
> +        .caps = caps,
> +        .def = def,
> +        .xmlconf = xmlconf,
> +    };
> +
> +    /* call the domain config callback */
> +    if (xmlconf && xmlconf->config.domainConfigCallback) {
> +        ret = xmlconf->config.domainConfigCallback(def, caps,
> +                                                   xmlconf->config.priv);
> +        if (ret < 0)
> +            return ret;
> +    }
> +
> +    /* iterate the devices */
> +    if ((ret = virDomainDeviceInfoIterate(def,
> +                                          virDomainDefPostParseDeviceIterator,
> +                                          &data)) < 0)
> +        return ret;
> +
> +    return 0;
> +}
> +
> +
>  void virDomainDefClearPCIAddresses(virDomainDefPtr def)
>  {
>      virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearPCIAddress, NULL);
> @@ -8385,6 +8470,7 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt,
>
>  virDomainDeviceDefPtr
>  virDomainDeviceDefParse(virCapsPtr caps,
> +                        virDomainXMLConfPtr xmlconf,
>                          virDomainDefPtr def,
>                          const char *xmlStr,
>                          unsigned int flags)
> @@ -8471,6 +8557,10 @@ virDomainDeviceDefParse(virCapsPtr caps,
>          goto error;
>      }
>
> +    /* callback to fill driver specific device aspects */
> +    if (virDomainDeviceDefPostParse(xmlconf, dev, def,  caps) < 0)
> +        goto error;
> +
>  cleanup:
>      xmlFreeDoc(xml);
>      xmlXPathFreeContext(ctxt);
> @@ -10992,6 +11082,10 @@ virDomainDefParseXML(virCapsPtr caps,
>      if (virDomainDefAddImplicitControllers(def) < 0)
>          goto error;
>
> +    /* callback to fill driver specific domain aspects */
> +    if (virDomainDefPostParse(xmlconf, def, caps) < 0)
> +        goto error;
> +
>      virBitmapFree(bootMap);
>
>      return def;
> @@ -16387,6 +16481,7 @@ virDomainNetFind(virDomainDefPtr def, const char *device)
>   */
>  virDomainDeviceDefPtr
>  virDomainDeviceDefCopy(virCapsPtr caps,
> +                       virDomainXMLConfPtr xmlconf,
>                         const virDomainDefPtr def,
>                         virDomainDeviceDefPtr src)
>  {
> @@ -16455,7 +16550,7 @@ virDomainDeviceDefCopy(virCapsPtr caps,
>          goto cleanup;
>
>      xmlStr = virBufferContentAndReset(&buf);
> -    ret = virDomainDeviceDefParse(caps, def, xmlStr, flags);
> +    ret = virDomainDeviceDefParse(caps, xmlconf, def, xmlStr, flags);
>
>  cleanup:
>      VIR_FREE(xmlStr);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 96f11ba..4995da5 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1954,6 +1954,24 @@ typedef void (*virDomainXMLPrivateDataFreeFunc)(void *);
>  typedef int (*virDomainXMLPrivateDataFormatFunc)(virBufferPtr, void *);
>  typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr, void *);
>
> +typedef int (*virDomainDefPostParseCallback)(virDomainDefPtr def,
> +                                             virCapsPtr caps,
> +                                             void *opaque);
> +typedef int (*virDomainDeviceDefPostParseCallback)(virDomainDeviceDefPtr dev,
> +                                                  virDomainDefPtr def,
> +                                                  virCapsPtr caps,
> +                                                  void *opaque);
> +
> +typedef struct _virDomainDefParserConfig virDomainDefParserConfig;
> +typedef virDomainDefParserConfig *virDomainDefParserConfigPtr;
> +struct _virDomainDefParserConfig {
> +    virDomainDefPostParseCallback domainConfigCallback;
> +    virDomainDeviceDefPostParseCallback devicesConfigCallback;

Yeah, here's the definition. I think domainConfigCallback should be
called domainPostParseCallback and devicesConfigCallback should be
called devicesPostParseCallback. That would help quite a lot to
un-confuse me.


> +
> +    void *priv;
> +    virFreeCallback privFree;
> +};
> +
>  typedef struct _virDomainXMLPrivateDataCallbacks virDomainXMLPrivateDataCallbacks;
>  typedef virDomainXMLPrivateDataCallbacks *virDomainXMLPrivateDataCallbacksPtr;
>  struct _virDomainXMLPrivateDataCallbacks {
> @@ -1963,9 +1981,10 @@ struct _virDomainXMLPrivateDataCallbacks {
>      virDomainXMLPrivateDataParseFunc  parse;
>  };
>
> -virDomainXMLConfPtr
> -virDomainXMLConfNew(virDomainXMLPrivateDataCallbacksPtr priv,
> -                    virDomainXMLNamespacePtr xmlns);
> +
> +virDomainXMLConfPtr virDomainXMLConfNew(virDomainDefParserConfigPtr config,
> +                                        virDomainXMLPrivateDataCallbacksPtr priv,
> +                                        virDomainXMLNamespacePtr xmlns);
>
>  virDomainXMLNamespacePtr
>  virDomainXMLConfGetNamespace(virDomainXMLConfPtr xmlconf)
> @@ -2025,6 +2044,7 @@ void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def);
>  void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def);
>  void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
>  virDomainDeviceDefPtr virDomainDeviceDefCopy(virCapsPtr caps,
> +                                             virDomainXMLConfPtr xmlconf,
>                                               const virDomainDefPtr def,
>                                               virDomainDeviceDefPtr src)

Going through these patches, I was a bit bothered by the difference in
ordering of args to different functions. In some cases, domain and/or
device is first, then caps (for example the post-parse callbacks
themselves), in others it's like the above - caps and xmlconf first,
domain and device later. We should try to standardize on one ordering of
these args:


   domain
   device
   caps
   xmlconf

Personally, I think of the caps and xmlconf as modifiers similar to
flags, so I think they should go later.

(an aside - I'm still trying to think of something that's less confusing
than "conf" for xmlconf and virDomainXMLConfPtr - this dual usage (for
both domain config that we're parsing, and the parser config) requires
too much concentration from my brain :-). I haven't come up with a
better idea yet, though...)
 
>  int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
> @@ -2088,6 +2108,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
>                              virDomainObjPtr dom);
>
>  virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
> +                                              virDomainXMLConfPtr xmlconf,
>                                                virDomainDefPtr def,
>                                                const char *xmlStr,
>                                                unsigned int flags);

For example, here - it would be very easy for someone to think that
xmlconf was "the domain's xml config" (although they would figure out
that's not right as soon as they investigated more closely, it makes it
cumbersome to read).

> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index fc8a3ae..2bff60e 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -1100,7 +1100,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
>          goto cleanup;
>      }
>
> -    if (!(priv->xmlconf = virDomainXMLConfNew(NULL, NULL)))
> +    if (!(priv->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL)))
>          goto cleanup;
>
>      conn->privateData = priv;
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 40a7a6b..fd69637 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1239,7 +1239,8 @@ libxlStartup(bool privileged,
>          goto error;
>      }
>
> -    if (!(libxl_driver->xmlconf = virDomainXMLConfNew(&libxlDomainXMLPrivateDataCallbacks,
> +    if (!(libxl_driver->xmlconf = virDomainXMLConfNew(NULL,
> +                                                      &libxlDomainXMLPrivateDataCallbacks,
>                                                        NULL)))
>          goto error;
>
> @@ -3556,7 +3557,8 @@ libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>      priv = vm->privateData;
>
>      if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> -        if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> +        if (!(dev = virDomainDeviceDefParse(driver->caps, driver->xmlconf,
> +                                            vm->def, xml,
>                                              VIR_DOMAIN_XML_INACTIVE)))
>              goto cleanup;
>
> @@ -3586,7 +3588,8 @@ libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>      if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
>          /* If dev exists it was created to modify the domain config. Free it. */
>          virDomainDeviceDefFree(dev);
> -        if (!(dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> +        if (!(dev = virDomainDeviceDefParse(driver->caps, driver->xmlconf,
> +                                            vm->def, xml,
>                                              VIR_DOMAIN_XML_INACTIVE)))
>              goto cleanup;
>
> diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
> index c723e77..dbc0b42 100644
> --- a/src/lxc/lxc_conf.c
> +++ b/src/lxc/lxc_conf.c
> @@ -159,7 +159,9 @@ error:
>  virDomainXMLConfPtr
>  lxcDomainXMLConfInit(void)
>  {
> -    return virDomainXMLConfNew(&virLXCDriverPrivateDataCallbacks, NULL);
> +    return virDomainXMLConfNew(NULL,
> +                               &virLXCDriverPrivateDataCallbacks,
> +                               NULL);
>  }
>
>  int lxcLoadDriverConfig(virLXCDriverPtr driver)
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 8603078..6a9d2b5 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -4349,7 +4349,8 @@ lxcDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>           goto cleanup;
>      }
>
> -    dev = dev_copy = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> +    dev = dev_copy = virDomainDeviceDefParse(driver->caps, driver->xmlconf,
> +                                             vm->def, xml,
>                                               VIR_DOMAIN_XML_INACTIVE);
>      if (dev == NULL)
>          goto cleanup;
> @@ -4360,7 +4361,8 @@ lxcDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>           * create a deep copy of device as adding
>           * to CONFIG takes one instance.
>           */
> -        dev_copy = virDomainDeviceDefCopy(driver->caps, vm->def, dev);
> +        dev_copy = virDomainDeviceDefCopy(driver->caps, driver->xmlconf,
> +                                          vm->def, dev);
>          if (!dev_copy)
>              goto cleanup;
>      }
> diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
> index f175655..e3e64e5 100644
> --- a/src/openvz/openvz_conf.c
> +++ b/src/openvz/openvz_conf.c
> @@ -174,6 +174,7 @@ static int openvzDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED,
>      return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ;
>  }
>
> +
>  virCapsPtr openvzCapsInit(void)

Spurious extra line :-)


>  {
>      virCapsPtr caps;
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 67d66ae..a6f4c66 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -1453,7 +1453,7 @@ static virDrvOpenStatus openvzOpen(virConnectPtr conn,
>      if (!(driver->caps = openvzCapsInit()))
>          goto cleanup;
>
> -    if (!(driver->xmlconf = virDomainXMLConfNew(NULL, NULL)))
> +    if (!(driver->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL)))
>          goto cleanup;
>
>      if (openvzLoadDomains(driver) < 0)
> @@ -2085,8 +2085,8 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
>                                          &vmdef) < 0)
>          goto cleanup;
>
> -    dev = virDomainDeviceDefParse(driver->caps, vmdef, xml,
> -                                  VIR_DOMAIN_XML_INACTIVE);
> +    dev = virDomainDeviceDefParse(driver->caps, driver->xmlconf,
> +                                  vmdef, xml, VIR_DOMAIN_XML_INACTIVE);
>      if (!dev)
>          goto cleanup;
>
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> index 88f41f7..ffb86dc 100644
> --- a/src/parallels/parallels_driver.c
> +++ b/src/parallels/parallels_driver.c
> @@ -929,7 +929,7 @@ parallelsOpenDefault(virConnectPtr conn)
>      if (!(privconn->caps = parallelsBuildCapabilities()))
>          goto error;
>
> -    if (!(privconn->xmlconf = virDomainXMLConfNew(NULL, NULL)))
> +    if (!(privconn->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL)))
>          goto error;
>
>      if (!(privconn->domains = virDomainObjListNew()))
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index 59cc1ca..6063256 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
> @@ -1204,7 +1204,7 @@ phypOpen(virConnectPtr conn,
>          goto failure;
>      }
>
> -    if (!(phyp_driver->xmlconf = virDomainXMLConfNew(NULL, NULL)))
> +    if (!(phyp_driver->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL)))
>          goto failure;
>
>      conn->privateData = phyp_driver;
> @@ -1754,8 +1754,8 @@ phypAttachDevice(virDomainPtr domain, const char *xml)
>          goto cleanup;
>      }
>
> -    dev = virDomainDeviceDefParse(phyp_driver->caps, def, xml,
> -                                  VIR_DOMAIN_XML_INACTIVE);
> +    dev = virDomainDeviceDefParse(phyp_driver->caps, NULL,
> +                                  def, xml, VIR_DOMAIN_XML_INACTIVE);
>      if (!dev) {
>          goto cleanup;
>      }
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index c2e2e10..d67debd 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -554,7 +554,8 @@ virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
>  virDomainXMLConfPtr
>  virQEMUDriverCreateXMLConf(void)
>  {
> -    return virDomainXMLConfNew(&virQEMUDriverPrivateDataCallbacks,
> +    return virDomainXMLConfNew(NULL,
> +                               &virQEMUDriverPrivateDataCallbacks,
>                                 &virQEMUDriverDomainXMLNamespace);
>  }
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9cd9e44..1c02a7c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5785,7 +5785,8 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
>          tmp = dev->data.disk;
>          dev->data.disk = orig_disk;
>
> -        if (!(dev_copy = virDomainDeviceDefCopy(caps, vm->def, dev))) {
> +        if (!(dev_copy = virDomainDeviceDefCopy(caps, driver->xmlconf,
> +                                                vm->def, dev))) {
>              dev->data.disk = tmp;
>              goto end;
>          }
> @@ -6061,7 +6062,8 @@ qemuDomainChangeDiskMediaLive(virDomainObjPtr vm,
>          tmp = dev->data.disk;
>          dev->data.disk = orig_disk;
>
> -        if (!(dev_copy = virDomainDeviceDefCopy(caps, vm->def, dev))) {
> +        if (!(dev_copy = virDomainDeviceDefCopy(caps, driver->xmlconf,
> +                                                vm->def, dev))) {
>              dev->data.disk = tmp;
>              goto end;
>          }
> @@ -6460,7 +6462,8 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>           goto endjob;
>      }
>
> -    dev = dev_copy = virDomainDeviceDefParse(caps, vm->def, xml,
> +    dev = dev_copy = virDomainDeviceDefParse(caps, driver->xmlconf,
> +                                             vm->def, xml,
>                                               VIR_DOMAIN_XML_INACTIVE);
>      if (dev == NULL)
>          goto endjob;
> @@ -6471,7 +6474,7 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
>           * create a deep copy of device as adding
>           * to CONFIG takes one instance.
>           */
> -        dev_copy = virDomainDeviceDefCopy(caps, vm->def, dev);
> +        dev_copy = virDomainDeviceDefCopy(caps, driver->xmlconf, vm->def, dev);
>          if (!dev_copy)
>              goto endjob;
>      }
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index c1a3ec9..866eac3 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -711,7 +711,7 @@ get_definition(vahControl * ctl, const char *xmlStr)
>          goto exit;
>      }
>
> -    if (!(ctl->xmlconf = virDomainXMLConfNew(NULL, NULL))) {
> +    if (!(ctl->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) {
>          vah_error(ctl, 0, _("Failed to create XML config object"));
>          goto exit;
>      }
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index c5fffb9..76e04c3 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -162,7 +162,7 @@ testBuildXMLConfig(void)
>  {
>      virDomainXMLPrivateDataCallbacks priv = { .alloc = testDomainObjPrivateAlloc,
>                                                .free = testDomainObjPrivateFree };
> -    return virDomainXMLConfNew(&priv, NULL);
> +    return virDomainXMLConfNew(NULL, &priv, NULL);
>  }
>
>
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index b08212d..480fc3d 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -505,8 +505,7 @@ umlStartup(bool privileged,
>      if ((uml_driver->caps = umlCapsInit()) == NULL)
>          goto out_of_memory;
>
> -    if (!(uml_driver->xmlconf = virDomainXMLConfNew(&privcb,
> -                                                    NULL)))
> +    if (!(uml_driver->xmlconf = virDomainXMLConfNew(NULL, &privcb, NULL)))
>          goto error;
>
>      if ((uml_driver->inotifyFD = inotify_init()) < 0) {
> @@ -2080,7 +2079,7 @@ static int umlDomainAttachDevice(virDomainPtr dom, const char *xml)
>          goto cleanup;
>      }
>
> -    dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> +    dev = virDomainDeviceDefParse(driver->caps, driver->xmlconf, vm->def, xml,
>                                    VIR_DOMAIN_XML_INACTIVE);
>
>      if (dev == NULL)
> @@ -2198,7 +2197,7 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) {
>          goto cleanup;
>      }
>
> -    dev = virDomainDeviceDefParse(driver->caps, vm->def, xml,
> +    dev = virDomainDeviceDefParse(driver->caps, driver->xmlconf, vm->def, xml,
>                                    VIR_DOMAIN_XML_INACTIVE);
>      if (dev == NULL)
>          goto cleanup;
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index aa7466b..dd96e7b 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -854,7 +854,7 @@ static int vboxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED,
>  static virDomainXMLConfPtr
>  vboxXMLConfInit(void)
>  {
> -    return virDomainXMLConfNew(NULL, NULL);
> +    return virDomainXMLConfNew(NULL, NULL, NULL);
>  }
>
>
> @@ -5396,8 +5396,8 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom,
>          goto cleanup;
>      }
>
> -    dev = virDomainDeviceDefParse(data->caps, def, xml,
> -                                  VIR_DOMAIN_XML_INACTIVE);
> +    dev = virDomainDeviceDefParse(data->caps, data->xmlconf,
> +                                  def, xml, VIR_DOMAIN_XML_INACTIVE);
>      if (dev == NULL)
>          goto cleanup;
>
> @@ -5631,8 +5631,8 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml) {
>          goto cleanup;
>      }
>
> -    dev = virDomainDeviceDefParse(data->caps, def, xml,
> -                                  VIR_DOMAIN_XML_INACTIVE);
> +    dev = virDomainDeviceDefParse(data->caps, data->xmlconf,
> +                                  def, xml, VIR_DOMAIN_XML_INACTIVE);
>      if (dev == NULL)
>          goto cleanup;
>
> diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
> index 6d82532..bf4c1ff 100644
> --- a/src/vmware/vmware_driver.c
> +++ b/src/vmware/vmware_driver.c
> @@ -78,7 +78,7 @@ vmwareDomainXMLConfigInit(void)
>      virDomainXMLPrivateDataCallbacks priv = { .alloc = vmwareDataAllocFunc,
>                                                .free = vmwareDataFreeFunc };
>
> -    return virDomainXMLConfNew(&priv, NULL);
> +    return virDomainXMLConfNew(NULL, &priv, NULL);
>  }
>
>  static virDrvOpenStatus
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index fd20b73..2ef3609 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -401,7 +401,7 @@ xenUnifiedOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
>          goto fail;
>      }
>
> -    if (!(priv->xmlconf = virDomainXMLConfNew(NULL, NULL)))
> +    if (!(priv->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL)))
>          goto fail;
>
>  #if WITH_XEN_INOTIFY
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 398da0d..93ba456 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -2519,7 +2519,7 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain,
>                                       NULL)))
>          goto cleanup;
>
> -    if (!(dev = virDomainDeviceDefParse(priv->caps,
> +    if (!(dev = virDomainDeviceDefParse(priv->caps, priv->xmlconf,
>                                          def, xml, VIR_DOMAIN_XML_INACTIVE)))
>          goto cleanup;
>
> @@ -2679,7 +2679,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain,
>                                       NULL)))
>          goto cleanup;
>
> -    if (!(dev = virDomainDeviceDefParse(priv->caps,
> +    if (!(dev = virDomainDeviceDefParse(priv->caps, priv->xmlconf,
>                                          def, xml, VIR_DOMAIN_XML_INACTIVE)))
>          goto cleanup;
>
> @@ -2786,7 +2786,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain,
>                                       NULL)))
>          goto cleanup;
>
> -    if (!(dev = virDomainDeviceDefParse(priv->caps,
> +    if (!(dev = virDomainDeviceDefParse(priv->caps, priv->xmlconf,
>                                          def, xml, VIR_DOMAIN_XML_INACTIVE)))
>          goto cleanup;
>
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index f6a3593..2a42e0a 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -1311,6 +1311,7 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain,
>      def = entry->def;
>
>      if (!(dev = virDomainDeviceDefParse(priv->caps,
> +                                        priv->xmlconf,
>                                          entry->def,
>                                          xml, VIR_DOMAIN_XML_INACTIVE)))
>          goto cleanup;
> @@ -1404,6 +1405,7 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain,
>      def = entry->def;
>
>      if (!(dev = virDomainDeviceDefParse(priv->caps,
> +                                        priv->xmlconf,
>                                          entry->def,
>                                          xml, VIR_DOMAIN_XML_INACTIVE)))
>          goto cleanup;
> diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
> index b368f48..941f3bd 100644
> --- a/src/xenapi/xenapi_driver.c
> +++ b/src/xenapi/xenapi_driver.c
> @@ -169,7 +169,7 @@ xenapiOpen(virConnectPtr conn, virConnectAuthPtr auth,
>          goto error;
>      }
>
> -    if (!(privP->xmlconf = virDomainXMLConfNew(NULL, NULL))) {
> +    if (!(privP->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) {
>          xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR,
>                                    _("Failed to create XML conf object"));
>          goto error;
> diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
> index 201ea2a..479eec3 100644
> --- a/tests/testutilsxen.c
> +++ b/tests/testutilsxen.c
> @@ -18,7 +18,7 @@ static int testXenDefaultConsoleType(const char *ostype,
>  virDomainXMLConfPtr
>  testXenXMLConfInit(void)
>  {
> -    return virDomainXMLConfNew(NULL, NULL);
> +    return virDomainXMLConfNew(NULL, NULL, NULL);
>  }
>
>  virCapsPtr testXenCapsInit(void) {
> diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c
> index 800fd2c..c606036 100644
> --- a/tests/xml2vmxtest.c
> +++ b/tests/xml2vmxtest.c
> @@ -240,7 +240,7 @@ mymain(void)
>          return EXIT_FAILURE;
>      }
>
> -    if (!(xmlconf = virDomainXMLConfNew(NULL, NULL)))
> +    if (!(xmlconf = virDomainXMLConfNew(NULL, NULL, NULL)))
>          return EXIT_FAILURE;
>
>      ctx.opaque = NULL;

The naming and arg ordering issues are the only problems that I see here.




More information about the libvir-list mailing list