[Libvir] [PATCH] Guard a negative value of "virsh setmem" and "virsh setmexmem"
Hi "virsh setmem" and "virsh setmexmem" have the problem of setting a very big value in the domain when -1 is set in the domain. As a result, I detected the problem of doing abnormal operation of Xen in 64bit machine. (virsh setmem) Therefore, I contribute the patch that corrects the argument of the function to guard a negative value. This treatment is based on setMemoryTarget@XendDomainInfo.py. Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. ------------------------------------------------------------------------------- Index: include/libvirt/libvirt.h =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h,v retrieving revision 1.41 diff -u -p -r1.41 libvirt.h --- include/libvirt/libvirt.h 15 Mar 2007 17:24:57 -0000 1.41 +++ include/libvirt/libvirt.h 16 Mar 2007 01:37:25 -0000 @@ -314,9 +314,9 @@ int virDomainGetUUIDString (virDomainP char * virDomainGetOSType (virDomainPtr domain); unsigned long virDomainGetMaxMemory (virDomainPtr domain); int virDomainSetMaxMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainSetMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainGetMaxVcpus (virDomainPtr domain); /* Index: include/libvirt/libvirt.h.in =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h.in,v retrieving revision 1.26 diff -u -p -r1.26 libvirt.h.in --- include/libvirt/libvirt.h.in 15 Mar 2007 17:24:57 -0000 1.26 +++ include/libvirt/libvirt.h.in 16 Mar 2007 01:37:26 -0000 @@ -314,9 +314,9 @@ int virDomainGetUUIDString (virDomainP char * virDomainGetOSType (virDomainPtr domain); unsigned long virDomainGetMaxMemory (virDomainPtr domain); int virDomainSetMaxMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainSetMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainGetMaxVcpus (virDomainPtr domain); /* Index: src/libvirt.c =================================================================== RCS file: /data/cvs/libvirt/src/libvirt.c,v retrieving revision 1.63 diff -u -p -r1.63 libvirt.c --- src/libvirt.c 15 Mar 2007 17:24:57 -0000 1.63 +++ src/libvirt.c 16 Mar 2007 01:37:31 -0000 @@ -1499,7 +1499,7 @@ virDomainGetMaxMemory(virDomainPtr domai * Returns 0 in case of success and -1 in case of failure. */ int -virDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) +virDomainSetMaxMemory(virDomainPtr domain, int memory) { int ret = -1 , i; virConnectPtr conn; @@ -1553,7 +1553,7 @@ virDomainSetMaxMemory(virDomainPtr domai * Returns 0 in case of success and -1 in case of failure. */ int -virDomainSetMemory(virDomainPtr domain, unsigned long memory) +virDomainSetMemory(virDomainPtr domain, int memory) { int ret = -1 , i; virConnectPtr conn; -------------------------------------------------------------------------------
On Fri, Mar 16, 2007 at 12:26:18PM +0900, Masayuki Sunou wrote:
Hi
"virsh setmem" and "virsh setmexmem" have the problem of setting a very big value in the domain when -1 is set in the domain.
As a result, I detected the problem of doing abnormal operation of Xen in 64bit machine. (virsh setmem)
Therefore, I contribute the patch that corrects the argument of the function to guard a negative value. [...] unsigned long virDomainGetMaxMemory (virDomainPtr domain); int virDomainSetMaxMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainSetMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainGetMaxVcpus (virDomainPtr domain);
Sorry you really cannot do that: - it breaks the API (by changing a public interface) - it breaks the ABI (by changing the parameter size on 64bit boxes) - and it makes Set and Get routines inconsistent. This kind of errors must be caught when calling the API, if one casts from a signed to the unsigned long a check must be done at that point for negative value by the caller. I'm unsure in what environment you got the problem, but that's not the right way to fix it :-) 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
This kind of errors must be caught when calling the API, if one casts from a signed to the unsigned long a check must be done at that point for negative value by the caller. I'm unsure in what environment you got the problem, but that's not the right way to fix it :-)
This patch intends to add a check of the "bytes" value on virsh. Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. ------------------------------------------------------------------------------- Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.64 diff -u -p -r1.64 virsh.c --- src/virsh.c 15 Mar 2007 17:24:57 -0000 1.64 +++ src/virsh.c 19 Mar 2007 00:14:12 -0000 @@ -1459,7 +1459,7 @@ cmdSetmem(vshControl * ctl, vshCmd * cmd return FALSE; bytes = vshCommandOptInt(cmd, "bytes", &bytes); - if (!bytes) { + if (bytes <= 0) { virDomainFree(dom); return FALSE; } @@ -1502,7 +1502,7 @@ cmdSetmaxmem(vshControl * ctl, vshCmd * return FALSE; bytes = vshCommandOptInt(cmd, "bytes", &bytes); - if (!bytes) { + if (bytes <= 0) { virDomainFree(dom); return FALSE; } ------------------------------------------------------------------------------- In message <20070316103843.GP3127@redhat.com> "Re: [Libvir] [PATCH] Guard a negative value of "virsh setmem" and "virsh setmexmem"" "Daniel Veillard <veillard@redhat.com>" wrote:
On Fri, Mar 16, 2007 at 12:26:18PM +0900, Masayuki Sunou wrote:
Hi
"virsh setmem" and "virsh setmexmem" have the problem of setting a very big value in the domain when -1 is set in the domain.
As a result, I detected the problem of doing abnormal operation of Xen in 64bit machine. (virsh setmem)
Therefore, I contribute the patch that corrects the argument of the function to guard a negative value. [...] unsigned long virDomainGetMaxMemory (virDomainPtr domain); int virDomainSetMaxMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainSetMemory (virDomainPtr domain, - unsigned long memory); + int memory); int virDomainGetMaxVcpus (virDomainPtr domain);
Sorry you really cannot do that: - it breaks the API (by changing a public interface) - it breaks the ABI (by changing the parameter size on 64bit boxes) - and it makes Set and Get routines inconsistent.
This kind of errors must be caught when calling the API, if one casts from a signed to the unsigned long a check must be done at that point for negative value by the caller. I'm unsure in what environment you got the problem, but that's not the right way to fix it :-)
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 Mon, Mar 19, 2007 at 09:48:03AM +0900, Masayuki Sunou wrote:
Hi Daniel
This kind of errors must be caught when calling the API, if one casts from a signed to the unsigned long a check must be done at that point for negative value by the caller. I'm unsure in what environment you got the problem, but that's not the right way to fix it :-)
This patch intends to add a check of the "bytes" value on virsh.
Right, this makes sense, we need to add the check too in the python accessor but that's generated code (libvirt_virDomainSetMemory in libvirt-py.c) so I will keep this as a TODO, until we desactivate the automatic bindings generation (or I find a good way to automatically patch the generated bindings when the generator is run). Applied and commited to CVS, 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 (2)
-
Daniel Veillard -
Masayuki Sunou