[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#687160: patch review

hey Dmitrijs!
 I prepared a similar fix for this a couple of weeks ago that has been
in testing at a customer site. Looks like we crossed paths, but our
fixes are very similar. My patch has been working well for my test
case, and I've verified yours does too. However, I've a question on
your patch by way of review.. and selfishly to find out if I've an
issue in my implementation.

Is the vgdisplay bit known to be necessary, or is it just
precautionary? I wouldn't think that vgdisplay did anything more than
vgs with respect to scanning - if that is true, it should should make
the vgdisplay redundant with the vgs call in this call path:

  -> pv_on_device()
    -> pvs())

If you can dispense w/ the vgdisplay, I'd propose moving the
update-dev and device_remove_lvm calls into the preceding loop instead
of adding a new one, just to improve readability.

Reply to: