[libvirt] [Q]Why does libvirt check size of memory in xenStoreDriver?
Hi, I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c? In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated. -------------------- # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory # echo $? 0 -------------------- As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c. Thanks, Shigeki Sakamoto.
Hi, Shigeki At first, libvirt.c should not check VMM-arch specific value. In this meaning, xs_internal.c do the right thing. The problem is error handling on libvirt.c(Xen Store error is not handled properly). Do you investigate it? Thanks Atsushi SAKAI "S.Sakamoto" <fj0588di@aa.jp.fujitsu.com> wrote:
Hi,
I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c?
In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated.
-------------------- # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory
# echo $? 0 --------------------
As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c.
Thanks, Shigeki Sakamoto.
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi, I make the patch. This patch adds the check of memory size in xenXMDriver, to handle the xenStoreDriver error when the set value by setmem(setmaxmem) is 4096-65535 definitely. Thanks, Shigeki Sakamoto. Index: src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.95 diff -u -p -r1.95 xm_internal.c --- src/xm_internal.c 24 Oct 2008 11:20:08 -0000 1.95 +++ src/xm_internal.c 31 Oct 2008 05:52:27 -0000 @@ -1273,6 +1273,8 @@ int xenXMDomainSetMemory(virDomainPtr do return (-1); if (domain->id != -1) return (-1); + if (memory < 1024 * MIN_XEN_GUEST_SIZE); + return (-1); if (!(filename = virHashLookup(nameConfigMap, domain->name))) return (-1);
At first, libvirt.c should not check VMM-arch specific value. In this meaning, xs_internal.c do the right thing. The problem is error handling on libvirt.c(Xen Store error is not handled properly). Do you investigate it?
Thanks Atsushi SAKAI
"S.Sakamoto" <fj0588di@aa.jp.fujitsu.com> wrote:
Hi,
I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c?
In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated.
-------------------- # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory
# echo $? 0 --------------------
As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c.
Thanks, Shigeki Sakamoto.
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi, Daniel, This patch itself is OK. But I also think MIN_XEN_GUEST_SIZE should be defined in xen_unified.h not src/internal.h. Since this variable is Xen specific. How do you think? Thanks Atsushi SAKAI "S.Sakamoto" <fj0588di@aa.jp.fujitsu.com> wrote:
Hi,
I make the patch. This patch adds the check of memory size in xenXMDriver, to handle the xenStoreDriver error when the set value by setmem(setmaxmem) is 4096-65535 definitely.
Thanks, Shigeki Sakamoto.
Index: src/xm_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xm_internal.c,v retrieving revision 1.95 diff -u -p -r1.95 xm_internal.c --- src/xm_internal.c 24 Oct 2008 11:20:08 -0000 1.95 +++ src/xm_internal.c 31 Oct 2008 05:52:27 -0000 @@ -1273,6 +1273,8 @@ int xenXMDomainSetMemory(virDomainPtr do return (-1); if (domain->id != -1) return (-1); + if (memory < 1024 * MIN_XEN_GUEST_SIZE); + return (-1);
if (!(filename = virHashLookup(nameConfigMap, domain->name))) return (-1);
At first, libvirt.c should not check VMM-arch specific value. In this meaning, xs_internal.c do the right thing. The problem is error handling on libvirt.c(Xen Store error is not handled properly). Do you investigate it?
Thanks Atsushi SAKAI
"S.Sakamoto" <fj0588di@aa.jp.fujitsu.com> wrote:
Hi,
I have a question. Why does libvirt check size of memory in xenStoreDomainSetMemory of xs_internal.c?
In the xen not exclude lifecycle, when I do the following command for the domain that is shutoff, error message is output and return value is 0 and the definition file(=/etc/xen/XXX) is updated.
-------------------- # virsh setmem(or setmaxmem) guestdom 65535(4096-65535) libvir: Xen Store error : invalid argument in xenStoreDomainSetMemory
# echo $? 0 --------------------
As a result that I research this, this error message is output by the check of the memory size in xenStoreDriver. Because libvirt checks the memory size of under 4096 in libvirt.c, I think that libvirt should check memory size range of 4096-65535 in libvirt.c.
Thanks, Shigeki Sakamoto.
-- 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
On Fri, Oct 31, 2008 at 05:34:24PM +0900, Atsushi SAKAI wrote:
Hi, Daniel,
This patch itself is OK.
But I also think MIN_XEN_GUEST_SIZE should be defined in xen_unified.h not src/internal.h. Since this variable is Xen specific.
Yes, I believe one of my patches from yesterday moves this. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Oct 31, 2008 at 05:34:24PM +0900, Atsushi SAKAI wrote:
Hi, Daniel,
This patch itself is OK.
Yes except for the extra ; after the if making it return -1 all the time, but once fixed sure :-) Patch applied and commited [...]
+ if (memory < 1024 * MIN_XEN_GUEST_SIZE);
if (memory < 1024 * MIN_XEN_GUEST_SIZE)
+ return (-1);
Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Atsushi SAKAI -
Daniel P. Berrange -
Daniel Veillard -
S.Sakamoto