On Wed, Apr 09, 2014 at 03:11:48PM +0400, Roman Bogorodskiy wrote:
> Daniel P. Berrange wrote:
>
> > On Wed, Apr 09, 2014 at 07:52:12AM +0200, Wojciech Macek wrote:
> > > Implement bhyveDomainCreteXML function.
> > > ---
> > > src/bhyve/bhyve_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 70 insertions(+)
> > >
> > > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> > > index 23e7112..1357e07 100644
> > > --- a/src/bhyve/bhyve_driver.c
> > > +++ b/src/bhyve/bhyve_driver.c
> > > @@ -1107,6 +1107,75 @@ bhyveConnectCompareCPU(virConnectPtr conn,
> > > return ret;
> > > }
> > >
> > > +static virDomainPtr
> > > +bhyveDomainCreateXML(virConnectPtr conn,
> > > + const char *xml,
> > > + unsigned int flags)
> > > +{
> > > + bhyveConnPtr privconn = conn->privateData;
> > > + virDomainPtr dom = NULL;
> > > + virDomainDefPtr def = NULL;
> > > + virDomainObjPtr vm = NULL;
> > > + virCapsPtr caps = NULL;
> > > + int ret;
> > > + unsigned int start_flags = 0;
> > > +
> > > + virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL);
> > > +
> > > + if (flags & VIR_DOMAIN_START_AUTODESTROY)
> > > + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
> > > +
> > > + caps = bhyveDriverGetCapabilities(privconn);
> > > + if (!caps)
> > > + return NULL;
> > > +
> > > + if ((def = virDomainDefParseString(xml, caps, privconn->xmlopt,
> > > + 1 << VIR_DOMAIN_VIRT_BHYVE,
> > > + VIR_DOMAIN_XML_INACTIVE)) == NULL)
> > > + goto cleanup;
> > > +
> > > + if (virDomainCreateXMLEnsureACL(conn, def) < 0)
> > > + goto cleanup;
> > > +
> > > + if (!(vm = virDomainObjListAdd(privconn->domains, def,
> > > + privconn->xmlopt,
> > > + 0, NULL)))
> >
> > You should pass VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, rather than 0.
> > This ensures you don't overwrite the config of an existing runing
> > guest
> >
> > > + goto cleanup;
> > > + def = NULL;
> > > + vm->persistent = 0;
> >
> > Don't reset the persistent flag. The guest may already exist
> > as an inactive persistent guest, and you're causing that to be
> > lost.
> >
> > > +
> > > + dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
> > > + if (!dom)
> > > + goto cleanup;
> > > +
> > > + dom->id = vm->def->id;
> >
> > Err, the 'id' won't be set until you've started the guest,
> > so this fnuction call & set needs to be the last thing you
> > do
> >
> > > +
> > > + if (flags & VIR_DOMAIN_START_AUTODESTROY)
> > > + start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY;
> >
> > You've already done this above.
> >
> > > +
> > > + if (virDomainObjIsActive(vm)) {
> > > + virReportError(VIR_ERR_OPERATION_INVALID,
> > > + "%s", _("Domain is already running"));
> > > + goto cleanup;
> > > + }
> >
> > This is not required if you pass the right flags to the
> > virDomainObjListAdd method
> >
> > > +
> > > + ret = virBhyveProcessStart(dom->conn, privconn, vm,
> > > + VIR_DOMAIN_RUNNING_BOOTED,
> > > + start_flags);
> > > + if (ret) {
>
> As discussed on IRC now, I think we need to call
> virDomainObjListRemove() if the domain is not persistent in case if
> virBhyveProcessStart() fails.
>
> Daniel, please correct me if I'm wrong.
You are right.
Regards,
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list