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

[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

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

[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

[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