Re: [libvirt] [PATCH] add the check of memory size in xenXMDriver forsetmem of Xen domain

2008-10-31 Thread Atsushi SAKAI 
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 [EMAIL PROTECTED] 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 -  1.95
 +++ src/xm_internal.c 31 Oct 2008 05:52:27 -
 @@ -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 [EMAIL PROTECTED] 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


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] add the check of memory size in xenXMDriver forsetmem of Xen domain

2008-10-31 Thread Daniel P. Berrange 
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 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] add the check of memory size in xenXMDriver forsetmem of Xen domain

2008-10-31 Thread Daniel Veillard 
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/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list