Re: [Libvir] [PATCH] setmem checks
On 6/15/07, Daniel Veillard <veillard(a)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?
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 :-)
Mark.