Re: [patch 1/7] Merge the active and inactive network/guest lists

[Mark McLoughlin <markmc@xxxxxxxxxx>]

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.