On Fri, Jul 28, 2017 at 10:02:34AM +0200, Pino Toscano wrote:
On Tuesday, 18 July 2017 19:59:20 CEST Richard W.M. Jones wrote:
> In SUSE guests, handle the case where
> Bootloader::Tools::GetDefaultSection () returns undef.
>
> Previously this would return an empty string and cause a bogus error
> in subsequent code:
>
> virt-v2v: error: libguestfs error: statns: statns_stub: path must start
> with a / character
> ---
I saw this was pushed, even though v1 was NACKed by me, and there was
no further information on actual guests (i.e. real world, not the one
that comes out of the virt-builder templates) affected by this.
The pushed patch is basically dead code, and I don't see why the urge
to have it regardless.
Furthermore, I don't see the logic in have me re-NACK every new version
of a patch series that I NACKed, while the new series is merely a
repost of the old one, or in general the reasons of the NACK did not
change at all.
I disagree that this is dead code. Two commits add a regression test
of v2v on the opensuse guests (as we already have for Fedora and other
guests), and one of those commits is this patch which is needed for
that regression test to work. Regression tests are a good thing -
they run on every release (as indeed I am running them right now) and
ensure that other aspects of the code don't regress because of some
unintentional change.
Also I don't think the patch is incorrect, even if it is only
applicable for certain types of "just installed / never booted" guests
that we probably wouldn't see outside of the regression testing.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/