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 <fj1826dm@xxxxxxxxxxxxxxxxx>
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 <20070301200929.GK6079@xxxxxxxxxx>
"Re: [PATCH] check the maximum of virtual CPU"
""Daniel P. Berrange" <berrange@xxxxxxxxxx>" 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 -=|
>