Re: [Libvir] [patch 1/7] Merge the active and inactive network/guest lists
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 :-)
net = vm->def->nets; @@ -867,6 +846,7 @@ int qemudShutdownVMDaemon(struct qemud_s
vm->pid = -1; vm->id = -1; + vm->active = 0;
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 -=|
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.
participants (2)
-
Daniel P. Berrange -
Mark McLoughlin