Re: [Libvir] [patch 1/7] Merge the active and inactive network/guest lists
On Tue, 2007-02-20 at 19:40 +0000, Daniel P. Berrange wrote:
On Tue, Feb 20, 2007 at 07:26:03PM +0000, Mark McLoughlin wrote:
> Merge the ->activevms and ->inactivevms into a single
> ->vms list where each VM has a "active" flag in order
> to make things easier to manage.
I'm sure I had some interesting reason for keeping the lists separate
when i wrote this. Damned if I can think of what it was now :-) The
patch does seem to make things simpler. Having an explicit 'active'
flag though is redundant. Simply check for 'vm->id == -1' or equivalently
the 'vm->pid == -1'. We don't need a 3rd flag to mark inactivity :-)
Dunno, I much prefer the active flag purely for readability.
e.g.
while (vm) {
if (vm->id != -1)
shutdown(vm);
vm = vm->next;
}
would make me carefully check whether ->id means active the first time
I see it and
while (vm) {
if (vm->pid != -1)
shutdown(vm);
vm = vm->next;
}
would make me check *every* time I see it, whereas I'd never feel the
need to verify this:
while (vm) {
if (vm->active)
shutdown(vm);
vm = vm->next;
}
i.e. several times already when tracking down bugs I've felt suspicious
about whether ((id != -1) != active) in places ...
Perhaps we just need a macro or static inline:
static inline
qemudIsActiveVM(struct qemud_vm *vm)
{
return vm->id != -1;
}
Cheers,
Mark.