[libvirt] [libvirt-php] bug in libvirt_domain_change_vcpus()
Hello everyone, I think there is a bug in the current implementation of function libvirt_domain_change_vcpu(). When using this function (with second parameter = 2), I have the following error : Error setting maxvcpus=2: (domain_definition):8: Specification mandate value for attribute curre <vcpu>2</vcpu> --^ If I'm right, the equivalent with 'virsh' binary is 'setvcpus', and this is working. If I look into source code (http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=7ce2d...) it seems this operation is done by rewriting the whole XML. I think we should use instead C function virDomainSetVcpus http://libvirt.org/html/libvirt-libvirt.html#virDomainSetVcpus What do you think ? Olivier
Hi Olivier, this is not really a bug as it's intended. The libvirt_domain_change_vcpus() is not the same as (yet unimplemented) virDomainSetVcpus() API so it's not bug. I realize that naming could be a little confusing so if you like, feel free to rename libvirt_domain_change_vcpus() to libvirt_domain_xml_change_vcpus() as this API should be used to change number of vCPUS in the domain XML itself and implement libvirt_domain_set_vcpus() calling real virDomainSetVcpus() API function. Thanks, Michal On 09/09/2013 06:03 PM, Olivier Doucet wrote:
Hello everyone,
I think there is a bug in the current implementation of function libvirt_domain_change_vcpu(). When using this function (with second parameter = 2), I have the following error :
Error setting maxvcpus=2: (domain_definition):8: Specification mandate value for attribute curre <vcpu>2</vcpu> --^
If I'm right, the equivalent with 'virsh' binary is 'setvcpus', and this is working.
If I look into source code (http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=7ce2d...) it seems this operation is done by rewriting the whole XML.
I think we should use instead C function virDomainSetVcpus http://libvirt.org/html/libvirt-libvirt.html#virDomainSetVcpus
What do you think ?
Olivier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org
On 09/09/2013 06:51 PM, Michal Novotny wrote:
Hi Olivier, this is not really a bug as it's intended. The libvirt_domain_change_vcpus() is not the same as (yet unimplemented) virDomainSetVcpus() API so it's not bug. I realize that naming could be a little confusing so if you like, feel free to rename libvirt_domain_change_vcpus() to libvirt_domain_xml_change_vcpus() as this API should be used to change number of vCPUS in the domain XML itself and implement libvirt_domain_set_vcpus() calling real virDomainSetVcpus() API function.
Alternatively, for more control, we should implement virDomainSetVcpusFlags() API [1] instead that can affect live domain and also defined persistent domain according to the flags passed. Michal [1] http://libvirt.org/html/libvirt-libvirt.html#virDomainSetVcpusFlags
Thanks, Michal
On 09/09/2013 06:03 PM, Olivier Doucet wrote:
Hello everyone,
I think there is a bug in the current implementation of function libvirt_domain_change_vcpu(). When using this function (with second parameter = 2), I have the following error :
Error setting maxvcpus=2: (domain_definition):8: Specification mandate value for attribute curre <vcpu>2</vcpu> --^
If I'm right, the equivalent with 'virsh' binary is 'setvcpus', and this is working.
If I look into source code (http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=7ce2d...) it seems this operation is done by rewriting the whole XML.
I think we should use instead C function virDomainSetVcpus http://libvirt.org/html/libvirt-libvirt.html#virDomainSetVcpus
What do you think ?
Olivier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org
On Mon, Sep 09, 2013 at 06:51:54PM +0200, Michal Novotny wrote:
Hi Olivier, this is not really a bug as it's intended. The libvirt_domain_change_vcpus() is not the same as (yet unimplemented) virDomainSetVcpus() API so it's not bug. I realize that naming could be a little confusing so if you like, feel free to rename libvirt_domain_change_vcpus() to libvirt_domain_xml_change_vcpus() as this API should be used to change number of vCPUS in the domain XML itself and implement libvirt_domain_set_vcpus() calling real virDomainSetVcpus() API function.
That function is also buggy in that it will silently turn a transient guest into a persistent guest, which is almost certainly not what a caller would expect Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 09/10/2013 10:09 AM, Daniel P. Berrange wrote:
On Mon, Sep 09, 2013 at 06:51:54PM +0200, Michal Novotny wrote:
Hi Olivier, this is not really a bug as it's intended. The libvirt_domain_change_vcpus() is not the same as (yet unimplemented) virDomainSetVcpus() API so it's not bug. I realize that naming could be a little confusing so if you like, feel free to rename libvirt_domain_change_vcpus() to libvirt_domain_xml_change_vcpus() as this API should be used to change number of vCPUS in the domain XML itself and implement libvirt_domain_set_vcpus() calling real virDomainSetVcpus() API function. That function is also buggy in that it will silently turn a transient guest into a persistent guest, which is almost certainly not what a caller would expect
Daniel
Ok, I guess dropping libvirt_domain_xml_change_vcpus() and implementing libvirt_domain_xml_set_vcpus() using the virDomainSetVcpus() API function is much better. Thanks, Michal -- Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org
On 09/10/2013 10:09 AM, Daniel P. Berrange wrote:
On Mon, Sep 09, 2013 at 06:51:54PM +0200, Michal Novotny wrote:
Hi Olivier, this is not really a bug as it's intended. The libvirt_domain_change_vcpus() is not the same as (yet unimplemented) virDomainSetVcpus() API so it's not bug. I realize that naming could be a little confusing so if you like, feel free to rename libvirt_domain_change_vcpus() to libvirt_domain_xml_change_vcpus() as this API should be used to change number of vCPUS in the domain XML itself and implement libvirt_domain_set_vcpus() calling real virDomainSetVcpus() API function. That function is also buggy in that it will silently turn a transient guest into a persistent guest, which is almost certainly not what a caller would expect
Daniel Thanks for information Daniel. You are correct. I did change the implementation to use virDomainSetVcpusFlags() API function instead. It's already committed to git [1].
Michal [1] http://libvirt.org/git/?p=libvirt-php.git;a=commit;h=cff010a36e382ee26c10a8e... -- Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat Virtualization | libvirt-php bindings | php-virt-control.org
participants (3)
-
Daniel P. Berrange -
Michal Novotny -
Olivier Doucet