Code review comment for lp:~nuclearbob/utah/vm-teardown-fix

Revision history for this message
Javier Collado (javier.collado) wrote :

@Max

Thanks for the update.

Looking at the code, something I find a little bit weird is that activecheck
and _start call themselves recursively and there isn't a clear base case defined
in the code to stop the recursion.

What I mean is that if lv.lookupByName misbehaves and returns None, then _load
will be useless and activecheck/_start will call themselves indefinitely until
a stack overflow exception is raised.

Aside from that, I've seen that lv.lookupByName might raise libvirtError and
indeed there's a try/except block to detect that when calling _load from _provision.
Shouldn't activecheck and _start take that into account as well?

review: Needs Information

« Back to merge proposal