[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH v3 0/3] v2v: simplify driver copying from virtio-win iso



On Thu, Oct 15, 2015 at 10:40:15AM +0100, Richard W.M. Jones wrote:
> On Thu, Oct 15, 2015 at 12:27:58PM +0300, Roman Kagan wrote:
> > On Thu, Oct 15, 2015 at 10:07:45AM +0100, Richard W.M. Jones wrote:
> > > On Wed, Oct 14, 2015 at 07:55:41PM +0300, Roman Kagan wrote:
> > > > Libguestfs supports passing an ISO image as a source of virtio windows
> > > > drivers to v2v.
> > > > 
> > > > This series attempts to make it simpler and better scoped.
> > > 
> > > Looks good (apart from patch 3 - the Gc.compact is necessary!)
> > 
> > Why?  As a matter of fact that was the primary reason I even started
> > with this patchset: that Gc.compact was sitting in the middle of the
> > main v2v function and was yet another obstacle in the refactoring I was
> > after.
> > 
> > Now that the extra guestfs handle doesn't survive beyond
> > copy_virtio_drivers() what is the need for that explicit GC (which was
> > introduced at the time exactly to clear up that handle in commit
> > 47b5f245bec908f803f0a89c3b1e3166cfe33aad)?
> 
> Just because the handle becomes unreachable doesn't mean the GC will
> immediately collect it.  Objects are only required to be finalized
> when you invoke Gc.full_major (Gc.compact calls Gc.full_major).
> 
> Previous to these patches, the handle was still unreachable, but
> Gc.compact didn't free it because of a different problem.  That
> different issue has been fixed by:
> 
> https://github.com/libguestfs/libguestfs/commit/8bbc5e73cb5b56b5cfbe979ac0e1c14d1701a0d8
> 
> In summary, we still need to call Gc.compact before doing the
> long-running copy operation, to make sure that the resources used by
> virt-v2v are minimized during the copy.

What resources are you talking about?  The only costly resource eater is
the auxiliary hypervisor backend instance; it used to be terminated
indirectly via this forced GC here (after your fix), but now it's shut
down explictly before leaving copy_virtio_drivers().  Anything else that
can get collected here is negligible compared to what remains.

Roman.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]