From nobody Thu May 15 03:08:51 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 1512638645615641.6394684531086; Thu, 7 Dec 2017 01:24:05 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9EED490901; Thu, 7 Dec 2017 09:24:03 +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 E64A969411; Thu, 7 Dec 2017 09:24:02 +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 1F7951800BDE; Thu, 7 Dec 2017 09:24:01 +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 vB79N9q9007265 for ; Thu, 7 Dec 2017 04:23:09 -0500 Received: by smtp.corp.redhat.com (Postfix) id 00E45183A6; Thu, 7 Dec 2017 09:23:09 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.192]) by smtp.corp.redhat.com (Postfix) with ESMTP id 05070183A2; Thu, 7 Dec 2017 09:23:07 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Thu, 7 Dec 2017 10:22:56 +0100 Message-Id: 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 1/7] Make installation_get_xml hurt my eyes less. 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.15 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:24:04 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This function has a lot of problems. Fix some of them: 1) long lines 2) useless argument @step 3) unclear domain XML 4) unclear difference in generated XMLs for step 1 and step 2 5) static 32KB buffer(!) Signed-off-by: Michal Privoznik --- src/libvirt-domain.c | 8 +-- src/libvirt-php.c | 194 +++++++++++++++++++++++++++--------------------= ---- src/libvirt-php.h | 2 +- 3 files changed, 107 insertions(+), 97 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 18f6888..c517dfd 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -145,8 +145,8 @@ PHP_FUNCTION(libvirt_domain_new) =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(1, - conn->conn, tmpname, memMB, maxmemMB, NULL = /* arch */, NULL, vcpus, iso_image, + tmp =3D installation_get_xml(conn->conn, tmpname, memMB, maxmemMB, + NULL /* arch */, NULL, vcpus, iso_image, vmDisks, numDisks, vmNetworks, numNets, flags TSRMLS_CC); if (tmp =3D=3D NULL) { @@ -194,8 +194,8 @@ PHP_FUNCTION(libvirt_domain_new) =20 set_vnc_location(vncl TSRMLS_CC); =20 - tmp =3D installation_get_xml(2, - conn->conn, name, memMB, maxmemMB, NULL /* = arch */, NULL, vcpus, iso_image, + tmp =3D installation_get_xml(conn->conn, name, memMB, maxmemMB, + NULL /* arch */, NULL, vcpus, NULL, vmDisks, numDisks, vmNetworks, numNets, flags TSRMLS_CC); if (tmp =3D=3D NULL) { diff --git a/src/libvirt-php.c b/src/libvirt-php.c index efbef58..51534a5 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -2248,9 +2248,11 @@ char *get_network_xml(char *mac, char *network, char= *model) /* * Private function name: installation_get_xml * Since version: 0.4.5 - * Description: Function is used to generate the installation = XML description to install a new domain - * Arguments: @step [int]: number of step for XML output (1 = or 2) - * @conn [virConnectPtr]: libvirt connection poin= ter + * Description: Function is used to generate the installation = XML + * description to install a new domain. If @iso_i= mage + * is not NULL, domain XML is created so that it = boots + * from it. + * Arguments: @conn [virConnectPtr]: libvirt connection poin= ter * @name [string]: name of the new virtual machine * @memMB [int]: memory in Megabytes * @maxmemMB [int]: maximum memory in Megabytes @@ -2265,17 +2267,20 @@ char *get_network_xml(char *mac, char *network, cha= r *model) * @domain_flags [int]: flags for the domain inst= allation * Returns: full XML output for installation */ -char *installation_get_xml(int step, virConnectPtr conn, char *name, int m= emMB, int maxmemMB, char *arch, char *uuid, int vCpus, char *iso_image, - tVMDisk *disks, int numDisks, tVMNetwork *netwo= rks, int numNetworks, int domain_flags TSRMLS_DC) +char *installation_get_xml(virConnectPtr conn, char *name, int memMB, + int maxmemMB, char *arch, char *uuid, int vCpus, + char *iso_image, tVMDisk *disks, int numDisks, + tVMNetwork *networks, int numNetworks, int + domain_flags TSRMLS_DC) { int i; - char xml[32768] =3D { 0 }; + char *xml =3D NULL; char disks_xml[16384] =3D { 0 }; char networks_xml[16384] =3D { 0 }; char features[128] =3D { 0 }; char *tmp =3D NULL; char type[64] =3D { 0 }; - // virDomainPtr domain=3DNULL; + int rv; =20 if (conn =3D=3D NULL) { DPRINTF("%s: Invalid libvirt connection pointer\n", __FUNCTION__); @@ -2297,7 +2302,7 @@ char *installation_get_xml(int step, virConnectPtr co= nn, char *name, int memMB, DPRINTF("%s: No architecture defined, got host arch of '%s'\n", __= FUNCTION__, arch); } =20 - if (access(iso_image, R_OK) !=3D 0) { + if (iso_image && access(iso_image, R_OK) !=3D 0) { DPRINTF("%s: Installation image %s doesn't exist\n", __FUNCTION__,= iso_image); return NULL; } @@ -2324,93 +2329,98 @@ char *installation_get_xml(int step, virConnectPtr = conn, char *name, int memMB, free(network); } =20 - if (step =3D=3D 1) - snprintf(xml, sizeof(xml), "\n" - "\t%s\n" - "\t%d\n" - "\t%d\n" - "\t%s\n" - "\t\n" - "\t\thvm\n" - "\t\t\n" - "\t\t\n" - "\t\n" - "\t\n" - "\t\t%s\n" - "\t\n" - "\t\n" - "\tdestroy\n" - "\tdestroy\n" - "\tdestroy\n" - "\t%d\n" - "\t\n" - "\t\t%s\n" - "%s" - "\t\t\n" - "\t\t\t\n" - "\t\t\t\n" - "\t\t\t\n" - "\t\t\t\n" - "\t\t\n" - "%s" - "\t\t\n" - "\t\t\n" - "\t\t\n" - "%s" - "\t\t\n" - "\t\n" - "", + if (iso_image) { + rv =3D asprintf(&xml, + "\n" + " %s\n" + " %d\n" + " %d\n" + " %s\n" + " \n" + " hvm\n" + " \n" + " \n" + " \n" + " \n" + " %s\n" + " \n" + " \n" + " destroy\n" + " destroy\n" + " destroy\n" + " %d\n" + " \n" + " %s\n" + " %s" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + " %s" + " \n" + " \n" + " \n" + " %s" + " \n" + " \n" + "", type, name, memMB * 1024, maxmemMB * 1024, uuid, arch, feature= s, (domain_flags & DOMAIN_FLAG_CLOCK_LOCALTIME ? "localtime" : "u= tc"), - vCpus, connection_get_emulator(conn, arch TSRMLS_CC), disks_xm= l, iso_image, networks_xml, - (domain_flags & DOMAIN_FLAG_SOUND_AC97 ? "\t\t\n" : "") - ); - else - if (step =3D=3D 2) - snprintf(xml, sizeof(xml), "\n" - "\t%s\n" - "\t%d\n" - "\t%d\n" - "\t%s\n" - "\t\n" - "\t\thvm\n" - "\t\t\n" - "\t\n" - "\t\n" - "\t\t%s\n" - "\t\n" - "\t\n" - "\tdestroy\n" - "\tdestroy\n" - "\tdestroy\n" - "\t%d\n" - "\t\n" - "\t\t%s\n" - "%s" - "\t\t\n" - "\t\t\t\n" - "\t\t\t\n" - "\t\t\t\n" - "\t\t\n" - "%s" - "\t\t\n" - "\t\t\n" - "\t\t\n" - "%s" - "\t\t\n" - "\t\n" - "", - type, name, memMB * 1024, maxmemMB * 1024, uuid, arch, fea= tures, - (domain_flags & DOMAIN_FLAG_CLOCK_LOCALTIME ? "localtime" = : "utc"), - vCpus, connection_get_emulator(conn, arch TSRMLS_CC), disk= s_xml, networks_xml, - (domain_flags & DOMAIN_FLAG_SOUND_AC97 ? "\t\t\n" : "") - ); + vCpus, connection_get_emulator(conn, arch TSRMLS_CC), disks_xm= l, + iso_image, networks_xml, + (domain_flags & DOMAIN_FLAG_SOUND_AC97 ? "\n" : "")); + } else { + rv =3D asprintf(&xml, + "\n" + " %s\n" + " %d\n" + " %d\n" + " %s\n" + " \n" + " hvm\n" + " \n" + " \n" + " \n" + " %s\n" + " \n" + " \n" + " destroy\n" + " destroy\n" + " destroy\n" + " %d\n" + " \n" + " %s\n" + " %s" + " \n" + " \n" + " \n" + " \n" + " \n" + " %s" + " \n" + " \n" + " \n" + " %s" + " \n" + " \n" + "", + type, name, memMB * 1024, maxmemMB * 1024, uuid, arch, feature= s, + (domain_flags & DOMAIN_FLAG_CLOCK_LOCALTIME ? "localtime" : "u= tc"), + vCpus, connection_get_emulator(conn, arch TSRMLS_CC), disks_xm= l, + networks_xml, + (domain_flags & DOMAIN_FLAG_SOUND_AC97 ? "\n" : "")); + } =20 - return (strlen(xml) > 0) ? strdup(xml) : NULL; + if (rv < 0) + return NULL; + + return xml; } =20 /* diff --git a/src/libvirt-php.h b/src/libvirt-php.h index aea43a2..e6c22b2 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -175,7 +175,7 @@ int set_logfile(char *filename, long maxsize TSRMLS_DC); char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVa= l); char **get_array_from_xpath(char *xml, char *xpath, int *num); void parse_array(zval *arr, tVMDisk *disk, tVMNetwork *network); -char *installation_get_xml(int step, virConnectPtr conn, char *name, int m= emMB, +char *installation_get_xml(virConnectPtr conn, char *name, int memMB, int maxmemMB, char *arch, char *uuid, int vCpus, char *iso_image, tVMDisk *disks, int numDisks, tVMNetwork *networks, int numNetworks, --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list