Re: [Libguestfs] [PATCH v2 01/17] v2v: debug gc via at_exit hook
On Thu, Aug 27, 2015 at 08:12:11PM +0300, Roman Kagan wrote:
On Thu, Aug 27, 2015 at 03:50:11PM +0100, Richard W.M. Jones wrote:
> On Tue, Aug 11, 2015 at 08:00:20PM +0300, Roman Kagan wrote:
> > debub_gc (coming from the command line) indicates that gc should be
> > forced on program exit. Instead of sticking it on every exit path,
> > register it as an at_exit hook once.
>
> Was this change necessary as part of this patch series?
I think so.
The goal of the refactoring part of the series was to reduce the amount
of detail in main(), and only leave coarse steps, making it easy to see
the whole scenario.
Originally every exit path had a clause
if debug_gc then
Gc.compact ()
There were two of them, and I was about to add another one.
OK - I see, although only every non-error exit path. But adding an
atexit handler ought to be safe, since the garbage collector should
never be in an inconsistent state, and also the GC should never itself
call exit.
> The --debug-gc option is used across most of the virt-* tools in
the
> internal tests, and those tests shouldn't exit with an error when the
> option is used.
This patch doesn't affect any other virt-* tool. Also I'm failing to
see why it would cause tests to fail: it doesn't change the behavior on
regular exit paths. Needless to say that I run (the relevant subset of)
the tests and they passed.
I don't mean your patch would cause tests to fail, I mean the tests
should only use --debug-gc when the virt-* tool is expected to use a
non-failing path. Anyway this is going down a rabbit hole of detail
that doesn't matter for the virt-v2v --in-place patch series.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v