From nobody Mon Dec 15 01:54:25 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 15126387553111001.4957423983157; Thu, 7 Dec 2017 01:25:55 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C625C5D3; Thu, 7 Dec 2017 09:25:53 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9DEED78430; Thu, 7 Dec 2017 09:25:53 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 4F75D1800FC4; Thu, 7 Dec 2017 09:25:53 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vB79NAvg007274 for ; Thu, 7 Dec 2017 04:23:10 -0500 Received: by smtp.corp.redhat.com (Postfix) id 23B75183A6; Thu, 7 Dec 2017 09:23:10 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4F29E183A2; Thu, 7 Dec 2017 09:23:09 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Thu, 7 Dec 2017 10:22:57 +0100 Message-Id: <6164d3f4ea28bd3889b9e68678b25975e7852243.1512638349.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Cc: drewwynne@gmail.com, dzamirski@datto.com Subject: [libvirt] [libvirt-php][PATCH 2/7] Rework libvirt_domain_new a bit X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 07 Dec 2017 09:25:54 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Firstly, this API is creating $domName-install for installation and at the same time it defines $domName (but never runs it). This is not very optimal - libvirt can handle two definitions for a single domain (active and inactive ones). Secondly, this function is leaking domain objects on any error. Signed-off-by: Michal Privoznik --- src/libvirt-domain.c | 65 +++++++++++++++++++++++++++++++++++-------------= ---- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index c517dfd..3c4c683 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -63,8 +63,8 @@ PHP_FUNCTION(libvirt_domain_new) { php_libvirt_connection *conn =3D NULL; php_libvirt_domain *res_domain =3D NULL; - virDomainPtr domain2 =3D NULL; virDomainPtr domain =3D NULL; + virDomainPtr domainUpdated =3D NULL; zval *zconn; char *arch =3D NULL; strsize_t arch_len; @@ -85,10 +85,9 @@ PHP_FUNCTION(libvirt_domain_new) zval *data; HashPosition pointer; char vncl[2048] =3D { 0 }; - char tmpname[1024] =3D { 0 }; char *xml =3D NULL; int retval =3D 0; - char *uuid =3D NULL; + char uuid[VIR_UUID_STRING_BUFLEN] =3D { 0 }; zend_long flags =3D 0; int fd =3D -1; =20 @@ -143,9 +142,7 @@ PHP_FUNCTION(libvirt_domain_new) } VIRT_FOREACH_END(); numNets =3D i; =20 - snprintf(tmpname, sizeof(tmpname), "%s-install", name); - DPRINTF("%s: Name is '%s', memMB is %d, maxmemMB is %d\n", PHPFUNC, tm= pname, (int) memMB, (int) maxmemMB); - tmp =3D installation_get_xml(conn->conn, tmpname, memMB, maxmemMB, + tmp =3D installation_get_xml(conn->conn, name, memMB, maxmemMB, NULL /* arch */, NULL, vcpus, iso_image, vmDisks, numDisks, vmNetworks, numNets, flags TSRMLS_CC); @@ -155,25 +152,37 @@ PHP_FUNCTION(libvirt_domain_new) RETURN_FALSE; } =20 - domain =3D virDomainCreateXML(conn->conn, tmp, 0); + domain =3D virDomainDefineXML(conn->conn, tmp); if (domain =3D=3D NULL) { - set_error_if_unset("Cannot create installation domain from the XML= description" TSRMLS_CC); - DPRINTF("%s: Cannot create installation domain from the XML descri= ption (%s): %s\n", PHPFUNC, LIBVIRT_G(last_error), tmp); + set_error_if_unset("Cannot define domain from the XML description"= TSRMLS_CC); + DPRINTF("%s: Cannot define domain from the XML description (%s): %= s\n", PHPFUNC, LIBVIRT_G(last_error), tmp); RETURN_FALSE; } =20 + if (virDomainCreate(domain) < 0) { + DPRINTF("%s: Cannot create domain: %s\n", PHPFUNC, LIBVIRT_G(last_= error)); + set_error_if_unset("Cannot create domain" TSRMLS_CC); + goto error; + } + xml =3D virDomainGetXMLDesc(domain, 0); if (!xml) { - DPRINTF("%s: Cannot get the XML description\n", PHPFUNC); + DPRINTF("%s: Cannot get the XML description: %s\n", PHPFUNC, LIBVI= RT_G(last_error)); set_error_if_unset("Cannot get the XML description" TSRMLS_CC); - RETURN_FALSE; + goto error; + } + + if (virDomainGetUUIDString(domain, uuid) < 0) { + DPRINTF("%s: Cannot get domain UUID: %s\n", PHPFUNC, LIBVIRT_G(las= t_error)); + set_error_if_unset("Cannot get domain UUID" TSRMLS_CC); + goto error; } =20 tmp =3D get_string_from_xpath(xml, "//domain/devices/graphics[@type=3D= 'vnc']/@port", NULL, &retval); if (retval < 0) { DPRINTF("%s: Cannot get port from XML description\n", PHPFUNC); set_error_if_unset("Cannot get port from XML description" TSRMLS_C= C); - RETURN_FALSE; + goto error; } =20 snprintf(vncl, sizeof(vncl), "%s:%s", virConnectGetHostname(conn->conn= ), tmp); @@ -195,23 +204,25 @@ PHP_FUNCTION(libvirt_domain_new) set_vnc_location(vncl TSRMLS_CC); =20 tmp =3D installation_get_xml(conn->conn, name, memMB, maxmemMB, - NULL /* arch */, NULL, vcpus, NULL, + NULL /* arch */, uuid, vcpus, NULL, vmDisks, numDisks, vmNetworks, numNets, flags TSRMLS_CC); if (tmp =3D=3D NULL) { DPRINTF("%s: Cannot get installation XML, step 2\n", PHPFUNC); set_error("Cannot get installation XML, step 2" TSRMLS_CC); - virDomainFree(domain); - RETURN_FALSE; + goto error; } =20 - domain2 =3D virDomainDefineXML(conn->conn, tmp); - if (domain2 =3D=3D NULL) { - set_error_if_unset("Cannot define domain from the XML description"= TSRMLS_CC); - DPRINTF("%s: Cannot define domain from the XML description (name = =3D '%s', uuid =3D '%s', error =3D '%s')\n", PHPFUNC, name, uuid, LIBVIRT_G= (last_error)); - RETURN_FALSE; + domainUpdated =3D virDomainDefineXML(conn->conn, tmp); + if (domainUpdated =3D=3D NULL) { + set_error_if_unset("Cannot update domain definition" TSRMLS_CC); + DPRINTF("%s: Cannot update domain definition " + "(name =3D '%s', uuid =3D '%s', error =3D '%s')\n", + PHPFUNC, name, uuid, LIBVIRT_G(last_error)); + goto error; } - virDomainFree(domain2); + virDomainFree(domainUpdated); + domainUpdated =3D NULL; =20 res_domain =3D (php_libvirt_domain *)emalloc(sizeof(php_libvirt_domain= )); res_domain->domain =3D domain; @@ -221,6 +232,18 @@ PHP_FUNCTION(libvirt_domain_new) resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, res_domain->d= omain, 1 TSRMLS_CC); =20 VIRT_REGISTER_RESOURCE(res_domain, le_libvirt_domain); + return; + + error: + if (domain) { + if (virDomainIsActive(domain) > 0) + virDomainDestroy(domain); + virDomainUndefine(domain); + virDomainFree(domain); + } + if (domainUpdated) + virDomainFree(domainUpdated); + RETURN_FALSE; } =20 /* --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list