[libvirt] libvirt-php: various fixes
While building a libvirt-php package for Gentoo Linux I had to workaround a couple of build issues. The first two patches are the results. The last patch is a fix for the case libvirt has been built without Xen support. In this case libvirt_connect (and phpinfo()) fails because virGetVersion is called with a non-NULL typeVer argument and libvirt therefore checks for Xen version. I guess the documentation for virGetVersion should be updated since it also returns 0 if both type and typeVer are NULL. [PATCH 1/3] Fix building with threaded php. [PATCH 2/3] add --with-php-config flag [PATCH 3/3] fix libvirt_connect failure without Xen
--- src/libvirt.c | 74 ++++++++++++++++++++++++++++++--------------------------- 1 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 1ba2662..ce39a28 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -195,7 +195,7 @@ PHP_MINFO_FUNCTION(libvirt) Arguments: @msg [string]: error message string Returns: None */ -void set_error(char *msg) +void set_error(char *msg TSRMLS_DC) { php_error_docref(NULL TSRMLS_CC, E_WARNING,"%s",msg); if (LIBVIRT_G (last_error)!=NULL) efree(LIBVIRT_G (last_error)); @@ -203,10 +203,11 @@ void set_error(char *msg) } /* Error handler for receiving libvirt errors */ -static void catch_error(void *userData ATTRIBUTE_UNUSED, +static void catch_error(void *userData, virErrorPtr error) { - set_error(error->message); + TSRMLS_FETCH_FROM_CTX(userData); + set_error(error->message TSRMLS_CC); } @@ -414,7 +415,10 @@ PHP_MINIT_FUNCTION(libvirt) /* Initialize libvirt and set up error callback */ virInitialize(); - virSetErrorFunc(NULL, catch_error); + + void *thread_ctx = NULL; + TSRMLS_SET_CTX(thread_ctx); + virSetErrorFunc(thread_ctx, catch_error); return SUCCESS; } @@ -597,7 +601,7 @@ PHP_FUNCTION(libvirt_connect) if (libVer<6002) { - set_error("Only libvirt 0.6.2 and higher supported. Please upgrade your libvirt"); + set_error("Only libvirt 0.6.2 and higher supported. Please upgrade your libvirt" TSRMLS_CC); RETURN_FALSE; } @@ -1503,7 +1507,7 @@ PHP_FUNCTION(libvirt_domain_memory_stats) #else PHP_FUNCTION(libvirt_domain_memory_stats) { - set_error("Only libvirt 0.7.5 and higher supports getting the job information"); + set_error("Only libvirt 0.7.5 and higher supports getting the job information" TSRMLS_CC); } #endif @@ -1534,7 +1538,7 @@ PHP_FUNCTION(libvirt_domain_update_device) #else PHP_FUNCTION(libvirt_domain_update_device) { - set_error("Only libvirt 0.8.0 and higher supports updating the device information"); + set_error("Only libvirt 0.8.0 and higher supports updating the device information" TSRMLS_CC); } #endif @@ -1591,19 +1595,19 @@ PHP_FUNCTION(libvirt_domain_get_network_info) { /* Get XML for the domain */ xml=virDomainGetXMLDesc(domain->domain, VIR_DOMAIN_XML_INACTIVE); if (xml==NULL) { - set_error("Cannot get domain XML"); + set_error("Cannot get domain XML" TSRMLS_CC); RETURN_FALSE; } snprintf(fnpath, sizeof(fnpath), "//domain/devices/interface[@type='network']/mac[@address='%s']/../source/@network", mac); tmp = get_string_from_xpath(xml, fnpath, NULL, &retval); if (tmp == NULL) { - set_error("Invalid XPath node for source network"); + set_error("Invalid XPath node for source network" TSRMLS_CC); RETURN_FALSE; } if (retval < 0) { - set_error("Cannot get XPath expression result for network source"); + set_error("Cannot get XPath expression result for network source" TSRMLS_CC); RETURN_FALSE; } @@ -1644,7 +1648,7 @@ PHP_FUNCTION(libvirt_domain_get_block_info) { /* Get XML for the domain */ xml=virDomainGetXMLDesc(domain->domain, VIR_DOMAIN_XML_INACTIVE); if (xml==NULL) { - set_error("Cannot get domain XML"); + set_error("Cannot get domain XML" TSRMLS_CC); RETURN_FALSE; } @@ -1653,7 +1657,7 @@ PHP_FUNCTION(libvirt_domain_get_block_info) { tmp = get_string_from_xpath(xml, fnpath, NULL, &retval); if (retval < 0) { - set_error("Cannot get XPath expression result for device storage"); + set_error("Cannot get XPath expression result for device storage" TSRMLS_CC); RETURN_FALSE; } @@ -1661,21 +1665,21 @@ PHP_FUNCTION(libvirt_domain_get_block_info) { snprintf(fnpath, sizeof(fnpath), "//domain/devices/disk/target[@dev='%s']/../source/@file", dev); tmp = get_string_from_xpath(xml, fnpath, NULL, &retval); if (retval < 0) { - set_error("Cannot get XPath expression result for file storage"); + set_error("Cannot get XPath expression result for file storage" TSRMLS_CC); RETURN_FALSE; } isFile = 1; } if (retval == 0) { - set_error("No relevant node found"); + set_error("No relevant node found" TSRMLS_CC); RETURN_FALSE; } retval=virDomainGetBlockInfo(domain->domain, tmp, &info,0); if (retval == -1) { - set_error("Cannot get domain block information"); + set_error("Cannot get domain block information" TSRMLS_CC); RETURN_FALSE; } @@ -1700,7 +1704,7 @@ PHP_FUNCTION(libvirt_domain_get_block_info) { #else PHP_FUNCTION(libvirt_domain_get_block_info) { - set_error("Only libvirt 0.8.0 and higher supports getting the block information"); + set_error("Only libvirt 0.8.0 and higher supports getting the block information" TSRMLS_CC); RETURN_FALSE; } #endif @@ -1912,7 +1916,7 @@ PHP_FUNCTION(libvirt_domain_get_job_info) #else PHP_FUNCTION(libvirt_domain_get_job_info) { - set_error("Only libvirt 0.7.7 and higher supports getting the job information"); + set_error("Only libvirt 0.7.7 and higher supports getting the job information" TSRMLS_CC); RETURN_FALSE; } #endif @@ -2902,7 +2906,7 @@ PHP_FUNCTION(libvirt_nodedev_get) GET_CONNECTION_FROM_ARGS("rs",&zconn,&name,&name_len); if ((dev = virNodeDeviceLookupByName(conn->conn, name)) == NULL) { - set_error("Cannot get find requested node device"); + set_error("Cannot get find requested node device" TSRMLS_CC); RETURN_FALSE; } @@ -2961,7 +2965,7 @@ PHP_FUNCTION(libvirt_nodedev_get_xml_desc) xml=virNodeDeviceGetXMLDesc(nodedev->device, 0); if ( xml == NULL ) { - set_error("Cannot get the device XML information"); + set_error("Cannot get the device XML information" TSRMLS_CC); RETURN_FALSE; } @@ -2987,7 +2991,7 @@ PHP_FUNCTION(libvirt_nodedev_get_information) xml=virNodeDeviceGetXMLDesc(nodedev->device, 0); if ( xml == NULL ) { - set_error("Cannot get the device XML information"); + set_error("Cannot get the device XML information" TSRMLS_CC); RETURN_FALSE; } @@ -2996,12 +3000,12 @@ PHP_FUNCTION(libvirt_nodedev_get_information) /* Get name */ tmp = get_string_from_xpath(xml, "//device/name", NULL, &retval); if (tmp == NULL) { - set_error("Invalid XPath node for device name"); + set_error("Invalid XPath node for device name" TSRMLS_CC); RETURN_FALSE; } if (retval < 0) { - set_error("Cannot get XPath expression result for device name"); + set_error("Cannot get XPath expression result for device name" TSRMLS_CC); RETURN_FALSE; } @@ -3117,7 +3121,7 @@ PHP_FUNCTION(libvirt_network_get) GET_CONNECTION_FROM_ARGS("rs",&zconn,&name,&name_len); if ((net = virNetworkLookupByName(conn->conn, name)) == NULL) { - set_error("Cannot get find requested network"); + set_error("Cannot get find requested network" TSRMLS_CC); RETURN_FALSE; } @@ -3145,7 +3149,7 @@ PHP_FUNCTION(libvirt_network_get_bridge) name = virNetworkGetBridgeName(network->network); if (name == NULL) { - set_error("Cannot get network bridge name"); + set_error("Cannot get network bridge name" TSRMLS_CC); RETURN_FALSE; } @@ -3169,7 +3173,7 @@ PHP_FUNCTION(libvirt_network_get_active) res = virNetworkIsActive(network->network); if (res == -1) { - set_error("Error getting virtual network state"); + set_error("Error getting virtual network state" TSRMLS_CC); RETURN_FALSE; } @@ -3197,7 +3201,7 @@ PHP_FUNCTION(libvirt_network_get_information) xml=virNetworkGetXMLDesc(network->network, 0); if (xml==NULL) { - set_error("Cannot get network XML"); + set_error("Cannot get network XML" TSRMLS_CC); RETURN_FALSE; } @@ -3206,12 +3210,12 @@ PHP_FUNCTION(libvirt_network_get_information) /* Get name */ tmp = get_string_from_xpath(xml, "//network/name", NULL, &retval); if (tmp == NULL) { - set_error("Invalid XPath node for network name"); + set_error("Invalid XPath node for network name" TSRMLS_CC); RETURN_FALSE; } if (retval < 0) { - set_error("Cannot get XPath expression result for network name"); + set_error("Cannot get XPath expression result for network name" TSRMLS_CC); RETURN_FALSE; } @@ -3220,12 +3224,12 @@ PHP_FUNCTION(libvirt_network_get_information) /* Get gateway IP address */ tmp = get_string_from_xpath(xml, "//network/ip/@address", NULL, &retval); if (tmp == NULL) { - set_error("Invalid XPath node for network gateway IP address"); + set_error("Invalid XPath node for network gateway IP address" TSRMLS_CC); RETURN_FALSE; } if (retval < 0) { - set_error("Cannot get XPath expression result for network gateway IP address"); + set_error("Cannot get XPath expression result for network gateway IP address" TSRMLS_CC); RETURN_FALSE; } @@ -3234,12 +3238,12 @@ PHP_FUNCTION(libvirt_network_get_information) /* Get netmask */ tmp2 = get_string_from_xpath(xml, "//network/ip/@netmask", NULL, &retval); if (tmp2 == NULL) { - set_error("Invalid XPath node for network mask"); + set_error("Invalid XPath node for network mask" TSRMLS_CC); RETURN_FALSE; } if (retval < 0) { - set_error("Cannot get XPath expression result for network mask"); + set_error("Cannot get XPath expression result for network mask" TSRMLS_CC); RETURN_FALSE; } @@ -3289,7 +3293,7 @@ PHP_FUNCTION(libvirt_network_set_active) GET_NETWORK_FROM_ARGS("rl",&znetwork,&act); if ((act != 0) && (act != 1)) { - set_error("Invalid network activity state"); + set_error("Invalid network activity state" TSRMLS_CC); RETURN_FALSE; } @@ -3331,7 +3335,7 @@ PHP_FUNCTION(libvirt_network_get_xml_desc) xml=virNetworkGetXMLDesc(network->network, 0); if (xml==NULL) { - set_error("Cannot get network XML"); + set_error("Cannot get network XML" TSRMLS_CC); RETURN_FALSE; } @@ -3416,7 +3420,7 @@ PHP_FUNCTION(libvirt_check_version) RETURN_TRUE; } else - set_error("Invalid version type"); + set_error("Invalid version type" TSRMLS_CC); RETURN_FALSE; } -- 1.7.4.1
php extensions can usually be configured to use a specific php-config using --with-php-config to explicitly specify a path to php-config. Moved phpize-detection after php-config and use php-config's prefix path to look for phpize first. --- configure.ac | 18 ++++++++++++++---- src/Makefile.am | 6 +++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 6abf568..8149349 100644 --- a/configure.ac +++ b/configure.ac @@ -109,16 +109,26 @@ AC_ARG_WITH([html-subdir], [AC_HELP_STRING([--with-html-subdir=path], [HTML_DIR="$HTML_DIR/\$(PACKAGE)-\$(VERSION)/html"]) AC_SUBST([HTML_DIR]) -AC_PATH_PROG([PHPIZE], [phpize], [no]) -if test "x$PHPIZE" = "xno"; then - AC_MSG_ERROR([phpize not found; please install the PHP SDK]) +AC_ARG_WITH([php-config], [AC_HELP_STRING([--with-php-config=path], + [path to php-config, default search all paths])]) + +if test "x$with_php_config" != "x" ; then + AC_MSG_CHECKING(for php-config) + PHPCONFIG=$with_php_config + AC_MSG_RESULT($with_php_config) +else + AC_PATH_PROG([PHPCONFIG], [php-config], [no]) fi -AC_PATH_PROG([PHPCONFIG], [php-config], [no]) if test "x$PHPCONFIG" = "xno"; then AC_MSG_ERROR([php-config not found; please install the PHP SDK]) fi +AC_PATH_PROG([PHPIZE], [phpize], [no], [`php-config --prefix`/bin$PATH_SEPARATOR$PATH]) +if test "x$PHPIZE" = "xno"; then + AC_MSG_ERROR([phpize not found; please install the PHP SDK]) +fi + AC_SUBST([PHPIZE]) AC_SUBST([PHPCONFIG]) AC_CONFIG_HEADERS([config.h]) diff --git a/src/Makefile.am b/src/Makefile.am index 9c7101d..9bc62dd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,6 +1,6 @@ -PHPINC=$(shell php-config --includes) -PHPEDIR=$(shell php-config --extension-dir) -PHPCDIR=$(shell php-config --configure-options | sed -n 's|.*--with-config-file-scan-dir=\([^ ]*\).*|\1|p') +PHPINC=$(shell $(PHPCONFIG) --includes) +PHPEDIR=$(shell $(PHPCONFIG) --extension-dir) +PHPCDIR=$(shell $(PHPCONFIG) --configure-options | sed -n 's|.*--with-config-file-scan-dir=\([^ ]*\).*|\1|p') DEFINES=-DHAVE_CONFIG_H EXTRA_DIST = libvirt.c libvirt_php.h -- 1.7.4.1
Currently libvirt_connect fails if libvirt has no Xen support. This is because virGetVersion checks for Xen if typeVer!=NULL. --- src/libvirt.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ce39a28..9a1b51a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -173,13 +173,12 @@ PHP_RSHUTDOWN_FUNCTION(libvirt) PHP_MINFO_FUNCTION(libvirt) { unsigned long libVer; - unsigned long typeVer; char *version; php_info_print_table_start(); php_info_print_table_row(2, "Libvirt support", "enabled"); php_info_print_table_row(2, "Extension version", PHP_LIBVIRT_WORLD_VERSION); - if (virGetVersion(&libVer,NULL,&typeVer)== 0) + if (virGetVersion(&libVer,NULL,NULL)== 0) { version=emalloc(100); snprintf(version, 100, "%i.%i.%i", (long)((libVer/1000000) % 1000),(long)((libVer/1000) % 1000),(long)(libVer % 1000)); @@ -590,13 +589,12 @@ PHP_FUNCTION(libvirt_connect) unsigned long index; unsigned long libVer; - unsigned long typeVer; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|sba", &url,&url_len,&readonly,&zcreds) == FAILURE) { RETURN_FALSE; } - if (virGetVersion(&libVer,NULL,&typeVer)!= 0) + if (virGetVersion(&libVer,NULL,NULL)!= 0) RETURN_FALSE; if (libVer<6002) -- 1.7.4.1
This is a revised patch to also fix the call to libvirt_version without arguments and without Xen. Currently libvirt_connect fails if libvirt has no Xen support. This is because virGetVersion checks for Xen if typeVer!=NULL. The same applies for libvirt_version if called without arguments and no Xen available. --- src/libvirt.c | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ce39a28..9636166 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -173,13 +173,12 @@ PHP_RSHUTDOWN_FUNCTION(libvirt) PHP_MINFO_FUNCTION(libvirt) { unsigned long libVer; - unsigned long typeVer; char *version; php_info_print_table_start(); php_info_print_table_row(2, "Libvirt support", "enabled"); php_info_print_table_row(2, "Extension version", PHP_LIBVIRT_WORLD_VERSION); - if (virGetVersion(&libVer,NULL,&typeVer)== 0) + if (virGetVersion(&libVer,NULL,NULL)== 0) { version=emalloc(100); snprintf(version, 100, "%i.%i.%i", (long)((libVer/1000000) % 1000),(long)((libVer/1000) % 1000),(long)(libVer % 1000)); @@ -590,13 +589,12 @@ PHP_FUNCTION(libvirt_connect) unsigned long index; unsigned long libVer; - unsigned long typeVer; if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|sba", &url,&url_len,&readonly,&zcreds) == FAILURE) { RETURN_FALSE; } - if (virGetVersion(&libVer,NULL,&typeVer)!= 0) + if (virGetVersion(&libVer,NULL,NULL)!= 0) RETURN_FALSE; if (libVer<6002) @@ -3358,8 +3356,13 @@ PHP_FUNCTION(libvirt_version) RETURN_FALSE; } - if (virGetVersion(&libVer,type,&typeVer) != 0) - RETURN_FALSE; + if (ZEND_NUM_ARGS() == 0) { + if (virGetVersion(&libVer,NULL,NULL) != 0) + RETURN_FALSE; + } else { + if (virGetVersion(&libVer,type,&typeVer) != 0) + RETURN_FALSE; + } /* The version is returned as: major * 1,000,000 + minor * 1,000 + release. */ array_init(return_value); @@ -3372,9 +3375,12 @@ PHP_FUNCTION(libvirt_version) add_assoc_long(return_value, "connector.major", VERSION_MAJOR); add_assoc_long(return_value, "connector.minor", VERSION_MINOR); add_assoc_long(return_value, "connector.release", VERSION_MICRO); - add_assoc_long(return_value, "type.release",(long)(typeVer %1000)); - add_assoc_long(return_value, "type.minor",(long)((typeVer/1000) % 1000)); - add_assoc_long(return_value, "type.major",(long)((typeVer/1000000) % 1000)); + + if (ZEND_NUM_ARGS() > 0) { + add_assoc_long(return_value, "type.release",(long)(typeVer %1000)); + add_assoc_long(return_value, "type.minor",(long)((typeVer/1000) % 1000)); + add_assoc_long(return_value, "type.major",(long)((typeVer/1000000) % 1000)); + } } /* -- 1.7.4.1
Hi Tiziano, thanks a lot for your patches. I pushed them already. Just one more thing: By libvirt-php prefix I didn't mean to start subject with libvirt-php directly but to format patches using `git format-patch -X --subject-prefix="libvirt-php PATCH"` or you can set it directly in the .git/config file in your cloned repository. You can use git send-email since your version of git should support it however the resulting subject will be: [libvirt] [libvirt-php PATCH] something and now it's still: [libvirt] [PATCH] something So if you don't put "libvirt-php" in the beginning of your commit line I could get confused whether it's for libvirt-php or libvirt itself. You can set it in your config also using the following command in the libvirt-php cloned repository: git config --local format.subjectprefix "libvirt-php PATCH" (or just "libvirt-php" is OK too). This is mentioned on the contributions page of libvirt-php available at [1]. Thanks, Michal [1] http://libvirt.org/php/contributions.html On 03/08/2011 11:00 AM, Tiziano Mueller wrote:
This is a revised patch to also fix the call to libvirt_version without arguments and without Xen.
Currently libvirt_connect fails if libvirt has no Xen support. This is because virGetVersion checks for Xen if typeVer!=NULL. The same applies for libvirt_version if called without arguments and no Xen available.
--- src/libvirt.c | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index ce39a28..9636166 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -173,13 +173,12 @@ PHP_RSHUTDOWN_FUNCTION(libvirt) PHP_MINFO_FUNCTION(libvirt) { unsigned long libVer; - unsigned long typeVer; char *version; php_info_print_table_start(); php_info_print_table_row(2, "Libvirt support", "enabled"); php_info_print_table_row(2, "Extension version", PHP_LIBVIRT_WORLD_VERSION);
- if (virGetVersion(&libVer,NULL,&typeVer)== 0) + if (virGetVersion(&libVer,NULL,NULL)== 0) { version=emalloc(100); snprintf(version, 100, "%i.%i.%i", (long)((libVer/1000000) % 1000),(long)((libVer/1000) % 1000),(long)(libVer % 1000)); @@ -590,13 +589,12 @@ PHP_FUNCTION(libvirt_connect) unsigned long index;
unsigned long libVer; - unsigned long typeVer;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|sba",&url,&url_len,&readonly,&zcreds) == FAILURE) { RETURN_FALSE; }
- if (virGetVersion(&libVer,NULL,&typeVer)!= 0) + if (virGetVersion(&libVer,NULL,NULL)!= 0) RETURN_FALSE;
if (libVer<6002) @@ -3358,8 +3356,13 @@ PHP_FUNCTION(libvirt_version) RETURN_FALSE; }
- if (virGetVersion(&libVer,type,&typeVer) != 0) - RETURN_FALSE; + if (ZEND_NUM_ARGS() == 0) { + if (virGetVersion(&libVer,NULL,NULL) != 0) + RETURN_FALSE; + } else { + if (virGetVersion(&libVer,type,&typeVer) != 0) + RETURN_FALSE; + }
/* The version is returned as: major * 1,000,000 + minor * 1,000 + release. */ array_init(return_value); @@ -3372,9 +3375,12 @@ PHP_FUNCTION(libvirt_version) add_assoc_long(return_value, "connector.major", VERSION_MAJOR); add_assoc_long(return_value, "connector.minor", VERSION_MINOR); add_assoc_long(return_value, "connector.release", VERSION_MICRO); - add_assoc_long(return_value, "type.release",(long)(typeVer %1000)); - add_assoc_long(return_value, "type.minor",(long)((typeVer/1000) % 1000)); - add_assoc_long(return_value, "type.major",(long)((typeVer/1000000) % 1000)); + + if (ZEND_NUM_ARGS()> 0) { + add_assoc_long(return_value, "type.release",(long)(typeVer %1000)); + add_assoc_long(return_value, "type.minor",(long)((typeVer/1000) % 1000)); + add_assoc_long(return_value, "type.major",(long)((typeVer/1000000) % 1000)); + } }
/*
-- Michal Novotny<minovotn@redhat.com>, RHCE Virtualization Team (xen userspace), Red Hat
participants (2)
-
Michal Novotny -
Tiziano Mueller