Hi, Dan
Masayuki changes the function
from virGetCpuMax to virDomainGetMaxVcpus.
but not changes xenHypervisorGetCpuMax.
This is based on your suggestion.
Is this code satify your criteria?
If this code satisfied, he will make final code for the commiting.
Thanks
Atsushi SAKAI
Masayuki Sunou <[EMAIL PROTECTED]> wrote:
> Hi Dan
>
> > 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.
> >
> I agree.
> I will consider it as a back log.
>
> > 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
> >
> I contribute the patch that corrects the following again.
> ・ Correction of name of method and argument
> --> Isn't the name bad?
> ・ Correction of position of method
>
>
> This patch is over 200lines.
> If you request, I am ready to repost it with split this patch.
>
> Signed-off-by: Masayuki Sunou <[EMAIL PROTECTED]>
>
> Thanks,
> Masayuki Sunou.
>
>
> libvirt-0.2.0
> ----------------------------------------------------------------------
> diff -uprN 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-03 00:07:15.000000000 +0900
> @@ -310,6 +310,8 @@ int virDomainSetMaxMemory
> (virDomainPt
> unsigned long memory);
> int virDomainSetMemory (virDomainPtr domain,
> unsigned long memory);
> +int virDomainGetMaxVcpus (virDomainPtr domain);
> +
> /*
> * XML domain description
> */
> diff -uprN a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> --- a/include/libvirt/libvirt.h.in 2007-02-23 17:51:30.000000000 +0900
> +++ b/include/libvirt/libvirt.h.in 2007-03-02 20:37:55.000000000 +0900
> @@ -310,6 +310,8 @@ int virDomainSetMaxMemory
> (virDomainPt
> unsigned long memory);
> int virDomainSetMemory (virDomainPtr domain,
> unsigned long memory);
> +int virDomainGetMaxVcpus (virDomainPtr domain);
> +
> /*
> * XML domain description
> */
> diff -uprN a/src/driver.h b/src/driver.h
> --- a/src/driver.h 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/driver.h 2007-03-02 23:33:20.000000000 +0900
> @@ -129,6 +129,8 @@ typedef int
> unsigned char *cpumaps,
> int maplen);
> typedef int
> + (*virDrvDomainGetMaxVcpus) (virDomainPtr domain);
> +typedef int
> (*virDrvDomainAttachDevice) (virDomainPtr domain,
> char *xml);
> typedef int
> @@ -181,6 +183,7 @@ struct _virDriver {
> virDrvDomainSetVcpus domainSetVcpus;
> virDrvDomainPinVcpu domainPinVcpu;
> virDrvDomainGetVcpus domainGetVcpus;
> + virDrvDomainGetMaxVcpus domainGetMaxVcpus;
> virDrvDomainDumpXML domainDumpXML;
> virDrvListDefinedDomains listDefinedDomains;
> virDrvNumOfDefinedDomains numOfDefinedDomains;
> diff -uprN a/src/libvirt.c b/src/libvirt.c
> --- a/src/libvirt.c 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/libvirt.c 2007-03-03 00:10:50.000000000 +0900
> @@ -2110,6 +2110,40 @@ virDomainGetVcpus(virDomainPtr domain, v
> }
>
> /**
> + * virDomainGetMaxVcpus:
> + * @domain: pointer to domain object
> + *
> + * Returns the maximum of virtual CPU of Domain.
> + *
> + * Returns the maximum of virtual CPU or 0 in case of error.
> + */
> +int
> +virDomainGetMaxVcpus(virDomainPtr domain) {
> + int i;
> + int ret = 0;
> + virConnectPtr conn;
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> + virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + return (0);
> + }
> +
> + conn = domain->conn;
> +
> + for (i = 0;i < conn->nb_drivers;i++) {
> + if ((conn->drivers[i] != NULL) &&
> + (conn->drivers[i]->domainGetMaxVcpus != NULL)) {
> + ret = conn->drivers[i]->domainGetMaxVcpus(domain);
> + if (ret != 0)
> + return(ret);
> + }
> + }
> + virLibConnError(conn, VIR_ERR_CALL_FAILED, __FUNCTION__);
> + return (-1);
> +}
> +
> +
> +/**
> * virDomainAttachDevice:
> * @domain: pointer to domain object
> * @xml: pointer to XML description of one device
> diff -uprN a/src/libvirt_sym.version b/src/libvirt_sym.version
> --- a/src/libvirt_sym.version 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/libvirt_sym.version 2007-03-02 20:38:53.000000000 +0900
> @@ -56,6 +56,7 @@
> virDomainSetVcpus;
> virDomainPinVcpu;
> virDomainGetVcpus;
> + virDomainGetMaxVcpus;
>
> virDomainAttachDevice;
> virDomainDetachDevice;
> diff -uprN a/src/proxy_internal.c b/src/proxy_internal.c
> --- a/src/proxy_internal.c 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/proxy_internal.c 2007-03-02 18:42:42.000000000 +0900
> @@ -73,6 +73,7 @@ static virDriver xenProxyDriver = {
> NULL, /* domainSetVcpus */
> NULL, /* domainPinVcpu */
> NULL, /* domainGetVcpus */
> + NULL, /* domainGetMaxVcpus */
> xenProxyDomainDumpXML, /* domainDumpXML */
> NULL, /* listDefinedDomains */
> NULL, /* numOfDefinedDomains */
> diff -uprN 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-02 23:34:08.000000000 +0900
> @@ -1190,6 +1190,7 @@ static virDriver qemuDriver = {
> NULL, /* domainSetVcpus */
> NULL, /* domainPinVcpu */
> NULL, /* domainGetVcpus */
> + NULL, /* domainGetMaxVcpus */
> qemuDomainDumpXML, /* domainDumpXML */
> qemuListDefinedDomains, /* listDomains */
> qemuNumOfDefinedDomains, /* numOfDomains */
> diff -uprN a/src/test.c b/src/test.c
> --- a/src/test.c 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/test.c 2007-03-02 23:36:07.000000000 +0900
> @@ -117,6 +117,7 @@ static virDriver testDriver = {
> testSetVcpus, /* domainSetVcpus */
> NULL, /* domainPinVcpu */
> NULL, /* domainGetVcpus */
> + NULL, /* domainGetMaxVcpus */
> testDomainDumpXML, /* domainDumpXML */
> testListDefinedDomains, /* listDefinedDomains */
> testNumOfDefinedDomains, /* numOfDefinedDomains */
> diff -uprN a/src/virsh.c b/src/virsh.c
> --- a/src/virsh.c 2007-02-28 00:35:50.000000000 +0900
> +++ b/src/virsh.c 2007-03-02 20:35:28.000000000 +0900
> @@ -1383,6 +1383,7 @@ cmdSetvcpus(vshControl * ctl, vshCmd * c
> {
> virDomainPtr dom;
> int count;
> + int maxcpu;
> int ret = TRUE;
>
> if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
> @@ -1397,6 +1398,18 @@ cmdSetvcpus(vshControl * ctl, vshCmd * c
> return FALSE;
> }
>
> + maxcpu = virDomainGetMaxVcpus(dom);
> + if (!maxcpu) {
> + virDomainFree(dom);
> + return FALSE;
> + }
> +
> + if (count > maxcpu) {
> + vshError(ctl, FALSE, _("Too many virtual CPU's."));
> + virDomainFree(dom);
> + return FALSE;
> + }
> +
> if (virDomainSetVcpus(dom, count) != 0) {
> ret = FALSE;
> }
> diff -uprN a/src/xen_internal.c b/src/xen_internal.c
> --- a/src/xen_internal.c 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/xen_internal.c 2004-07-03 05:43:41.000000000 +0900
> @@ -446,6 +446,7 @@ static virDriver xenHypervisorDriver = {
> xenHypervisorSetVcpus, /* domainSetVcpus */
> xenHypervisorPinVcpu, /* domainPinVcpu */
> xenHypervisorGetVcpus, /* domainGetVcpus */
> + xenHypervisorGetVcpuMax, /* domainGetMaxVcpus */
> NULL, /* domainDumpXML */
> NULL, /* listDefinedDomains */
> NULL, /* numOfDefinedDomains */
> @@ -1824,6 +1825,17 @@ xenHypervisorGetVcpus(virDomainPtr domai
> }
> #endif
>
> +/**
> + * xend_get_cpu_max:
> + *
> + * Returns the maximum of CPU defined by Xen.
> + */
> +int
> +xenHypervisorGetVcpuMax(virDomainPtr domain)
> +{
> + return MAX_VIRT_CPUS;
> +}
> +
> /*
> * Local variables:
> * indent-tabs-mode: nil
> diff -uprN a/src/xen_internal.h b/src/xen_internal.h
> --- a/src/xen_internal.h 2006-08-04 19:41:05.000000000 +0900
> +++ b/src/xen_internal.h 2004-07-03 05:44:03.000000000 +0900
> @@ -55,6 +55,7 @@ int xenHypervisorGetVcpus (virDomainPtr
> int maxinfo,
> unsigned char *cpumaps,
> int maplen);
> +int xenHypervisorGetVcpuMax (virDomainPtr domain);
>
> #ifdef __cplusplus
> }
> diff -uprN a/src/xend_internal.c b/src/xend_internal.c
> --- a/src/xend_internal.c 2007-03-02 08:24:09.000000000 +0900
> +++ b/src/xend_internal.c 2007-03-02 23:34:44.000000000 +0900
> @@ -89,6 +89,7 @@ static virDriver xenDaemonDriver = {
> xenDaemonDomainSetVcpus, /* domainSetVcpus */
> xenDaemonDomainPinVcpu, /* domainPinVcpu */
> xenDaemonDomainGetVcpus, /* domainGetVcpus */
> + NULL, /* domainGetMaxVcpus */
> xenDaemonDomainDumpXML, /* domainDumpXML */
> xenDaemonListDefinedDomains, /* listDefinedDomains */
> xenDaemonNumOfDefinedDomains, /* numOfDefinedDomains */
> @@ -511,10 +512,10 @@ xend_post(virConnectPtr xend, const char
> } else if ((ret == 202) && (strstr(content, "failed") != NULL)) {
> virXendError(xend, VIR_ERR_POST_FAILED, content);
> ret = -1;
> - } else if (((ret >= 200) && (ret <= 202)) && (strstr(content,
> "xend.err") != NULL)) {
> - /* This is to catch case of things like 'virsh dump Domain-0 foo'
> - * which returns a success code, but the word 'xend.err'
> - * in body to indicate error :-(
> + } else if ((ret == 202) && (strstr(content, "Cannot") != NULL)) {
> + /* This is to catch case of 'virsh dump Domain-0 foo'
> + * which returns a success code, but the word 'Cannot'
> + * in body to indicate error
> */
> virXendError(xend, VIR_ERR_POST_FAILED, content);
> ret = -1;
> diff -uprN a/src/xm_internal.c b/src/xm_internal.c
> --- a/src/xm_internal.c 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/xm_internal.c 2007-03-02 23:35:29.000000000 +0900
> @@ -98,6 +98,7 @@ static virDriver xenXMDriver = {
> xenXMDomainSetVcpus, /* domainSetVcpus */
> NULL, /* domainPinVcpu */
> NULL, /* domainGetVcpus */
> + NULL, /* domainGetMaxVcpus */
> xenXMDomainDumpXML, /* domainDumpXML */
> xenXMListDefinedDomains, /* listDefinedDomains */
> xenXMNumOfDefinedDomains, /* numOfDefinedDomains */
> diff -uprN a/src/xs_internal.c b/src/xs_internal.c
> --- a/src/xs_internal.c 2007-02-23 17:51:30.000000000 +0900
> +++ b/src/xs_internal.c 2007-03-02 23:36:22.000000000 +0900
> @@ -67,6 +67,7 @@ static virDriver xenStoreDriver = {
> NULL, /* domainSetVcpus */
> NULL, /* domainPinVcpu */
> NULL, /* domainGetVcpus */
> + NULL, /* domainGetMaxVcpus */
> NULL, /* domainDumpXML */
> NULL, /* listDefinedDomains */
> NULL, /* numOfDefinedDomains */
> ----------------------------------------------------------------------
>
>
> In message <[EMAIL PROTECTED]>
> "Re: [Libvir] [PATCH] check the maximum of virtual CPU"
> ""Daniel P. Berrange" <[EMAIL PROTECTED]>" wrote:
>
> > 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
> > -=|
> >
>
> --
> Libvir-list mailing list
> Libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
