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

[Fernando Casas Schössow]

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 at googlemail.com> wrote:
> 2016-09-07 14:20 GMT+02:00 Fernando Casas Schössow 
> <casasfernando at hotmail.com>:
>>  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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160907/b3aec49d/attachment-0001.htm>