[libvirt] [PATCHv4 0/9] Introduce driver specific callbacks and get rid of irrelevant data in virCaps

This series now splits out almost everything from the virCaps object (except for the defaultConsoleTargetType callback that I will post later as It requires more tweaking). See notes in individual patches for change summary. Peter Krempa (9): conf: Add post XML parse callbacks and prepare for cleaning of virCaps qemu: Record the default NIC model in the domain XML virCaps: get rid of "defaultInitPath" value in the virCaps struct virCaps: get rid of defaultDiskDriverName virCaps: get rid of emulatorRequired conf: Enforce ranges on cputune variables virCaps: remove defaultDiskDriverType from the struct virCaps: Get rid of hasWideScsiBus virCaps: get rid of macPrefix field src/conf/capabilities.c | 24 --- src/conf/capabilities.h | 20 -- src/conf/domain_conf.c | 239 ++++++++++++++++----- src/conf/domain_conf.h | 40 +++- src/esx/esx_driver.c | 13 +- src/libvirt_private.syms | 6 +- src/libvirt_vmx.syms | 2 + src/libxl/libxl_conf.c | 2 - src/libxl/libxl_driver.c | 13 +- src/lxc/lxc_conf.c | 11 +- src/lxc/lxc_domain.c | 17 ++ src/lxc/lxc_domain.h | 1 + src/lxc/lxc_driver.c | 6 +- src/openvz/openvz_conf.c | 4 +- src/openvz/openvz_driver.c | 32 ++- src/parallels/parallels_driver.c | 12 +- src/phyp/phyp_driver.c | 10 +- src/qemu/qemu_capabilities.c | 6 - src/qemu/qemu_command.c | 15 +- src/qemu/qemu_conf.c | 14 +- src/qemu/qemu_conf.h | 3 +- src/qemu/qemu_domain.c | 89 ++++++++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 13 +- src/security/virt-aa-helper.c | 2 +- src/test/test_driver.c | 2 +- src/uml/uml_driver.c | 7 +- src/vbox/vbox_tmpl.c | 18 +- src/vmware/vmware_conf.c | 4 +- src/vmware/vmware_driver.c | 8 +- src/vmx/vmx.c | 39 ++-- src/vmx/vmx.h | 12 +- src/xen/xen_driver.c | 7 +- src/xen/xen_hypervisor.c | 2 - src/xen/xend_internal.c | 6 +- src/xen/xm_internal.c | 2 + src/xenapi/xenapi_driver.c | 2 +- tests/domainsnapshotxml2xmltest.c | 2 +- tests/lxcxml2xmldata/lxc-hostdev.xml | 1 + tests/lxcxml2xmldata/lxc-systemd.xml | 1 + tests/qemuargv2xmltest.c | 2 +- tests/qemumonitorjsontest.c | 2 +- .../qemuxml2argv-disk-drive-network-nbd.args | 5 +- .../qemuxml2argv-disk-drive-network-nbd.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd-auth.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.args | 3 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 1 + .../qemuxml2argv-net-bandwidth.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-client.args | 4 +- .../qemuxml2argv-net-eth-ifname.args | 4 +- .../qemuxml2argv-net-eth-ifname.xml | 1 + .../qemuxml2argv-net-eth-names.args | 8 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args | 4 +- .../qemuxml2argv-net-openvswitch.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-server.args | 4 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 1 + .../qemuxml2argv-net-virtio-network-portgroup.xml | 2 + tests/qemuxml2argvtest.c | 2 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 1 + tests/qemuxml2xmltest.c | 2 +- tests/qemuxmlnstest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- tests/testutilsxen.c | 2 +- tests/vmx2xmltest.c | 11 +- tests/xml2vmxtest.c | 8 +- 73 files changed, 548 insertions(+), 251 deletions(-) -- 1.8.1.5

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); + 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; + + 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); 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); 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) { 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; -- 1.8.1.5

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.

On 03/25/2013 02:25 PM, Laine Stump wrote:
+ 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.).
+ +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.
(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...)
Maybe virDomainXMLOptionPtr. Leave 'conf' for items related to parsing a user's libvirtd.conf and qemu.conf files, and 'xmlopt' instead of 'xmlconf' for items related to options that the driver passed in for impacting xml parsing/validation. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This patch implements the devices post parse cllback and uses it to fill the default qemu network card model into the XML if none is specified. Libvirt assumes that the network card model for qemu is the "rtl8139". Record this in the XML using the new callback to avoid user confusion. --- Notes: Version 4: - tweaked naming after previous changes src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 26 ++++++++++++++++++++++ src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-net-bandwidth.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-client.args | 4 ++-- .../qemuxml2argv-net-eth-ifname.args | 4 ++-- .../qemuxml2argv-net-eth-ifname.xml | 1 + .../qemuxml2argv-net-eth-names.args | 8 +++---- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args | 4 ++-- .../qemuxml2argv-net-openvswitch.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-server.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 1 + .../qemuxml2argv-net-virtio-network-portgroup.xml | 2 ++ .../qemuxml2xmlout-graphics-spice-timeout.xml | 1 + 18 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d67debd..128baf8 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -554,7 +554,7 @@ virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver) virDomainXMLConfPtr virQEMUDriverCreateXMLConf(void) { - return virDomainXMLConfNew(NULL, + return virDomainXMLConfNew(&virQEMUDriverDomainDefParserConfig, &virQEMUDriverPrivateDataCallbacks, &virQEMUDriverDomainXMLNamespace); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c79b05d..51db3da 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -647,6 +647,7 @@ qemuDomainDefNamespaceFormatXML(virBufferPtr buf, return 0; } + static const char * qemuDomainDefNamespaceHref(void) { @@ -662,6 +663,31 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { }; +static int +qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (!dev->data.net->model && + !(dev->data.net->model = strdup("rtl8139"))) + goto no_memory; + } + return 0; + +no_memory: + virReportOOMError(); + return -1; +} + + +virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { + .devicesConfigCallback = qemuDomainDeviceDefPostParse, +}; + + static void qemuDomainObjSaveJob(virQEMUDriverPtr driver, virDomainObjPtr obj) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 26d5859..089ced0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -342,5 +342,6 @@ void qemuDomainCleanupRun(virQEMUDriverPtr driver, extern virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks; extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace; +extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig; #endif /* __QEMU_DOMAIN_H__ */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml index bf7dde5..885e854 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml @@ -44,6 +44,7 @@ <interface type='network'> <mac address='52:54:00:24:a5:9f'/> <source network='default'/> + <model type='rtl8139'/> <bandwidth> <inbound average='1000' peak='4000' burst='1024'/> <outbound average='128' peak='256' burst='32768'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-client.args b/tests/qemuxml2argvdata/qemuxml2argv-net-client.args index 7974f2e..34fab0a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-client.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-client.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=52:54:00:8c:b9:05,vlan=0 -net socket,connect=192.168.0.1:5558,vlan=0 \ --serial none -parallel none +macaddr=52:54:00:8c:b9:05,vlan=0,model=rtl8139 -net socket,\ +connect=192.168.0.1:5558,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args index cced5d5..6aef307 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net tap,ifname=nic02,script=/etc/qemu-ifup,\ -vlan=0 -serial none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net tap,ifname=nic02,\ +script=/etc/qemu-ifup,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml index 04a4ca4..b150371 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml @@ -25,6 +25,7 @@ <mac address='00:11:22:33:44:55'/> <script path='/etc/qemu-ifup'/> <target dev='nic02'/> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args index dc15f63..57761c5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0,name=net0 -net tap,script=/etc/qemu-ifup,\ -vlan=0,name=hostnet0 -net nic,macaddr=00:11:22:33:44:56,vlan=1,model=e1000,\ -name=net1 -net tap,script=/etc/qemu-ifup,vlan=1,name=hostnet1 -serial none \ --parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139,name=net0 -net tap,\ +script=/etc/qemu-ifup,vlan=0,name=hostnet0 -net nic,macaddr=00:11:22:33:44:56,\ +vlan=1,model=e1000,name=net1 -net tap,script=/etc/qemu-ifup,vlan=1,name=hostnet1 \ +-serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args index a482193..877dac2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net tap,script=/etc/qemu-ifup,vlan=0 -serial \ -none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net tap,script=/etc/qemu-ifup,\ +vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml index 87dd65f..eca5da5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml @@ -24,6 +24,7 @@ <interface type='ethernet'> <mac address='00:11:22:33:44:55'/> <script path='/etc/qemu-ifup'/> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml index 81f70d0..9be0d2d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -32,6 +32,7 @@ <virtualport type='802.1Qbg'> <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args b/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args index fc2091b..ed4f01e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=52:54:00:8c:b9:05,vlan=0 -net socket,mcast=192.0.0.1:5558,vlan=0 \ --serial none -parallel none +macaddr=52:54:00:8c:b9:05,vlan=0,model=rtl8139 -net socket,mcast=192.0.0.1:5558,\ +vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml index ff09844..9c2c5dc 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml @@ -32,6 +32,7 @@ <virtualport type='openvswitch'> <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f' profileid='bob'/> </virtualport> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-server.args b/tests/qemuxml2argvdata/qemuxml2argv-net-server.args index 7c9d735..c92a3ff 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-server.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-server.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=52:54:00:8c:b9:05,vlan=0 -net socket,listen=192.168.0.1:5558,vlan=0 \ --serial none -parallel none +macaddr=52:54:00:8c:b9:05,vlan=0,model=rtl8139 -net socket,\ +listen=192.168.0.1:5558,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args index 7364281..814167b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,4 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net user,vlan=0 -serial none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net user,vlan=0 -serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..fe3a271 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,7 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml index c84ed3f..0fb9b2c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml @@ -38,6 +38,7 @@ <virtualport> <parameters instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f' interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> + <model type='rtl8139'/> </interface> <interface type='network'> <mac address='22:11:22:33:44:55'/> @@ -45,6 +46,7 @@ <virtualport type='802.1Qbh'> <parameters profileid='testhis99'/> </virtualport> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml index cd19b64..54b68fa 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml @@ -62,6 +62,7 @@ <interface type='ethernet'> <mac address='52:54:00:71:70:89'/> <script path='/etc/qemu-ifup'/> + <model type='rtl8139'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <serial type='pty'> -- 1.8.1.5

On 03/15/2013 11:26 AM, Peter Krempa wrote:
This patch implements the devices post parse cllback and uses it to fill the default qemu network card model into the XML if none is specified.
Libvirt assumes that the network card model for qemu is the "rtl8139". Record this in the XML using the new callback to avoid user confusion.
As I recall (from a previous Fedora-specific patch I had to make that forced all "fedora-13" machinetypes in configs to be changed to "pc-0.14"), just causing the parser to fill this in will not cause the default value to actually be written to the config files. So when we get to the point where we want to change the default, we won't have actually addressed the issue of making sure that existing configs continue to use rtl8139 rather than abruptly changing the guest's hardware at next boot (or when migrated to another host). If we want to assure that existing guests have their default netdev model written to config, we'll need to actually force the config to be rewritten to disk. Take a look at the patch named libvirt-qemu-replace-deprecated-fedora-13-machine.patch in fedora-git's f16 branch for libvirt to see what I'm talking about. It's really quite ugly :-(
---
Notes: Version 4: - tweaked naming after previous changes
src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_domain.c | 26 ++++++++++++++++++++++ src/qemu/qemu_domain.h | 1 + .../qemuxml2argv-net-bandwidth.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-client.args | 4 ++-- .../qemuxml2argv-net-eth-ifname.args | 4 ++-- .../qemuxml2argv-net-eth-ifname.xml | 1 + .../qemuxml2argv-net-eth-names.args | 8 +++---- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args | 4 ++-- .../qemuxml2argv-net-openvswitch.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-net-server.args | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 1 + .../qemuxml2argv-net-virtio-network-portgroup.xml | 2 ++ .../qemuxml2xmlout-graphics-spice-timeout.xml | 1 + 18 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d67debd..128baf8 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -554,7 +554,7 @@ virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver) virDomainXMLConfPtr virQEMUDriverCreateXMLConf(void) { - return virDomainXMLConfNew(NULL, + return virDomainXMLConfNew(&virQEMUDriverDomainDefParserConfig, &virQEMUDriverPrivateDataCallbacks, &virQEMUDriverDomainXMLNamespace); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c79b05d..51db3da 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -647,6 +647,7 @@ qemuDomainDefNamespaceFormatXML(virBufferPtr buf, return 0; }
+ static const char * qemuDomainDefNamespaceHref(void) { @@ -662,6 +663,31 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { };
+static int +qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_NET && + dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (!dev->data.net->model && + !(dev->data.net->model = strdup("rtl8139"))) + goto no_memory; + } + return 0; + +no_memory: + virReportOOMError(); + return -1; +} + + +virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { + .devicesConfigCallback = qemuDomainDeviceDefPostParse, +}; + + static void qemuDomainObjSaveJob(virQEMUDriverPtr driver, virDomainObjPtr obj) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 26d5859..089ced0 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -342,5 +342,6 @@ void qemuDomainCleanupRun(virQEMUDriverPtr driver,
extern virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks; extern virDomainXMLNamespace virQEMUDriverDomainXMLNamespace; +extern virDomainDefParserConfig virQEMUDriverDomainDefParserConfig;
#endif /* __QEMU_DOMAIN_H__ */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml index bf7dde5..885e854 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml @@ -44,6 +44,7 @@ <interface type='network'> <mac address='52:54:00:24:a5:9f'/> <source network='default'/> + <model type='rtl8139'/> <bandwidth> <inbound average='1000' peak='4000' burst='1024'/> <outbound average='128' peak='256' burst='32768'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-client.args b/tests/qemuxml2argvdata/qemuxml2argv-net-client.args index 7974f2e..34fab0a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-client.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-client.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=52:54:00:8c:b9:05,vlan=0 -net socket,connect=192.168.0.1:5558,vlan=0 \ --serial none -parallel none +macaddr=52:54:00:8c:b9:05,vlan=0,model=rtl8139 -net socket,\ +connect=192.168.0.1:5558,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args index cced5d5..6aef307 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net tap,ifname=nic02,script=/etc/qemu-ifup,\ -vlan=0 -serial none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net tap,ifname=nic02,\ +script=/etc/qemu-ifup,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml index 04a4ca4..b150371 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml @@ -25,6 +25,7 @@ <mac address='00:11:22:33:44:55'/> <script path='/etc/qemu-ifup'/> <target dev='nic02'/> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args index dc15f63..57761c5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0,name=net0 -net tap,script=/etc/qemu-ifup,\ -vlan=0,name=hostnet0 -net nic,macaddr=00:11:22:33:44:56,vlan=1,model=e1000,\ -name=net1 -net tap,script=/etc/qemu-ifup,vlan=1,name=hostnet1 -serial none \ --parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139,name=net0 -net tap,\ +script=/etc/qemu-ifup,vlan=0,name=hostnet0 -net nic,macaddr=00:11:22:33:44:56,\ +vlan=1,model=e1000,name=net1 -net tap,script=/etc/qemu-ifup,vlan=1,name=hostnet1 \ +-serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args index a482193..877dac2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net tap,script=/etc/qemu-ifup,vlan=0 -serial \ -none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net tap,script=/etc/qemu-ifup,\ +vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml index 87dd65f..eca5da5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml @@ -24,6 +24,7 @@ <interface type='ethernet'> <mac address='00:11:22:33:44:55'/> <script path='/etc/qemu-ifup'/> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml index 81f70d0..9be0d2d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml @@ -32,6 +32,7 @@ <virtualport type='802.1Qbg'> <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args b/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args index fc2091b..ed4f01e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=52:54:00:8c:b9:05,vlan=0 -net socket,mcast=192.0.0.1:5558,vlan=0 \ --serial none -parallel none +macaddr=52:54:00:8c:b9:05,vlan=0,model=rtl8139 -net socket,mcast=192.0.0.1:5558,\ +vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml index ff09844..9c2c5dc 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml @@ -32,6 +32,7 @@ <virtualport type='openvswitch'> <parameters interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f' profileid='bob'/> </virtualport> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-server.args b/tests/qemuxml2argvdata/qemuxml2argv-net-server.args index 7c9d735..c92a3ff 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-server.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-server.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=52:54:00:8c:b9:05,vlan=0 -net socket,listen=192.168.0.1:5558,vlan=0 \ --serial none -parallel none +macaddr=52:54:00:8c:b9:05,vlan=0,model=rtl8139 -net socket,\ +listen=192.168.0.1:5558,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args index 7364281..814167b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,4 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net user,vlan=0 -serial none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net user,vlan=0 -serial none \ +-parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..fe3a271 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,7 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml index c84ed3f..0fb9b2c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-network-portgroup.xml @@ -38,6 +38,7 @@ <virtualport> <parameters instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f' interfaceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/> </virtualport> + <model type='rtl8139'/> </interface> <interface type='network'> <mac address='22:11:22:33:44:55'/> @@ -45,6 +46,7 @@ <virtualport type='802.1Qbh'> <parameters profileid='testhis99'/> </virtualport> + <model type='rtl8139'/> </interface> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml index cd19b64..54b68fa 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml @@ -62,6 +62,7 @@ <interface type='ethernet'> <mac address='52:54:00:71:70:89'/> <script path='/etc/qemu-ifup'/> + <model type='rtl8139'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </interface> <serial type='pty'>

On 03/25/13 21:34, Laine Stump wrote:
On 03/15/2013 11:26 AM, Peter Krempa wrote:
This patch implements the devices post parse cllback and uses it to fill the default qemu network card model into the XML if none is specified.
Libvirt assumes that the network card model for qemu is the "rtl8139". Record this in the XML using the new callback to avoid user confusion.
As I recall (from a previous Fedora-specific patch I had to make that forced all "fedora-13" machinetypes in configs to be changed to "pc-0.14"), just causing the parser to fill this in will not cause the default value to actually be written to the config files. So when we get to the point where we want to change the default, we won't have actually addressed the issue of making sure that existing configs continue to use rtl8139 rather than abruptly changing the guest's hardware at next boot (or when migrated to another host).
This isn't an issue in this case. The default is and always was to use the realtek card. We can't change that unfortunately for anything better :(. This is just to notify the user what is the actual model we are using in this case so I think it isn't that important to rewrite the XML file. In case it's missing in the config, it will be always parsed to the default value and when the config will be saved in the future it will be written. But the value will still be used.
If we want to assure that existing guests have their default netdev model written to config, we'll need to actually force the config to be rewritten to disk. Take a look at the patch named
I don't think this is really needed, but I will have a look if this is feasible in this case.
libvirt-qemu-replace-deprecated-fedora-13-machine.patch
in fedora-git's f16 branch for libvirt to see what I'm talking about. It's really quite ugly :-(
Peter

On 03/26/2013 12:51 PM, Peter Krempa wrote:
On 03/25/13 21:34, Laine Stump wrote:
On 03/15/2013 11:26 AM, Peter Krempa wrote:
This patch implements the devices post parse cllback and uses it to fill the default qemu network card model into the XML if none is specified.
Libvirt assumes that the network card model for qemu is the "rtl8139". Record this in the XML using the new callback to avoid user confusion.
As I recall (from a previous Fedora-specific patch I had to make that forced all "fedora-13" machinetypes in configs to be changed to "pc-0.14"), just causing the parser to fill this in will not cause the default value to actually be written to the config files. So when we get to the point where we want to change the default, we won't have actually addressed the issue of making sure that existing configs continue to use rtl8139 rather than abruptly changing the guest's hardware at next boot (or when migrated to another host).
This isn't an issue in this case. The default is and always was to use the realtek card. We can't change that unfortunately for anything better :(. This is just to notify the user what is the actual model we are using in this case so I think it isn't that important to rewrite the XML file.
In case it's missing in the config, it will be always parsed to the default value and when the config will be saved in the future it will be written. But the value will still be used.
If we want to assure that existing guests have their default netdev model written to config, we'll need to actually force the config to be rewritten to disk. Take a look at the patch named
I don't think this is really needed, but I will have a look if this is feasible in this case.
Actually, I think this *is* necessary, because part of the reason for explicitly adding the model to the config even when it is the default is that there is talk afloat to *change* libvirt's default interface model from rtl8139 to e1000. When/if this happens, existing domains will need to continue using the old default after upgrading libvirt and restarting the guest. In order for that to work, the model needs to be written to the persistent config on disk.

This gets rid of the parameter in favor of using the new callback infrastructure to do the same stuff. This patch implements the domain adjustment callback in the openVZ driver and moves the check from the parser to a new validation method in the callback infrastructure. --- Notes: Version 4: - tweaked naming do comply with other changes - v3 ACKed Version 3: - new in series src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 33 +++++++++++++++++++++------------ src/openvz/openvz_conf.c | 1 - src/openvz/openvz_driver.c | 26 +++++++++++++++++++++++++- 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index a70896a..43ace12 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -168,7 +168,6 @@ struct _virCaps { int defaultDiskDriverType; /* enum virStorageFileFormat */ int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); bool hasWideScsiBus; - const char *defaultInitPath; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a1b634b..bd3cd26 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2487,6 +2487,22 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, } +/* this is a place for global assumption checks */ +static int +virDomainDefPostParseInternal(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED) +{ + /* verify init path for container based domains */ + if (STREQ(def->os.type, "exe") && !def->os.init) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + return -1; + } + + return 0; +} + + static int virDomainDeviceDefPostParse(virDomainXMLConfPtr xmlconf, virDomainDeviceDefPtr dev, @@ -2506,6 +2522,7 @@ virDomainDeviceDefPostParse(virDomainXMLConfPtr xmlconf, } + struct virDomainDefPostParseDeviceIteratorData { virCapsPtr caps; virDomainDefPtr def; @@ -2550,6 +2567,10 @@ virDomainDefPostParse(virDomainXMLConfPtr xmlconf, &data)) < 0) return ret; + + if ((ret = virDomainDefPostParseInternal(def, caps)) < 0) + return ret; + return 0; } @@ -10297,18 +10318,6 @@ virDomainDefParseXML(virCapsPtr caps, if (STREQ(def->os.type, "exe")) { def->os.init = virXPathString("string(./os/init[1])", ctxt); - if (!def->os.init) { - if (caps->defaultInitPath) { - def->os.init = strdup(caps->defaultInitPath); - if (!def->os.init) { - goto no_memory; - } - } else { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("init binary must be specified")); - goto error; - } - } def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); if ((n = virXPathNodeSet("./os/initarg", ctxt, &nodes)) < 0) { diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index e3e64e5..05c6113 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -206,7 +206,6 @@ virCapsPtr openvzCapsInit(void) NULL) == NULL) goto no_memory; - caps->defaultInitPath = "/sbin/init"; caps->defaultConsoleTargetType = openvzDefaultConsoleType; return caps; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index a6f4c66..de6198c 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -97,6 +97,29 @@ static void cmdExecFree(const char *cmdExec[]) } } + +static int +openvzDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + /* fill the init path */ + if (STREQ(def->os.type, "exe") && !def->os.init) { + if (!(def->os.init = strdup("/sbin/init"))) { + virReportOOMError(); + return -1; + } + } + + return 0; +} + + +virDomainDefParserConfig openvzDomainDefParserConfig = { + .domainConfigCallback = openvzDomainDefPostParse, +}; + + /* generate arguments to create OpenVZ container return -1 - error 0 - OK @@ -1453,7 +1476,8 @@ static virDrvOpenStatus openvzOpen(virConnectPtr conn, if (!(driver->caps = openvzCapsInit())) goto cleanup; - if (!(driver->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) + if (!(driver->xmlconf = virDomainXMLConfNew(&openvzDomainDefParserConfig, + NULL, NULL))) goto cleanup; if (openvzLoadDomains(driver) < 0) -- 1.8.1.5

On 03/15/2013 09:26 AM, Peter Krempa wrote:
This gets rid of the parameter in favor of using the new callback infrastructure to do the same stuff.
This patch implements the domain adjustment callback in the openVZ driver and moves the check from the parser to a new validation method in the callback infrastructure. ---
Notes: Version 4: - tweaked naming do comply with other changes - v3 ACKed
+++ b/src/conf/domain_conf.c
@@ -2506,6 +2522,7 @@ virDomainDeviceDefPostParse(virDomainXMLConfPtr xmlconf, }
+ struct virDomainDefPostParseDeviceIteratorData {
Spurious newline addition? Other than that, still looks good. I'm assuming this series is waiting until after 1.0.4. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This patch removes the defaultDiskDriverName from the virCaps structure. This particular default value is used only in the qemu driver so this patch uses the recently added callback to fill the driver name if it's needed instead of propagating it through virCaps. --- Notes: Version 4: - tweaked naming to comply - v3 was already ACKed Version 3: - new in series src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 8 -------- src/qemu/qemu_conf.c | 5 ++--- src/qemu/qemu_conf.h | 3 ++- src/qemu/qemu_domain.c | 25 ++++++++++++++++++++++--- src/qemu/qemu_driver.c | 2 +- tests/domainsnapshotxml2xmltest.c | 2 +- tests/qemuargv2xmltest.c | 2 +- tests/qemumonitorjsontest.c | 2 +- tests/qemuxml2argvtest.c | 2 +- tests/qemuxml2xmltest.c | 2 +- tests/qemuxmlnstest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- 13 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 43ace12..5fd1bb5 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -164,7 +164,6 @@ struct _virCaps { /* Move to virDomainXMLConf later */ unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; unsigned int emulatorRequired : 1; - const char *defaultDiskDriverName; int defaultDiskDriverType; /* enum virStorageFileFormat */ int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); bool hasWideScsiBus; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd3cd26..7f8f96e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4830,11 +4830,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, def->format = caps->defaultDiskDriverType; } - if (!def->driverName && - caps->defaultDiskDriverName && - !(def->driverName = strdup(caps->defaultDiskDriverName))) - goto no_memory; - if (mirrorFormat) { def->mirrorFormat = virStorageFileFormatTypeFromString(mirrorFormat); if (def->mirrorFormat <= 0) { @@ -4897,9 +4892,6 @@ cleanup: ctxt->node = save_ctxt; return def; -no_memory: - virReportOOMError(); - error: virDomainDiskDefFree(def); def = NULL; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 128baf8..aab3375 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -552,8 +552,9 @@ virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver) } virDomainXMLConfPtr -virQEMUDriverCreateXMLConf(void) +virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver) { + virQEMUDriverDomainDefParserConfig.priv = driver; return virDomainXMLConfNew(&virQEMUDriverDomainDefParserConfig, &virQEMUDriverPrivateDataCallbacks, &virQEMUDriverDomainXMLNamespace); @@ -574,10 +575,8 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) goto no_memory; if (cfg->allowDiskFormatProbing) { - caps->defaultDiskDriverName = NULL; caps->defaultDiskDriverType = VIR_STORAGE_FILE_AUTO; } else { - caps->defaultDiskDriverName = "qemu"; caps->defaultDiskDriverType = VIR_STORAGE_FILE_RAW; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c5ddaad..4b917a7 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -301,6 +301,7 @@ void qemuSharedDiskEntryFree(void *payload, const void *name) ATTRIBUTE_NONNULL(1); int qemuDriverAllocateID(virQEMUDriverPtr driver); -virDomainXMLConfPtr virQEMUDriverCreateXMLConf(void); +virDomainXMLConfPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); + #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 51db3da..6397809 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -667,19 +667,38 @@ static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainDefPtr def ATTRIBUTE_UNUSED, virCapsPtr caps ATTRIBUTE_UNUSED, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + int ret = -1; + virQEMUDriverPtr driver = opaque; + virQEMUDriverConfigPtr cfg = NULL; + if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { if (!dev->data.net->model && !(dev->data.net->model = strdup("rtl8139"))) goto no_memory; } - return 0; + + if (dev->type == VIR_DOMAIN_DEVICE_DISK && + !dev->data.disk->driverName && + driver && + (cfg = virQEMUDriverGetConfig(driver))) { + if (!cfg->allowDiskFormatProbing && + !(dev->data.disk->driverName = strdup("qemu"))) { + goto no_memory; + } + } + + ret = 0; + +cleanup: + virObjectUnref(cfg); + return ret; no_memory: virReportOOMError(); - return -1; + goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1c02a7c..c8c759f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -723,7 +723,7 @@ qemuStartup(bool privileged, if ((qemu_driver->caps = virQEMUDriverCreateCapabilities(qemu_driver)) == NULL) goto error; - if (!(qemu_driver->xmlconf = virQEMUDriverCreateXMLConf())) + if (!(qemu_driver->xmlconf = virQEMUDriverCreateXMLConf(qemu_driver))) goto error; /* If hugetlbfs is present, then we need to create a sub-directory within diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 7d95310..e6bd3a6 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -91,7 +91,7 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(driver.xmlconf = virQEMUDriverCreateXMLConf())) + if (!(driver.xmlconf = virQEMUDriverCreateXMLConf(&driver))) return EXIT_FAILURE; # define DO_TEST(name, uuid, internal) \ diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index f7d6019..d216e78 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -120,7 +120,7 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(driver.xmlconf = virQEMUDriverCreateXMLConf())) + if (!(driver.xmlconf = virQEMUDriverCreateXMLConf(&driver))) return EXIT_FAILURE; # define DO_TEST_FULL(name, extraFlags, migrateFrom) \ diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 639eeb1..d6f1d64 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -451,7 +451,7 @@ mymain(void) #endif if (virThreadInitialize() < 0 || - !(xmlconf = virQEMUDriverCreateXMLConf())) + !(xmlconf = virQEMUDriverCreateXMLConf(NULL))) return EXIT_FAILURE; virEventRegisterDefaultImpl(); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c77b73f..f31bf0c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -294,7 +294,7 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(driver.xmlconf = virQEMUDriverCreateXMLConf())) + if (!(driver.xmlconf = virQEMUDriverCreateXMLConf(&driver))) return EXIT_FAILURE; VIR_FREE(driver.config->stateDir); if ((driver.config->stateDir = strdup("/nowhere")) == NULL) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d77da4e..313d688 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -106,7 +106,7 @@ mymain(void) if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(driver.xmlconf = virQEMUDriverCreateXMLConf())) + if (!(driver.xmlconf = virQEMUDriverCreateXMLConf(&driver))) return EXIT_FAILURE; # define DO_TEST_FULL(name, is_different, when) \ diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 76a715d..de46993 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -204,7 +204,7 @@ mymain(void) driver.config = virQEMUDriverConfigNew(false); if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(driver.xmlconf = virQEMUDriverCreateXMLConf())) + if (!(driver.xmlconf = virQEMUDriverCreateXMLConf(&driver))) return EXIT_FAILURE; if (virAsprintf(&map, "%s/src/cpu/cpu_map.xml", abs_top_srcdir) < 0 || cpuMapOverride(map) < 0) { diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 405bc2b..f3e8d7c 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -328,7 +328,7 @@ mymain(void) if ((caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(xmlconf = virQEMUDriverCreateXMLConf())) + if (!(xmlconf = virQEMUDriverCreateXMLConf(NULL))) return EXIT_FAILURE; #define DO_TEST_LABELING(name) \ -- 1.8.1.5

On 03/15/2013 09:26 AM, Peter Krempa wrote:
This patch removes the defaultDiskDriverName from the virCaps structure. This particular default value is used only in the qemu driver so this patch uses the recently added callback to fill the driver name if it's needed instead of propagating it through virCaps. ---
Notes: Version 4: - tweaked naming to comply - v3 was already ACKed
Still looks good. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This patch removes the emulatorRequired field and associated infrastructure from the virCaps object. Instead the driver specific callbacks are used as this field isn't enforced by all drivers. This patch implements the appropriate callbacks in the qemu and lxc driver and moves to check to that location. --- Notes: Version 4: - rename virDomainDefDefaultEmulator to virDomainDefGetDefaultEmulator - changed names of functions to comply with the rest Version 3: - new in the series src/conf/capabilities.c | 10 ---------- src/conf/capabilities.h | 7 ------- src/conf/domain_conf.c | 16 ++++++---------- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 3 +-- src/lxc/lxc_conf.c | 6 ++---- src/lxc/lxc_domain.c | 17 +++++++++++++++++ src/lxc/lxc_domain.h | 1 + src/qemu/qemu_capabilities.c | 3 --- src/qemu/qemu_domain.c | 16 ++++++++++++++++ tests/lxcxml2xmldata/lxc-hostdev.xml | 1 + tests/lxcxml2xmldata/lxc-systemd.xml | 1 + 12 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index d53d5a3..1d29ce6 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -935,13 +935,3 @@ virCapabilitiesGenerateMac(virCapsPtr caps, { virMacAddrGenerate(caps->macPrefix, mac); } - -extern void -virCapabilitiesSetEmulatorRequired(virCapsPtr caps) { - caps->emulatorRequired = 1; -} - -extern unsigned int -virCapabilitiesIsEmulatorRequired(virCapsPtr caps) { - return caps->emulatorRequired; -} diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 5fd1bb5..dcf38a8 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -163,7 +163,6 @@ struct _virCaps { /* Move to virDomainXMLConf later */ unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; - unsigned int emulatorRequired : 1; int defaultDiskDriverType; /* enum virStorageFileFormat */ int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); bool hasWideScsiBus; @@ -186,12 +185,6 @@ extern void virCapabilitiesGenerateMac(virCapsPtr caps, virMacAddrPtr mac); -extern void -virCapabilitiesSetEmulatorRequired(virCapsPtr caps); - -extern unsigned int -virCapabilitiesIsEmulatorRequired(virCapsPtr caps); - extern int virCapabilitiesAddHostFeature(virCapsPtr caps, const char *name); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7f8f96e..fde88b2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9166,8 +9166,9 @@ virDomainLeaseRemove(virDomainDefPtr def, } -static char *virDomainDefDefaultEmulator(virDomainDefPtr def, - virCapsPtr caps) { +char * +virDomainDefGetDefaultEmulator(virDomainDefPtr def, + virCapsPtr caps) { const char *type; const char *emulator; char *retemu; @@ -9186,13 +9187,13 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def, if (!emulator) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("no emulator for domain %s os type %s on architecture %s"), + _("no emulator for domain %s os type %s " + "on architecture %s"), type, def->os.type, virArchToString(def->os.arch)); return NULL; } - retemu = strdup(emulator); - if (!retemu) + if (!(retemu = strdup(emulator))) virReportOOMError(); return retemu; @@ -10350,11 +10351,6 @@ virDomainDefParseXML(virCapsPtr caps, } def->emulator = virXPathString("string(./devices/emulator[1])", ctxt); - if (!def->emulator && virCapabilitiesIsEmulatorRequired(caps)) { - def->emulator = virDomainDefDefaultEmulator(def, caps); - if (!def->emulator) - goto error; - } /* analysis of the disk devices */ if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4995da5..a82a432 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2490,4 +2490,6 @@ int virDomainObjListExport(virDomainObjListPtr doms, virDomainVcpuPinDefPtr virDomainLookupVcpuPin(virDomainDefPtr def, int vcpuid); +char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5cad990..fb05d59 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -21,9 +21,7 @@ virCapabilitiesFormatXML; virCapabilitiesFreeMachines; virCapabilitiesFreeNUMAInfo; virCapabilitiesGenerateMac; -virCapabilitiesIsEmulatorRequired; virCapabilitiesNew; -virCapabilitiesSetEmulatorRequired; virCapabilitiesSetHostCPU; virCapabilitiesSetMacPrefix; @@ -118,6 +116,7 @@ virDomainDefCopy; virDomainDefFormat; virDomainDefFormatInternal; virDomainDefFree; +virDomainDefGetDefaultEmulator; virDomainDefGetSecurityLabelDef; virDomainDefParseFile; virDomainDefParseNode; diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index dbc0b42..71b8916 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -119,9 +119,6 @@ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) goto error; } - /* LXC Requires an emulator in the XML */ - virCapabilitiesSetEmulatorRequired(caps); - if (driver) { /* Security driver data */ const char *doi, *model; @@ -159,11 +156,12 @@ error: virDomainXMLConfPtr lxcDomainXMLConfInit(void) { - return virDomainXMLConfNew(NULL, + return virDomainXMLConfNew(&virLXCDriverDomainDefParserConfig, &virLXCDriverPrivateDataCallbacks, NULL); } + int lxcLoadDriverConfig(virLXCDriverPtr driver) { char *filename; diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 08cf8f6..673be3a 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -79,3 +79,20 @@ virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks = { .format = virLXCDomainObjPrivateXMLFormat, .parse = virLXCDomainObjPrivateXMLParse, }; + +static int +virLXCDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + void *opaque ATTRIBUTE_UNUSED) +{ + /* check for emulator and create a default one if needed */ + if (!def->emulator && + !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) + return -1; + + return 0; +} + +virDomainDefParserConfig virLXCDriverDomainDefParserConfig = { + .domainConfigCallback = virLXCDomainDefPostParse, +}; diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h index 007ea84..12753aa 100644 --- a/src/lxc/lxc_domain.h +++ b/src/lxc/lxc_domain.h @@ -39,5 +39,6 @@ struct _virLXCDomainObjPrivate { }; extern virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks; +extern virDomainDefParserConfig virLXCDriverDomainDefParserConfig; #endif /* __LXC_DOMAIN_H__ */ diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 519d2c5..4ef7092 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -904,9 +904,6 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache) i) < 0) goto error; - /* QEMU Requires an emulator in the XML */ - virCapabilitiesSetEmulatorRequired(caps); - caps->defaultConsoleTargetType = virQEMUCapsDefaultConsoleType; return caps; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6397809..58f9d27 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -664,10 +664,25 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { static int +qemuDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + void *opaque ATTRIBUTE_UNUSED) +{ + /* check for emulator and create a default one if needed */ + if (!def->emulator && + !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) + return -1; + + return 0; +} + + +static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainDefPtr def ATTRIBUTE_UNUSED, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque) + { int ret = -1; virQEMUDriverPtr driver = opaque; @@ -703,6 +718,7 @@ no_memory: virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { + .domainConfigCallback = qemuDomainDefPostParse, .devicesConfigCallback = qemuDomainDeviceDefPostParse, }; diff --git a/tests/lxcxml2xmldata/lxc-hostdev.xml b/tests/lxcxml2xmldata/lxc-hostdev.xml index 02a87a7..b022cc7 100644 --- a/tests/lxcxml2xmldata/lxc-hostdev.xml +++ b/tests/lxcxml2xmldata/lxc-hostdev.xml @@ -13,6 +13,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> <filesystem type='mount' accessmode='passthrough'> <source dir='/root/container'/> <target dir='/'/> diff --git a/tests/lxcxml2xmldata/lxc-systemd.xml b/tests/lxcxml2xmldata/lxc-systemd.xml index 2f36eee..fd3a606 100644 --- a/tests/lxcxml2xmldata/lxc-systemd.xml +++ b/tests/lxcxml2xmldata/lxc-systemd.xml @@ -15,6 +15,7 @@ <on_reboot>restart</on_reboot> <on_crash>destroy</on_crash> <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> <filesystem type='mount' accessmode='passthrough'> <source dir='/root/container'/> <target dir='/'/> -- 1.8.1.5

On 03/15/2013 09:26 AM, Peter Krempa wrote:
This patch removes the emulatorRequired field and associated infrastructure from the virCaps object. Instead the driver specific callbacks are used as this field isn't enforced by all drivers.
This patch implements the appropriate callbacks in the qemu and lxc driver and moves to check to that location. ---
Notes: Version 4: - rename virDomainDefDefaultEmulator to virDomainDefGetDefaultEmulator - changed names of functions to comply with the rest Version 3: - new in the series
+static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainDefPtr def ATTRIBUTE_UNUSED, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque) + {
Spurious newline addition. ACK with that cleaned up (again, assuming this series is post-release). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The limits are documented at http://libvirt.org/formatdomain.html#elementsCPUTuning . Enforce them when going through XML parsing in addition to being enforced by the API. --- Notes: Version 4: - changed error from VIR_ERR_XML_ERROR to VIR_ERR_CONFIG_UNSUPPORTED Version 3: - new in series src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fde88b2..5a59e3f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2499,6 +2499,43 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } + /* enforce range checks for cputune values */ + /* these are not represented in the XML schema, but are documented */ + if (def->cputune.period > 0 && + (def->cputune.period < 1000 || def->cputune.period > 1000000)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune period must be in range " + "[1000, 1000000]")); + return -1; + } + + if (def->cputune.emulator_period > 0 && + (def->cputune.emulator_period < 1000 || + def->cputune.emulator_period > 1000000)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune emulator_period must be in range " + "[1000, 1000000]")); + return -1; + } + + if (def->cputune.quota > 0 && + (def->cputune.quota < 1000 || + def->cputune.quota > 18446744073709551)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune quota must be in range " + "[1000, 18446744073709551]")); + return -1; + } + + if (def->cputune.emulator_quota > 0 && + (def->cputune.emulator_quota < 1000 || + def->cputune.emulator_quota > 18446744073709551)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune emulator_quota must be in range " + "[1000, 18446744073709551]")); + return -1; + } + return 0; } -- 1.8.1.5

On 03/15/2013 11:26 AM, Peter Krempa wrote:
The limits are documented at http://libvirt.org/formatdomain.html#elementsCPUTuning . Enforce them when going through XML parsing in addition to being enforced by the API.
What's the rationale for doing this validation during the post-parse rather than just doing it as the cputune elements are being parsed? They don't depend on any device that might be modified during a post-parse callback (or any other unrelated part of the domain). My opinion is that a separate post-parse validation should only be done if: 1) the validation depends on hypervisor (in which case it will be done in a hypervisor-specific callback) 2) the validation depends on some other element of the domain object (in which case it would be done in virDomainDefPostParseInternal, as you've done here) or 2a) the validation depends on some other element of the domain that could be changed by a hypervisor-specific post-parse validation function. Doing it in a separate function when none of the above is true just has the effect of spreading out the parsing of a single element into multiple places, making it more difficult to understand and maintain the code.
---
Notes: Version 4: - changed error from VIR_ERR_XML_ERROR to VIR_ERR_CONFIG_UNSUPPORTED Version 3: - new in series
src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fde88b2..5a59e3f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2499,6 +2499,43 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; }
+ /* enforce range checks for cputune values */ + /* these are not represented in the XML schema, but are documented */ + if (def->cputune.period > 0 && + (def->cputune.period < 1000 || def->cputune.period > 1000000)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune period must be in range " + "[1000, 1000000]")); + return -1; + } + + if (def->cputune.emulator_period > 0 && + (def->cputune.emulator_period < 1000 || + def->cputune.emulator_period > 1000000)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune emulator_period must be in range " + "[1000, 1000000]")); + return -1; + } + + if (def->cputune.quota > 0 && + (def->cputune.quota < 1000 || + def->cputune.quota > 18446744073709551)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune quota must be in range " + "[1000, 18446744073709551]")); + return -1; + } + + if (def->cputune.emulator_quota > 0 && + (def->cputune.emulator_quota < 1000 || + def->cputune.emulator_quota > 18446744073709551)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Value of cputune emulator_quota must be in range " + "[1000, 18446744073709551]")); + return -1; + } + return 0; }

On 03/26/13 01:19, Laine Stump wrote:
On 03/15/2013 11:26 AM, Peter Krempa wrote:
The limits are documented at http://libvirt.org/formatdomain.html#elementsCPUTuning . Enforce them when going through XML parsing in addition to being enforced by the API.
What's the rationale for doing this validation during the post-parse rather than just doing it as the cputune elements are being parsed? They
I wanted to keep the parser clean of stuff that can't or isn't represented in the XML schema. In this case, this information is "enforced" by the docs and probably also could be represented in the schema. Thus it might be worth moving this to the parser.
don't depend on any device that might be modified during a post-parse callback (or any other unrelated part of the domain). My opinion is that a separate post-parse validation should only be done if:
1) the validation depends on hypervisor (in which case it will be done in a hypervisor-specific callback)
2) the validation depends on some other element of the domain object (in which case it would be done in virDomainDefPostParseInternal, as you've done here)
or
2a) the validation depends on some other element of the domain that could be changed by a hypervisor-specific post-parse validation function.
Doing it in a separate function when none of the above is true just has the effect of spreading out the parsing of a single element into multiple places, making it more difficult to understand and maintain the code.
I agree on those points and I'll move that stuff to the parser.
---
Notes: Version 4: - changed error from VIR_ERR_XML_ERROR to VIR_ERR_CONFIG_UNSUPPORTED Version 3: - new in series
src/conf/domain_conf.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
Peter

Use the qemu specific callback to fill this data in the qemu driver as it's the only place where it was used and fix tests as the qemu test capability object didn't configure the defaults for the tests. --- Notes: Version 4: - new in series src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 5 --- src/qemu/qemu_conf.c | 6 ---- src/qemu/qemu_domain.c | 42 ++++++++++++++++++---- .../qemuxml2argv-disk-drive-network-nbd.args | 5 +-- .../qemuxml2argv-disk-drive-network-nbd.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd-auth.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.args | 3 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 1 + 13 files changed, 47 insertions(+), 25 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index dcf38a8..22a7a3d 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -163,7 +163,6 @@ struct _virCaps { /* Move to virDomainXMLConf later */ unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; - int defaultDiskDriverType; /* enum virStorageFileFormat */ int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); bool hasWideScsiBus; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5a59e3f..6cebe67 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4862,9 +4862,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, driverType); goto error; } - } else if (def->type == VIR_DOMAIN_DISK_TYPE_FILE || - def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { - def->format = caps->defaultDiskDriverType; } if (mirrorFormat) { @@ -4875,8 +4872,6 @@ virDomainDiskDefParseXML(virCapsPtr caps, driverType); goto error; } - } else if (def->mirror) { - def->mirrorFormat = caps->defaultDiskDriverType; } if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index aab3375..c1ab3c9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -574,12 +574,6 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) if (!(caps = virQEMUCapsInit(driver->qemuCapsCache))) goto no_memory; - if (cfg->allowDiskFormatProbing) { - caps->defaultDiskDriverType = VIR_STORAGE_FILE_AUTO; - } else { - caps->defaultDiskDriverType = VIR_STORAGE_FILE_RAW; - } - if (virGetHostUUID(caps->host.host_uuid)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot get the host uuid")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 58f9d27..f7df4a9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -695,13 +695,41 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, goto no_memory; } - if (dev->type == VIR_DOMAIN_DEVICE_DISK && - !dev->data.disk->driverName && - driver && - (cfg = virQEMUDriverGetConfig(driver))) { - if (!cfg->allowDiskFormatProbing && - !(dev->data.disk->driverName = strdup("qemu"))) { - goto no_memory; + /* set default disk types and drivers */ + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + virDomainDiskDefPtr disk = dev->data.disk; + + /* both of these require data from the driver config */ + if (driver && (cfg = virQEMUDriverGetConfig(driver))) { + /* assign default storage format and driver according to config */ + if (cfg->allowDiskFormatProbing) { + /* default disk format for drives */ + if (disk->format == VIR_STORAGE_FILE_NONE && + (disk->type == VIR_DOMAIN_DISK_TYPE_FILE || + disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK)) + disk->format = VIR_STORAGE_FILE_AUTO; + + /* default disk format for mirrored drive */ + if (disk->mirror && + disk->mirrorFormat == VIR_STORAGE_FILE_NONE) + disk->mirrorFormat = VIR_STORAGE_FILE_AUTO; + } else { + /* default driver if probing is forbidden */ + if (!disk->driverName && + !(disk->driverName = strdup("qemu"))) + goto no_memory; + + /* default disk format for drives */ + if (disk->format == VIR_STORAGE_FILE_NONE && + (disk->type == VIR_DOMAIN_DISK_TYPE_FILE || + disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK)) + disk->format = VIR_STORAGE_FILE_RAW; + + /* default disk format for mirrored drive */ + if (disk->mirror && + disk->mirrorFormat == VIR_STORAGE_FILE_NONE) + disk->mirrorFormat = VIR_STORAGE_FILE_RAW; + } } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args index 856217b..8d0f69f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ -file=nbd:example.org:6000,if=virtio,format=raw -net none -serial none \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw \ +-drive file=nbd:example.org:6000,if=virtio,format=raw -net none -serial none \ -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml index e31da87..36301a9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args index 4f3ccd5..6714553 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ -file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw -drive \ 'file=rbd:pool/image:\ id=myname:\ key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args index 0c67229..6623161 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ -file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw -drive \ 'file=rbd:pool/image:auth_supported=none:\ mon_host=[\:\:1]\:6321\;example.org\:6789\;\ [ffff\:1234\:567\:abc\:\:0f]\:6322\;\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml index a2ca2d2..be4edbf 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args index ad3ed3e..cf433a3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args @@ -1,7 +1,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ -file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw -drive \ 'file=rbd:pool/image:auth_supported=none:\ mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\ mon3.example.org\:6322,\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml index 8309cae..081f9a6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args index 3e9d913..e0a5cfa 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,,,,1,if=ide,bus=0,unit=0 \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMU,,Guest,,,,1,if=ide,bus=0,unit=0,format=raw \ -drive file=sheepdog:example.org:6000:image,,with,,commas,if=virtio,format=raw \ -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml index 7917357..ac89dd7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMU,Guest,,1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> -- 1.8.1.5

On 03/15/13 16:26, Peter Krempa wrote:
Use the qemu specific callback to fill this data in the qemu driver as it's the only place where it was used and fix tests as the qemu test capability object didn't configure the defaults for the tests. ---
Notes: Version 4: - new in series
src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 5 --- src/qemu/qemu_conf.c | 6 ---- src/qemu/qemu_domain.c | 42 ++++++++++++++++++---- .../qemuxml2argv-disk-drive-network-nbd.args | 5 +-- .../qemuxml2argv-disk-drive-network-nbd.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd-auth.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.args | 3 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 1 + 13 files changed, 47 insertions(+), 25 deletions(-)
After recent Paolo's NBD patches this addition to the testsuite will be needed: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args index bc9d93d..ca70ce4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw \ -drive file=nbd:example.org:6000:exportname=bar,if=virtio,format=raw \ -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args index a942935..d103abf 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw \ -drive 'file=nbd+tcp://[::1]:6000/bar,if=virtio,format=raw' -net none \ -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args index 7cdbdd1..a03c4e8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw \ -drive 'file=nbd+tcp://[::1]:6000,if=virtio,format=raw' -net none \ -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args index 977b68f..84cae4a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw \ -drive file=nbd:unix:/var/run/nbdsock:exportname=bar,if=virtio,format=raw \ -net none -serial none -parallel none I already squashed that to my local tree. Peter

On 03/21/13 18:07, Peter Krempa wrote:
On 03/15/13 16:26, Peter Krempa wrote:
Use the qemu specific callback to fill this data in the qemu driver as it's the only place where it was used and fix tests as the qemu test capability object didn't configure the defaults for the tests. ---
Notes: Version 4: - new in series
src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 5 --- src/qemu/qemu_conf.c | 6 ---- src/qemu/qemu_domain.c | 42 ++++++++++++++++++---- .../qemuxml2argv-disk-drive-network-nbd.args | 5 +-- .../qemuxml2argv-disk-drive-network-nbd.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd-auth.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.args | 3 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 1 + 13 files changed, 47 insertions(+), 25 deletions(-)
After recent Paolo's NBD patches this addition to the testsuite will be needed:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args index bc9d93d..ca70ce4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -usb -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 \ +-no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0,format=raw \ -drive file=nbd:example.org:6000:exportname=bar,if=virtio,format=raw \ -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.args index a942935..d103abf 100644
and also the corresponding XML changes: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml index f2b5ca4..7a84604 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-export.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml index 595d7ea..c063db8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6-export.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml index 3c5c99d..540aa02 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-ipv6.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml index 46114d5..a4126f5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml @@ -15,6 +15,7 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> ...
I already squashed that to my local tree.
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 03/15/2013 09:26 AM, Peter Krempa wrote:
Use the qemu specific callback to fill this data in the qemu driver as it's the only place where it was used and fix tests as the qemu test capability object didn't configure the defaults for the tests. ---
Notes: Version 4: - new in series
src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 5 --- src/qemu/qemu_conf.c | 6 ---- src/qemu/qemu_domain.c | 42 ++++++++++++++++++---- .../qemuxml2argv-disk-drive-network-nbd.args | 5 +-- .../qemuxml2argv-disk-drive-network-nbd.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd-auth.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd.args | 2 +- .../qemuxml2argv-disk-drive-network-rbd.xml | 1 + .../qemuxml2argv-disk-drive-network-sheepdog.args | 3 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 1 + 13 files changed, 47 insertions(+), 25 deletions(-)
This plus your followups looks fine. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Use the virDomainXMLConf structure to hold this data. --- Notes: Version 4: - new in series src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 13 +++++++------ src/conf/domain_conf.h | 8 +++++++- src/esx/esx_driver.c | 12 +++++------- src/libvirt_vmx.syms | 2 ++ src/qemu/qemu_command.c | 9 +++++---- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 6 +++--- src/vmx/vmx.c | 38 ++++++++++++++++++++++++++------------ src/vmx/vmx.h | 12 +++++++----- tests/vmx2xmltest.c | 10 +++++++--- tests/xml2vmxtest.c | 7 +++---- 12 files changed, 73 insertions(+), 47 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 22a7a3d..f4cf8f3 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -164,7 +164,6 @@ struct _virCaps { /* Move to virDomainXMLConf later */ unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); - bool hasWideScsiBus; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6cebe67..cad5387 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3640,7 +3640,8 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def, } int -virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) +virDomainDiskDefAssignAddress(virDomainXMLConfPtr xmlconf, + virDomainDiskDefPtr def) { int idx = virDiskNameToIndex(def->dst); if (idx < 0) { @@ -3654,7 +3655,7 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) case VIR_DOMAIN_DISK_BUS_SCSI: def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - if (caps->hasWideScsiBus) { + if (xmlconf->config.hasWideScsiBus) { /* For a wide SCSI bus we define the default mapping to be * 16 units per bus, 1 bus per controller, many controllers. * Unit 7 is the SCSI controller itself. Therefore unit 7 @@ -4093,7 +4094,7 @@ cleanup: * @param node XML nodeset to parse for disk definition */ static virDomainDiskDefPtr -virDomainDiskDefParseXML(virCapsPtr caps, +virDomainDiskDefParseXML(virDomainXMLConfPtr xmlconf, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, @@ -4875,7 +4876,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, } if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE - && virDomainDiskDefAssignAddress(caps, def) < 0) + && virDomainDiskDefAssignAddress(xmlconf, def) < 0) goto error; cleanup: @@ -8537,7 +8538,7 @@ virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; - if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, + if (!(dev->data.disk = virDomainDiskDefParseXML(xmlconf, node, ctxt, NULL, def->seclabels, def->nseclabels, flags))) @@ -10392,7 +10393,7 @@ virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, + virDomainDiskDefPtr disk = virDomainDiskDefParseXML(xmlconf, nodes[i], ctxt, bootMap, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a82a432..a253438 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1965,11 +1965,16 @@ typedef int (*virDomainDeviceDefPostParseCallback)(virDomainDeviceDefPtr dev, typedef struct _virDomainDefParserConfig virDomainDefParserConfig; typedef virDomainDefParserConfig *virDomainDefParserConfigPtr; struct _virDomainDefParserConfig { + /* driver domain definition callbacks */ virDomainDefPostParseCallback domainConfigCallback; virDomainDeviceDefPostParseCallback devicesConfigCallback; + /* private data for the callbacks */ void *priv; virFreeCallback privFree; + + /* data */ + bool hasWideScsiBus; }; typedef struct _virDomainXMLPrivateDataCallbacks virDomainXMLPrivateDataCallbacks; @@ -2166,7 +2171,8 @@ int virDomainDiskInsert(virDomainDefPtr def, virDomainDiskDefPtr disk); void virDomainDiskInsertPreAlloced(virDomainDefPtr def, virDomainDiskDefPtr disk); -int virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def); +int virDomainDiskDefAssignAddress(virDomainXMLConfPtr xmlconf, + virDomainDiskDefPtr def); virDomainDiskDefPtr virDomainDiskRemove(virDomainDefPtr def, size_t i); diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 2bff60e..ff2a9b8 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -601,7 +601,6 @@ esxCapsInit(esxPrivate *priv) virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "vpxmigr"); - caps->hasWideScsiBus = true; caps->defaultConsoleTargetType = esxDefaultConsoleType; if (esxLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) { @@ -888,7 +887,6 @@ esxConnectToVCenter(esxPrivate *priv, } - /* * URI format: {vpx|esx|gsx}://[<username>@]<hostname>[:<port>]/[<path>][?<query parameter>...] * <path> = [<folder>/...]<datacenter>/[<folder>/...]<computeresource>[/<hostsystem>] @@ -1100,7 +1098,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth, goto cleanup; } - if (!(priv->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) + if (!(priv->xmlconf = virVMXDomainXMLConfInit())) goto cleanup; conn->privateData = priv; @@ -2786,7 +2784,7 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) ctx.formatFileName = NULL; ctx.autodetectSCSIControllerModel = NULL; - def = virVMXParseConfig(&ctx, priv->caps, vmx); + def = virVMXParseConfig(&ctx, priv->xmlconf, vmx); if (def != NULL) { if (powerState != esxVI_VirtualMachinePowerState_PoweredOff) { @@ -2845,7 +2843,7 @@ esxDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, ctx.formatFileName = NULL; ctx.autodetectSCSIControllerModel = NULL; - def = virVMXParseConfig(&ctx, priv->caps, nativeConfig); + def = virVMXParseConfig(&ctx, priv->xmlconf, nativeConfig); if (def != NULL) { xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); @@ -2902,7 +2900,7 @@ esxDomainXMLToNative(virConnectPtr conn, const char *nativeFormat, ctx.formatFileName = esxFormatVMXFileName; ctx.autodetectSCSIControllerModel = esxAutodetectSCSIControllerModel; - vmx = virVMXFormatConfig(&ctx, priv->caps, def, virtualHW_version); + vmx = virVMXFormatConfig(&ctx, priv->xmlconf, def, virtualHW_version); virDomainDefFree(def); @@ -3149,7 +3147,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) ctx.formatFileName = esxFormatVMXFileName; ctx.autodetectSCSIControllerModel = esxAutodetectSCSIControllerModel; - vmx = virVMXFormatConfig(&ctx, priv->caps, def, virtualHW_version); + vmx = virVMXFormatConfig(&ctx, priv->xmlconf, def, virtualHW_version); if (vmx == NULL) { goto cleanup; diff --git a/src/libvirt_vmx.syms b/src/libvirt_vmx.syms index 0b15f49..a29073f 100644 --- a/src/libvirt_vmx.syms +++ b/src/libvirt_vmx.syms @@ -4,6 +4,7 @@ # vmx/vmx.h virVMXConvertToUTF8; +virVMXDomainXMLConfInit; virVMXEscapeHex; virVMXFormatCDROM; virVMXFormatConfig; @@ -22,6 +23,7 @@ virVMXParseSerial; virVMXParseVNC; virVMXUnescapeHex; + # Let emacs know we want case-insensitive sorting # Local Variables: # sort-fold-case: t diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dc49d44..ea99d69 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7743,7 +7743,7 @@ error: * Will fail if not using the 'index' keyword */ static virDomainDiskDefPtr -qemuParseCommandLineDisk(virCapsPtr qemuCaps, +qemuParseCommandLineDisk(virDomainXMLConfPtr xmlconf, const char *val, int nvirtiodisk, bool old_style_ceph_args) @@ -8063,7 +8063,7 @@ qemuParseCommandLineDisk(virCapsPtr qemuCaps, else def->dst[2] = 'a' + idx; - if (virDomainDiskDefAssignAddress(qemuCaps, def) < 0) { + if (virDomainDiskDefAssignAddress(xmlconf, def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid device name '%s'"), def->dst); virDomainDiskDefFree(def); @@ -9147,7 +9147,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, !disk->dst) goto no_memory; - if (virDomainDiskDefAssignAddress(qemuCaps, disk) < 0) { + if (virDomainDiskDefAssignAddress(xmlconf, disk) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Cannot assign address for device name '%s'"), disk->dst); @@ -9369,7 +9369,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, } } else if (STREQ(arg, "-drive")) { WANT_VALUE(); - if (!(disk = qemuParseCommandLineDisk(qemuCaps, val, nvirtiodisk, + if (!(disk = qemuParseCommandLineDisk(xmlconf, val, + nvirtiodisk, ceph_args != NULL))) goto error; if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 19be62a..e5e8c40 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -174,7 +174,7 @@ vmwareLoadDomains(struct vmware_driver *driver) goto cleanup; if ((vmdef = - virVMXParseConfig(&ctx, driver->caps, vmx)) == NULL) { + virVMXParseConfig(&ctx, driver->xmlconf, vmx)) == NULL) { goto cleanup; } diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index bf4c1ff..683abd5 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -330,7 +330,7 @@ vmwareDomainDefineXML(virConnectPtr conn, const char *xml) goto cleanup; /* generate vmx file */ - vmx = virVMXFormatConfig(&ctx, driver->caps, vmdef, 7); + vmx = virVMXFormatConfig(&ctx, driver->xmlconf, vmdef, 7); if (vmx == NULL) goto cleanup; @@ -601,7 +601,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, goto cleanup; /* generate vmx file */ - vmx = virVMXFormatConfig(&ctx, driver->caps, vmdef, 7); + vmx = virVMXFormatConfig(&ctx, driver->xmlconf, vmdef, 7); if (vmx == NULL) goto cleanup; @@ -943,7 +943,7 @@ vmwareDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, ctx.parseFileName = vmwareCopyVMXFileName; - def = virVMXParseConfig(&ctx, driver->caps, nativeConfig); + def = virVMXParseConfig(&ctx, driver->xmlconf, nativeConfig); if (def != NULL) xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index c604bd2..1761a80 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -519,6 +519,18 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, * Helpers */ +virDomainDefParserConfig virVMXDomainDefParserConfig = { + .hasWideScsiBus = true, +}; + + +virDomainXMLConfPtr +virVMXDomainXMLConfInit(void) +{ + return virDomainXMLConfNew(&virVMXDomainDefParserConfig, + NULL, NULL); +} + char * virVMXEscapeHex(const char *string, char escape, const char *special) { @@ -934,7 +946,7 @@ virVMXFloppyDiskNameToUnit(const char *name, int *unit) static int -virVMXVerifyDiskAddress(virCapsPtr caps, virDomainDiskDefPtr disk) +virVMXVerifyDiskAddress(virDomainXMLConfPtr xmlconf, virDomainDiskDefPtr disk) { virDomainDiskDef def; virDomainDeviceDriveAddressPtr drive; @@ -953,7 +965,7 @@ virVMXVerifyDiskAddress(virCapsPtr caps, virDomainDiskDefPtr disk) def.dst = disk->dst; def.bus = disk->bus; - if (virDomainDiskDefAssignAddress(caps, &def) < 0) { + if (virDomainDiskDefAssignAddress(xmlconf, &def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not verify disk address")); return -1; @@ -1210,7 +1222,9 @@ virVMXGatherSCSIControllers(virVMXContext *ctx, virDomainDefPtr def, */ virDomainDefPtr -virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) +virVMXParseConfig(virVMXContext *ctx, + virDomainXMLConfPtr xmlconf, + const char *vmx) { bool success = false; virConfPtr conf = NULL; @@ -1587,7 +1601,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) continue; } - if (virVMXParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_DISK, + if (virVMXParseDisk(ctx, xmlconf, conf, VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_BUS_SCSI, controller, unit, &def->disks[def->ndisks]) < 0) { goto cleanup; @@ -1598,7 +1612,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) continue; } - if (virVMXParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, + if (virVMXParseDisk(ctx, xmlconf, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, VIR_DOMAIN_DISK_BUS_SCSI, controller, unit, &def->disks[def->ndisks]) < 0) { goto cleanup; @@ -1613,7 +1627,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) /* def:disks (ide) */ for (bus = 0; bus < 2; ++bus) { for (unit = 0; unit < 2; ++unit) { - if (virVMXParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_DISK, + if (virVMXParseDisk(ctx, xmlconf, conf, VIR_DOMAIN_DISK_DEVICE_DISK, VIR_DOMAIN_DISK_BUS_IDE, bus, unit, &def->disks[def->ndisks]) < 0) { goto cleanup; @@ -1624,7 +1638,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) continue; } - if (virVMXParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, + if (virVMXParseDisk(ctx, xmlconf, conf, VIR_DOMAIN_DISK_DEVICE_CDROM, VIR_DOMAIN_DISK_BUS_IDE, bus, unit, &def->disks[def->ndisks]) < 0) { goto cleanup; @@ -1638,7 +1652,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char *vmx) /* def:disks (floppy) */ for (unit = 0; unit < 2; ++unit) { - if (virVMXParseDisk(ctx, caps, conf, VIR_DOMAIN_DISK_DEVICE_FLOPPY, + if (virVMXParseDisk(ctx, xmlconf, conf, VIR_DOMAIN_DISK_DEVICE_FLOPPY, VIR_DOMAIN_DISK_BUS_FDC, 0, unit, &def->disks[def->ndisks]) < 0) { goto cleanup; @@ -1949,7 +1963,7 @@ virVMXParseSCSIController(virConfPtr conf, int controller, bool *present, int -virVMXParseDisk(virVMXContext *ctx, virCapsPtr caps, virConfPtr conf, +virVMXParseDisk(virVMXContext *ctx, virDomainXMLConfPtr xmlconf, virConfPtr conf, int device, int busType, int controllerOrBus, int unit, virDomainDiskDefPtr *def) { @@ -2283,7 +2297,7 @@ virVMXParseDisk(virVMXContext *ctx, virCapsPtr caps, virConfPtr conf, goto cleanup; } - if (virDomainDiskDefAssignAddress(caps, *def) < 0) { + if (virDomainDiskDefAssignAddress(xmlconf, *def) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not assign address to disk '%s'"), (*def)->src); goto cleanup; @@ -3021,7 +3035,7 @@ virVMXParseSVGA(virConfPtr conf, virDomainVideoDefPtr *def) */ char * -virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, +virVMXFormatConfig(virVMXContext *ctx, virDomainXMLConfPtr xmlconf, virDomainDefPtr def, int virtualHW_version) { char *vmx = NULL; @@ -3230,7 +3244,7 @@ virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, virDomainDefPtr def, /* def:disks */ for (i = 0; i < def->ndisks; ++i) { - if (virVMXVerifyDiskAddress(caps, def->disks[i]) < 0 || + if (virVMXVerifyDiskAddress(xmlconf, def->disks[i]) < 0 || virVMXHandleLegacySCSIDiskDriverName(def, def->disks[i]) < 0) { goto cleanup; } diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h index f4877b1..b9d623d 100644 --- a/src/vmx/vmx.h +++ b/src/vmx/vmx.h @@ -29,6 +29,7 @@ typedef struct _virVMXContext virVMXContext; +virDomainXMLConfPtr virVMXDomainXMLConfInit(void); /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * @@ -78,7 +79,8 @@ char *virVMXConvertToUTF8(const char *encoding, const char *string); * VMX -> Domain XML */ -virDomainDefPtr virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, +virDomainDefPtr virVMXParseConfig(virVMXContext *ctx, + virDomainXMLConfPtr xmlconf, const char *vmx); int virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def); @@ -86,9 +88,9 @@ int virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def); int virVMXParseSCSIController(virConfPtr conf, int controller, bool *present, int *virtualDev); -int virVMXParseDisk(virVMXContext *ctx, virCapsPtr caps, virConfPtr conf, - int device, int busType, int controllerOrBus, int unit, - virDomainDiskDefPtr *def); +int virVMXParseDisk(virVMXContext *ctx, virDomainXMLConfPtr xmlconf, + virConfPtr conf, int device, int busType, + int controllerOrBus, int unit, virDomainDiskDefPtr *def); int virVMXParseFileSystem(virConfPtr conf, int number, virDomainFSDefPtr *def); @@ -108,7 +110,7 @@ int virVMXParseSVGA(virConfPtr conf, virDomainVideoDefPtr *def); * Domain XML -> VMX */ -char *virVMXFormatConfig(virVMXContext *ctx, virCapsPtr caps, +char *virVMXFormatConfig(virVMXContext *ctx, virDomainXMLConfPtr xmlconf, virDomainDefPtr def, int virtualHW_version); int virVMXFormatVNC(virDomainGraphicsDefPtr def, virBufferPtr buffer); diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index cee4c39..d73c6d2 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -12,6 +12,7 @@ # include "vmx/vmx.h" static virCapsPtr caps; +static virDomainXMLConfPtr xmlconf; static virVMXContext ctx; static int testDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, @@ -20,6 +21,7 @@ static int testDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } + static void testCapsInit(void) { @@ -36,8 +38,6 @@ testCapsInit(void) virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); - caps->hasWideScsiBus = true; - /* i686 guest */ guest = virCapabilitiesAddGuest(caps, "hvm", @@ -93,7 +93,7 @@ testCompareFiles(const char *vmx, const char *xml) goto failure; } - def = virVMXParseConfig(&ctx, caps, vmxData); + def = virVMXParseConfig(&ctx, xmlconf, vmxData); if (def == NULL) { err = virGetLastError(); @@ -221,6 +221,9 @@ mymain(void) return EXIT_FAILURE; } + if (!(xmlconf = virVMXDomainXMLConfInit())) + return EXIT_FAILURE; + ctx.opaque = NULL; ctx.parseFileName = testParseVMXFileName; ctx.formatFileName = NULL; @@ -296,6 +299,7 @@ mymain(void) DO_TEST("svga", "svga"); virObjectUnref(caps); + virObjectUnref(xmlconf); return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index c606036..303cb22 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -37,7 +37,6 @@ testCapsInit(void) virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); - caps->hasWideScsiBus = true; /* i686 guest */ guest = @@ -73,7 +72,6 @@ testCapsInit(void) failure: virObjectUnref(caps); - virObjectUnref(xmlconf); caps = NULL; } @@ -102,7 +100,7 @@ testCompareFiles(const char *xml, const char *vmx, int virtualHW_version) goto failure; } - formatted = virVMXFormatConfig(&ctx, caps, def, virtualHW_version); + formatted = virVMXFormatConfig(&ctx, xmlconf, def, virtualHW_version); if (formatted == NULL) { goto failure; @@ -240,7 +238,7 @@ mymain(void) return EXIT_FAILURE; } - if (!(xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) + if (!(xmlconf = virVMXDomainXMLConfInit())) return EXIT_FAILURE; ctx.opaque = NULL; @@ -312,6 +310,7 @@ mymain(void) DO_TEST("svga", "svga", 4); virObjectUnref(caps); + virObjectUnref(xmlconf); return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.1.5

On 03/15/2013 09:26 AM, Peter Krempa wrote:
Use the virDomainXMLConf structure to hold this data. ---
Notes: Version 4: - new in series
src/conf/capabilities.h | 1 - src/conf/domain_conf.c | 13 +++++++------ src/conf/domain_conf.h | 8 +++++++- src/esx/esx_driver.c | 12 +++++------- src/libvirt_vmx.syms | 2 ++ src/qemu/qemu_command.c | 9 +++++---- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 6 +++--- src/vmx/vmx.c | 38 ++++++++++++++++++++++++++------------ src/vmx/vmx.h | 12 +++++++----- tests/vmx2xmltest.c | 10 +++++++--- tests/xml2vmxtest.c | 7 +++---- 12 files changed, 73 insertions(+), 47 deletions(-)
ACK (modulo any naming changes propagating from earlier in the series on a respin). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Use the virDomainXMLConf structure to hold this data and tweak the code to avoid semantic change. Without configuration the KVM mac prefix is used by default. I chose it as it's in the privately administered segment so it should be usable for any purposes. --- Notes: Version 4: - new in series src/conf/capabilities.c | 14 -------------- src/conf/capabilities.h | 9 --------- src/conf/domain_conf.c | 26 ++++++++++++++++++++++---- src/conf/domain_conf.h | 3 +++ src/esx/esx_driver.c | 1 - src/libvirt_private.syms | 3 +-- src/libxl/libxl_conf.c | 2 -- src/libxl/libxl_driver.c | 6 +++++- src/lxc/lxc_conf.c | 3 --- src/openvz/openvz_conf.c | 2 -- src/openvz/openvz_driver.c | 2 +- src/parallels/parallels_driver.c | 12 ++++++++---- src/phyp/phyp_driver.c | 4 ---- src/qemu/qemu_capabilities.c | 3 --- src/qemu/qemu_command.c | 6 +++--- src/vbox/vbox_tmpl.c | 10 +++++++--- src/vmware/vmware_conf.c | 2 -- src/vmx/vmx.c | 1 + src/xen/xen_driver.c | 7 ++++++- src/xen/xen_hypervisor.c | 2 -- tests/vmx2xmltest.c | 1 - tests/xml2vmxtest.c | 1 - 22 files changed, 57 insertions(+), 63 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 1d29ce6..c7ec92f 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -921,17 +921,3 @@ virCapabilitiesFormatXML(virCapsPtr caps) return virBufferContentAndReset(&xml); } - -extern void -virCapabilitiesSetMacPrefix(virCapsPtr caps, - const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN]) -{ - memcpy(caps->macPrefix, prefix, sizeof(caps->macPrefix)); -} - -extern void -virCapabilitiesGenerateMac(virCapsPtr caps, - virMacAddrPtr mac) -{ - virMacAddrGenerate(caps->macPrefix, mac); -} diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index f4cf8f3..6b65e6a 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -162,7 +162,6 @@ struct _virCaps { virCapsGuestPtr *guests; /* Move to virDomainXMLConf later */ - unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); }; @@ -175,14 +174,6 @@ virCapabilitiesNew(virArch hostarch, extern void virCapabilitiesFreeNUMAInfo(virCapsPtr caps); -extern void -virCapabilitiesSetMacPrefix(virCapsPtr caps, - const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN]); - -extern void -virCapabilitiesGenerateMac(virCapsPtr caps, - virMacAddrPtr mac); - extern int virCapabilitiesAddHostFeature(virCapsPtr caps, const char *name); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cad5387..a26bc7b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -800,6 +800,16 @@ virDomainXMLConfNew(virDomainDefParserConfigPtr config, if (xmlns) xmlconf->ns = *xmlns; + /* Technically this forbids to use one of Xerox's MAC address prefixes in + * our hypervisor drivers. This shouldn't ever be a problem. + * + * Use the KVM prefix as default as it's in the privately administered + * range */ + if (memcmp(xmlconf->config.macPrefix, + (unsigned char[]) {0x00, 0x00, 0x00}, 3)) + memcpy(xmlconf->config.macPrefix, + (unsigned char[]) {0x54, 0x52, 0x00}, 3); + return xmlconf; } @@ -5077,6 +5087,14 @@ cleanup: } +void +virDomainNetGenerateMAC(virDomainXMLConfPtr xmlconf, + virMacAddrPtr mac) +{ + virMacAddrGenerate(xmlconf->config.macPrefix, mac); +} + + /* Parse a value located at XPATH within CTXT, and store the * result into val. If REQUIRED, then the value must exist; * otherwise, the value is optional. The value is in bytes. @@ -5445,7 +5463,7 @@ error: * @return 0 on success, -1 on failure */ static virDomainNetDefPtr -virDomainNetDefParseXML(virCapsPtr caps, +virDomainNetDefParseXML(virDomainXMLConfPtr xmlconf, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, @@ -5630,7 +5648,7 @@ virDomainNetDefParseXML(virCapsPtr caps, goto error; } } else { - virCapabilitiesGenerateMac(caps, &def->mac); + virDomainNetGenerateMAC(xmlconf, &def->mac); } if (devaddr) { @@ -8553,7 +8571,7 @@ virDomainDeviceDefParse(virCapsPtr caps, goto error; } else if (xmlStrEqual(node->name, BAD_CAST "interface")) { dev->type = VIR_DOMAIN_DEVICE_NET; - if (!(dev->data.net = virDomainNetDefParseXML(caps, node, ctxt, + if (!(dev->data.net = virDomainNetDefParseXML(xmlconf, node, ctxt, NULL, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "input")) { @@ -10493,7 +10511,7 @@ virDomainDefParseXML(virCapsPtr caps, if (n && VIR_ALLOC_N(def->nets, n) < 0) goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainNetDefPtr net = virDomainNetDefParseXML(caps, + virDomainNetDefPtr net = virDomainNetDefParseXML(xmlconf, nodes[i], ctxt, bootMap, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a253438..6f24528 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1975,6 +1975,7 @@ struct _virDomainDefParserConfig { /* data */ bool hasWideScsiBus; + unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; }; typedef struct _virDomainXMLPrivateDataCallbacks virDomainXMLPrivateDataCallbacks; @@ -1991,6 +1992,8 @@ virDomainXMLConfPtr virDomainXMLConfNew(virDomainDefParserConfigPtr config, virDomainXMLPrivateDataCallbacksPtr priv, virDomainXMLNamespacePtr xmlns); +void virDomainNetGenerateMAC(virDomainXMLConfPtr xmlconf, virMacAddrPtr mac); + virDomainXMLNamespacePtr virDomainXMLConfGetNamespace(virDomainXMLConfPtr xmlconf) ATTRIBUTE_NONNULL(1); diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index ff2a9b8..aa6f8c1 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -598,7 +598,6 @@ esxCapsInit(esxPrivate *priv) return NULL; } - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "vpxmigr"); caps->defaultConsoleTargetType = esxDefaultConsoleType; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fb05d59..5326766 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -20,10 +20,8 @@ virCapabilitiesDefaultGuestMachine; virCapabilitiesFormatXML; virCapabilitiesFreeMachines; virCapabilitiesFreeNUMAInfo; -virCapabilitiesGenerateMac; virCapabilitiesNew; virCapabilitiesSetHostCPU; -virCapabilitiesSetMacPrefix; # conf/cpu_conf.h @@ -238,6 +236,7 @@ virDomainMemDumpTypeToString; virDomainNetDefFree; virDomainNetFind; virDomainNetFindIdx; +virDomainNetGenerateMAC; virDomainNetGetActualBandwidth; virDomainNetGetActualBridgeName; virDomainNetGetActualDirectDev; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b208dd8..ed3e832 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -85,8 +85,6 @@ libxlBuildCapabilities(virArch hostarch, if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x16, 0x3e }); - if (host_pae && virCapabilitiesAddHostFeature(caps, "pae") < 0) goto no_memory; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index fd69637..cfc2cfe 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -431,6 +431,10 @@ virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = { .free = libxlDomainObjPrivateFree, }; +virDomainDefParserConfig libxlDomainDefParserConfig = { + .macPrefix = { 0x00, 0x16, 0x3e }, +}; + /* driver must be locked before calling */ static void libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) @@ -1239,7 +1243,7 @@ libxlStartup(bool privileged, goto error; } - if (!(libxl_driver->xmlconf = virDomainXMLConfNew(NULL, + if (!(libxl_driver->xmlconf = virDomainXMLConfNew(&libxlDomainDefParserConfig, &libxlDomainXMLPrivateDataCallbacks, NULL))) goto error; diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 71b8916..d835139 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -79,9 +79,6 @@ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) goto error; } - /* XXX shouldn't 'borrow' KVM's prefix */ - virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 }); - if ((guest = virCapabilitiesAddGuest(caps, "exe", caps->host.arch, diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 05c6113..83d8d18 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -187,8 +187,6 @@ virCapsPtr openvzCapsInit(void) if (nodeCapsInitNUMA(caps) < 0) goto no_memory; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); - if ((guest = virCapabilitiesAddGuest(caps, "exe", caps->host.arch, diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index de6198c..350c8c7 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -832,7 +832,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, } virMacAddrFormat(&net->mac, macaddr); - virCapabilitiesGenerateMac(driver->caps, &host_mac); + virDomainNetGenerateMAC(driver->xmlconf, &host_mac); virMacAddrFormat(&host_mac, host_macaddr); if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ffb86dc..b137b66 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -129,9 +129,6 @@ parallelsBuildCapabilities(void) if (nodeCapsInitNUMA(caps) < 0) goto no_memory; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]) { - 0x42, 0x1C, 0x00}); - if ((guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_X86_64, "parallels", @@ -911,6 +908,12 @@ parallelsLoadDomains(parallelsConnPtr privconn, const char *domain_name) return ret; } + +virDomainDefParserConfig parallelsDomainDefParserConfig = { + .macPrefix = {0x42, 0x1C, 0x00}, +}; + + static int parallelsOpenDefault(virConnectPtr conn) { @@ -929,7 +932,8 @@ parallelsOpenDefault(virConnectPtr conn) if (!(privconn->caps = parallelsBuildCapabilities())) goto error; - if (!(privconn->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) + if (!(privconn->xmlconf = virDomainXMLConfNew(¶llelsDomainDefParserConfig, + NULL, NULL))) goto error; if (!(privconn->domains = virDomainObjListNew())) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 6063256..8b3fdb6 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -327,10 +327,6 @@ phypCapsInit(void) ("Failed to query host NUMA topology, disabling NUMA capabilities"); } - /* XXX shouldn't 'borrow' KVM's prefix */ - virCapabilitiesSetMacPrefix(caps, (unsigned char[]) { - 0x52, 0x54, 0x00}); - if ((guest = virCapabilitiesAddGuest(caps, "linux", caps->host.arch, diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ef7092..f3517c1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -871,9 +871,6 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache) 1, 1)) == NULL) goto error; - /* Using KVM's mac prefix for QEMU too */ - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); - /* Some machines have problematic NUMA toplogy causing * unexpected failures. We don't want to break the QEMU * driver in this scenario, so log errors & carry on diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea99d69..aba6eb1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8129,7 +8129,7 @@ qemuFindNICForVLAN(int nnics, * match up against. Horribly complicated stuff */ static virDomainNetDefPtr -qemuParseCommandLineNet(virCapsPtr qemuCaps, +qemuParseCommandLineNet(virDomainXMLConfPtr xmlconf, const char *val, int nnics, const char **nics) @@ -8263,7 +8263,7 @@ qemuParseCommandLineNet(virCapsPtr qemuCaps, } if (genmac) - virCapabilitiesGenerateMac(qemuCaps, &def->mac); + virDomainNetGenerateMAC(xmlconf, &def->mac); cleanup: for (i = 0 ; i < nkeywords ; i++) { @@ -9359,7 +9359,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, WANT_VALUE(); if (!STRPREFIX(val, "nic") && STRNEQ(val, "none")) { virDomainNetDefPtr net; - if (!(net = qemuParseCommandLineNet(qemuCaps, val, nnics, nics))) + if (!(net = qemuParseCommandLineNet(xmlconf, val, nnics, nics))) goto error; if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) { virDomainNetDefFree(net); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index dd96e7b..6f2dcf0 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -851,10 +851,16 @@ static int vboxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, } +static virDomainDefParserConfig vboxDomainDefParserConfig = { + .macPrefix = { 0x08, 0x00, 0x27 }, +}; + + static virDomainXMLConfPtr vboxXMLConfInit(void) { - return virDomainXMLConfNew(NULL, NULL, NULL); + return virDomainXMLConfNew(&vboxDomainDefParserConfig, + NULL, NULL); } @@ -870,8 +876,6 @@ static virCapsPtr vboxCapsInit(void) if (nodeCapsInitNUMA(caps) < 0) goto no_memory; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x08, 0x00, 0x27 }); - if ((guest = virCapabilitiesAddGuest(caps, "hvm", caps->host.arch, diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index e5e8c40..ab06c66 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -72,8 +72,6 @@ vmwareCapsInit(void) if (nodeCapsInitNUMA(caps) < 0) goto error; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]) {0x52, 0x54, 0x00}); - /* i686 guests are always supported */ if ((guest = virCapabilitiesAddGuest(caps, "hvm", diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 1761a80..a211eb6 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -521,6 +521,7 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, virDomainDefParserConfig virVMXDomainDefParserConfig = { .hasWideScsiBus = true, + .macPrefix = {0x00, 0x0c, 0x29}, }; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 2ef3609..b13dcec 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -265,6 +265,10 @@ xenUnifiedXendProbe(void) #endif +virDomainDefParserConfig xenDomainDefParserConfig = { + .macPrefix = { 0x00, 0x16, 0x3e }, +}; + static virDrvOpenStatus xenUnifiedOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) @@ -401,7 +405,8 @@ xenUnifiedOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) goto fail; } - if (!(priv->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) + if (!(priv->xmlconf = virDomainXMLConfNew(&xenDomainDefParserConfig, + NULL, NULL))) goto fail; #if WITH_XEN_INOTIFY diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index d803972..69bc6cd 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2303,8 +2303,6 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, virArch hostarch, if ((caps = virCapabilitiesNew(hostarch, 1, 1)) == NULL) goto no_memory; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x16, 0x3e }); - if (hvm_type && STRNEQ(hvm_type, "") && virCapabilitiesAddHostFeature(caps, hvm_type) < 0) goto no_memory; diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index d73c6d2..7f2c73b 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -35,7 +35,6 @@ testCapsInit(void) caps->defaultConsoleTargetType = testDefaultConsoleType; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); /* i686 guest */ diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index 303cb22..b62d214 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -34,7 +34,6 @@ testCapsInit(void) caps->defaultConsoleTargetType = testDefaultConsoleType; - virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 }); virCapabilitiesAddHostMigrateTransport(caps, "esx"); -- 1.8.1.5

On 03/15/2013 09:26 AM, Peter Krempa wrote:
Use the virDomainXMLConf structure to hold this data and tweak the code to avoid semantic change.
Without configuration the KVM mac prefix is used by default. I chose it as it's in the privately administered segment so it should be usable for any purposes. ---
Notes: Version 4: - new in series
src/conf/capabilities.c | 14 -------------- src/conf/capabilities.h | 9 --------- src/conf/domain_conf.c | 26 ++++++++++++++++++++++---- src/conf/domain_conf.h | 3 +++ src/esx/esx_driver.c | 1 - src/libvirt_private.syms | 3 +-- src/libxl/libxl_conf.c | 2 -- src/libxl/libxl_driver.c | 6 +++++- src/lxc/lxc_conf.c | 3 --- src/openvz/openvz_conf.c | 2 -- src/openvz/openvz_driver.c | 2 +- src/parallels/parallels_driver.c | 12 ++++++++---- src/phyp/phyp_driver.c | 4 ---- src/qemu/qemu_capabilities.c | 3 --- src/qemu/qemu_command.c | 6 +++--- src/vbox/vbox_tmpl.c | 10 +++++++--- src/vmware/vmware_conf.c | 2 -- src/vmx/vmx.c | 1 + src/xen/xen_driver.c | 7 ++++++- src/xen/xen_hypervisor.c | 2 -- tests/vmx2xmltest.c | 1 - tests/xml2vmxtest.c | 1 - 22 files changed, 57 insertions(+), 63 deletions(-)
+++ b/src/conf/domain_conf.c @@ -800,6 +800,16 @@ virDomainXMLConfNew(virDomainDefParserConfigPtr config, if (xmlns) xmlconf->ns = *xmlns;
+ /* Technically this forbids to use one of Xerox's MAC address prefixes in + * our hypervisor drivers. This shouldn't ever be a problem. + * + * Use the KVM prefix as default as it's in the privately administered + * range */ + if (memcmp(xmlconf->config.macPrefix, + (unsigned char[]) {0x00, 0x00, 0x00}, 3)) + memcpy(xmlconf->config.macPrefix, + (unsigned char[]) {0x54, 0x52, 0x00}, 3);
I don't see this C99 construct used very often; would it be any more straightforward to write: if (xmlconf->conf.macPrefix[0] == 0 && xmlconf->conf.macPrefix[1] == 0 && xmlconf->conf.macPrefix[2] == 0) { xmlconf->conf.macPrefix[0] = 0x54; xmlconf->conf.macPrefix[1] = 0x52; } But that's bikeshedding, so you don't necessarily have to agree. More importantly, why the magic number '3', instead of...
+++ b/src/conf/domain_conf.h @@ -1975,6 +1975,7 @@ struct _virDomainDefParserConfig {
/* data */ bool hasWideScsiBus; + unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN];
...the symbolic constant VIR_MAC_PREFIX_BUFLEN?
+++ b/src/esx/esx_driver.c @@ -598,7 +598,6 @@ esxCapsInit(esxPrivate *priv) return NULL; }
- virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 });
You are removing this prefix from esx://, ...
+virDomainDefParserConfig libxlDomainDefParserConfig = { + .macPrefix = { 0x00, 0x16, 0x3e }, +}; +
- /* XXX shouldn't 'borrow' KVM's prefix */ - virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 });
This is a change to the default lxc:// prefix; since that is a semantic change, it should probably be a separate patch.
+++ b/src/vmx/vmx.c @@ -521,6 +521,7 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
virDomainDefParserConfig virVMXDomainDefParserConfig = { .hasWideScsiBus = true, + .macPrefix = {0x00, 0x0c, 0x29}, };
...but adding it to vmx://. Are you sure you did this correctly? I think you need to fix the esx vs. vmx issue in v5. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/29/13 18:39, Eric Blake wrote:
On 03/15/2013 09:26 AM, Peter Krempa wrote:
Use the virDomainXMLConf structure to hold this data and tweak the code to avoid semantic change.
Without configuration the KVM mac prefix is used by default. I chose it as it's in the privately administered segment so it should be usable for any purposes. ---
Notes: Version 4: - new in series
src/conf/capabilities.c | 14 -------------- src/conf/capabilities.h | 9 --------- src/conf/domain_conf.c | 26 ++++++++++++++++++++++---- src/conf/domain_conf.h | 3 +++ src/esx/esx_driver.c | 1 - src/libvirt_private.syms | 3 +-- src/libxl/libxl_conf.c | 2 -- src/libxl/libxl_driver.c | 6 +++++- src/lxc/lxc_conf.c | 3 --- src/openvz/openvz_conf.c | 2 -- src/openvz/openvz_driver.c | 2 +- src/parallels/parallels_driver.c | 12 ++++++++---- src/phyp/phyp_driver.c | 4 ---- src/qemu/qemu_capabilities.c | 3 --- src/qemu/qemu_command.c | 6 +++--- src/vbox/vbox_tmpl.c | 10 +++++++--- src/vmware/vmware_conf.c | 2 -- src/vmx/vmx.c | 1 + src/xen/xen_driver.c | 7 ++++++- src/xen/xen_hypervisor.c | 2 -- tests/vmx2xmltest.c | 1 - tests/xml2vmxtest.c | 1 - 22 files changed, 57 insertions(+), 63 deletions(-)
+++ b/src/conf/domain_conf.c @@ -800,6 +800,16 @@ virDomainXMLConfNew(virDomainDefParserConfigPtr config, if (xmlns) xmlconf->ns = *xmlns;
+ /* Technically this forbids to use one of Xerox's MAC address prefixes in + * our hypervisor drivers. This shouldn't ever be a problem. + * + * Use the KVM prefix as default as it's in the privately administered + * range */ + if (memcmp(xmlconf->config.macPrefix, + (unsigned char[]) {0x00, 0x00, 0x00}, 3)) + memcpy(xmlconf->config.macPrefix, + (unsigned char[]) {0x54, 0x52, 0x00}, 3);
Regarding a comment later in the review. This line is flawed and the default and most commonly used prefix should be 0x52, 0x54. ...
I don't see this C99 construct used very often; would it be any more straightforward to write:
if (xmlconf->conf.macPrefix[0] == 0 && xmlconf->conf.macPrefix[1] == 0 && xmlconf->conf.macPrefix[2] == 0) { xmlconf->conf.macPrefix[0] = 0x54; xmlconf->conf.macPrefix[1] = 0x52; }
But that's bikeshedding, so you don't necessarily have to agree.
More importantly, why the magic number '3', instead of...
+++ b/src/conf/domain_conf.h @@ -1975,6 +1975,7 @@ struct _virDomainDefParserConfig {
/* data */ bool hasWideScsiBus; + unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN];
...the symbolic constant VIR_MAC_PREFIX_BUFLEN?
+++ b/src/esx/esx_driver.c @@ -598,7 +598,6 @@ esxCapsInit(esxPrivate *priv) return NULL; }
- virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x00, 0x0c, 0x29 });
You are removing this prefix from esx://, ...
esx uses the VMX backend to do the parsing and other stuff ... [2]
+virDomainDefParserConfig libxlDomainDefParserConfig = { + .macPrefix = { 0x00, 0x16, 0x3e }, +}; +
- /* XXX shouldn't 'borrow' KVM's prefix */ - virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 });
This is a change to the default lxc:// prefix; since that is a semantic change, it should probably be a separate patch.
... thus this doesn't make a semantic change. The funny stuff is that the test suite doesn't care about this kind of mistake ...
+++ b/src/vmx/vmx.c @@ -521,6 +521,7 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
virDomainDefParserConfig virVMXDomainDefParserConfig = { .hasWideScsiBus = true, + .macPrefix = {0x00, 0x0c, 0x29}, };
...but adding it to vmx://. Are you sure you did this correctly?
[2] ... so adding here is in fact accurate and also the changes in the tests confirm this.
I think you need to fix the esx vs. vmx issue in v5.
Peter

On 03/15/13 16:26, Peter Krempa wrote:
This series now splits out almost everything from the virCaps object (except for the defaultConsoleTargetType callback that I will post later as It requires more tweaking).
See notes in individual patches for change summary.
Ping? Anybody that could spare some time to do a review? Thanks. Peter
Peter Krempa (9): conf: Add post XML parse callbacks and prepare for cleaning of virCaps qemu: Record the default NIC model in the domain XML virCaps: get rid of "defaultInitPath" value in the virCaps struct virCaps: get rid of defaultDiskDriverName virCaps: get rid of emulatorRequired conf: Enforce ranges on cputune variables virCaps: remove defaultDiskDriverType from the struct virCaps: Get rid of hasWideScsiBus virCaps: get rid of macPrefix field

This patch refactors various places to allow removing of the defaultConsoleTargetType callback from the virCaps structure. A new console character device target type is introduced - VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE - to mark that no type was specified in the XML. This type is at the end converted to the standard VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL. Other types that are different from this default have to be processed separately in the device post parse callback. --- Notes: Version 4: - new in series src/conf/capabilities.h | 3 - src/conf/domain_conf.c | 269 +++++++++++++++++++++------------------ src/conf/domain_conf.h | 3 +- src/esx/esx_driver.c | 8 -- src/libxl/libxl_conf.c | 11 -- src/libxl/libxl_driver.c | 19 +++ src/lxc/lxc_conf.c | 8 -- src/lxc/lxc_domain.c | 17 +++ src/openvz/openvz_conf.c | 10 +- src/openvz/openvz_driver.c | 16 +++ src/parallels/parallels_driver.c | 7 - src/phyp/phyp_driver.c | 9 -- src/qemu/qemu_capabilities.c | 13 -- src/qemu/qemu_domain.c | 7 + src/security/virt-aa-helper.c | 7 - src/test/test_driver.c | 9 -- src/uml/uml_conf.c | 9 -- src/uml/uml_driver.c | 24 +++- src/vbox/vbox_tmpl.c | 9 -- src/vmware/vmware_conf.c | 9 -- src/xen/xen_driver.c | 28 +++- src/xen/xen_driver.h | 2 + src/xen/xen_hypervisor.c | 11 -- src/xenapi/xenapi_driver.c | 28 ++-- tests/testutilslxc.c | 9 -- tests/testutilsqemu.c | 11 -- tests/testutilsxen.c | 16 --- tests/vmx2xmltest.c | 7 - tests/xmconfigtest.c | 2 +- tests/xml2sexprtest.c | 3 +- tests/xml2vmxtest.c | 7 - 31 files changed, 278 insertions(+), 313 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 6b65e6a..abcf6de 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -160,9 +160,6 @@ struct _virCaps { size_t nguests; size_t nguests_max; virCapsGuestPtr *guests; - - /* Move to virDomainXMLConf later */ - int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7fb282..70da739 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -385,6 +385,7 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_ENUM_IMPL(virDomainChrConsoleTarget, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, + "none", "serial", "xen", "uml", @@ -2345,9 +2346,11 @@ virDomainDeviceInfoClearCCWAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } -int virDomainDeviceInfoIterate(virDomainDefPtr def, - virDomainDeviceInfoCallback cb, - void *opaque) +static int +virDomainDeviceInfoIterateInternal(virDomainDefPtr def, + virDomainDeviceInfoCallback cb, + bool all, + void *opaque) { int i; virDomainDeviceDef device; @@ -2411,9 +2414,11 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, return -1; } for (i = 0; i < def->nconsoles ; i++) { - if (i == 0 && - def->consoles[i]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && - STREQ_NULLABLE(def->os.type, "hvm")) + if (!all && + i == 0 && + (def->consoles[i]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL || + def->consoles[i]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) && + STREQ_NULLABLE(def->os.type, "hvm")) continue; device.data.chr = def->consoles[i]; if (cb(def, &device, &def->consoles[i]->info, opaque) < 0) @@ -2489,11 +2494,22 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, } +int +virDomainDeviceInfoIterate(virDomainDefPtr def, + virDomainDeviceInfoCallback cb, + void *opaque) +{ + return virDomainDeviceInfoIterateInternal(def, cb, false, opaque); +} + + /* this is a place for global assumption checks */ static int virDomainDefPostParseInternal(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED) { + int i; + /* verify init path for container based domains */ if (STREQ(def->os.type, "exe") && !def->os.init) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -2538,6 +2554,91 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } + /* + * Some really crazy backcompat stuff for consoles + * + * Historically the first (and only) '<console>' element in an HVM guest + * was treated as being an alias for a <serial> device. + * + * So if we see that this console device should be a serial device, then we + * move the config over to def->serials[0] (or discard it if that already + * exists). However, given console can already be filled with aliased data + * of def->serials[0]. Keep it then. + * + * We then fill def->consoles[0] with a stub just so we get sequencing + * correct for consoles > 0 + */ + if (def->nconsoles > 0 && STREQ(def->os.type, "hvm") && + (def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL || + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)) { + /* First verify that only the first console is of type serial */ + for (i = 1; i < def->nconsoles; i++) { + virDomainChrDefPtr cons = def->consoles[i]; + + if (cons->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only the first console can be a serial port")); + return -1; + } + } + + /* If there isn't a corresponding serial port: + * - create one and set, the console to be an alias for it + * + * If there is a corresponding serial port: + * - Check if the source definition is equal: + * - if yes: leave it as-is + * - if no: change the console to be alias of the serial port + */ + + /* create the serial port definition from the console definition */ + if (def->nserials == 0) { + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, + def->consoles[0]) < 0) + goto no_memory; + + /* modify it to be a serial port */ + def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; + def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + def->serials[0]->target.port = 0; + } else { + /* if the console source does't match */ + if (!virDomainChrSourceDefIsEqual(&def->serials[0]->source, + &def->consoles[0]->source)) { + virDomainChrDefFree(def->consoles[0]); + def->consoles[0] = NULL; + } + } + + if (!def->consoles[0]) { + /* allocate a new console type for the stolen one */ + if (VIR_ALLOC(def->consoles[0]) < 0) + goto no_memory; + + /* Create an console alias for the serial port */ + def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + } + } + + return 0; + +no_memory: + virReportOOMError(); + return -1; +} + + +static int +virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + return 0; } @@ -2557,6 +2658,9 @@ virDomainDeviceDefPostParse(virDomainXMLConfPtr xmlconf, return ret; } + if ((ret = virDomainDeviceDefPostParseInternal(dev, def, caps)) < 0) + return ret; + return 0; } @@ -2601,9 +2705,10 @@ virDomainDefPostParse(virDomainXMLConfPtr xmlconf, } /* iterate the devices */ - if ((ret = virDomainDeviceInfoIterate(def, - virDomainDefPostParseDeviceIterator, - &data)) < 0) + if ((ret = virDomainDeviceInfoIterateInternal(def, + virDomainDefPostParseDeviceIterator, + true, + &data)) < 0) return ret; @@ -5960,87 +6065,66 @@ error: } static int -virDomainChrDefaultTargetType(virCapsPtr caps, - virDomainDefPtr def, - int devtype) { - - int target = -1; - - switch (devtype) { +virDomainChrDefaultTargetType(int devtype) { + switch ((enum virDomainChrDeviceType) devtype) { case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: virReportError(VIR_ERR_XML_ERROR, _("target type must be specified for %s device"), virDomainChrDeviceTypeToString(devtype)); - break; + return -1; case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: - if (!caps->defaultConsoleTargetType) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Driver does not have a default console type set")); - return -1; - } - target = caps->defaultConsoleTargetType(def->os.type, def->os.arch); - break; + return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - target = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; - break; + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: - default: + case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: /* No target type yet*/ - target = 0; break; } - return target; + return 0; } static int -virDomainChrTargetTypeFromString(virCapsPtr caps, - virDomainDefPtr vmdef, - virDomainChrDefPtr def, +virDomainChrTargetTypeFromString(virDomainChrDefPtr def, int devtype, const char *targetType) { int ret = -1; - int target = 0; - if (!targetType) { - target = virDomainChrDefaultTargetType(caps, vmdef, devtype); - goto out; - } + if (!targetType) + return virDomainChrDefaultTargetType(devtype); - switch (devtype) { + switch ((enum virDomainChrDeviceType) devtype) { case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: - target = virDomainChrChannelTargetTypeFromString(targetType); + ret = virDomainChrChannelTargetTypeFromString(targetType); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: - target = virDomainChrConsoleTargetTypeFromString(targetType); + ret = virDomainChrConsoleTargetTypeFromString(targetType); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - target = virDomainChrSerialTargetTypeFromString(targetType); + ret = virDomainChrSerialTargetTypeFromString(targetType); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: - default: + case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: /* No target type yet*/ + ret = 0; break; } def->targetTypeAttr = true; -out: - ret = target; return ret; } static int -virDomainChrDefParseTargetXML(virCapsPtr caps, - virDomainDefPtr vmdef, - virDomainChrDefPtr def, +virDomainChrDefParseTargetXML(virDomainChrDefPtr def, xmlNodePtr cur) { int ret = -1; @@ -6050,8 +6134,8 @@ virDomainChrDefParseTargetXML(virCapsPtr caps, const char *portStr = NULL; if ((def->targetType = - virDomainChrTargetTypeFromString(caps, vmdef, def, - def->deviceType, targetType)) < 0) { + virDomainChrTargetTypeFromString(def, def->deviceType, + targetType)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown target type '%s' specified for character device"), targetType); @@ -6410,9 +6494,7 @@ virDomainChrDefNew(void) { * */ static virDomainChrDefPtr -virDomainChrDefParseXML(virCapsPtr caps, - virDomainDefPtr vmdef, - xmlXPathContextPtr ctxt, +virDomainChrDefParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node, virSecurityLabelDefPtr* vmSeclabels, int nvmSeclabels, @@ -6456,7 +6538,7 @@ virDomainChrDefParseXML(virCapsPtr caps, if (cur->type == XML_ELEMENT_NODE) { if (xmlStrEqual(cur->name, BAD_CAST "target")) { seenTarget = true; - if (virDomainChrDefParseTargetXML(caps, vmdef, def, cur) < 0) { + if (virDomainChrDefParseTargetXML(def, cur) < 0) { goto error; } } @@ -6466,7 +6548,7 @@ virDomainChrDefParseXML(virCapsPtr caps, } if (!seenTarget && - ((def->targetType = virDomainChrDefaultTargetType(caps, vmdef, def->deviceType)) < 0)) + ((def->targetType = virDomainChrDefaultTargetType(def->deviceType)) < 0)) goto cleanup; if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { @@ -10558,9 +10640,7 @@ virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, - def, - ctxt, + virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt, nodes[i], def->seclabels, def->nseclabels, @@ -10588,9 +10668,7 @@ virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, - def, - ctxt, + virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt, nodes[i], def->seclabels, def->nseclabels, @@ -10620,10 +10698,7 @@ virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { - bool create_stub = true; - virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, - def, - ctxt, + virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt, nodes[i], def->seclabels, def->nseclabels, @@ -10631,64 +10706,7 @@ virDomainDefParseXML(virCapsPtr caps, if (!chr) goto error; - /* - * Some really crazy backcompat stuff for consoles - * - * Historically the first (and only) '<console>' - * element in an HVM guest was treated as being - * an alias for a <serial> device. - * - * So if we see that this console device should - * be a serial device, then we move the config - * over to def->serials[0] (or discard it if - * that already exists). However, given console - * can already be filled with aliased data of - * def->serials[0]. Keep it then. - * - * We then fill def->consoles[0] with a stub - * just so we get sequencing correct for consoles - * > 0 - */ - if (STREQ(def->os.type, "hvm") && - (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)) { - if (i != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only the first console can be a serial port")); - virDomainChrDefFree(chr); - goto error; - } - - /* Either discard or move this chr to the serial config */ - if (def->nserials != 0) { - if (virDomainChrSourceDefIsEqual(&def->serials[0]->source, - &chr->source)) { - /* Alias to def->serial[0]. Skip it */ - create_stub = false; - } else { - virDomainChrDefFree(chr); - } - } else { - if (VIR_ALLOC_N(def->serials, 1) < 0) { - virDomainChrDefFree(chr); - goto no_memory; - } - chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - def->nserials = 1; - def->serials[0] = chr; - chr->target.port = 0; - } - - if (create_stub) { - /* And create a stub placeholder */ - if (VIR_ALLOC(chr) < 0) - goto no_memory; - chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - } - } - chr->target.port = i; - def->consoles[def->nconsoles++] = chr; } VIR_FREE(nodes); @@ -10700,9 +10718,7 @@ virDomainDefParseXML(virCapsPtr caps, goto no_memory; for (i = 0 ; i < n ; i++) { - virDomainChrDefPtr chr = virDomainChrDefParseXML(caps, - def, - ctxt, + virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt, nodes[i], def->seclabels, def->nseclabels, @@ -15201,7 +15217,6 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) goto error; - for (n = 0 ; n < def->nnets ; n++) if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) goto error; @@ -15224,7 +15239,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, * if it is type == serial */ if (STREQ(def->os.type, "hvm") && - (def->consoles[n]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) && + (def->consoles[n]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL || + def->consoles[n]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) && (n < def->nserials)) { memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; @@ -15241,6 +15257,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainChrDef console; memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; if (virDomainChrDefFormat(buf, &console, flags) < 0) goto error; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1d059a8..5da664f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -956,7 +956,8 @@ enum virDomainChrChannelTargetType { }; enum virDomainChrConsoleTargetType { - VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL = 0, + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE = 0, + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO, diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index aa6f8c1..a4ea0fb 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -569,13 +569,6 @@ esxLookupHostSystemBiosUuid(esxPrivate *priv, unsigned char *uuid) } -static int esxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} - - static virCapsPtr esxCapsInit(esxPrivate *priv) { @@ -600,7 +593,6 @@ esxCapsInit(esxPrivate *priv) virCapabilitiesAddHostMigrateTransport(caps, "vpxmigr"); - caps->defaultConsoleTargetType = esxDefaultConsoleType; if (esxLookupHostSystemBiosUuid(priv, caps->host.host_uuid) < 0) { goto failure; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index ed3e832..7e0753a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -64,15 +64,6 @@ static const char *xen_cap_re = "(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(x86_32|x static regex_t xen_cap_rec; -static int libxlDefaultConsoleType(const char *ostype, - virArch arch ATTRIBUTE_UNUSED) -{ - if (STREQ(ostype, "hvm")) - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - else - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; -} - static virCapsPtr libxlBuildCapabilities(virArch hostarch, int host_pae, @@ -162,8 +153,6 @@ libxlBuildCapabilities(virArch hostarch, } } - caps->defaultConsoleTargetType = libxlDefaultConsoleType; - return caps; no_memory: diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index cfc2cfe..7b49e9b 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -431,10 +431,29 @@ virDomainXMLPrivateDataCallbacks libxlDomainXMLPrivateDataCallbacks = { .free = libxlDomainObjPrivateFree, }; + +static int +libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && + STRNEQ(def->os.type, "hvm")) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + + return 0; +} + + virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, + .devicesConfigCallback = libxlDomainDeviceDefPostParse, }; + /* driver must be locked before calling */ static void libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event) diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index d835139..63a77a7 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -41,12 +41,6 @@ #define VIR_FROM_THIS VIR_FROM_LXC -static int lxcDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC; -} - /* Functions */ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) @@ -59,8 +53,6 @@ virCapsPtr lxcCapsInit(virLXCDriverPtr driver) 0, 0)) == NULL) goto error; - caps->defaultConsoleTargetType = lxcDefaultConsoleType; - /* Some machines have problematic NUMA toplogy causing * unexpected failures. We don't want to break the QEMU * driver in this scenario, so log errors & carry on diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 673be3a..8615789 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -93,6 +93,23 @@ virLXCDomainDefPostParse(virDomainDefPtr def, return 0; } + +static int +virLXCDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC; + + return 0; +} + + virDomainDefParserConfig virLXCDriverDomainDefParserConfig = { .domainConfigCallback = virLXCDomainDefPostParse, + .devicesConfigCallback = virLXCDomainDeviceDefPostParse, }; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 83d8d18..5bca3fc 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -168,13 +168,6 @@ error: } -static int openvzDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ; -} - - virCapsPtr openvzCapsInit(void) { virCapsPtr caps; @@ -204,9 +197,8 @@ virCapsPtr openvzCapsInit(void) NULL) == NULL) goto no_memory; - caps->defaultConsoleTargetType = openvzDefaultConsoleType; - return caps; + no_memory: virObjectUnref(caps); return NULL; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 350c8c7..a3cc961 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -115,8 +115,24 @@ openvzDomainDefPostParse(virDomainDefPtr def, } +static int +openvzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ; + + return 0; +} + + virDomainDefParserConfig openvzDomainDefParserConfig = { .domainConfigCallback = openvzDomainDefPostParse, + .devicesConfigCallback = openvzDomainDeviceDefPostParse, }; diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index b137b66..c592dcf 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -96,12 +96,6 @@ parallelsDriverUnlock(parallelsConnPtr driver) virMutexUnlock(&driver->lock); } -static int -parallelsDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} static void parallelsDomObjFreePrivate(void *p) @@ -149,7 +143,6 @@ parallelsBuildCapabilities(void) "parallels", NULL, NULL, 0, NULL) == NULL) goto no_memory; - caps->defaultConsoleTargetType = parallelsDefaultConsoleType; return caps; no_memory: diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8b3fdb6..467d0b1 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -300,13 +300,6 @@ phypGetVIOSPartitionID(virConnectPtr conn) } -static int phypDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} - - static virCapsPtr phypCapsInit(void) { @@ -337,8 +330,6 @@ phypCapsInit(void) "phyp", NULL, NULL, 0, NULL) == NULL) goto no_memory; - caps->defaultConsoleTargetType = phypDefaultConsoleType; - return caps; no_memory: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index caa77a4..ca97f7d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -853,17 +853,6 @@ error: } -static int virQEMUCapsDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch) -{ - if (arch == VIR_ARCH_S390 || - arch == VIR_ARCH_S390X) - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; - else - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} - - virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache) { virCapsPtr caps; @@ -903,8 +892,6 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache) i) < 0) goto error; - caps->defaultConsoleTargetType = virQEMUCapsDefaultConsoleType; - return caps; error: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f7df4a9..7bec908 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -733,6 +733,13 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } + /* set the default console type for S390 arches */ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && + (def->os.arch == VIR_ARCH_S390 || def->os.arch == VIR_ARCH_S390X)) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; + ret = 0; cleanup: diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 2f2bc71..a5d84c5 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -687,11 +687,6 @@ caps_mockup(vahControl * ctl, const char *xmlStr) return rc; } -static int aaDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} static int get_definition(vahControl * ctl, const char *xmlStr) @@ -716,8 +711,6 @@ get_definition(vahControl * ctl, const char *xmlStr) goto exit; } - ctl->caps->defaultConsoleTargetType = aaDefaultConsoleType; - if ((guest = virCapabilitiesAddGuest(ctl->caps, ctl->hvm, ctl->arch, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 76e04c3..a7cde58 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -150,13 +150,6 @@ static void testDomainObjPrivateFree(void *data) } -static int testDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} - - static virDomainXMLConfPtr testBuildXMLConfig(void) { @@ -177,8 +170,6 @@ testBuildCapabilities(virConnectPtr conn) { if ((caps = virCapabilitiesNew(VIR_ARCH_I686, 0, 0)) == NULL) goto no_memory; - caps->defaultConsoleTargetType = testDefaultConsoleType; - if (virCapabilitiesAddHostFeature(caps, "pae") < 0) goto no_memory; if (virCapabilitiesAddHostFeature(caps ,"nonpae") < 0) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index b3ac326..598eb76 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -52,13 +52,6 @@ #define VIR_FROM_THIS VIR_FROM_UML -static int umlDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML; -} - - virCapsPtr umlCapsInit(void) { virCapsPtr caps; virCapsGuestPtr guest; @@ -102,8 +95,6 @@ virCapsPtr umlCapsInit(void) { NULL) == NULL) goto error; - caps->defaultConsoleTargetType = umlDefaultConsoleType; - return caps; error: diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 480fc3d..d3ec7d7 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -420,6 +420,27 @@ cleanup: umlDriverUnlock(driver); } + +static int +umlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML; + + return 0; +} + + +virDomainDefParserConfig umlDriverDomainDefParserConfig = { + .devicesConfigCallback = umlDomainDeviceDefPostParse, +}; + + /** * umlStartup: * @@ -505,7 +526,8 @@ umlStartup(bool privileged, if ((uml_driver->caps = umlCapsInit()) == NULL) goto out_of_memory; - if (!(uml_driver->xmlconf = virDomainXMLConfNew(NULL, &privcb, NULL))) + if (!(uml_driver->xmlconf = virDomainXMLConfNew(¨DriverDomainDefParserConfig, + &privcb, NULL))) goto error; if ((uml_driver->inotifyFD = inotify_init()) < 0) { diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 6f2dcf0..0ce43eb 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -844,13 +844,6 @@ cleanup: } -static int vboxDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} - - static virDomainDefParserConfig vboxDomainDefParserConfig = { .macPrefix = { 0x08, 0x00, 0x27 }, }; @@ -893,8 +886,6 @@ static virCapsPtr vboxCapsInit(void) NULL) == NULL) goto no_memory; - caps->defaultConsoleTargetType = vboxDefaultConsoleType; - return caps; no_memory: diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index ab06c66..c7f69f7 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -50,13 +50,6 @@ vmwareFreeDriver(struct vmware_driver *driver) } -static int vmwareDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} - - virCapsPtr vmwareCapsInit(void) { @@ -122,8 +115,6 @@ vmwareCapsInit(void) goto error; } - caps->defaultConsoleTargetType = vmwareDefaultConsoleType; - cleanup: virCPUDefFree(cpu); if (caps) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index b13dcec..c5b55f2 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -265,11 +265,36 @@ xenUnifiedXendProbe(void) #endif +static int +xenDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && + STRNEQ(def->os.type, "hvm")) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + + return 0; +} + + virDomainDefParserConfig xenDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, + .devicesConfigCallback = xenDomainDeviceDefPostParse, }; +virDomainXMLConfPtr +xenDomainXMLConfInit(void) +{ + return virDomainXMLConfNew(&xenDomainDefParserConfig, + NULL, NULL); +} + + static virDrvOpenStatus xenUnifiedOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) { @@ -405,8 +430,7 @@ xenUnifiedOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) goto fail; } - if (!(priv->xmlconf = virDomainXMLConfNew(&xenDomainDefParserConfig, - NULL, NULL))) + if (!(priv->xmlconf = xenDomainXMLConfInit())) goto fail; #if WITH_XEN_INOTIFY diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 8e43db3..568de07 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -226,6 +226,8 @@ typedef struct _xenUnifiedPrivate *xenUnifiedPrivatePtr; char *xenDomainUsedCpus(virDomainPtr dom); +virDomainXMLConfPtr xenDomainXMLConfInit(void); + void xenUnifiedDomainInfoListFree(xenUnifiedDomainInfoListPtr info); int xenUnifiedAddDomainInfo(xenUnifiedDomainInfoListPtr info, int id, char *name, diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 69bc6cd..e16fffe 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2279,15 +2279,6 @@ struct guest_arch { }; -static int xenDefaultConsoleType(const char *ostype, - virArch arch ATTRIBUTE_UNUSED) -{ - if (STREQ(ostype, "hvm")) - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - else - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; -} - static virCapsPtr xenHypervisorBuildCapabilities(virConnectPtr conn, virArch hostarch, int host_pae, @@ -2414,8 +2405,6 @@ xenHypervisorBuildCapabilities(virConnectPtr conn, virArch hostarch, } - caps->defaultConsoleTargetType = xenDefaultConsoleType; - return caps; no_memory: diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 941f3bd..8a58e7b 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -43,16 +43,27 @@ #define VIR_FROM_THIS VIR_FROM_XENAPI -static int xenapiDefaultConsoleType(const char *ostype, - virArch arch ATTRIBUTE_UNUSED) +static int +xenapiDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, + virDomainDefPtr def, + virCapsPtr caps ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) { - if (STREQ(ostype, "hvm")) - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - else - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + if (dev->type == VIR_DOMAIN_DEVICE_CHR && + dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE && + STRNEQ(def->os.type, "hvm")) + dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; + + return 0; } +virDomainDefParserConfig xenapiDomainDefParserConfig = { + .devicesConfigCallback = xenapiDomainDeviceDefPostParse, +}; + + /* * getCapsObject * @@ -83,8 +94,6 @@ getCapsObject(void) if (!domain2) goto error_cleanup; - caps->defaultConsoleTargetType = xenapiDefaultConsoleType; - return caps; error_cleanup: @@ -169,7 +178,8 @@ xenapiOpen(virConnectPtr conn, virConnectAuthPtr auth, goto error; } - if (!(privP->xmlconf = virDomainXMLConfNew(NULL, NULL, NULL))) { + if (!(privP->xmlconf = virDomainXMLConfNew(&xenapiDomainDefParserConfig, + NULL, NULL))) { xenapiSessionErrorHandler(conn, VIR_ERR_INTERNAL_ERROR, _("Failed to create XML conf object")); goto error; diff --git a/tests/testutilslxc.c b/tests/testutilslxc.c index 0c2170c..1bc6feb 100644 --- a/tests/testutilslxc.c +++ b/tests/testutilslxc.c @@ -8,13 +8,6 @@ # include "domain_conf.h" -static int testLXCDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC; -} - - virCapsPtr testLXCCapsInit(void) { virCapsPtr caps; virCapsGuestPtr guest; @@ -23,8 +16,6 @@ virCapsPtr testLXCCapsInit(void) { 0, 0)) == NULL) return NULL; - caps->defaultConsoleTargetType = testLXCDefaultConsoleType; - if ((guest = virCapabilitiesAddGuest(caps, "exe", VIR_ARCH_I686, "/usr/libexec/libvirt_lxc", NULL, 0, NULL)) == NULL) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index db15ee6..fba17a3 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -55,15 +55,6 @@ static virCapsGuestMachinePtr *testQemuAllocNewerMachines(int *nmachines) return machines; } -static int testQemuDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch) -{ - if (arch == VIR_ARCH_S390 || - arch == VIR_ARCH_S390X) - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; - else - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} static int testQemuAddPPC64Guest(virCapsPtr caps) { @@ -200,8 +191,6 @@ virCapsPtr testQemuCapsInit(void) { 0, 0)) == NULL) return NULL; - caps->defaultConsoleTargetType = testQemuDefaultConsoleType; - if ((caps->host.cpu = virCPUDefCopy(&host_cpu)) == NULL || (machines = testQemuAllocMachines(&nmachines)) == NULL) goto cleanup; diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c index 479eec3..1b5ee79 100644 --- a/tests/testutilsxen.c +++ b/tests/testutilsxen.c @@ -6,20 +6,6 @@ #include "testutilsxen.h" #include "domain_conf.h" -static int testXenDefaultConsoleType(const char *ostype, - virArch arch ATTRIBUTE_UNUSED) -{ - if (STREQ(ostype, "hvm")) - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; - else - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; -} - -virDomainXMLConfPtr -testXenXMLConfInit(void) -{ - return virDomainXMLConfNew(NULL, NULL, NULL); -} virCapsPtr testXenCapsInit(void) { struct utsname utsname; @@ -39,8 +25,6 @@ virCapsPtr testXenCapsInit(void) { 0, 0)) == NULL) return NULL; - caps->defaultConsoleTargetType = testXenDefaultConsoleType; - nmachines = ARRAY_CARDINALITY(x86_machines); if ((machines = virCapabilitiesAllocMachines(x86_machines, nmachines)) == NULL) goto cleanup; diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 7f2c73b..1a0c97a 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -15,11 +15,6 @@ static virCapsPtr caps; static virDomainXMLConfPtr xmlconf; static virVMXContext ctx; -static int testDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} static void @@ -33,8 +28,6 @@ testCapsInit(void) return; } - caps->defaultConsoleTargetType = testDefaultConsoleType; - virCapabilitiesAddHostMigrateTransport(caps, "esx"); /* i686 guest */ diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index a32c11a..c8cb6f5 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -196,7 +196,7 @@ mymain(void) if (!(caps = testXenCapsInit())) return EXIT_FAILURE; - if (!(xmlconf = testXenXMLConfInit())) + if (!(xmlconf = xenDomainXMLConfInit())) return EXIT_FAILURE; #define DO_TEST(name, version) \ diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c index 364601a..77f2620 100644 --- a/tests/xml2sexprtest.c +++ b/tests/xml2sexprtest.c @@ -10,6 +10,7 @@ #include "internal.h" #include "xen/xend_internal.h" +#include "xen/xen_driver.h" #include "xenxs/xen_sxpr.h" #include "testutils.h" #include "testutilsxen.h" @@ -104,7 +105,7 @@ mymain(void) if (!(caps = testXenCapsInit())) return EXIT_FAILURE; - if (!(xmlconf = testXenXMLConfInit())) + if (!(xmlconf = xenDomainXMLConfInit())) return EXIT_FAILURE; DO_TEST("pv", "pv", "pvtest", 1); diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index b62d214..204a57c 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -15,11 +15,6 @@ static virCapsPtr caps; static virVMXContext ctx; static virDomainXMLConfPtr xmlconf; -static int testDefaultConsoleType(const char *ostype ATTRIBUTE_UNUSED, - virArch arch ATTRIBUTE_UNUSED) -{ - return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; -} static void testCapsInit(void) @@ -32,8 +27,6 @@ testCapsInit(void) return; } - caps->defaultConsoleTargetType = testDefaultConsoleType; - virCapabilitiesAddHostMigrateTransport(caps, "esx"); -- 1.8.1.5

On 03/25/2013 10:17 AM, Peter Krempa wrote:
This patch refactors various places to allow removing of the defaultConsoleTargetType callback from the virCaps structure.
A new console character device target type is introduced - VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE - to mark that no type was specified in the XML. This type is at the end converted to the standard VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL. Other types that are different from this default have to be processed separately in the device post parse callback. ---
Notes: Version 4: - new in series
@@ -2538,6 +2554,91 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; }
+ /* + * Some really crazy backcompat stuff for consoles
No joke! Even it if it is the same comment just moved from elsewhere
+ /* create the serial port definition from the console definition */ + if (def->nserials == 0) { + if (VIR_APPEND_ELEMENT(def->serials, def->nserials, + def->consoles[0]) < 0) + goto no_memory; + + /* modify it to be a serial port */ + def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; + def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + def->serials[0]->target.port = 0; + } else { + /* if the console source does't match */
s/does't/doesn't/ ACK with the typo fix (and modulo any renaming earlier in the series) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
Laine Stump
-
Peter Krempa