[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] setmem checks



On Fri, Jun 15, 2007 at 09:48:42AM -0400, Daniel Veillard wrote:
> On Fri, Jun 15, 2007 at 07:52:32AM -0400, Mark Johnson wrote:
> > On 6/15/07, Daniel Veillard <veillard redhat com> wrote:
> > >On Fri, Jun 15, 2007 at 08:59:50AM +0100, Richard W.M. Jones wrote:
> > >> Mark Johnson wrote:
> > >> >1673a1675,1677
> > >> >>    if (virDomainGetInfo(dom, &info) != 0) {
> > >> >>      info.maxMem = 0;
> > >> >>    }
> > >> >1675c1679
> > >> ><     if (kilobytes <= 0) {
> > >> >---
> > >> >>    if ((kilobytes <= 0)  || (kilobytes > info.maxMem)) {
> > >>
> > >> I don't understand this bit.  If virDomainGetInfo fails then it'll
> > >> always give an error because kilobytes > info.maxMem (== 0) ?
> > >
> > >  Agreed, we probably need to better handle the case where virDomainGetInfo
> > >fails, there was an actual scenario where we still wanted to try to set the
> > >memory anyway, but I can't remember. But the idea of the patch is fine...
> > >  I'm not too fond of
> > >    info.memory = 0x7fffffff;
> > >can we express that value like (((unsigned int) 1 << 31) -1 ) or a standard
> > >macro value for MAX_INT ? but that's cosmetic.
> > 
> > I agree that both of these are very ugly.  For both cases I didn't 
> > understand
> > how vshCommandOptDomain() would suceed and then virDomainGetInfo()
> > would fail.
> >
> > And if virDomainGetInfo*() would fail, would it be better to not
> > continue on, and
> > why virDomainSetMemory() would then pass if we did?
> 
>   I think it was releated to the case where hypervisor access would not
> work (non-root, no proxy), but doing the RPC to xend for setting up the
> amount of memory would actually work. It definitely was an edge case.
>   
> > It would have been better if I returned FALSE instead to forcing a
> > failure. I was just
> > doing that to get an error message out and not create yet another string 
> > which
> > would have to be localized. I'm happy to do what folks think is the right
> > thing to do here :-)
> 
>   Let's not be afraid of having strings to localize :-)

Yeah, localization is not a problem - there's a huge team of people in 
Fedora who get us a very quick turn around on translations.

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  -=| 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]