[Libguestfs] [PATCH v2 1/2] v2v: bootloaders: Handle no Bootloader::Tools default section (RHBZ#1472208).

[Pino Toscano]

On Friday, 28 July 2017 10:16:33 CEST Richard W.M. Jones wrote:
> 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.

Regression test is good when it tests actual stuff, not unrealistic
scenarios that (at least so far) haven't happened in real world yet.
Also, as mentioned in my previous replies, before applying any kind of
workaround, we need to check what is the actual problem at hand.

IOW, the v2v regression testing with those openSUSE templates is
partially flawed, because surely the content is not 100% what is going
to be in a real-world guest.

> 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.

I didn't say the patch is incorrect, but that is dead code. This adds
checks for a situation that so far will not ever happen in real guests,
but only in that very specific guest that virt-builder produces from
the openSUSE templates.  Just booting the guest once fixes the grub2
configuration into something standard, and even installing openSUSE
from the install media produces a working grub2 configuration.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170728/5c4e88cc/attachment.sig>