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