On Tue, Jul 28, 2015 at 02:51:39PM +0300, Roman Kagan wrote: > First off, thanks for the review. IIUC you are not opposed to the idea > of adding the in-place operation mode in general, are you? I'll work > through your comments to make the patch fit better with the rest of the > code.
I'd definitely want to see the final patch, but I'm not opposed in principle. > On Tue, Jul 28, 2015 at 11:54:59AM +0100, Richard W.M. Jones wrote: > > On Mon, Jul 27, 2015 at 07:29:57PM +0300, Roman Kagan wrote: > > > --- a/tests/guests/guests.xml.in > > > +++ b/tests/guests/guests.xml.in > > > > Let's not add this to tests/guests, since this disk image is only used > > for a single test in the v2v/ subdirectory. > > > > You can change the new test in the v2v/ subdirectory so it creates (or > > is supplied with) the XML. There are various tests which work like > > that already - eg: v2v/test-v2v-cdrom.* > > OK, makes sense, thanks for the suggestion. > > While at this: do you think adding the test for the newly added option > should go in the same commit with the option itself? It may be easier > to review separately, but YMMV, so please let me know which way you'd > like it better. Either way you like. Sometimes we add tests as separate commits, and sometimes in the same commit. The only hard rule for commits is: don't break bisection. 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 _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
