Re: [libvirt-php] libvirt_list_active_domains returns garbage querying a Hyper-V host

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for the tip Matthias.

I found some time to check the code and came up with this patch:

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 2045c59..8e215f4 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -9105,17 +9105,21 @@ PHP_FUNCTION(libvirt_list_active_domains)
     int count=-1;
     int expectedcount=-1;
     int *ids;
- int i;
- virDomainPtr domain = NULL;
     const char *name;
+ int i, rv;
+ virDomainPtr domain=NULL;
 
     GET_CONNECTION_FROM_ARGS("r",&zconn);
 
     if ((expectedcount=virConnectNumOfDomains (conn->conn)) < 0)
         RETURN_FALSE;
 
+ DPRINTF("%s: Found %d domains\n", PHPFUNC, expectedcount);
+
     ids=(int *)emalloc(sizeof(int)*expectedcount);
     count=virConnectListDomains (conn->conn,ids,expectedcount);
+ DPRINTF("%s: virConnectListDomains returned %d domains\n", PHPFUNC, count);
+
     if ((count != expectedcount) || (count<0))
     {
         efree (ids);
@@ -9126,21 +9130,26 @@ PHP_FUNCTION(libvirt_list_active_domains)
     for (i=0;i<count;i++)
     {
         domain=virDomainLookupByID(conn->conn,ids[i]);
+ resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 1 TSRMLS_CC);
         if (domain!=NULL)
         {
             name=virDomainGetName(domain);
-
- if (virDomainFree (domain))
- resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC);
-
- if (name==NULL)
- {
- efree (ids);
- RETURN_FALSE;
+ if (name != NULL) {
+ DPRINTF("%s: Found running domain %s with ID = %d\n", PHPFUNC, name, ids[i]);
+ VIRT_ADD_NEXT_INDEX_STRING(return_value, name);
             }
-
- VIRT_ADD_NEXT_INDEX_STRING(return_value, name);
+ else
+ DPRINTF("%s: Cannot get ID for running domain %d\n", PHPFUNC, ids[i]);
         }
+ rv = virDomainFree (domain);
+ if (rv != 0) {
+ php_error_docref(NULL TSRMLS_CC, E_WARNING,"virDomainFree failed with %i on list_domain: %s",
+ rv, LIBVIRT_G (last_error));
+ }
+ else {
+ resource_change_counter(INT_RESOURCE_DOMAIN, conn->conn, domain, 0 TSRMLS_CC);
+ }
+ domain = NULL;
     }
     efree(ids);
 }


Basically I took the code from libvirt_list_domains and fit it in.
I'm almost sure it's not the most elegant and clean solution but it works fine in my tests.
Hopefully a fix will be included in the next libvirt-php release.

Thanks.

Fer

On mié, sep 7, 2016 at 3:58 , Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx> wrote:
2016-09-07 14:20 GMT+02:00 Fernando Casas Schössow <casasfernando@xxxxxxxxxxx>:
Hi all, I'm using libvirt-php to manage a Hyper-V 2012 R2 host. When trying to retrieve the active VMs using the function libvirt_list_active_domains() I get an array with the correct amount of items but with garbage instead of the domain names: Array ( [0] => P’*µ [1] => +µ [2] => `„*µ [3] => °Ø*µ [4] => Є*µ [5] => `†*µ [6] => [7] => `†*µ [8] => [9] => p +µ [10] => p"+µ [11] => P#+µ [12] => ß*µ ) On the other hand libvirt_list_inactive_domains() and libvirt_list_domains() work as expected returning the right amount of items and the domain names correctly: libvirt_list_inactive_domains() output: Array ( [0] => DCHOMELAB01 [1] => VMFCSW7 [2] => REMOTEAPP01 [3] => MPSSD01 [4] => CS16SVR )
The problem is here http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=2045c59644075c00a22695d7b8534ff593120f14;hb=HEAD#l9131 libvirt_list_active_domains gets the domain name using virDomainGetName, then frees the domain object using virDomainFree (this also frees the domain name) and afterwards tries to use the already freed domain name. libvirt_list_active_domains needs to add the domian name to the result list and then free the domain. For example, libvirt_list_domains does it in the correct order.
--
Matthias Bolte http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]