Code review comment for lp:~raharper/curtin/trunk.fix-bcache-sysfs-write-failures-on-remove

Revision history for this message
Ryan Harper (raharper) wrote :

On Thu, Oct 12, 2017 at 2:56 PM, Scott Moser <email address hidden> wrote:

> Review: Needs Information
>
>
>
> Diff comments:
>
> > === modified file 'curtin/block/clear_holders.py'
> > --- curtin/block/clear_holders.py 2017-05-24 18:42:40 +0000
> > +++ curtin/block/clear_holders.py 2017-10-12 16:29:11 +0000
> > @@ -132,8 +132,13 @@
> > os.path.basename(bcache_cache_sysfs))
> > else:
> > LOG.info('stopping bcache cacheset at: %s', bcache_cache_sysfs)
> > - util.write_file(os.path.join(bcache_cache_sysfs, 'stop'),
> > - '1', mode=None)
> > + bcache_stop = os.path.join(bcache_cache_sysfs, 'stop')
> > + try:
> > + util.write_file(bcache_stop, '1', mode=None)
> > + except (IOError, OSError) as e:
> > + if e.errno == errno.ENOENT:
> > + LOG.debug('bcache stop file %s missing, device removed:
> %s',
> > + bcache_stop, e)
>
> dont you want:
> else:
> raise e
> ?
> or did you intend to be completely silent on failure except in the ENOENT
> case.
>

This is the only case where we want to eat the exception; the write to the
file
failed due to bcache kernel bits stopping. I'll add a comment to make it
more clear.

>
> > try:
> > util.wait_for_removal(bcache_cache_sysfs,
> retries=removal_retries)
> > except OSError:
>
>
> --
> https://code.launchpad.net/~raharper/curtin/trunk.fix-
> bcache-sysfs-write-failures-on-remove/+merge/330758
> You are the owner of lp:~raharper/curtin/trunk.fix-
> bcache-sysfs-write-failures-on-remove.
>

« Back to merge proposal