[Libvir] [PATCH] check the maximum of virtual CPU

[Daniel P. Berrange]

On Thu, Mar 01, 2007 at 10:02:03AM +0900, Masayuki Sunou wrote:
> Hi
> 
> The maximum of virtual CPU is not guarded in virsh setvcpus now.
> Then, when 32767 was set to virtual CPU of virsh setvcpus, the problem
> that Xend became abnormal was detected.
> 
> example:
> ----------------------------------------------------------------------
>   # virsh setvcpus 0 32767
>   libvir: Xen Daemon error : POST operation failed: (xend.err "(9, 'Bad file descriptor')")
>   libvir: Xen error : failed Xen syscall  ioctl  3166208
>   libvir: error : library call virDomainSetVcpus failed, possibly not supported
>   
>   # xm list -l
>   Error: (9, 'Bad file descriptor')
>   Usage: xm list [options] [Domain, ...]

This looks like a bug in XenD that should be reported upstrem. If the hypercall
is given an invalid value it should reject it and not screw up the whole host.

> Therefore, I propose the correction that adjusts the maximum of virtual
> CPU to the same value as Xen. 

Ordinarily I'd say just fix Xen, but given that there are many broken versions
of Xen out there, I agree it is worth putting a max-vcpus check into libvirt to
protect users.

> libvirt-0.2.0
> ----------------------------------------------------------------------
> diff -rup a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> --- a/include/libvirt/libvirt.h	2007-02-23 17:51:30.000000000 +0900
> +++ b/include/libvirt/libvirt.h	2007-03-01 03:02:20.000000000 +0900
> @@ -233,6 +233,7 @@ int			virConnectGetVersion	(virConnectPt
>  						 unsigned long *hvVer);
>  int			virNodeGetInfo		(virConnectPtr conn,
>  						 virNodeInfoPtr info);
> +int			virGetCpuMax		(virConnectPtr conn);

If we add this against the virConnectPtr object, we should name it 

    virConnectGetVcpuMax()

For consistency with other VCPU method naming.  I wonder though, if we should
instead make it a method which takes a virDomainPtr instead - semantically we
are asking for the VCPU limit which can be applied to a domain. What do people
think ?


> diff -rup a/src/qemu_internal.c b/src/qemu_internal.c
> --- a/src/qemu_internal.c	2007-02-23 21:46:35.000000000 +0900
> +++ b/src/qemu_internal.c	2007-03-01 02:18:02.000000000 +0900
> @@ -1200,6 +1200,7 @@ static virDriver qemuDriver = {
>      NULL, /* domainDetachDevice */
>      qemuDomainGetAutostart, /* domainGetAutostart */
>      qemuDomainSetAutostart, /* domainSetAutostart */
> +    NULL, /* cpumax */
>  };

W e should an impl for QEMU, and in fact this makes me think that we should
definitely make it virDomainGetMaxVcpus(virDomainPtr dom) because in the QEMU
case the limit is different depending on what type of domain we've created.
ie, a QEMU or KQEMU domain can have many VCPUs, but a KVM domain can currently
only have 1 VCPU. So we need the virDomainptr object to implement this logic.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|