[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/6] Only assign newDef when we have a new def.



On Fri, Apr 02, 2010 at 09:45:58PM -0400, Chris Lalancette wrote:
> While playing around with def/newDef with the qemu code,
> I noticed that newDef was *always* getting set to a value,
> even when I didn't redefine the domain.  I think the problem
> is the virDomainLoadConfig is always doing virDomainAssignDef
> regardless of whether the domain already exists in the hashtable.
> In turn, virDomainAssignDef is assigning the definition (which
> is actually a duplicate) to newDef.  Fix this so that newDef stays
> NULL until we actually have a new def.
> 
> Signed-off-by: Chris Lalancette <clalance redhat com>
> ---
>  src/conf/domain_conf.c |   23 +++++++++++++----------
>  1 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 41c83fd..3cd43eb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6151,22 +6151,25 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps,
>  
>      if ((configFile = virDomainConfigFile(configDir, name)) == NULL)
>          goto error;
> -    if ((autostartLink = virDomainConfigFile(autostartDir, name)) == NULL)
> -        goto error;
> -
> -    if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0)
> -        goto error;
> -
>      if (!(def = virDomainDefParseFile(caps, configFile,
>                                        VIR_DOMAIN_XML_INACTIVE)))
>          goto error;
>  
> -    if ((dom = virDomainFindByName(doms, def->name))) {
> -        virDomainObjUnlock(dom);
> -        dom = NULL;
> -        newVM = 0;
> +    /* if the domain is already in our hashtable, we don't need to do
> +     * anything further
> +     */
> +    if ((dom = virDomainFindByUUID(doms, def->uuid))) {
> +        VIR_FREE(configFile);
> +        virDomainDefFree(def);
> +        return dom;
>      }
>  
> +    if ((autostartLink = virDomainConfigFile(autostartDir, name)) == NULL)
> +        goto error;
> +
> +    if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0)
> +        goto error;
> +
>      if (!(dom = virDomainAssignDef(caps, doms, def, false)))
>          goto error;

  More a bug fix than purely part of the patch set,

  ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]