From mboxrd@z Thu Jan 1 00:00:00 1970 From: Martin Kletzander Subject: Re: [libvirt] [PATCH 1/3] libxl: Move job acquisition in libxlDomainStart to callers Date: Wed, 1 Apr 2015 11:55:01 +0200 Message-ID: <20150401095501.GB30610@wheatley> References: <1427314116-14451-1-git-send-email-jfehlig@suse.com> <1427314116-14451-2-git-send-email-jfehlig@suse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3502219730017994997==" Return-path: In-Reply-To: <1427314116-14451-2-git-send-email-jfehlig@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jim Fehlig Cc: libvir-list@redhat.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org --===============3502219730017994997== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9zSXsLTf0vkW971A" Content-Disposition: inline --9zSXsLTf0vkW971A Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On Wed, Mar 25, 2015 at 02:08:34PM -0600, Jim Fehlig wrote: >Let callers of libxlDomainStart decide when it is appropriate to >acquire a job on the associated virDomainObj. > This makes sense, I see many bugs this fixes, but how come libxlDomainShutdownThread(), libxlDomainRestoreFlags() and libxlDoMigrateReceive() don't need to start the job when they all call libxlDomainStart()? >Signed-off-by: Jim Fehlig >--- > src/libxl/libxl_domain.c | 24 ++++++++-------------- > src/libxl/libxl_driver.c | 53 +++++++++++++++++++++++++++++++++++++++--------- > 2 files changed, 52 insertions(+), 25 deletions(-) > >diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >index 05f6eb1..da4c1c7 100644 >--- a/src/libxl/libxl_driver.c >+++ b/src/libxl/libxl_driver.c >@@ -885,17 +893,26 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, > goto cleanup; > def = NULL; > >+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { >+ virDomainObjListRemove(driver->domains, vm); >+ vm = NULL; >+ goto cleanup; >+ } >+ This should be acquired before virDomainObjListAdd() since you need to check whether it's active after creating the job. If I'm wrong, then virDomainObjListRemove() should be only called if the vm is not persistent (as CreateXML can be called on persistent ones as well), shouldn't it? [...] >@@ -1695,11 +1712,20 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, > > def = NULL; > >+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { >+ if (!vm->persistent) { >+ virDomainObjListRemove(driver->domains, vm); >+ vm = NULL; >+ } >+ goto cleanup; >+ } >+ Same here, I guess. > ret = libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, fd); >- if (ret < 0 && !vm->persistent) { >+ if (ret < 0 && !vm->persistent) > virDomainObjListRemove(driver->domains, vm); >+ >+ if (!libxlDomainObjEndJob(driver, vm)) > vm = NULL; >- } > > cleanup: > if (VIR_CLOSE(fd) < 0) >@@ -2567,17 +2593,24 @@ libxlDomainCreateWithFlags(virDomainPtr dom, > if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) > goto cleanup; > >+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) >+ goto cleanup; >+ > if (virDomainObjIsActive(vm)) { > virReportError(VIR_ERR_OPERATION_INVALID, > "%s", _("Domain is already running")); >- goto cleanup; >+ goto endjob; > } > > ret = libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, -1); > if (ret < 0) >- goto cleanup; >+ goto endjob; > dom->id = vm->def->id; > >+ endjob: >+ if (!libxlDomainObjEndJob(driver, vm)) >+ vm = NULL; >+ > cleanup: > if (vm) > virObjectUnlock(vm); >-- >1.8.4.5 > >-- >libvir-list mailing list >libvir-list@redhat.com >https://www.redhat.com/mailman/listinfo/libvir-list --9zSXsLTf0vkW971A Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJVG8B1AAoJEAgfwp8kF4bdG54P/2D52xCUd1T/GISpEYAbfst8 ubUBEBtw4uhRnCOoKwm10G4lj7vNWYBBG88RhXmG9iLX1hy8biVX+WitmUixdJdw PEa4dkoY20ScUkGmQyBcmZnRwCiALNOuxU0GyseMhDbsnnyYAJhDHV60z48vt2pY Bw/crMR6PVZfCNB5hDQ/iNPtmk0b/KmMKnCcNEkxDSu1QGB1aV1NjcySu0xXSHTA wGNdfWWqZaoDB6RoS7W1dJ038fRdf3BcEK7RfGLdI5HpDh++qiioXcRKCzpZflNJ jfZ0TBDb6p+TME+sLuQtc9bL8Oh8ay8jPxukYJ38fTl8EUpZKGaBt7F4C6aMVjs6 rLK9QWBpTx8y/re5iS3T898cLt4GYb8wFKqCEfnS1JlbIZOv43/CpkLZuVudFlAp GWtPs9zegnw09x8WJx2fnPr27DNCPYbJ/TnmbDmD8o7DJFOHkKw1wWjlOMZyTy3g MAnraJxcIzJq3RO9Go/L5JM0QkLvJqZB60isKktC6MjpcDXPnYrnilBo1qEqKu0e VVrXO79cSx9zbH9FkldOfee4qLOv1ldYQNn9lCnIvXsPNSsuyJfip5uT3n/NsGF4 FC8OxZRk6DcgfcD9jZxmd9I+i+nATxRBM5kUgU8TWOARAWKH9R/kPyYYSY8VuNl9 EVd5240PjDuxrOZVJCyb =MlIN -----END PGP SIGNATURE----- --9zSXsLTf0vkW971A-- --===============3502219730017994997== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3502219730017994997==--