The message didn't seem to be much helpful --- src/conf/domain_conf.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f2887c6..bd1ea25 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9265,16 +9265,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; /* and info about it */ - tmp = virXPathString("string(./memory[1]/@dumpCore)", ctxt); - if (tmp) { - def->mem.dump_core = virDomainMemDumpTypeFromString(tmp); - - if (def->mem.dump_core <= 0) { - virReportError(VIR_ERR_XML_ERROR, _("Bad value '%s'"), tmp); - goto error; - } - VIR_FREE(tmp); + if ((tmp = virXPathString("string(./memory[1]/@dumpCore)", ctxt)) && + (def->mem.dump_core = virDomainMemDumpTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid memory core dump attribute value '%s'"), tmp); + goto error; } + VIR_FREE(tmp); if (def->mem.cur_balloon > def->mem.max_balloon) { /* Older libvirt could get into this situation due to -- 1.8.1.1

Change the error label to "error" and simplify some error paths. --- src/qemu/qemu_conf.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8299b79..a2a05d4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -543,11 +543,8 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); /* Basic host arch / guest machine capabilities */ - if (!(caps = virQEMUCapsInit(driver->qemuCapsCache))) { - virReportOOMError(); - virObjectUnref(cfg); - return NULL; - } + if (!(caps = virQEMUCapsInit(driver->qemuCapsCache))) + goto no_memory; if (cfg->allowDiskFormatProbing) { caps->defaultDiskDriverName = NULL; @@ -563,14 +560,12 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) if (virGetHostUUID(caps->host.host_uuid)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot get the host uuid")); - goto err_exit; + goto error; } /* access sec drivers and create a sec model for each one */ - sec_managers = virSecurityManagerGetNested(driver->securityManager); - if (sec_managers == NULL) { - goto err_exit; - } + if (!(sec_managers = virSecurityManagerGetNested(driver->securityManager))) + goto error; /* calculate length */ for (i = 0; sec_managers[i]; i++) @@ -597,7 +592,7 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) no_memory: virReportOOMError(); -err_exit: +error: VIR_FREE(sec_managers); virObjectUnref(caps); virObjectUnref(cfg); -- 1.8.1.1

Use the correct type and get rid of "default" label in switch to make the compiler complain if a new device type is added. --- src/conf/domain_conf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd1ea25..218a28b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16030,7 +16030,7 @@ virDomainDeviceDefCopy(virCapsPtr caps, char *xmlStr = NULL; int rc = -1; - switch (src->type) { + switch ((virDomainDeviceType) src->type) { case VIR_DOMAIN_DEVICE_DISK: rc = virDomainDiskDefFormat(&buf, src->data.disk, flags); break; @@ -16070,7 +16070,11 @@ virDomainDeviceDefCopy(virCapsPtr caps, case VIR_DOMAIN_DEVICE_REDIRDEV: rc = virDomainRedirdevDefFormat(&buf, src->data.redirdev, flags); break; - default: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("Copying definition of '%d' type " "is not implemented yet."), -- 1.8.1.1

The label named "cleanup" was used in error cases only. Change it to "error". --- src/conf/domain_conf.c | 72 +++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 218a28b..e6b1f7c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14267,7 +14267,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (!(type = virDomainVirtTypeToString(def->virtType))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected domain type %d"), def->virtType); - goto cleanup; + goto error; } if (def->id == -1) @@ -14307,7 +14307,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferGetIndent(buf, false) / 2 + 1, 1) < 0) { xmlBufferFree(xmlbuf); xmlIndentTreeOutput = oldIndentTreeOutput; - goto cleanup; + goto error; } virBufferAsprintf(buf, " %s\n", (char *) xmlBufferContent(xmlbuf)); xmlBufferFree(xmlbuf); @@ -14395,7 +14395,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cpumask && !virBitmapIsAllSet(def->cpumask)) { char *cpumask = NULL; if ((cpumask = virBitmapFormat(def->cpumask)) == NULL) - goto cleanup; + goto error; virBufferAsprintf(buf, " cpuset='%s'", cpumask); VIR_FREE(cpumask); } @@ -14448,7 +14448,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (cpumask == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format cpuset for vcpupin")); - goto cleanup; + goto error; } virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); @@ -14463,7 +14463,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (cpumask == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format cpuset for emulator")); - goto cleanup; + goto error; } virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); @@ -14493,7 +14493,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format nodeset for " "NUMA memory tuning")); - goto cleanup; + goto error; } virBufferAsprintf(buf, "nodeset='%s'/>\n", nodemask); VIR_FREE(nodemask); @@ -14557,7 +14557,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected boot device type %d"), def->os.bootDevs[n]); - goto cleanup; + goto error; } virBufferAsprintf(buf, " <boot dev='%s'/>\n", boottype); } @@ -14590,7 +14590,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (mode == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected smbios mode %d"), def->os.smbios_mode); - goto cleanup; + goto error; } virBufferAsprintf(buf, " <smbios mode='%s'/>\n", mode); } @@ -14605,7 +14605,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (!name) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected feature %d"), i); - goto cleanup; + goto error; } virBufferAsprintf(buf, " <%s", name); if (i == VIR_DOMAIN_FEATURE_APIC && def->apic_eoi) { @@ -14640,7 +14640,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAdjustIndent(buf, 2); if (virCPUDefFormatBufFull(buf, def->cpu, flags) < 0) - goto cleanup; + goto error; virBufferAdjustIndent(buf, -2); virBufferAsprintf(buf, " <clock offset='%s'", @@ -14666,7 +14666,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, ">\n"); for (n = 0; n < def->clock.ntimers; n++) { if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) - goto cleanup; + goto error; } virBufferAddLit(buf, " </clock>\n"); } @@ -14674,20 +14674,20 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (virDomainEventActionDefFormat(buf, def->onPoweroff, "on_poweroff", virDomainLifecycleTypeToString) < 0) - goto cleanup; + goto error; if (virDomainEventActionDefFormat(buf, def->onReboot, "on_reboot", virDomainLifecycleTypeToString) < 0) - goto cleanup; + goto error; if (virDomainEventActionDefFormat(buf, def->onCrash, "on_crash", virDomainLifecycleCrashTypeToString) < 0) - goto cleanup; + goto error; if (def->onLockFailure != VIR_DOMAIN_LOCK_FAILURE_DEFAULT && virDomainEventActionDefFormat(buf, def->onLockFailure, "on_lockfailure", virDomainLockFailureTypeToString) < 0) - goto cleanup; + goto error; if (def->pm.s3 || def->pm.s4) { virBufferAddLit(buf, " <pm>\n"); @@ -14709,36 +14709,36 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (n = 0 ; n < def->ndisks ; n++) if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->ncontrollers ; n++) if (virDomainControllerDefFormat(buf, def->controllers[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nleases ; n++) if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nfss ; n++) if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nnets ; n++) if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nsmartcards ; n++) if (virDomainSmartcardDefFormat(buf, def->smartcards[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nserials ; n++) if (virDomainChrDefFormat(buf, def->serials[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nparallels ; n++) if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nconsoles ; n++) { virDomainChrDef console; @@ -14755,7 +14755,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, memcpy(&console, def->consoles[n], sizeof(console)); } if (virDomainChrDefFormat(buf, &console, flags) < 0) - goto cleanup; + goto error; } if (STREQ(def->os.type, "hvm") && def->nconsoles == 0 && @@ -14764,17 +14764,17 @@ virDomainDefFormatInternal(virDomainDefPtr def, memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; if (virDomainChrDefFormat(buf, &console, flags) < 0) - goto cleanup; + goto error; } for (n = 0 ; n < def->nchannels ; n++) if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->ninputs ; n++) if (def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_USB && virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) - goto cleanup; + goto error; if (def->ngraphics > 0) { /* If graphics is enabled, add the implicit mouse */ @@ -14786,20 +14786,20 @@ virDomainDefFormatInternal(virDomainDefPtr def, }; if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->ngraphics ; n++) if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) - goto cleanup; + goto error; } for (n = 0 ; n < def->nsounds ; n++) if (virDomainSoundDefFormat(buf, def->sounds[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nvideos ; n++) if (virDomainVideoDefFormat(buf, def->videos[n], flags) < 0) - goto cleanup; + goto error; for (n = 0 ; n < def->nhostdevs ; n++) { /* If parent.type != NONE, this is just a pointer to the @@ -14808,20 +14808,20 @@ virDomainDefFormatInternal(virDomainDefPtr def, */ if (def->hostdevs[n]->parent.type == VIR_DOMAIN_DEVICE_NONE && virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) { - goto cleanup; + goto error; } } for (n = 0 ; n < def->nredirdevs ; n++) if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], flags) < 0) - goto cleanup; + goto error; if (def->redirfilter) virDomainRedirFilterDefFormat(buf, def->redirfilter); for (n = 0 ; n < def->nhubs ; n++) if (virDomainHubDefFormat(buf, def->hubs[n], flags) < 0) - goto cleanup; + goto error; if (def->watchdog) virDomainWatchdogDefFormat(buf, def->watchdog, flags); @@ -14838,7 +14838,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->namespaceData && def->ns.format) { if ((def->ns.format)(buf, def->namespaceData) < 0) - goto cleanup; + goto error; } virBufferAddLit(buf, "</domain>\n"); @@ -14850,7 +14850,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, no_memory: virReportOOMError(); - cleanup: + error: virBufferFreeAndReset(buf); return -1; } -- 1.8.1.1

Many of the headers were using the old style and even overflowing the 80 column mark. --- src/conf/domain_conf.c | 341 +++++++++++++++++++++++++++++-------------------- 1 file changed, 201 insertions(+), 140 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e6b1f7c..fad293b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8093,12 +8093,13 @@ error: return NULL; } -static int virDomainEventActionParseXML(xmlXPathContextPtr ctxt, - const char *name, - const char *xpath, - int *val, - int defaultVal, - virEventActionFromStringFunc convFunc) +static int +virDomainEventActionParseXML(xmlXPathContextPtr ctxt, + const char *name, + const char *xpath, + int *val, + int defaultVal, + virEventActionFromStringFunc convFunc) { char *tmp = virXPathString(xpath, ctxt); if (tmp == NULL) { @@ -8138,10 +8139,11 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt, return ret; } -virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, - virDomainDefPtr def, - const char *xmlStr, - unsigned int flags) +virDomainDeviceDefPtr +virDomainDeviceDefParse(virCapsPtr caps, + virDomainDefPtr def, + const char *xmlStr, + unsigned int flags) { xmlDocPtr xml; xmlNodePtr node; @@ -9129,12 +9131,13 @@ cleanup: } -static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, - xmlDocPtr xml, - xmlNodePtr root, - xmlXPathContextPtr ctxt, - unsigned int expectedVirtTypes, - unsigned int flags) +static virDomainDefPtr +virDomainDefParseXML(virCapsPtr caps, + xmlDocPtr xml, + xmlNodePtr root, + xmlXPathContextPtr ctxt, + unsigned int expectedVirtTypes, + unsigned int flags) { xmlNodePtr *nodes = NULL, node = NULL; char *tmp = NULL; @@ -10739,11 +10742,12 @@ no_memory: } -static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, - xmlDocPtr xml, - xmlXPathContextPtr ctxt, - unsigned int expectedVirtTypes, - unsigned int flags) +static virDomainObjPtr +virDomainObjParseXML(virCapsPtr caps, + xmlDocPtr xml, + xmlXPathContextPtr ctxt, + unsigned int expectedVirtTypes, + unsigned int flags) { char *tmp = NULL; long val; @@ -10857,28 +10861,31 @@ virDomainDefParse(const char *xmlStr, return def; } -virDomainDefPtr virDomainDefParseString(virCapsPtr caps, - const char *xmlStr, - unsigned int expectedVirtTypes, - unsigned int flags) +virDomainDefPtr +virDomainDefParseString(virCapsPtr caps, + const char *xmlStr, + unsigned int expectedVirtTypes, + unsigned int flags) { return virDomainDefParse(xmlStr, NULL, caps, expectedVirtTypes, flags); } -virDomainDefPtr virDomainDefParseFile(virCapsPtr caps, - const char *filename, - unsigned int expectedVirtTypes, - unsigned int flags) +virDomainDefPtr +virDomainDefParseFile(virCapsPtr caps, + const char *filename, + unsigned int expectedVirtTypes, + unsigned int flags) { return virDomainDefParse(NULL, filename, caps, expectedVirtTypes, flags); } -virDomainDefPtr virDomainDefParseNode(virCapsPtr caps, - xmlDocPtr xml, - xmlNodePtr root, - unsigned int expectedVirtTypes, - unsigned int flags) +virDomainDefPtr +virDomainDefParseNode(virCapsPtr caps, + xmlDocPtr xml, + xmlNodePtr root, + unsigned int expectedVirtTypes, + unsigned int flags) { xmlXPathContextPtr ctxt = NULL; virDomainDefPtr def = NULL; @@ -10961,8 +10968,9 @@ virDomainObjParseFile(virCapsPtr caps, } -static bool virDomainTimerDefCheckABIStability(virDomainTimerDefPtr src, - virDomainTimerDefPtr dst) +static bool +virDomainTimerDefCheckABIStability(virDomainTimerDefPtr src, + virDomainTimerDefPtr dst) { bool identical = false; @@ -11005,8 +11013,9 @@ cleanup: } -static bool virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, - virDomainDeviceInfoPtr dst) +static bool +virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, + virDomainDeviceInfoPtr dst) { bool identical = false; @@ -11084,8 +11093,9 @@ cleanup: } -static bool virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, - virDomainDiskDefPtr dst) +static bool +virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, + virDomainDiskDefPtr dst) { bool identical = false; @@ -11135,8 +11145,9 @@ cleanup: } -static bool virDomainControllerDefCheckABIStability(virDomainControllerDefPtr src, - virDomainControllerDefPtr dst) +static bool +virDomainControllerDefCheckABIStability(virDomainControllerDefPtr src, + virDomainControllerDefPtr dst) { bool identical = false; @@ -11188,8 +11199,9 @@ cleanup: } -static bool virDomainFsDefCheckABIStability(virDomainFSDefPtr src, - virDomainFSDefPtr dst) +static bool +virDomainFsDefCheckABIStability(virDomainFSDefPtr src, + virDomainFSDefPtr dst) { bool identical = false; @@ -11216,8 +11228,9 @@ cleanup: } -static bool virDomainNetDefCheckABIStability(virDomainNetDefPtr src, - virDomainNetDefPtr dst) +static bool +virDomainNetDefCheckABIStability(virDomainNetDefPtr src, + virDomainNetDefPtr dst) { bool identical = false; @@ -11249,8 +11262,9 @@ cleanup: } -static bool virDomainInputDefCheckABIStability(virDomainInputDefPtr src, - virDomainInputDefPtr dst) +static bool +virDomainInputDefCheckABIStability(virDomainInputDefPtr src, + virDomainInputDefPtr dst) { bool identical = false; @@ -11280,8 +11294,9 @@ cleanup: } -static bool virDomainSoundDefCheckABIStability(virDomainSoundDefPtr src, - virDomainSoundDefPtr dst) +static bool +virDomainSoundDefCheckABIStability(virDomainSoundDefPtr src, + virDomainSoundDefPtr dst) { bool identical = false; @@ -11303,8 +11318,9 @@ cleanup: } -static bool virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, - virDomainVideoDefPtr dst) +static bool +virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, + virDomainVideoDefPtr dst) { bool identical = false; @@ -11363,8 +11379,9 @@ cleanup: } -static bool virDomainHostdevDefCheckABIStability(virDomainHostdevDefPtr src, - virDomainHostdevDefPtr dst) +static bool +virDomainHostdevDefCheckABIStability(virDomainHostdevDefPtr src, + virDomainHostdevDefPtr dst) { bool identical = false; @@ -11396,8 +11413,9 @@ cleanup: } -static bool virDomainSmartcardDefCheckABIStability(virDomainSmartcardDefPtr src, - virDomainSmartcardDefPtr dst) +static bool +virDomainSmartcardDefCheckABIStability(virDomainSmartcardDefPtr src, + virDomainSmartcardDefPtr dst) { bool identical = false; @@ -11411,8 +11429,9 @@ cleanup: } -static bool virDomainSerialDefCheckABIStability(virDomainChrDefPtr src, - virDomainChrDefPtr dst) +static bool +virDomainSerialDefCheckABIStability(virDomainChrDefPtr src, + virDomainChrDefPtr dst) { bool identical = false; @@ -11433,8 +11452,9 @@ cleanup: } -static bool virDomainParallelDefCheckABIStability(virDomainChrDefPtr src, - virDomainChrDefPtr dst) +static bool +virDomainParallelDefCheckABIStability(virDomainChrDefPtr src, + virDomainChrDefPtr dst) { bool identical = false; @@ -11455,8 +11475,9 @@ cleanup: } -static bool virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, - virDomainChrDefPtr dst) +static bool +virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, + virDomainChrDefPtr dst) { bool identical = false; @@ -11511,8 +11532,9 @@ cleanup: } -static bool virDomainConsoleDefCheckABIStability(virDomainChrDefPtr src, - virDomainChrDefPtr dst) +static bool +virDomainConsoleDefCheckABIStability(virDomainChrDefPtr src, + virDomainChrDefPtr dst) { bool identical = false; @@ -11534,8 +11556,9 @@ cleanup: } -static bool virDomainWatchdogDefCheckABIStability(virDomainWatchdogDefPtr src, - virDomainWatchdogDefPtr dst) +static bool +virDomainWatchdogDefCheckABIStability(virDomainWatchdogDefPtr src, + virDomainWatchdogDefPtr dst) { bool identical = false; @@ -11557,8 +11580,9 @@ cleanup: } -static bool virDomainMemballoonDefCheckABIStability(virDomainMemballoonDefPtr src, - virDomainMemballoonDefPtr dst) +static bool +virDomainMemballoonDefCheckABIStability(virDomainMemballoonDefPtr src, + virDomainMemballoonDefPtr dst) { bool identical = false; @@ -11580,8 +11604,9 @@ cleanup: } -static bool virDomainHubDefCheckABIStability(virDomainHubDefPtr src, - virDomainHubDefPtr dst) +static bool +virDomainHubDefCheckABIStability(virDomainHubDefPtr src, + virDomainHubDefPtr dst) { bool identical = false; @@ -11662,8 +11687,9 @@ cleanup: * which will affect the guest ABI. This is primarily to allow * validation of custom XML config passed in during migration */ -bool virDomainDefCheckABIStability(virDomainDefPtr src, - virDomainDefPtr dst) +bool +virDomainDefCheckABIStability(virDomainDefPtr src, + virDomainDefPtr dst) { bool identical = false; int i; @@ -11969,9 +11995,10 @@ cleanup: } -static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, - int controllerType, - int diskBus) +static int +virDomainDefAddDiskControllersForType(virDomainDefPtr def, + int controllerType, + int diskBus) { int i; int maxController = -1; @@ -11996,7 +12023,8 @@ static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, } -static int virDomainDefMaybeAddVirtioSerialController(virDomainDefPtr def) +static int +virDomainDefMaybeAddVirtioSerialController(virDomainDefPtr def) { /* Look for any virtio serial or virtio console devs */ int i; @@ -12079,7 +12107,8 @@ virDomainDefMaybeAddSmartcardController(virDomainDefPtr def) * in the XML. This is for compat with existing apps which will * not know/care about <controller> info in the XML */ -int virDomainDefAddImplicitControllers(virDomainDefPtr def) +int +virDomainDefAddImplicitControllers(virDomainDefPtr def) { if (virDomainDefAddDiskControllersForType(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, @@ -12148,11 +12177,12 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, return NULL; } -int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, - int *nvcpupin, - unsigned char *cpumap, - int maplen, - int vcpu) +int +virDomainVcpuPinAdd(virDomainVcpuPinDefPtr **vcpupin_list, + int *nvcpupin, + unsigned char *cpumap, + int maplen, + int vcpu) { virDomainVcpuPinDefPtr vcpupin = NULL; @@ -12388,8 +12418,9 @@ virDomainLeaseDefFormat(virBufferPtr buf, return 0; } -static void virDomainDiskGeometryDefFormat(virBufferPtr buf, - virDomainDiskDefPtr def) +static void +virDomainDiskGeometryDefFormat(virBufferPtr buf, + virDomainDiskDefPtr def) { const char *trans = virDomainDiskGeometryTransTypeToString(def->geometry.trans); @@ -12409,8 +12440,9 @@ static void virDomainDiskGeometryDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } } -static void virDomainDiskBlockIoDefFormat(virBufferPtr buf, - virDomainDiskDefPtr def) +static void +virDomainDiskBlockIoDefFormat(virBufferPtr buf, + virDomainDiskDefPtr def) { if (def->blockio.logical_block_size > 0 || def->blockio.physical_block_size > 0) { @@ -14868,9 +14900,10 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int flags) } -static char *virDomainObjFormat(virCapsPtr caps, - virDomainObjPtr obj, - unsigned int flags) +static char * +virDomainObjFormat(virCapsPtr caps, + virDomainObjPtr obj, + unsigned int flags) { virBuffer buf = VIR_BUFFER_INITIALIZER; int state; @@ -14964,9 +14997,10 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, return 0; } -int virDomainSaveXML(const char *configDir, - virDomainDefPtr def, - const char *xml) +int +virDomainSaveXML(const char *configDir, + virDomainDefPtr def, + const char *xml) { char uuidstr[VIR_UUID_STRING_BUFLEN]; char *configFile = NULL; @@ -14992,8 +15026,9 @@ int virDomainSaveXML(const char *configDir, return ret; } -int virDomainSaveConfig(const char *configDir, - virDomainDefPtr def) +int +virDomainSaveConfig(const char *configDir, + virDomainDefPtr def) { int ret = -1; char *xml; @@ -15011,9 +15046,10 @@ cleanup: return ret; } -int virDomainSaveStatus(virCapsPtr caps, - const char *statusDir, - virDomainObjPtr obj) +int +virDomainSaveStatus(virCapsPtr caps, + const char *statusDir, + virDomainObjPtr obj) { unsigned int flags = (VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INTERNAL_STATUS | @@ -15130,14 +15166,15 @@ error: return NULL; } -int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, - virCapsPtr caps, - const char *configDir, - const char *autostartDir, - int liveStatus, - unsigned int expectedVirtTypes, - virDomainLoadConfigNotify notify, - void *opaque) +int +virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, + virCapsPtr caps, + const char *configDir, + const char *autostartDir, + int liveStatus, + unsigned int expectedVirtTypes, + virDomainLoadConfigNotify notify, + void *opaque) { DIR *dir; struct dirent *entry; @@ -15196,9 +15233,10 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, return 0; } -int virDomainDeleteConfig(const char *configDir, - const char *autostartDir, - virDomainObjPtr dom) +int +virDomainDeleteConfig(const char *configDir, + const char *autostartDir, + virDomainObjPtr dom) { char *configFile = NULL, *autostartLink = NULL; int ret = -1; @@ -15227,8 +15265,9 @@ cleanup: return ret; } -char *virDomainConfigFile(const char *dir, - const char *name) +char +*virDomainConfigFile(const char *dir, + const char *name) { char *ret = NULL; @@ -15248,9 +15287,10 @@ char *virDomainConfigFile(const char *dir, * @param devIdx parsed device number * @return 0 on success, -1 on failure */ -int virDiskNameToBusDeviceIndex(const virDomainDiskDefPtr disk, - int *busIdx, - int *devIdx) { +int +virDiskNameToBusDeviceIndex(const virDomainDiskDefPtr disk, + int *busIdx, + int *devIdx) { int idx = virDiskNameToIndex(disk->dst); if (idx < 0) @@ -15278,7 +15318,8 @@ int virDiskNameToBusDeviceIndex(const virDomainDiskDefPtr disk, return 0; } -virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def) +virDomainFSDefPtr +virDomainGetRootFilesystem(virDomainDefPtr def) { int i; @@ -15294,7 +15335,10 @@ virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def) } -static void virDomainObjListCountActive(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) +static void +virDomainObjListCountActive(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) { virDomainObjPtr obj = payload; int *count = data; @@ -15304,7 +15348,10 @@ static void virDomainObjListCountActive(void *payload, const void *name ATTRIBUT virObjectUnlock(obj); } -static void virDomainObjListCountInactive(void *payload, const void *name ATTRIBUTE_UNUSED, void *data) +static void +virDomainObjListCountInactive(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) { virDomainObjPtr obj = payload; int *count = data; @@ -15314,7 +15361,9 @@ static void virDomainObjListCountInactive(void *payload, const void *name ATTRIB virObjectUnlock(obj); } -int virDomainObjListNumOfDomains(virDomainObjListPtr doms, int active) +int +virDomainObjListNumOfDomains(virDomainObjListPtr doms, + int active) { int count = 0; virObjectLock(doms); @@ -15332,7 +15381,10 @@ struct virDomainIDData { int *ids; }; -static void virDomainObjListCopyActiveIDs(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) +static void +virDomainObjListCopyActiveIDs(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) { virDomainObjPtr obj = payload; struct virDomainIDData *data = opaque; @@ -15342,9 +15394,10 @@ static void virDomainObjListCopyActiveIDs(void *payload, const void *name ATTRIB virObjectUnlock(obj); } -int virDomainObjListGetActiveIDs(virDomainObjListPtr doms, - int *ids, - int maxids) +int +virDomainObjListGetActiveIDs(virDomainObjListPtr doms, + int *ids, + int maxids) { struct virDomainIDData data = { 0, maxids, ids }; virObjectLock(doms); @@ -15360,7 +15413,10 @@ struct virDomainNameData { char **const names; }; -static void virDomainObjListCopyInactiveNames(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) +static void +virDomainObjListCopyInactiveNames(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) { virDomainObjPtr obj = payload; struct virDomainNameData *data = opaque; @@ -15379,9 +15435,10 @@ static void virDomainObjListCopyInactiveNames(void *payload, const void *name AT } -int virDomainObjListGetInactiveNames(virDomainObjListPtr doms, - char **const names, - int maxnames) +int +virDomainObjListGetInactiveNames(virDomainObjListPtr doms, + char **const names, + int maxnames) { struct virDomainNameData data = { 0, 0, maxnames, names }; int i; @@ -15419,9 +15476,10 @@ virDomainObjListHelper(void *payload, data->ret = -1; } -int virDomainObjListForEach(virDomainObjListPtr doms, - virDomainObjListIterator callback, - void *opaque) +int +virDomainObjListForEach(virDomainObjListPtr doms, + virDomainObjListIterator callback, + void *opaque) { struct virDomainListIterData data = { callback, opaque, 0, @@ -15433,10 +15491,11 @@ int virDomainObjListForEach(virDomainObjListPtr doms, } -int virDomainChrDefForeach(virDomainDefPtr def, - bool abortOnError, - virDomainChrDefIterator iter, - void *opaque) +int +virDomainChrDefForeach(virDomainDefPtr def, + bool abortOnError, + virDomainChrDefIterator iter, + void *opaque) { int i; int rc = 0; @@ -15485,10 +15544,11 @@ done: } -int virDomainSmartcardDefForeach(virDomainDefPtr def, - bool abortOnError, - virDomainSmartcardDefIterator iter, - void *opaque) +int +virDomainSmartcardDefForeach(virDomainDefPtr def, + bool abortOnError, + virDomainSmartcardDefIterator iter, + void *opaque) { int i; int rc = 0; @@ -15513,10 +15573,11 @@ done: * ignoreOpenFailure determines whether to warn about a chain that * mentions a backing file without also having metadata on that * file. */ -int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, - bool ignoreOpenFailure, - virDomainDiskDefPathIterator iter, - void *opaque) +int +virDomainDiskDefForeachPath(virDomainDiskDefPtr disk, + bool ignoreOpenFailure, + virDomainDiskDefPathIterator iter, + void *opaque) { int ret = -1; size_t depth = 0; -- 1.8.1.1

Declare local variables at the start of the block and fix trivial formatting issues. --- src/conf/domain_conf.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fad293b..18df1bd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14463,9 +14463,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->cputune.emulator_quota); for (i = 0; i < def->cputune.nvcpupin; i++) { - /* Ignore the vcpupin which inherit from "cpuset" - * of "<vcpu>." - */ + char *cpumask; + /* Ignore the vcpupin which inherit from "cpuset of "<vcpu>." */ if (def->cpumask && virBitmapEqual(def->cpumask, def->cputune.vcpupin[i]->cpumask)) @@ -14474,10 +14473,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " <vcpupin vcpu='%u' ", def->cputune.vcpupin[i]->vcpuid); - char *cpumask = NULL; - cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask); - - if (cpumask == NULL) { + if (!(cpumask = virBitmapFormat(def->cputune.vcpupin[i]->cpumask))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format cpuset for vcpupin")); goto error; @@ -14488,11 +14484,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (def->cputune.emulatorpin) { + char *cpumask; virBufferAsprintf(buf, " <emulatorpin "); - char *cpumask = NULL; - cpumask = virBitmapFormat(def->cputune.emulatorpin->cpumask); - if (cpumask == NULL) { + if (!(cpumask = virBitmapFormat(def->cputune.emulatorpin->cpumask))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to format cpuset for emulator")); goto error; -- 1.8.1.1

This patch changes many unrelated places to simplify the code or update code style. This patch should not have any semantic impact on the code. --- src/conf/domain_conf.c | 137 +++++++++++++++++++++---------------------------- src/conf/domain_conf.h | 4 +- 2 files changed, 60 insertions(+), 81 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 18df1bd..b4340f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8150,9 +8150,9 @@ virDomainDeviceDefParse(virCapsPtr caps, xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) { + if (!(xml = virXMLParseStringCtxt(xmlStr, _("(device_definition)"), &ctxt))) goto error; - } + node = ctxt->node; if (VIR_ALLOC(dev) < 0) { @@ -8219,20 +8219,18 @@ virDomainDeviceDefParse(virCapsPtr caps, if (!(dev->data.redirdev = virDomainRedirdevDefParseXML(node, NULL, flags))) goto error; } else { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("unknown device type")); + virReportError(VIR_ERR_XML_ERROR, "%s", _("unknown device type")); goto error; } +cleanup: xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return dev; - error: - xmlFreeDoc(xml); - xmlXPathFreeContext(ctxt); +error: VIR_FREE(dev); - return NULL; + goto cleanup; } @@ -9296,8 +9294,7 @@ virDomainDefParseXML(virCapsPtr caps, def->mem.cur_balloon = def->mem.max_balloon; } - node = virXPathNode("./memoryBacking/hugepages", ctxt); - if (node) + if ((node = virXPathNode("./memoryBacking/hugepages", ctxt))) def->mem.hugepage_backed = true; /* Extract blkio cgroup tunables */ @@ -9652,36 +9649,34 @@ virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(nodes); - n = virXPathNodeSet("./features/*", ctxt, &nodes); - if (n < 0) + if ((n = virXPathNodeSet("./features/*", ctxt, &nodes)) < 0) goto error; - if (n) { - for (i = 0 ; i < n ; i++) { - int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name); - if (val < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected feature %s"), - nodes[i]->name); - goto error; - } - def->features |= (1 << val); - if (val == VIR_DOMAIN_FEATURE_APIC) { - tmp = virXPathString("string(./features/apic/@eoi)", ctxt); - if (tmp) { - int eoi; - if ((eoi = virDomainFeatureStateTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown value for attribute eoi: %s"), - tmp); - goto error; - } - def->apic_eoi = eoi; - VIR_FREE(tmp); + + for (i = 0 ; i < n ; i++) { + int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name); + if (val < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected feature %s"), + nodes[i]->name); + goto error; + } + def->features |= (1 << val); + if (val == VIR_DOMAIN_FEATURE_APIC) { + tmp = virXPathString("string(./features/apic/@eoi)", ctxt); + if (tmp) { + int eoi; + if ((eoi = virDomainFeatureStateTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown value for attribute eoi: %s"), + tmp); + goto error; } + def->apic_eoi = eoi; + VIR_FREE(tmp); } } - VIR_FREE(nodes); } + VIR_FREE(nodes); if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) { int feature; @@ -9769,17 +9764,14 @@ virDomainDefParseXML(virCapsPtr caps, &def->pm.s4) < 0) goto error; - tmp = virXPathString("string(./clock/@offset)", ctxt); - if (tmp) { - if ((def->clock.offset = virDomainClockOffsetTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown clock offset '%s'"), tmp); - goto error; - } - VIR_FREE(tmp); - } else { - def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; + if ((tmp = virXPathString("string(./clock/@offset)", ctxt)) && + (def->clock.offset = virDomainClockOffsetTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown clock offset '%s'"), tmp); + goto error; } + VIR_FREE(tmp); + switch (def->clock.offset) { case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: case VIR_DOMAIN_CLOCK_OFFSET_UTC: @@ -9838,11 +9830,12 @@ virDomainDefParseXML(virCapsPtr caps, break; } - if ((n = virXPathNodeSet("./clock/timer", ctxt, &nodes)) < 0) { + if ((n = virXPathNodeSet("./clock/timer", ctxt, &nodes)) < 0) goto error; - } + if (n && VIR_ALLOC_N(def->clock.timers, n) < 0) goto no_memory; + for (i = 0 ; i < n ; i++) { virDomainTimerDefPtr timer = virDomainTimerDefParseXML(nodes[i], ctxt); @@ -10690,8 +10683,8 @@ virDomainDefParseXML(virCapsPtr caps, } } } - tmp = virXPathString("string(./os/smbios/@mode)", ctxt); - if (tmp) { + + if ((tmp = virXPathString("string(./os/smbios/@mode)", ctxt))) { int mode; if ((mode = virDomainSmbiosModeTypeFromString(tmp)) < 0) { @@ -10701,27 +10694,22 @@ virDomainDefParseXML(virCapsPtr caps, } def->os.smbios_mode = mode; VIR_FREE(tmp); - } else { - def->os.smbios_mode = VIR_DOMAIN_SMBIOS_NONE; /* not present */ } /* Extract custom metadata */ - if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) { + if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) def->metadata = xmlCopyNode(node, 1); - } /* we have to make a copy of all of the callback pointers here since * we won't have the virCaps structure available during free */ def->ns = caps->ns; - if (def->ns.parse) { - if ((def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) - goto error; - } + if (def->ns.parse && + (def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) + goto error; - /* Auto-add any implied controllers which aren't present - */ + /* Auto-add any implied controllers which aren't present */ if (virDomainDefAddImplicitControllers(def) < 0) goto error; @@ -10731,9 +10719,7 @@ virDomainDefParseXML(virCapsPtr caps, no_memory: virReportOOMError(); - /* fallthrough */ - - error: +error: VIR_FREE(tmp); VIR_FREE(nodes); virBitmapFree(bootMap); @@ -10898,8 +10884,7 @@ virDomainDefParseNode(virCapsPtr caps, goto cleanup; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { + if (!(ctxt = xmlXPathNewContext(xml))) { virReportOOMError(); goto cleanup; } @@ -15028,8 +15013,7 @@ virDomainSaveConfig(const char *configDir, int ret = -1; char *xml; - if (!(xml = virDomainDefFormat(def, - VIR_DOMAIN_XML_WRITE_FLAGS))) + if (!(xml = virDomainDefFormat(def, VIR_DOMAIN_XML_WRITE_FLAGS))) goto cleanup; if (virDomainSaveXML(configDir, def, xml)) @@ -15238,7 +15222,8 @@ virDomainDeleteConfig(const char *configDir, if ((configFile = virDomainConfigFile(configDir, dom->def->name)) == NULL) goto cleanup; - if ((autostartLink = virDomainConfigFile(autostartDir, dom->def->name)) == NULL) + if ((autostartLink = virDomainConfigFile(autostartDir, + dom->def->name)) == NULL) goto cleanup; /* Not fatal if this doesn't work */ @@ -15264,12 +15249,10 @@ char *virDomainConfigFile(const char *dir, const char *name) { - char *ret = NULL; + char *ret; - if (virAsprintf(&ret, "%s/%s.xml", dir, name) < 0) { + if (virAsprintf(&ret, "%s/%s.xml", dir, name) < 0) virReportOOMError(); - return NULL; - } return ret; } @@ -15441,16 +15424,13 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms, virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data); virObjectUnlock(doms); if (data.oom) { + for (i = 0 ; i < data.numnames ; i++) + VIR_FREE(data.names[i]); virReportOOMError(); - goto cleanup; + return -1; } return data.numnames; - -cleanup: - for (i = 0 ; i < data.numnames ; i++) - VIR_FREE(data.names[i]); - return -1; } @@ -15620,8 +15600,7 @@ virDomainDefCopy(virCapsPtr caps, virDomainDefPtr src, bool migratable) write_flags |= VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_MIGRATABLE; /* Easiest to clone via a round-trip through XML. */ - xml = virDomainDefFormat(src, write_flags); - if (!xml) + if (!(xml = virDomainDefFormat(src, write_flags))) return NULL; ret = virDomainDefParseString(caps, xml, -1, read_flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4ffa4aa..cce53ca 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -783,7 +783,7 @@ struct _virDomainFSDef { }; -/* 5 different types of networking config */ +/* network config types */ enum virDomainNetType { VIR_DOMAIN_NET_TYPE_USER, VIR_DOMAIN_NET_TYPE_ETHERNET, @@ -1420,7 +1420,7 @@ struct _virDomainMemballoonDef { enum virDomainSmbiosMode { - VIR_DOMAIN_SMBIOS_NONE, + VIR_DOMAIN_SMBIOS_NONE = 0, VIR_DOMAIN_SMBIOS_EMULATE, VIR_DOMAIN_SMBIOS_HOST, VIR_DOMAIN_SMBIOS_SYSINFO, -- 1.8.1.1

Get rid of the "identical" variable in the ABI stability checks in favor of return statements and break or refactor very long lines where possible. --- src/conf/domain_conf.c | 469 +++++++++++++++++++++---------------------------- 1 file changed, 200 insertions(+), 269 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b4340f3..8024bff 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10957,21 +10957,19 @@ static bool virDomainTimerDefCheckABIStability(virDomainTimerDefPtr src, virDomainTimerDefPtr dst) { - bool identical = false; - if (src->name != dst->name) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target timer %s does not match source %s"), virDomainTimerNameTypeToString(dst->name), virDomainTimerNameTypeToString(src->name)); - goto cleanup; + return false; } if (src->present != dst->present) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target timer presence %d does not match source %d"), dst->present, src->present); - goto cleanup; + return false; } if (src->name == VIR_DOMAIN_TIMER_NAME_TSC) { @@ -10979,7 +10977,7 @@ virDomainTimerDefCheckABIStability(virDomainTimerDefPtr src, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target TSC frequency %lu does not match source %lu"), dst->frequency, src->frequency); - goto cleanup; + return false; } if (src->mode != dst->mode) { @@ -10987,14 +10985,11 @@ virDomainTimerDefCheckABIStability(virDomainTimerDefPtr src, _("Target TSC mode %s does not match source %s"), virDomainTimerModeTypeToString(dst->mode), virDomainTimerModeTypeToString(src->mode)); - goto cleanup; + return false; } } - identical = true; - -cleanup: - return identical; + return true; } @@ -11002,14 +10997,12 @@ static bool virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, virDomainDeviceInfoPtr dst) { - bool identical = false; - if (src->type != dst->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target device address type %s does not match source %s"), virDomainDeviceAddressTypeToString(dst->type), virDomainDeviceAddressTypeToString(src->type)); - goto cleanup; + return false; } switch (src->type) { @@ -11025,7 +11018,7 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, dst->addr.pci.slot, dst->addr.pci.function, src->addr.pci.domain, src->addr.pci.bus, src->addr.pci.slot, src->addr.pci.function); - goto cleanup; + return false; } break; @@ -11034,12 +11027,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, src->addr.drive.bus != dst->addr.drive.bus || src->addr.drive.unit != dst->addr.drive.unit) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target device drive address %d:%d:%d does not match source %d:%d:%d"), + _("Target device drive address %d:%d:%d " + "does not match source %d:%d:%d"), dst->addr.drive.controller, dst->addr.drive.bus, dst->addr.drive.unit, src->addr.drive.controller, src->addr.drive.bus, src->addr.drive.unit); - goto cleanup; + return false; } break; @@ -11048,12 +11042,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, src->addr.vioserial.bus != dst->addr.vioserial.bus || src->addr.vioserial.port != dst->addr.vioserial.port) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target device virtio serial address %d:%d:%d does not match source %d:%d:%d"), + _("Target device virtio serial address %d:%d:%d " + "does not match source %d:%d:%d"), dst->addr.vioserial.controller, dst->addr.vioserial.bus, dst->addr.vioserial.port, src->addr.vioserial.controller, src->addr.vioserial.bus, src->addr.vioserial.port); - goto cleanup; + return false; } break; @@ -11061,20 +11056,18 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src, if (src->addr.ccid.controller != dst->addr.ccid.controller || src->addr.ccid.slot != dst->addr.ccid.slot) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target device ccid address %d:%d does not match source %d:%d"), + _("Target device ccid address %d:%d " + "does not match source %d:%d"), dst->addr.ccid.controller, dst->addr.ccid.slot, src->addr.ccid.controller, src->addr.ccid.slot); - goto cleanup; + return false; } break; } - identical = true; - -cleanup: - return identical; + return true; } @@ -11082,14 +11075,12 @@ static bool virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, virDomainDiskDefPtr dst) { - bool identical = false; - if (src->device != dst->device) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target disk device %s does not match source %s"), virDomainDiskDeviceTypeToString(dst->device), virDomainDiskDeviceTypeToString(src->device)); - goto cleanup; + return false; } if (src->bus != dst->bus) { @@ -11097,36 +11088,33 @@ virDomainDiskDefCheckABIStability(virDomainDiskDefPtr src, _("Target disk bus %s does not match source %s"), virDomainDiskBusTypeToString(dst->bus), virDomainDiskBusTypeToString(src->bus)); - goto cleanup; + return false; } if (STRNEQ(src->dst, dst->dst)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target disk %s does not match source %s"), dst->dst, src->dst); - goto cleanup; + return false; } if (STRNEQ_NULLABLE(src->serial, dst->serial)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target disk serial %s does not match source %s"), NULLSTR(dst->serial), NULLSTR(src->serial)); - goto cleanup; + return false; } if (src->readonly != dst->readonly || src->shared != dst->shared) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target disk access mode does not match source")); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11134,28 +11122,26 @@ static bool virDomainControllerDefCheckABIStability(virDomainControllerDefPtr src, virDomainControllerDefPtr dst) { - bool identical = false; - if (src->type != dst->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target controller type %s does not match source %s"), virDomainControllerTypeToString(dst->type), virDomainControllerTypeToString(src->type)); - goto cleanup; + return false; } if (src->idx != dst->idx) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target controller index %d does not match source %d"), dst->idx, src->idx); - goto cleanup; + return false; } if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target controller model %d does not match source %d"), dst->model, src->model); - goto cleanup; + return false; } if (src->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) { @@ -11163,24 +11149,21 @@ virDomainControllerDefCheckABIStability(virDomainControllerDefPtr src, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target controller ports %d does not match source %d"), dst->opts.vioserial.ports, src->opts.vioserial.ports); - goto cleanup; + return false; } if (src->opts.vioserial.vectors != dst->opts.vioserial.vectors) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target controller vectors %d does not match source %d"), dst->opts.vioserial.vectors, src->opts.vioserial.vectors); - goto cleanup; + return false; } } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11188,28 +11171,23 @@ static bool virDomainFsDefCheckABIStability(virDomainFSDefPtr src, virDomainFSDefPtr dst) { - bool identical = false; - if (STRNEQ(src->dst, dst->dst)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target filesystem guest target %s does not match source %s"), dst->dst, src->dst); - goto cleanup; + return false; } if (src->readonly != dst->readonly) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target filesystem access mode does not match source")); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11217,8 +11195,6 @@ static bool virDomainNetDefCheckABIStability(virDomainNetDefPtr src, virDomainNetDefPtr dst) { - bool identical = false; - if (virMacAddrCmp(&src->mac, &dst->mac) != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card mac %02x:%02x:%02x:%02x:%02x:%02x" @@ -11227,23 +11203,20 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, dst->mac.addr[3], dst->mac.addr[4], dst->mac.addr[5], src->mac.addr[0], src->mac.addr[1], src->mac.addr[2], src->mac.addr[3], src->mac.addr[4], src->mac.addr[5]); - goto cleanup; + return false; } if (STRNEQ_NULLABLE(src->model, dst->model)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target network card model %s does not match source %s"), NULLSTR(dst->model), NULLSTR(src->model)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11251,14 +11224,12 @@ static bool virDomainInputDefCheckABIStability(virDomainInputDefPtr src, virDomainInputDefPtr dst) { - bool identical = false; - if (src->type != dst->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target input device type %s does not match source %s"), virDomainInputTypeToString(dst->type), virDomainInputTypeToString(src->type)); - goto cleanup; + return false; } if (src->bus != dst->bus) { @@ -11266,16 +11237,13 @@ virDomainInputDefCheckABIStability(virDomainInputDefPtr src, _("Target input device bus %s does not match source %s"), virDomainInputBusTypeToString(dst->bus), virDomainInputBusTypeToString(src->bus)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11283,23 +11251,18 @@ static bool virDomainSoundDefCheckABIStability(virDomainSoundDefPtr src, virDomainSoundDefPtr dst) { - bool identical = false; - if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target sound card model %s does not match source %s"), virDomainSoundModelTypeToString(dst->model), virDomainSoundModelTypeToString(src->model)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11307,35 +11270,33 @@ static bool virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, virDomainVideoDefPtr dst) { - bool identical = false; - if (src->type != dst->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card model %s does not match source %s"), virDomainVideoTypeToString(dst->type), virDomainVideoTypeToString(src->type)); - goto cleanup; + return false; } if (src->vram != dst->vram) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card vram %u does not match source %u"), dst->vram, src->vram); - goto cleanup; + return false; } if (src->heads != dst->heads) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card heads %u does not match source %u"), dst->heads, src->heads); - goto cleanup; + return false; } if ((src->accel && !dst->accel) || (!src->accel && dst->accel)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target video card acceleration does not match source")); - goto cleanup; + return false; } if (src->accel) { @@ -11343,24 +11304,21 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card 2d accel %u does not match source %u"), dst->accel->support2d, src->accel->support2d); - goto cleanup; + return false; } if (src->accel->support3d != dst->accel->support3d) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card 3d accel %u does not match source %u"), dst->accel->support3d, src->accel->support3d); - goto cleanup; + return false; } } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11368,33 +11326,27 @@ static bool virDomainHostdevDefCheckABIStability(virDomainHostdevDefPtr src, virDomainHostdevDefPtr dst) { - bool identical = false; - if (src->mode != dst->mode) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target host device mode %s does not match source %s"), virDomainHostdevModeTypeToString(dst->mode), virDomainHostdevModeTypeToString(src->mode)); - goto cleanup; + return false; } - if (src->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { - if (src->source.subsys.type != dst->source.subsys.type) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target host device subsystem %s does not match source %s"), - virDomainHostdevSubsysTypeToString(dst->source.subsys.type), - virDomainHostdevSubsysTypeToString(src->source.subsys.type)); - goto cleanup; - } + if (src->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + src->source.subsys.type != dst->source.subsys.type) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target host device subsystem %s does not match source %s"), + virDomainHostdevSubsysTypeToString(dst->source.subsys.type), + virDomainHostdevSubsysTypeToString(src->source.subsys.type)); + return false; } if (!virDomainDeviceInfoCheckABIStability(src->info, dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11402,15 +11354,10 @@ static bool virDomainSmartcardDefCheckABIStability(virDomainSmartcardDefPtr src, virDomainSmartcardDefPtr dst) { - bool identical = false; - if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11418,22 +11365,17 @@ static bool virDomainSerialDefCheckABIStability(virDomainChrDefPtr src, virDomainChrDefPtr dst) { - bool identical = false; - if (src->target.port != dst->target.port) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target serial port %d does not match source %d"), dst->target.port, src->target.port); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11441,22 +11383,17 @@ static bool virDomainParallelDefCheckABIStability(virDomainChrDefPtr src, virDomainChrDefPtr dst) { - bool identical = false; - if (src->target.port != dst->target.port) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target serial port %d does not match source %d"), dst->target.port, src->target.port); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11464,14 +11401,12 @@ static bool virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, virDomainChrDefPtr dst) { - bool identical = false; - if (src->targetType != dst->targetType) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target channel type %s does not match source %s"), virDomainChrChannelTargetTypeToString(dst->targetType), virDomainChrChannelTargetTypeToString(src->targetType)); - goto cleanup; + return false; } switch (src->targetType) { @@ -11480,7 +11415,7 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target channel name %s does not match source %s"), NULLSTR(dst->target.name), NULLSTR(src->target.name)); - goto cleanup; + return false; } if (src->source.type != dst->source.type && (src->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC || @@ -11489,7 +11424,7 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Changing device type to/from spicevmc would" " change default target channel name")); - goto cleanup; + return false; } break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: @@ -11502,18 +11437,15 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr src, NULLSTR(daddr), NULLSTR(saddr)); VIR_FREE(saddr); VIR_FREE(daddr); - goto cleanup; + return false; } break; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11521,23 +11453,18 @@ static bool virDomainConsoleDefCheckABIStability(virDomainChrDefPtr src, virDomainChrDefPtr dst) { - bool identical = false; - if (src->targetType != dst->targetType) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target console type %s does not match source %s"), virDomainChrConsoleTargetTypeToString(dst->targetType), virDomainChrConsoleTargetTypeToString(src->targetType)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11545,23 +11472,18 @@ static bool virDomainWatchdogDefCheckABIStability(virDomainWatchdogDefPtr src, virDomainWatchdogDefPtr dst) { - bool identical = false; - if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target watchdog model %s does not match source %s"), virDomainWatchdogModelTypeToString(dst->model), virDomainWatchdogModelTypeToString(src->model)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11569,23 +11491,18 @@ static bool virDomainMemballoonDefCheckABIStability(virDomainMemballoonDefPtr src, virDomainMemballoonDefPtr dst) { - bool identical = false; - if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target balloon model %s does not match source %s"), virDomainMemballoonModelTypeToString(dst->model), virDomainMemballoonModelTypeToString(src->model)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } @@ -11593,23 +11510,18 @@ static bool virDomainHubDefCheckABIStability(virDomainHubDefPtr src, virDomainHubDefPtr dst) { - bool identical = false; - if (src->type != dst->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target hub device type %s does not match source %s"), virDomainHubTypeToString(dst->type), virDomainHubTypeToString(src->type)); - goto cleanup; + return false; } if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) - goto cleanup; - - identical = true; + return false; -cleanup: - return identical; + return true; } static bool @@ -11617,14 +11529,13 @@ virDomainRedirFilterDefCheckABIStability(virDomainRedirFilterDefPtr src, virDomainRedirFilterDefPtr dst) { int i; - bool identical = false; if (src->nusbdevs != dst->nusbdevs) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target USB redirection filter rule " "count %zu does not match source %zu"), dst->nusbdevs, src->nusbdevs); - goto cleanup; + return false; } for (i = 0; i < src->nusbdevs; i++) { @@ -11633,25 +11544,25 @@ virDomainRedirFilterDefCheckABIStability(virDomainRedirFilterDefPtr src, if (srcUsbDev->usbClass != dstUsbDev->usbClass) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target USB Class code does not match source")); - goto cleanup; + return false; } if (srcUsbDev->vendor != dstUsbDev->vendor) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target USB vendor ID does not match source")); - goto cleanup; + return false; } if (srcUsbDev->product != dstUsbDev->product) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target USB product ID does not match source")); - goto cleanup; + return false; } if (srcUsbDev->version != dstUsbDev->version) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target USB version does not match source")); - goto cleanup; + return false; } if (srcUsbDev->allow != dstUsbDev->allow) { @@ -11659,13 +11570,11 @@ virDomainRedirFilterDefCheckABIStability(virDomainRedirFilterDefPtr src, _("Target USB allow '%s' does not match source '%s'"), dstUsbDev->allow ? "yes" : "no", srcUsbDev->allow ? "yes" : "no"); - goto cleanup; + return false; } } - identical = true; -cleanup: - return identical; + return true; } /* This compares two configurations and looks for any differences @@ -11676,7 +11585,6 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, virDomainDefPtr dst) { - bool identical = false; int i; if (src->virtType != dst->virtType) { @@ -11684,7 +11592,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src, _("Target domain virt type %s does not match source %s"), virDomainVirtTypeToString(dst->virtType), virDomainVirtTypeToString(src->virtType)); - goto cleanup; + return false; } if (memcmp(src->uuid, dst->uuid, VIR_UUID_BUFLEN) != 0) { @@ -11695,60 +11603,60 @@ virDomainDefCheckABIStability(virDomainDefPtr src, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain uuid %s does not match source %s"), uuiddst, uuidsrc); - goto cleanup; + return false; } if (src->mem.max_balloon != dst->mem.max_balloon) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain max memory %lld does not match source %lld"), dst->mem.max_balloon, src->mem.max_balloon); - goto cleanup; + return false; } if (src->mem.cur_balloon != dst->mem.cur_balloon) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain current memory %lld does not match source %lld"), dst->mem.cur_balloon, src->mem.cur_balloon); - goto cleanup; + return false; } if (src->mem.hugepage_backed != dst->mem.hugepage_backed) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain huge page backing %d does not match source %d"), dst->mem.hugepage_backed, src->mem.hugepage_backed); - goto cleanup; + return false; } if (src->vcpus != dst->vcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vpu count %d does not match source %d"), dst->vcpus, src->vcpus); - goto cleanup; + return false; } if (src->maxvcpus != dst->maxvcpus) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain vpu max %d does not match source %d"), dst->maxvcpus, src->maxvcpus); - goto cleanup; + return false; } if (STRNEQ(src->os.type, dst->os.type)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain OS type %s does not match source %s"), dst->os.type, src->os.type); - goto cleanup; + return false; } if (src->os.arch != dst->os.arch){ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain architecture %s does not match source %s"), virArchToString(dst->os.arch), virArchToString(src->os.arch)); - goto cleanup; + return false; } if (STRNEQ(src->os.machine, dst->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain OS type %s does not match source %s"), dst->os.machine, src->os.machine); - goto cleanup; + return false; } if (src->os.smbios_mode != dst->os.smbios_mode) { @@ -11756,227 +11664,250 @@ virDomainDefCheckABIStability(virDomainDefPtr src, _("Target domain SMBIOS mode %s does not match source %s"), virDomainSmbiosModeTypeToString(dst->os.smbios_mode), virDomainSmbiosModeTypeToString(src->os.smbios_mode)); - goto cleanup; + return false; } if (src->features != dst->features) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain features %d does not match source %d"), dst->features, src->features); - goto cleanup; + return false; } if (src->clock.ntimers != dst->clock.ntimers) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Target domain timers do not match source")); - goto cleanup; + return false; } for (i = 0 ; i < src->clock.ntimers ; i++) { - if (!virDomainTimerDefCheckABIStability(src->clock.timers[i], dst->clock.timers[i])) - goto cleanup; + if (!virDomainTimerDefCheckABIStability(src->clock.timers[i], + dst->clock.timers[i])) + return false; } if (!virCPUDefIsEqual(src->cpu, dst->cpu)) - goto cleanup; + return false; if (!virSysinfoIsEqual(src->sysinfo, dst->sysinfo)) - goto cleanup; + return false; if (src->ndisks != dst->ndisks) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain disk count %zu does not match source %zu"), dst->ndisks, src->ndisks); - goto cleanup; + return false; } for (i = 0 ; i < src->ndisks ; i++) if (!virDomainDiskDefCheckABIStability(src->disks[i], dst->disks[i])) - goto cleanup; + return false; if (src->ncontrollers != dst->ncontrollers) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain disk controller count %zu does not match source %zu"), + _("Target domain disk controller count %zu " + "does not match source %zu"), dst->ncontrollers, src->ncontrollers); - goto cleanup; + return false; } for (i = 0 ; i < src->ncontrollers ; i++) - if (!virDomainControllerDefCheckABIStability(src->controllers[i], dst->controllers[i])) - goto cleanup; + if (!virDomainControllerDefCheckABIStability(src->controllers[i], + dst->controllers[i])) + return false; if (src->nfss != dst->nfss) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain filesystem count %zu does not match source %zu"), + _("Target domain filesystem count %zu " + "does not match source %zu"), dst->nfss, src->nfss); - goto cleanup; + return false; } for (i = 0 ; i < src->nfss ; i++) if (!virDomainFsDefCheckABIStability(src->fss[i], dst->fss[i])) - goto cleanup; + return false; if (src->nnets != dst->nnets) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain net card count %zu does not match source %zu"), + _("Target domain net card count %zu " + "does not match source %zu"), dst->nnets, src->nnets); - goto cleanup; + return false; } for (i = 0 ; i < src->nnets ; i++) if (!virDomainNetDefCheckABIStability(src->nets[i], dst->nets[i])) - goto cleanup; + return false; if (src->ninputs != dst->ninputs) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain input device count %zu does not match source %zu"), + _("Target domain input device count %zu " + "does not match source %zu"), dst->ninputs, src->ninputs); - goto cleanup; + return false; } for (i = 0 ; i < src->ninputs ; i++) if (!virDomainInputDefCheckABIStability(src->inputs[i], dst->inputs[i])) - goto cleanup; + return false; if (src->nsounds != dst->nsounds) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain sound card count %zu does not match source %zu"), + _("Target domain sound card count %zu " + "does not match source %zu"), dst->nsounds, src->nsounds); - goto cleanup; + return false; } for (i = 0 ; i < src->nsounds ; i++) if (!virDomainSoundDefCheckABIStability(src->sounds[i], dst->sounds[i])) - goto cleanup; + return false; if (src->nvideos != dst->nvideos) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain video card count %zu does not match source %zu"), + _("Target domain video card count %zu " + "does not match source %zu"), dst->nvideos, src->nvideos); - goto cleanup; + return false; } for (i = 0 ; i < src->nvideos ; i++) if (!virDomainVideoDefCheckABIStability(src->videos[i], dst->videos[i])) - goto cleanup; + return false; if (src->nhostdevs != dst->nhostdevs) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain host device count %zu does not match source %zu"), + _("Target domain host device count %zu " + "does not match source %zu"), dst->nhostdevs, src->nhostdevs); - goto cleanup; + return false; } for (i = 0 ; i < src->nhostdevs ; i++) - if (!virDomainHostdevDefCheckABIStability(src->hostdevs[i], dst->hostdevs[i])) - goto cleanup; + if (!virDomainHostdevDefCheckABIStability(src->hostdevs[i], + dst->hostdevs[i])) + return false; if (src->nsmartcards != dst->nsmartcards) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain smartcard count %zu does not match source %zu"), + _("Target domain smartcard count %zu " + "does not match source %zu"), dst->nsmartcards, src->nsmartcards); - goto cleanup; + return false; } for (i = 0 ; i < src->nsmartcards ; i++) - if (!virDomainSmartcardDefCheckABIStability(src->smartcards[i], dst->smartcards[i])) - goto cleanup; + if (!virDomainSmartcardDefCheckABIStability(src->smartcards[i], + dst->smartcards[i])) + return false; if (src->nserials != dst->nserials) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain serial port count %zu does not match source %zu"), + _("Target domain serial port count %zu " + "does not match source %zu"), dst->nserials, src->nserials); - goto cleanup; + return false; } for (i = 0 ; i < src->nserials ; i++) - if (!virDomainSerialDefCheckABIStability(src->serials[i], dst->serials[i])) - goto cleanup; + if (!virDomainSerialDefCheckABIStability(src->serials[i], + dst->serials[i])) + return false; if (src->nparallels != dst->nparallels) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain parallel port count %zu does not match source %zu"), + _("Target domain parallel port count %zu " + "does not match source %zu"), dst->nparallels, src->nparallels); - goto cleanup; + return false; } for (i = 0 ; i < src->nparallels ; i++) - if (!virDomainParallelDefCheckABIStability(src->parallels[i], dst->parallels[i])) - goto cleanup; + if (!virDomainParallelDefCheckABIStability(src->parallels[i], + dst->parallels[i])) + return false; if (src->nchannels != dst->nchannels) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain channel count %zu does not match source %zu"), + _("Target domain channel count %zu " + "does not match source %zu"), dst->nchannels, src->nchannels); - goto cleanup; + return false; } for (i = 0 ; i < src->nchannels ; i++) - if (!virDomainChannelDefCheckABIStability(src->channels[i], dst->channels[i])) - goto cleanup; + if (!virDomainChannelDefCheckABIStability(src->channels[i], + dst->channels[i])) + return false; if (src->nconsoles != dst->nconsoles) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain console count %zu does not match source %zu"), + _("Target domain console count %zu " + "does not match source %zu"), dst->nconsoles, src->nconsoles); - goto cleanup; + return false; } for (i = 0 ; i < src->nconsoles ; i++) - if (!virDomainConsoleDefCheckABIStability(src->consoles[i], dst->consoles[i])) - goto cleanup; + if (!virDomainConsoleDefCheckABIStability(src->consoles[i], + dst->consoles[i])) + return false; if (src->nhubs != dst->nhubs) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain hub device count %zu does not match source %zu"), + _("Target domain hub device count %zu " + "does not match source %zu"), dst->nhubs, src->nhubs); - goto cleanup; + return false; } for (i = 0 ; i < src->nhubs ; i++) if (!virDomainHubDefCheckABIStability(src->hubs[i], dst->hubs[i])) - goto cleanup; + return false; if ((!src->redirfilter && dst->redirfilter) || (src->redirfilter && !dst->redirfilter)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain USB redirection filter count %d does not match source %d"), + _("Target domain USB redirection filter count %d " + "does not match source %d"), dst->redirfilter ? 1 : 0, src->redirfilter ? 1 : 0); - goto cleanup; + return false; } if (src->redirfilter && - !virDomainRedirFilterDefCheckABIStability(src->redirfilter, dst->redirfilter)) - goto cleanup; + !virDomainRedirFilterDefCheckABIStability(src->redirfilter, + dst->redirfilter)) + return false; if ((!src->watchdog && dst->watchdog) || (src->watchdog && !dst->watchdog)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain watchdog count %d does not match source %d"), + _("Target domain watchdog count %d " + "does not match source %d"), dst->watchdog ? 1 : 0, src->watchdog ? 1 : 0); - goto cleanup; + return false; } if (src->watchdog && !virDomainWatchdogDefCheckABIStability(src->watchdog, dst->watchdog)) - goto cleanup; + return false; if ((!src->memballoon && dst->memballoon) || (src->memballoon && !dst->memballoon)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain memory balloon count %d does not match source %d"), + _("Target domain memory balloon count %d " + "does not match source %d"), dst->memballoon ? 1 : 0, src->memballoon ? 1 : 0); - goto cleanup; + return false; } if (src->memballoon && - !virDomainMemballoonDefCheckABIStability(src->memballoon, dst->memballoon)) - goto cleanup; - - identical = true; + !virDomainMemballoonDefCheckABIStability(src->memballoon, + dst->memballoon)) + return false; -cleanup: - return identical; + return true; } -- 1.8.1.1

Make the iterator function usable in the next patches. Also refactor some parts to avoid strcmp if not necessary. --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8024bff..421492f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2311,8 +2311,9 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, return -1; } for (i = 0; i < def->nconsoles ; i++) { - if ((STREQ(def->os.type, "hvm")) && i == 0 && - def->consoles[i]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) + if (i == 0 && + def->consoles[i]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL && + STREQ_NULLABLE(def->os.type, "hvm")) continue; device.data.chr = def->consoles[i]; if (cb(def, &device, &def->consoles[i]->info, opaque) < 0) -- 1.8.1.1

This patch adds instrumentation that will ultimately allow to split out filling of defaults and input validation from the XML parser to separate functions. 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 exposed in the virCaps object: * virDriverDeviceDefCallback - called for a single device parsed and for every single device in a domain config. A virDomainDeviceDefPtr is passed. * virDriverDomainDefCallback - called if a domain definition is parsed device specific callbacks were called. A virDomainDefPtr is passed. Errors may be reported in those callbacks resulting in a XML parsing failure. Additionally two internal filler functions are added: virDomainDeviceDefUpdateDefaultsInternal and virDomainDefUpdateDefaultsInternal that are meant to be used for separating the validation and defaults assignment code from the parser function. --- src/conf/capabilities.h | 6 +++++ src/conf/domain_conf.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index cc01765..56cd09f 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -171,6 +171,12 @@ struct _virCaps { bool hasWideScsiBus; const char *defaultInitPath; + + /* these callbacks are called after a XML definition of a device or domain + * is parsed. They are meant to validate and fill driver-specific values. */ + int (*virDriverDeviceDefCallback)(void *); /* virDomainDeviceDefPtr is passed */ + int (*virDriverDomainDefCallback)(void *); /* virDomainDefPtr is passed */ + virDomainXMLNamespace ns; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 421492f..d881983 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2353,6 +2353,70 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, } +static int +virDomainDeviceDefUpdateDefaultsInternal(virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED) +{ + /* not in use yet */ + return 0; +} + + +static int +virDomainDefUpdateDefaultsInternal(virDomainDefPtr def ATTRIBUTE_UNUSED, + virCapsPtr caps ATTRIBUTE_UNUSED) +{ + /* not in use yet */ + return 0; +} + + +static int +virDomainDeviceDefUpdateDefaults(virDomainDeviceDefPtr dev, + virCapsPtr caps) +{ + int ret; + /* at first call the driver callback to update the device */ + if (caps->virDriverDeviceDefCallback && + (ret = (caps->virDriverDeviceDefCallback)(dev)) < 0) + return ret; + + /* then fill the parse defaults and do the checks */ + return virDomainDeviceDefUpdateDefaultsInternal(dev); +} + + +static int +virDomainDefUpdateDefaultsDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info ATTRIBUTE_UNUSED, + void *opaque) +{ + virCapsPtr caps = opaque; + return virDomainDeviceDefUpdateDefaults(dev, caps); +} + + +static int +virDomainDefUpdateDefaults(virDomainDefPtr def, + virCapsPtr caps) +{ + int ret; + + /* at first update all the devices , unfortunately they are separate */ + if ((ret = virDomainDeviceInfoIterate(def, + virDomainDefUpdateDefaultsDeviceIterator, + caps)) < 0) + return ret; + + /* call the driver callback to update the rest of the definition */ + if (caps->virDriverDomainDefCallback && + (ret = (caps->virDriverDomainDefCallback)(def)) < 0) + return ret; + + return virDomainDefUpdateDefaultsInternal(def, caps); +} + + void virDomainDefClearPCIAddresses(virDomainDefPtr def) { virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearPCIAddress, NULL); @@ -8224,6 +8288,10 @@ virDomainDeviceDefParse(virCapsPtr caps, goto error; } + /* callback to fill driver specific device aspects */ + if (virDomainDeviceDefUpdateDefaults(dev, caps) < 0) + goto error; + cleanup: xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); @@ -10714,6 +10782,10 @@ virDomainDefParseXML(virCapsPtr caps, if (virDomainDefAddImplicitControllers(def) < 0) goto error; + /* callback to fill driver specific domain aspects */ + if (virDomainDefUpdateDefaults(def, caps) < 0) + goto error; + virBitmapFree(bootMap); return def; -- 1.8.1.1

On 03/01/2013 02:10 PM, Laine Stump wrote: And I've already found a counter-example. We will want the PCI addresses of the devices (the ones which don't have an explicitly confugured address anyway) to be assigned by the device callback, and it's true that that will need the domain info to be properly setup (at least machinetype). However, we'll want to go through and auto-create all the required extra bridges during the domain callback, but knowing which bridges we need to create requires that we have already assigned PCI addresses to all of the devices. :-/

On Mon, Mar 04, 2013 at 04:04:43PM +0100, Peter Krempa wrote: This says to me that capabilities.h is not the right place for this data. Originally virCapabilities was just used to hold information about the guests/machine types / etc supported by a driver. Then we extended the domain XML parsers to require this object to be passed in. Then we added namespace hooks to it. Now we're adding yet more stuff. IMHO it is time to undo this mess. We need a virCapabilities object that just contains the data associated with the capabilities XML schema, nothing more. Then we need a virDomainParserConfigPtr struct that contains the information required by the virDomainDef parsers. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/20/2013 12:06 PM, Peter Krempa wrote: This is a good example of why the domain pointer and caps pointer are needed in the callback. The default netdev model cuold very well change depending on which hypervisor is used, the version/capabilities of the hypervisor, and the machinetype.

--- tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-client.args | 6 +----- tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args | 6 +----- tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args | 8 +------- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 6 +----- tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args | 6 +----- tests/qemuxml2argvdata/qemuxml2argv-net-openvswitch.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-server.args | 6 +----- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 5 +---- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 1 + .../qemuxml2argv-net-virtio-network-portgroup.xml | 2 ++ .../qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml | 1 + tests/testutilsqemu.c | 1 + 16 files changed, 17 insertions(+), 36 deletions(-) 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..64022d9 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-client.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-client.args @@ -1,5 +1 @@ -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 +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,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..67bf045 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.args @@ -1,5 +1 @@ -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 +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,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..73e5d5e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args @@ -1,7 +1 @@ -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 +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,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..2bb556e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args @@ -1,5 +1 @@ -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 +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,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..6a406b5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-mcast.args @@ -1,5 +1 @@ -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 +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,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..6b0c6d0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-server.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-server.args @@ -1,5 +1 @@ -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 +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,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..23f286d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,4 +1 @@ -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 +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,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'> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 966527c..0d9fb1d 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -175,6 +175,7 @@ virCapsPtr testQemuCapsInit(void) { goto cleanup; qemuDomainSetNamespaceHooks(caps); + qemuDomainSetDefHooks(caps); if ((guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_I686, "/usr/bin/qemu", NULL, -- 1.8.1.1

Move this to the after-parse updater. --- src/conf/domain_conf.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d881983..3c3172d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2362,10 +2362,13 @@ virDomainDeviceDefUpdateDefaultsInternal(virDomainDeviceDefPtr dev ATTRIBUTE_UNU static int -virDomainDefUpdateDefaultsInternal(virDomainDefPtr def ATTRIBUTE_UNUSED, +virDomainDefUpdateDefaultsInternal(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED) { - /* not in use yet */ + /* Auto-add any implied controllers which aren't present */ + if (virDomainDefAddImplicitControllers(def) < 0) + return -1; + return 0; } @@ -10778,10 +10781,6 @@ virDomainDefParseXML(virCapsPtr caps, (def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) goto error; - /* Auto-add any implied controllers which aren't present */ - if (virDomainDefAddImplicitControllers(def) < 0) - goto error; - /* callback to fill driver specific domain aspects */ if (virDomainDefUpdateDefaults(def, caps) < 0) goto error; -- 1.8.1.1

The parameters are initialized already so no need to do it again. --- src/conf/domain_conf.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3c3172d..da04b7b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9485,29 +9485,16 @@ virDomainDefParseXML(virCapsPtr caps, } /* Extract cpu tunables. */ - if (virXPathULong("string(./cputune/shares[1])", ctxt, - &def->cputune.shares) < 0) - def->cputune.shares = 0; + virXPathULong("string(./cputune/shares[1])", ctxt, &def->cputune.shares); + virXPathULongLong("string(./cputune/period[1])", ctxt, &def->cputune.period); + virXPathLongLong("string(./cputune/quota[1])", ctxt, &def->cputune.quota); + virXPathULongLong("string(./cputune/emulator_period[1])", ctxt, + &def->cputune.emulator_period); + virXPathLongLong("string(./cputune/emulator_quota[1])", ctxt, + &def->cputune.emulator_quota); - if (virXPathULongLong("string(./cputune/period[1])", ctxt, - &def->cputune.period) < 0) - def->cputune.period = 0; - - if (virXPathLongLong("string(./cputune/quota[1])", ctxt, - &def->cputune.quota) < 0) - def->cputune.quota = 0; - - if (virXPathULongLong("string(./cputune/emulator_period[1])", ctxt, - &def->cputune.emulator_period) < 0) - def->cputune.emulator_period = 0; - - if (virXPathLongLong("string(./cputune/emulator_quota[1])", ctxt, - &def->cputune.emulator_quota) < 0) - def->cputune.emulator_quota = 0; - - if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { + if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) goto error; - } if (n && VIR_ALLOC_N(def->cputune.vcpupin, n) < 0) goto no_memory; -- 1.8.1.1

On 03/01/13 20:29, Laine Stump wrote: For my defense, it used to be this way and I just simplified the code. I agree we should actually change this to provide meaningful errors. v2 coming soon. Peter

On 02/20/2013 12:06 PM, Peter Krempa wrote: This is a good example of something that isn't "updating defaults". (I think your implication here is that this validation should be hypervisor-specific, and so should end up being moved out into the hypervisor's parse callback, which is true, but it doesn't fit with the current named purpose of the function.)