Re: [libvirt] [PATCH] lxc: Seperate domain config loading

[Osier Yang <jyang redhat com>]

于 2011年05月20日 22:31, Daniel P. Berrange 写道:
> On Wed, May 18, 2011 at 04:57:53PM +0800, Osier Yang wrote:
> > This patch seperate the domain config loading just as qemu driver
> > does, first loading config of running or trasient domains, then
> > of persistent inactive domains. And only try to reconnect the
> > monitor of running domains, so that it won't always throws errors
> > saying can't connect to domain monitor.
> >
> > And as "virDomainLoadConfig->virDomainAssignDef->virDomainObjAssignDef",
> > already do things like "vm->newDef = def", removed the codes
> > in "lxcReconnectVM" that does the same work.
> > ---
> >   src/lxc/lxc_driver.c |   30 +++++++++++++++---------------
> >   1 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> > index 2bb592d..ccf20e4 100644
> > --- a/src/lxc/lxc_driver.c
> > +++ b/src/lxc/lxc_driver.c
> > @@ -1521,6 +1521,10 @@ static int lxcVmStart(virConnectPtr conn,
> >       if (virDomainObjSetDefTransient(driver->caps, vm, false)<  0)
> >           goto cleanup;
> >
> > +    /* Write domain status to disk. */
> > +    if (virDomainSaveStatus(driver->caps, driver->stateDir, vm)<  0)
> > +        goto cleanup;
> > +
> >       rc = 0;
>
> If we introduce that, then I think you need to have the inverse
> in  lxcVmCleanup to delete it.

Doesn't it already deleted in lxcVmCleanup?

<snip>
   virFileDeletePid(driver->stateDir, vm->def->name);
   virDomainDeleteConfig(driver->stateDir, NULL, vm);
</snip>

> >   cleanup:
> > @@ -1586,7 +1590,6 @@ static int lxcDomainStartWithFlags(virDomainPtr dom, unsigned int flags)
> >           event = virDomainEventNewFromObj(vm,
> >                                            VIR_DOMAIN_EVENT_STARTED,
> >                                            VIR_DOMAIN_EVENT_STARTED_BOOTED);
> > -
> >   cleanup:
> >       if (vm)
> >           virDomainObjUnlock(vm);
> > @@ -1925,18 +1928,6 @@ lxcReconnectVM(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque)
> >           goto cleanup;
> >       }
> >
> > -    if ((config = virDomainConfigFile(driver->stateDir,
> > -                                      vm->def->name)) == NULL)
> > -        goto cleanup;
> > -
> > -    /* Try and load the live config */
> > -    tmp = virDomainDefParseFile(driver->caps, config, 0);
> > -    VIR_FREE(config);
> > -    if (tmp) {
> > -        vm->newDef = vm->def;
> > -        vm->def = tmp;
> > -    }
> > -
> >       if (vm->pid != 0) {
> >           vm->def->id = vm->pid;
> >           virDomainObjSetState(vm, VIR_DOMAIN_RUNNING,
> > @@ -2029,6 +2020,17 @@ static int lxcStartup(int privileged)
> >       lxc_driver->caps->privateDataAllocFunc = lxcDomainObjPrivateAlloc;
> >       lxc_driver->caps->privateDataFreeFunc = lxcDomainObjPrivateFree;
> >
> > +    /* Get all the running persistent or transient configs first */
> > +    if (virDomainLoadAllConfigs(lxc_driver->caps,
> > +&lxc_driver->domains,
> > +                                lxc_driver->stateDir,
> > +                                NULL,
> > +                                1, NULL, NULL)<  0)
> > +        goto cleanup;
> > +
> > +    virHashForEach(lxc_driver->domains.objs, lxcReconnectVM, lxc_driver);
> > +
> > +    /* Then inactive persistent configs */
> >       if (virDomainLoadAllConfigs(lxc_driver->caps,
> >                                   &lxc_driver->domains,
> >                                   lxc_driver->configDir,
> > @@ -2036,8 +2038,6 @@ static int lxcStartup(int privileged)
> >                                   0, NULL, NULL)<  0)
> >           goto cleanup;
> >
> > -    virHashForEach(lxc_driver->domains.objs, lxcReconnectVM, lxc_driver);
> > -
> >       lxcDriverUnlock(lxc_driver);
> >
> >       lxcAutostartConfigs(lxc_driver);
>
>
> Looks good apart from the missing cleanup
>
> Daniel