Libguestfs - Aug 2016 - Re: [PATCH 1/2] v2v: Make fstrim warning clearer (RHBZ#1366456).

On Fri, Aug 12, 2016 at 04:50:31PM +0200, Pino Toscano
wrote:
> On Friday, 12 August 2016 10:37:29 CEST Richard W.M. Jones wrote:
> > This reverts the change made for RHBZ#1168144.  The warning is now
> > always displayed.
> > 
> > It would be nice to make the warning actionable, but there is not a
> > lot that end users can do since fstrim is such a complex topic
> > interacting with all filesystem and storage layers.
> > ---
> >  v2v/v2v.ml | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> > index e221f29..8365aae 100644
> > --- a/v2v/v2v.ml
> > +++ b/v2v/v2v.ml
> > @@ -397,11 +397,7 @@ and do_fstrim g inspect > >        if
mounted then (
> >          try g#fstrim "/"
> >          with G.Error msg ->
> > -          (* Only emit this warning when debugging, because otherwise
> > -           * it causes distress (RHBZ#1168144).
> > -           *)
> > -          if verbose () then
> > -            warning (f_"%s (ignored)") msg
> > +          warning (f_"fstrim on guest filesystem %s failed. 
This may mean that conversion takes longer than normal.  Usually you can ignore
this as fstrim is just an optimization.  Not all filesystems support trimming,
and it can depend on specific details of the filesystem, alignment and backing
storage.  Original message: %s") dev msg
> >        )
> 
> The idea is OK, but the message a bit too long perhaps? I'd keep the
> warning short here, and add the longer bit in an own paragraph in the
> manual.
I despair that no one reads the manual ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Friday, 12 August 2016 22:19:09 CEST Richard W.M. Jones
wrote:
> On Fri, Aug 12, 2016 at 04:50:31PM +0200, Pino Toscano wrote:
> > On Friday, 12 August 2016 10:37:29 CEST Richard W.M. Jones wrote:
> > > This reverts the change made for RHBZ#1168144.  The warning is
now
> > > always displayed.
> > > 
> > > It would be nice to make the warning actionable, but there is not
a
> > > lot that end users can do since fstrim is such a complex topic
> > > interacting with all filesystem and storage layers.
> > > ---
> > >  v2v/v2v.ml | 6 +-----
> > >  1 file changed, 1 insertion(+), 5 deletions(-)
> > > 
> > > diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> > > index e221f29..8365aae 100644
> > > --- a/v2v/v2v.ml
> > > +++ b/v2v/v2v.ml
> > > @@ -397,11 +397,7 @@ and do_fstrim g inspect > > >      
if mounted then (
> > >          try g#fstrim "/"
> > >          with G.Error msg ->
> > > -          (* Only emit this warning when debugging, because
otherwise
> > > -           * it causes distress (RHBZ#1168144).
> > > -           *)
> > > -          if verbose () then
> > > -            warning (f_"%s (ignored)") msg
> > > +          warning (f_"fstrim on guest filesystem %s failed.
This may mean that conversion takes longer than normal.  Usually you can ignore
this as fstrim is just an optimization.  Not all filesystems support trimming,
and it can depend on specific details of the filesystem, alignment and backing
storage.  Original message: %s") dev msg
> > >        )
> > 
> > The idea is OK, but the message a bit too long perhaps? I'd keep
the
> > warning short here, and add the longer bit in an own paragraph in the
> > manual.
> 
> I despair that no one reads the manual ...
Sure, but we can't just shove in the face long explanations to the
users, as they will produce a "TL;DR" effect making them ignored
likewise.  Since we have the manual, it makes sense to keep the long
explanations there.

-- 
Pino Toscano

On Mon, Aug 15, 2016 at 03:28:12PM +0200, Pino Toscano
wrote:
> On Friday, 12 August 2016 22:19:09 CEST Richard W.M. Jones wrote:
> > On Fri, Aug 12, 2016 at 04:50:31PM +0200, Pino Toscano wrote:
> > > On Friday, 12 August 2016 10:37:29 CEST Richard W.M. Jones wrote:
> > > > This reverts the change made for RHBZ#1168144.  The warning
is now
> > > > always displayed.
> > > > 
> > > > It would be nice to make the warning actionable, but there
is not a
> > > > lot that end users can do since fstrim is such a complex
topic
> > > > interacting with all filesystem and storage layers.
> > > > ---
> > > >  v2v/v2v.ml | 6 +-----
> > > >  1 file changed, 1 insertion(+), 5 deletions(-)
> > > > 
> > > > diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> > > > index e221f29..8365aae 100644
> > > > --- a/v2v/v2v.ml
> > > > +++ b/v2v/v2v.ml
> > > > @@ -397,11 +397,7 @@ and do_fstrim g inspect > > >
>        if mounted then (
> > > >          try g#fstrim "/"
> > > >          with G.Error msg ->
> > > > -          (* Only emit this warning when debugging, because
otherwise
> > > > -           * it causes distress (RHBZ#1168144).
> > > > -           *)
> > > > -          if verbose () then
> > > > -            warning (f_"%s (ignored)") msg
> > > > +          warning (f_"fstrim on guest filesystem %s
failed.  This may mean that conversion takes longer than normal.  Usually you
can ignore this as fstrim is just an optimization.  Not all filesystems support
trimming, and it can depend on specific details of the filesystem, alignment and
backing storage.  Original message: %s") dev msg
> > > >        )
> > > 
> > > The idea is OK, but the message a bit too long perhaps? I'd
keep the
> > > warning short here, and add the longer bit in an own paragraph in
the
> > > manual.
> > 
> > I despair that no one reads the manual ...
> 
> Sure, but we can't just shove in the face long explanations to the
> users, as they will produce a "TL;DR" effect making them ignored
> likewise.  Since we have the manual, it makes sense to keep the long
> explanations there.
I pushed most of the explanation into the manual, see final commit
here:

https://github.com/libguestfs/libguestfs/commit/66816e11332b07a07efd6821e7ecf090691ce441

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top