On 01/15/2016 09:05 AM, Nikolay Shirokovskiy wrote:
It's rather unexpected that function with such a name could return transient definition. This is possible if we call it for a transient active domain. But if it is called in such a conditions? No. So we would better fix this case to return NULL.
Calling conditions investigation.
There are 4 distinct callers:
1. migration - explicitly set 'persistent' before calling. 2. libxl_driver First it assures that (VIR_DOMAIN_VCPU_CONFIG && !persistent) is false. Then all usages of definition is under VIR_DOMAIN_VCPU_CONFIG flag, so 'persistent' is set.
3. virDomainObjCopyPersistentDef - all callers of this function has next structure: 1 call virDomainLiveConfigHelperMethod or virDomainObjUpdateModificationImpact thus we are sure that (VIR_DOMAIN_VCPU_CONFIG && !persistent) is false. 2. call function of interest under VIR_DOMAIN_VCPU_CONFIG, so 'persistent' flag is set.
4. virDomainLiveConfigHelperMethod - the same reason as in 3.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/conf/domain_conf.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
You've based your assumptions off of the change you made in 1/3 to virDomainObjUpdateModificationImpact regarding combining that if statement. Also even if the following would fail, you would have need to provide some sort of virReportError. John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e54c097..018c77e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2825,18 +2825,21 @@ virDomainObjSetDefTransient(virCapsPtr caps,
/* * Return the persistent domain configuration. If domain is transient, - * return the running config. + * return NULL. * * @param caps pointer to capabilities info * @param xmlopt pointer to XML parser configuration object * @param domain domain object pointer - * @return NULL on error, virDOmainDefPtr on success + * @return NULL on error or transient domains, virDOmainDefPtr on success */ virDomainDefPtr virDomainObjGetPersistentDef(virCapsPtr caps, virDomainXMLOptionPtr xmlopt, virDomainObjPtr domain) { + if (!domain->persistent) + return NULL; + if (virDomainObjSetDefTransient(caps, xmlopt, domain, false) < 0) return NULL;