[Libvir] [PATCH] check the maximum of virtual CPU
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, ...] List information about all/some domains. -l, --long Output all VM details in SXP --label Include security labels --state=<state> Select only VMs with the specified state ---------------------------------------------------------------------- Therefore, I propose the correction that adjusts the maximum of virtual CPU to the same value as Xen. This patch is over 200lines. If you request, I am ready to repost it with split this patch. Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. 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); /* * Gather list of running domains diff -rup 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-01 03:02:50.000000000 +0900 @@ -233,6 +233,7 @@ int virConnectGetVersion (virConnectPt unsigned long *hvVer); int virNodeGetInfo (virConnectPtr conn, virNodeInfoPtr info); +int virGetCpuMax (virConnectPtr conn); /* * Gather list of running domains diff -rup 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-01 03:09:32.000000000 +0900 @@ -140,6 +140,8 @@ typedef int typedef int (*virDrvDomainSetAutostart) (virDomainPtr domain, int autostart); +typedef int + (*virDrvGetCpuMax) (void); typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -191,6 +193,7 @@ struct _virDriver { virDrvDomainDetachDevice domainDetachDevice; virDrvDomainGetAutostart domainGetAutostart; virDrvDomainSetAutostart domainSetAutostart; + virDrvGetCpuMax cpumax; }; typedef int diff -rup 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-01 02:16:49.000000000 +0900 @@ -1670,6 +1670,35 @@ virNodeGetInfo(virConnectPtr conn, virNo return(0); } +/** + * virDrvGetCpuMax: + * + * Returns the maximum of CPU defined by Hypervisor. + * + * Returns the maximum of CPU or 0 in case of error. + */ +int +virGetCpuMax(virConnectPtr conn) { + int i; + int ret = -1; + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (0); + } + + for (i = 0;i < conn->nb_drivers;i++) { + if ((conn->drivers[i] != NULL) && + (conn->drivers[i]->cpumax != NULL)) { + ret = conn->drivers[i]->cpumax(); + if (ret != 0) + return(ret); + } + } + virLibConnError(conn, VIR_ERR_CALL_FAILED, __FUNCTION__); + return (-1); +} + /************************************************************************ * * * Handling of defined but not running domains * diff -rup 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-01 02:17:18.000000000 +0900 @@ -52,6 +52,7 @@ virConnResetLastError; virDefaultErrorFunc; virNodeGetInfo; + virGetCpuMax; virDomainSetVcpus; virDomainPinVcpu; diff -rup 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-01 02:17:40.000000000 +0900 @@ -83,6 +83,7 @@ static virDriver xenProxyDriver = { NULL, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ + NULL, /* cpumax */ }; /** 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 */ }; static virNetworkDriver qemuNetworkDriver = { diff -rup 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-01 02:18:53.000000000 +0900 @@ -127,6 +127,7 @@ static virDriver testDriver = { NULL, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ + NULL, /* cpumax */ }; typedef struct _testDev { diff -rup 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-01 02:19:59.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 = virGetCpuMax(ctl->conn); + 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 -rup a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c 2007-02-28 00:50:03.000000000 +0900 +++ b/src/xend_internal.c 2007-03-01 02:21:34.000000000 +0900 @@ -99,6 +99,7 @@ static virDriver xenDaemonDriver = { xenDaemonDetachDevice, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ + NULL, /* cpumax */ }; /** diff -rup 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 2007-02-28 18:06:08.000000000 +0900 @@ -456,6 +456,7 @@ static virDriver xenHypervisorDriver = { NULL, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ + xenHypervisorGetCpuMax, /* cpumax */ }; #endif /* !PROXY */ @@ -1824,6 +1825,17 @@ xenHypervisorGetVcpus(virDomainPtr domai } #endif +/** + * xend_get_cpu_max: + * + * Returns the maximum of CPU defined by Xen. + */ +int +xenHypervisorGetCpuMax(void) +{ + return MAX_VIRT_CPUS; +} + /* * Local variables: * indent-tabs-mode: nil diff -rup 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 2007-03-01 02:21:13.000000000 +0900 @@ -55,6 +55,7 @@ int xenHypervisorGetVcpus (virDomainPtr int maxinfo, unsigned char *cpumaps, int maplen); +int xenHypervisorGetCpuMax (void); #ifdef __cplusplus } diff -rup 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-01 02:23:54.000000000 +0900 @@ -108,6 +108,7 @@ static virDriver xenXMDriver = { NULL, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ + NULL, /* cpumax */ }; static void diff -rup 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-01 02:24:20.000000000 +0900 @@ -77,6 +77,7 @@ static virDriver xenStoreDriver = { NULL, /* domainDetachDevice */ NULL, /* domainGetAutostart */ NULL, /* domainSetAutostart */ + NULL, /* cpumax */ }; /**
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, ...]
List information about all/some domains. -l, --long Output all VM details in SXP --label Include security labels --state=<state> Select only VMs with the specified state ----------------------------------------------------------------------
Therefore, I propose the correction that adjusts the maximum of virtual CPU to the same value as Xen.
This patch is over 200lines. If you request, I am ready to repost it with split this patch.
The patch looks pretty sensible to me. Needs the same check added to virt-manager too? Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
Hi, Rich
The patch looks pretty sensible to me. Needs the same check added to virt-manager too?
Virt-manager seems to check maximums of the number of CPU. However, virt-manager checks the number of CPU by fixed value (32). Therefore, I think that the check of architecture is needed in the future. Thanks, Masayuki Sunou. In message <45E68CA1.3030406@redhat.com> "Re: [Libvir] [PATCH] check the maximum of virtual CPU" ""Richard W.M. Jones" <rjones@redhat.com>" wrote:
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, ...]
List information about all/some domains. -l, --long Output all VM details in SXP --label Include security labels --state=<state> Select only VMs with the specified state ----------------------------------------------------------------------
Therefore, I propose the correction that adjusts the maximum of virtual CPU to the same value as Xen.
This patch is over 200lines. If you request, I am ready to repost it with split this patch.
The patch looks pretty sensible to me. Needs the same check added to virt-manager too?
Rich.
-- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 "[Negative numbers] darken the very whole doctrines of the equations and make dark of the things which are in their nature excessively obvious and simple" (Francis Maseres FRS, mathematician, 1759)
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -=|
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@aa.jp.fujitsu.com> 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@redhat.com> "Re: [Libvir] [PATCH] check the maximum of virtual CPU" ""Daniel P. Berrange" <berrange@redhat.com>" 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 -=|
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 <fj1826dm@aa.jp.fujitsu.com> 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 <fj1826dm@aa.jp.fujitsu.com>
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@redhat.com> "Re: [Libvir] [PATCH] check the maximum of virtual CPU" ""Daniel P. Berrange" <berrange@redhat.com>" 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
On Fri, Mar 02, 2007 at 08:42:59PM +0900, Masayuki Sunou 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. ?$B!& Correction of name of method and argument --> Isn't the name bad? ?$B!& Correction of position of method
This patch looks good me - unless anyone else on the list has objections I'll commit it to CVS later today. (I'll tweak the name of the internal xenHypervisorGetMaxVcpus method to add in 'Domain') Regards, 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 -=|
Hi, Dan Please omit follwoing code from Masayuki's patch. It reverses Mizushima's @@ -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; Thanks Atsushi SAKAI "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Mar 02, 2007 at 08:42:59PM +0900, Masayuki Sunou 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. ?$B!& Correction of name of method and argument --> Isn't the name bad? ?$B!& Correction of position of method
This patch looks good me - unless anyone else on the list has objections I'll commit it to CVS later today. (I'll tweak the name of the internal xenHypervisorGetMaxVcpus method to add in 'Domain')
Regards, 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
On Fri, Mar 02, 2007 at 09:26:03PM +0900, Atsushi SAKAI wrote:
Hi, Dan
Please omit follwoing code from Masayuki's patch. It reverses Mizushima's
Yes, no problem - noticed that already. 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 -=|
I removed a needless part from the former patch. Add check the maxmum of virtual CPU. Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks ------------------------------------------------------------------------------- Index: libvirt/include/libvirt/libvirt.h =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h,v retrieving revision 1.38 diff -u -p -r1.38 libvirt.h --- libvirt/include/libvirt/libvirt.h 23 Feb 2007 08:51:30 -0000 1.38 +++ libvirt/include/libvirt/libvirt.h 5 Mar 2007 05:58:32 -0000 @@ -310,6 +310,8 @@ int virDomainSetMaxMemory (virDomainPt unsigned long memory); int virDomainSetMemory (virDomainPtr domain, unsigned long memory); +int virDomainGetMaxVcpus (virDomainPtr domain); + /* * XML domain description */ Index: libvirt/include/libvirt/libvirt.h.in =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h.in,v retrieving revision 1.23 diff -u -p -r1.23 libvirt.h.in --- libvirt/include/libvirt/libvirt.h.in 23 Feb 2007 08:51:30 -0000 1.23 +++ libvirt/include/libvirt/libvirt.h.in 5 Mar 2007 05:58:33 -0000 @@ -310,6 +310,8 @@ int virDomainSetMaxMemory (virDomainPt unsigned long memory); int virDomainSetMemory (virDomainPtr domain, unsigned long memory); +int virDomainGetMaxVcpus (virDomainPtr domain); + /* * XML domain description */ Index: libvirt/src/driver.h =================================================================== RCS file: /data/cvs/libvirt/src/driver.h,v retrieving revision 1.21 diff -u -p -r1.21 driver.h --- libvirt/src/driver.h 23 Feb 2007 08:51:30 -0000 1.21 +++ libvirt/src/driver.h 5 Mar 2007 05:58:33 -0000 @@ -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; Index: libvirt/src/libvirt.c =================================================================== RCS file: /data/cvs/libvirt/src/libvirt.c,v retrieving revision 1.58 diff -u -p -r1.58 libvirt.c --- libvirt/src/libvirt.c 23 Feb 2007 08:51:30 -0000 1.58 +++ libvirt/src/libvirt.c 5 Mar 2007 05:58:36 -0000 @@ -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 Index: libvirt/src/libvirt_sym.version =================================================================== RCS file: /data/cvs/libvirt/src/libvirt_sym.version,v retrieving revision 1.16 diff -u -p -r1.16 libvirt_sym.version --- libvirt/src/libvirt_sym.version 23 Feb 2007 08:51:30 -0000 1.16 +++ libvirt/src/libvirt_sym.version 5 Mar 2007 05:58:36 -0000 @@ -56,6 +56,7 @@ virDomainSetVcpus; virDomainPinVcpu; virDomainGetVcpus; + virDomainGetMaxVcpus; virDomainAttachDevice; virDomainDetachDevice; Index: libvirt/src/proxy_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/proxy_internal.c,v retrieving revision 1.22 diff -u -p -r1.22 proxy_internal.c --- libvirt/src/proxy_internal.c 23 Feb 2007 08:51:30 -0000 1.22 +++ libvirt/src/proxy_internal.c 5 Mar 2007 05:58:37 -0000 @@ -73,6 +73,7 @@ static virDriver xenProxyDriver = { NULL, /* domainSetVcpus */ NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ xenProxyDomainDumpXML, /* domainDumpXML */ NULL, /* listDefinedDomains */ NULL, /* numOfDefinedDomains */ Index: libvirt/src/qemu_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_internal.c,v retrieving revision 1.12 diff -u -p -r1.12 qemu_internal.c --- libvirt/src/qemu_internal.c 23 Feb 2007 12:46:35 -0000 1.12 +++ libvirt/src/qemu_internal.c 5 Mar 2007 05:58:38 -0000 @@ -1190,6 +1190,7 @@ static virDriver qemuDriver = { NULL, /* domainSetVcpus */ NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ qemuDomainDumpXML, /* domainDumpXML */ qemuListDefinedDomains, /* listDomains */ qemuNumOfDefinedDomains, /* numOfDomains */ Index: libvirt/src/test.c =================================================================== RCS file: /data/cvs/libvirt/src/test.c,v retrieving revision 1.20 diff -u -p -r1.20 test.c --- libvirt/src/test.c 23 Feb 2007 08:51:30 -0000 1.20 +++ libvirt/src/test.c 5 Mar 2007 05:58:38 -0000 @@ -117,6 +117,7 @@ static virDriver testDriver = { testSetVcpus, /* domainSetVcpus */ NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ testDomainDumpXML, /* domainDumpXML */ testListDefinedDomains, /* listDefinedDomains */ testNumOfDefinedDomains, /* numOfDefinedDomains */ Index: libvirt/src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.58 diff -u -p -r1.58 virsh.c --- libvirt/src/virsh.c 2 Mar 2007 14:22:33 -0000 1.58 +++ libvirt/src/virsh.c 5 Mar 2007 05:58:41 -0000 @@ -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; } Index: libvirt/src/xen_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xen_internal.c,v retrieving revision 1.58 diff -u -p -r1.58 xen_internal.c --- libvirt/src/xen_internal.c 23 Feb 2007 08:51:30 -0000 1.58 +++ libvirt/src/xen_internal.c 5 Mar 2007 05:58:42 -0000 @@ -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 Index: libvirt/src/xen_internal.h =================================================================== RCS file: /data/cvs/libvirt/src/xen_internal.h,v retrieving revision 1.14 diff -u -p -r1.14 xen_internal.h --- libvirt/src/xen_internal.h 4 Aug 2006 10:41:05 -0000 1.14 +++ libvirt/src/xen_internal.h 5 Mar 2007 05:58:42 -0000 @@ -55,6 +55,7 @@ int xenHypervisorGetVcpus (virDomainPtr int maxinfo, unsigned char *cpumaps, int maplen); +int xenHypervisorGetVcpuMax (virDomainPtr domain); #ifdef __cplusplus } Index: libvirt/src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.97 diff -u -p -r1.97 xend_internal.c --- libvirt/src/xend_internal.c 2 Mar 2007 20:19:08 -0000 1.97 +++ libvirt/src/xend_internal.c 5 Mar 2007 05:58:44 -0000 @@ -89,6 +89,7 @@ static virDriver xenDaemonDriver = { xenDaemonDomainSetVcpus, /* domainSetVcpus */ xenDaemonDomainPinVcpu, /* domainPinVcpu */ xenDaemonDomainGetVcpus, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ xenDaemonDomainDumpXML, /* domainDumpXML */ xenDaemonListDefinedDomains, /* listDefinedDomains */ xenDaemonNumOfDefinedDomains, /* numOfDefinedDomains */ Index: libvirt/src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.14 diff -u -p -r1.14 xm_internal.c --- libvirt/src/xm_internal.c 23 Feb 2007 08:51:30 -0000 1.14 +++ libvirt/src/xm_internal.c 5 Mar 2007 05:58:46 -0000 @@ -98,6 +98,7 @@ static virDriver xenXMDriver = { xenXMDomainSetVcpus, /* domainSetVcpus */ NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ xenXMDomainDumpXML, /* domainDumpXML */ xenXMListDefinedDomains, /* listDefinedDomains */ xenXMNumOfDefinedDomains, /* numOfDefinedDomains */ Index: libvirt/src/xs_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xs_internal.c,v retrieving revision 1.32 diff -u -p -r1.32 xs_internal.c --- libvirt/src/xs_internal.c 23 Feb 2007 08:51:30 -0000 1.32 +++ libvirt/src/xs_internal.c 5 Mar 2007 05:58:47 -0000 @@ -67,6 +67,7 @@ static virDriver xenStoreDriver = { NULL, /* domainSetVcpus */ NULL, /* domainPinVcpu */ NULL, /* domainGetVcpus */ + NULL, /* domainGetMaxVcpus */ NULL, /* domainDumpXML */ NULL, /* listDefinedDomains */ NULL, /* numOfDefinedDomains */ ------------------------------------------------------------------------------- In message <20070302123124.GA1660@redhat.com> "Re: [Libvir] [PATCH] check the maximum of virtual CPU" ""Daniel P. Berrange" <berrange@redhat.com>" wrote:
On Fri, Mar 02, 2007 at 09:26:03PM +0900, Atsushi SAKAI wrote:
Hi, Dan
Please omit follwoing code from Masayuki's patch. It reverses Mizushima's
Yes, no problem - noticed that already.
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 -=|
On Mon, Mar 05, 2007 at 03:42:35PM +0900, Masayuki Sunou wrote:
I removed a needless part from the former patch.
Add check the maxmum of virtual CPU.
I was thinking about how to make use of this new API in virt-manager when I came up with a further complication :-) There are three scenarios where I'd like to be able to find out the maximum VCPUs possible for a guest VM: 1. When initially creating a VM 2. When changing the config of an inactive VM 3. When changing the config of a running VM Thus far in this thread, we've focused on addressing the last point, but I think its worth trying address all 3 while we're looking at this area. In the first case, we do not have a virDomainPtr object at all yet, all we have is a virConnectPtr and info about the type of domain we're planning to create - Xen, QEMU, KVM, KQEMU. When creating a VM we need to apply some domain type specific limit. In the case of an inactive domain, we have a virDomainPtr object and from that the internal drivers can determine the domain type or Xen, QEMU, KVM, etc, and again apply some domain type specific limit. In the 3rd case of a running VM though, the hypervisor limit is typically not neccessarily the one that is relevant. While the hypervisor will still has its own limit of VCPUs, the effective limit is likely lower - ie fixed at the number of virtual CPUs which were allocated when the guest OS booted. So if you boot a guest with 4 cpus, you can hotplug remove & add CPUs, but you can never go above 4 until the guest is shutdown. So anyway, I think we need to add 2 apis to address this whole issue: - int virConnectGetMaxVcpus(virConnectPtr conn, const char *type); Returns the maximum number of virtual CPUs supported for a guest VM of a specific type. Thje 'type' parameter here corresponds to the 'type' attribute in the <domain> element of the XML. - int virDomainGetMaxVcpus(virDomainPtr dom); Returns the maximum number of virual CPUs supported for the guest VM. If the guest is inactive, this is basically the same as virConnectGetMaxVcpus. If the guest is running this will reflect the maximum number of VCPus the guest was booted with. In terms of the Xen implementation, the virConnectGetMaxVcpus method can simply return MAX_VIRT_VCPUS. Likewie virDomainGetMaxVcpus can do the same for inactive guests, but if the guest is running then we will need to call out to XenD to extract the max vcpus info. Any other thoughts from people on the list ??? 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 -=|
Hi, Dan
1. When initially creating a VM 2. When changing the config of an inactive VM 3. When changing the config of a running VM
Certainly, I had not considered concerning 1 and 2. So, I corrected the patch based on your proposal.
・ virDomainGetMaxVcpus Judge state (active/inactive) of the domain, and return information corresponding to each state. ・ virConnectGetMaxVcpus Add it as a method that returns the number of maximum CPUs defined by Xen. However, I only added virConnectGetMaxVcpus because I did not understand the use image of it. Is it added as a command of virsh? Or, is it used from virsh create and virsh start? Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks ---------------------------------------------------------------------- In message <20070306035647.GA31470@redhat.com> "Re: [Libvir] [PATCH] check the maximum of virtual CPU" ""Daniel P. Berrange" <berrange@redhat.com>" wrote:
On Mon, Mar 05, 2007 at 03:42:35PM +0900, Masayuki Sunou wrote:
I removed a needless part from the former patch.
Add check the maxmum of virtual CPU.
I was thinking about how to make use of this new API in virt-manager when I came up with a further complication :-) There are three scenarios where I'd like to be able to find out the maximum VCPUs possible for a guest VM:
1. When initially creating a VM 2. When changing the config of an inactive VM 3. When changing the config of a running VM
Thus far in this thread, we've focused on addressing the last point, but I think its worth trying address all 3 while we're looking at this area.
In the first case, we do not have a virDomainPtr object at all yet, all we have is a virConnectPtr and info about the type of domain we're planning to create - Xen, QEMU, KVM, KQEMU. When creating a VM we need to apply some domain type specific limit.
In the case of an inactive domain, we have a virDomainPtr object and from that the internal drivers can determine the domain type or Xen, QEMU, KVM, etc, and again apply some domain type specific limit.
In the 3rd case of a running VM though, the hypervisor limit is typically not neccessarily the one that is relevant. While the hypervisor will still has its own limit of VCPUs, the effective limit is likely lower - ie fixed at the number of virtual CPUs which were allocated when the guest OS booted. So if you boot a guest with 4 cpus, you can hotplug remove & add CPUs, but you can never go above 4 until the guest is shutdown.
So anyway, I think we need to add 2 apis to address this whole issue:
- int virConnectGetMaxVcpus(virConnectPtr conn, const char *type);
Returns the maximum number of virtual CPUs supported for a guest VM of a specific type. Thje 'type' parameter here corresponds to the 'type' attribute in the <domain> element of the XML.
- int virDomainGetMaxVcpus(virDomainPtr dom);
Returns the maximum number of virual CPUs supported for the guest VM. If the guest is inactive, this is basically the same as virConnectGetMaxVcpus. If the guest is running this will reflect the maximum number of VCPus the guest was booted with.
In terms of the Xen implementation, the virConnectGetMaxVcpus method can simply return MAX_VIRT_VCPUS. Likewie virDomainGetMaxVcpus can do the same for inactive guests, but if the guest is running then we will need to call out to XenD to extract the max vcpus info.
Any other thoughts from people on the list ???
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 -=|
On Tue, Mar 06, 2007 at 08:08:57PM +0900, Masayuki Sunou wrote:
Hi, Dan
1. When initially creating a VM 2. When changing the config of an inactive VM 3. When changing the config of a running VM
Certainly, I had not considered concerning 1 and 2. So, I corrected the patch based on your proposal.
・ virDomainGetMaxVcpus Judge state (active/inactive) of the domain, and return information corresponding to each state.
・ virConnectGetMaxVcpus Add it as a method that returns the number of maximum CPUs defined by Xen.
However, I only added virConnectGetMaxVcpus because I did not understand the use image of it.
Is it added as a command of virsh? Or, is it used from virsh create and virsh start?
Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>
Looks good to me, the new function comment should be fixed to avoid the 'Thje' typo, and 'make rebuild' should be run in the doc subdir to regenerate documentations, but it's IMHO ready to be applied. Thanks a lot ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Hi Daniel I forgot the spelling check of comment. I contribute the patch that corrects the comment again. Moreever, because some return values were illegal, I corrected it. Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks In message <20070307220239.GI1782@redhat.com> "Re: [Libvir] [PATCH] check the maximum of virtual CPU" "Daniel Veillard <veillard@redhat.com>" wrote:
On Tue, Mar 06, 2007 at 08:08:57PM +0900, Masayuki Sunou wrote:
Hi, Dan
1. When initially creating a VM 2. When changing the config of an inactive VM 3. When changing the config of a running VM
Certainly, I had not considered concerning 1 and 2. So, I corrected the patch based on your proposal.
・ virDomainGetMaxVcpus Judge state (active/inactive) of the domain, and return information corresponding to each state.
・ virConnectGetMaxVcpus Add it as a method that returns the number of maximum CPUs defined by Xen.
However, I only added virConnectGetMaxVcpus because I did not understand the use image of it.
Is it added as a command of virsh? Or, is it used from virsh create and virsh start?
Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>
Looks good to me, the new function comment should be fixed to avoid the 'Thje' typo, and 'make rebuild' should be run in the doc subdir to regenerate documentations, but it's IMHO ready to be applied.
Thanks a lot !
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Thu, Mar 08, 2007 at 10:03:25AM +0900, Masayuki Sunou wrote:
Hi Daniel
I forgot the spelling check of comment. I contribute the patch that corrects the comment again.
Moreever, because some return values were illegal, I corrected it.
Excellent, applied and commited, thanks a lot ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (5)
-
Atsushi SAKAI -
Daniel P. Berrange -
Daniel Veillard -
Masayuki Sunou -
Richard W.M. Jones