Created attachment 1286926 [details]
log.pvresize

The reproducer is:

  $ guestfish -N disk:10G pvcreate /dev/sda : pvresize-size /dev/sda 5G
  libguestfs: error: pvresize_size: /dev/sda: /dev/sda: Requested size 5.00 GiB is less than real size 10.00 GiB. Proceed?  [y/n]: [n]
    Physical Volume /dev/sda not resized.

Add -v -x to the command line to see the lvm2 commands being performed.

Actually the problem is not the kernel, but the lvm2 package.

WORKING => lvm2-2.02.166-1.el7_3.4.x86_64
 BROKEN => lvm2-2.02.171-4.el7.x86_64

Reproducible with lvm2-2.02.171-2.fc27.x86_64 (from Fedora Rawhide)
so I guess this is an upstream change.

Seems the upstream commit which changed this was:

  commit cbc69f8c693edf0d1307c9447e2e66d07a04bfe9
  Author: Alasdair G Kergon <agk>
  Date:   Thu Apr 27 02:36:34 2017 +0100

    pvresize: Prompt when non-default size supplied.
    
    Seek confirmation before changing the PV size to one that differs
    from the underlying block device.

Be nice if we didn't push breaking changes from less than two
months ago straight into RHEL.

I will add a workaround to libguestfs, but unfortunately it's too
late to get this into RHEL 7.4 GA.

See https://bugzilla.redhat.com/show_bug.cgi?id=1444130 for explanation. Especially Bug 1444130#c3.

I closed the original request as WONTFIX but was overridden - seems this is a frequently misused option, and ideally would not be used at all, except for testing.

But IMO as there is no risk if PV is smaller than physical device, creating smaller PV should not need a prompt.

lvm2 users should NOT use --setphysicalsize  in non-interactive scripts.

If user resizes underlying PV properly - then 'just' plain pvresize shall get the matching size right with auto-detection.

Anything that needs to override 'detected' defaults is going to need prompting, as we get far more problem from not prompting - compared with few brakes of basically broken scripts.

So my advice here is - to fix  libguestfs to NOT user --setphysicalsize - since this is purely admin command (just like you should mostly not use any --force inside scripts)

(In reply to Zdenek Kabelac from comment #6)
> lvm2 users should NOT use --setphysicalsize  in non-interactive scripts.
> [...]
> So my advice here is - to fix  libguestfs to NOT user --setphysicalsize -
> since this is purely admin command (just like you should mostly not use any
> --force inside scripts)

What are the script-friendly alternatives, then?

What is wrong  with plain 'pvresize  /dev/sda'  ?

Why is lvm2 not detecting proper size  ?

Why is passed differently sized device to lvm2 command ?

(In reply to Zdenek Kabelac from comment #8)
> What is wrong  with plain 'pvresize  /dev/sda'  ?
> 
> Why is lvm2 not detecting proper size  ?
> 
> Why is passed differently sized device to lvm2 command ?

The size can specified in case a particular size is wanted. And yes, there are uses cases for that.

https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=cbc69f8c693edf0d1307c9447e2e66d07a04bfe9

So the request is to revert this part of the patch in RHEL7.4, but to leave it upstream:

else if (new_size < size)
        if (!yes && yes_no_prompt("%s: Requested size %s is less than real size %s. Proceed?  [y/n]: ", pv_name, display_size(cmd, (uint64_t) new_size), display_size(cmd, (uint64_t) size)) == 'n')
                goto_out;

(In reply to Pino Toscano from comment #9)
> (In reply to Zdenek Kabelac from comment #8)
> > What is wrong  with plain 'pvresize  /dev/sda'  ?
> > 
> > Why is lvm2 not detecting proper size  ?
> > 
> > Why is passed differently sized device to lvm2 command ?
> 
> The size can specified in case a particular size is wanted. And yes, there
> are uses cases for that.

Such 'extra' use case is still 'supported' - but needs prompt confirmation as in most cases it's usually user's fault to use wrongly sized device - so the prompt got preference here - so user may realize if he really wants to use differently sized PV then the one lvm2 is really seeing in your system.

Note - there could be numerous problems with the missing physical space on device where lvm2 was the taken 'responsible' for letting happen this problem.

Please reconsider your tool workflow if you really need to pass in 'wrongly' sized device to lvm2 command.

(In reply to Zdenek Kabelac from comment #11)
> (In reply to Pino Toscano from comment #9)
> > (In reply to Zdenek Kabelac from comment #8)
> > > What is wrong  with plain 'pvresize  /dev/sda'  ?
> > > 
> > > Why is lvm2 not detecting proper size  ?
> > > 
> > > Why is passed differently sized device to lvm2 command ?
> > 
> > The size can specified in case a particular size is wanted. And yes, there
> > are uses cases for that.
> 
> Such 'extra' use case is still 'supported' - but needs prompt confirmation
> as in most cases it's usually user's fault to use wrongly sized device - so
> the prompt got preference here - so user may realize if he really wants to
> use differently sized PV then the one lvm2 is really seeing in your system.

Sure, hence my question above about the proper way to make *this* user case work in scripted scenarios, where this situation is known, and already taken care of.

The assumption has been that people would always resize the device first and THEN run pvresize to pick up the new size.

But if you're not sure that the part of the device being removed is completely unused (not in complete control of allocation and worried about something allocating in parallel to your resizing operations) then you might make the case that it's safer to do a 3-step process: pvresize explicitly to a size known to be smaller than the final size you want, reduce the real device size, run pvresize again (without --setphysicalvolumesize) to adjust back up to the actual size.

We do support generic  '--yes'   to answer Y to all prompts.

But this is kind of  'blank check' for anything 'strange' and prompted during cmdline processing.


So I'd most probably highly recommend to pass correctly sized device in the first place - so it's validated during all steps properly and there is minimized chance of an unexpected error.


So please reconsider the model workflow if you can pass correctly sized device when you run pvresize.

(In reply to Zdenek Kabelac from comment #14)
> We do support generic  '--yes'   to answer Y to all prompts.

And that's indeed what is going to be used soon.

> But this is kind of  'blank check' for anything 'strange' and prompted
> during cmdline processing.

Well, too bad (not for us, though).

> So please reconsider the model workflow if you can pass correctly sized
> device when you run pvresize.

There's a misunderstanding on your side about what we provide in libguestfs:
1) a 'pvresize' API, which just calls `pvresize` on the specified device
2) a 'pvresize_size' API, which allows to specify a size, and thus `pvresize --setphysicalvolumesize SIZE` is called

In both the cases, those are *user* APIs, i.e. the user invokes to do all the operation they need. Nothing directly in libguestfs itself uses it, and only "pvresize" is used in virt-resize to resize a PV.

As a consequence of the above: this is totally in control of the user, and thus the (broken record) suggestion "fix your workflow" does not make any sense, and would be nice if you could avoid mentioning it once again, thanks.

Providing  dangerous operation in API without any user confirmation is almost on pair  with running  i.e. 'mkfs' over existing formatted device and not asking for confirmation.


Clearly there are cases where a user may need to do it and pvresize the disk to a size mismatching the size reported by kernel.   But there is way way more users how actually do not want this to happen (often doing it in 'virtual' machines and passing wrongly sized device to pvresize and expecting things to work - since  'pvresize' has passed....)


So which API user does need to have it supported out-of-the box ?
The command really should require confirmation even from API user.

The intention lvm2 has is obviously totally defeated once user will convert every pvresize call to use  --yes    (it's even worse since more things will get 'green light' and some other suspicious actions  will go on without prompt)

Maybe something like:  --setphysicalsizedifferentfromkernel  might be the answer - so if user really really knows what is going on  - it might be some way.

Such option would likely need to check for different  size - and only in this case apply the size (otherwise again script writers would 'blindly'  convert existing option to more powerful one....

libguestfs provides APIs for creating and manipulating disk
images.  These APIs can be called from (eg) C programs, and many
other situations where there is no "user" to talk to.  And yes
we do very much provide a 'mkfs' operation which works without
prompting -- that is in fact the entire point.

Anyway, the issue here is that this change was made at the last
minute.  Upstream 6 weeks ago.  In RHEL 7.4, 5 weeks ago.

The change is still not in Fedora 26, but we're pushing it to
our customers anyway.

So please can you (in future) not make large changes like this
rebase long after the development phase of RHEL and before it has
had any significant testing anywhere.

I have (in bug 1460692) added the --yes flag to our invocation
of pvresize.

For 'scripts' running in  I-don't-care  mode  using  '--yes'  is clearly the way to go - as with ANY other lvm2 command.

Prompts are added regularly when some operation is seen for a regular user a dangerous one.

Although for normal 'scripts' - they should be living for default 'no' answer for non-interactive runs.

This brings however new 'light' -  lvm2 should consider storing such 'bypass' flags within it's physical on-disk metadata so it's known for analysis there has been such 'override' action provided.

(In reply to Pino Toscano from comment #15)

> 2) a 'pvresize_size' API, which allows to specify a size, and thus `pvresize
> --setphysicalvolumesize SIZE` is called

So what guidance is provided to people using this part of the libguestfs API to make sure they don't make the same mistakes that command line users make?  We have deliberately made it harder to use to try to stop people who don't need it from using it without thinking properly about it.  If they look, even with --yes, presumably API users will see our new warning message appearing in their output when there's a size mismatch.

log_warn("WARNING: %s: Pretending size is %" PRIu64 " not %" PRIu64 " sectors.", pv_name, new_size, size);

(In reply to Alasdair Kergon from comment #19)
> (In reply to Pino Toscano from comment #15)
> 
> > 2) a 'pvresize_size' API, which allows to specify a size, and thus `pvresize
> > --setphysicalvolumesize SIZE` is called
> 
> So what guidance is provided to people using this part of the libguestfs API
> to make sure they don't make the same mistakes that command line users make?

To unask the question, libguestfs is mostly used in situations where
you're automatically creating disk images, and in that case the program
has calculated the right size before calling pvresize_size.  If the
program calculates the wrong size then it has a bug.

I was going to say this API is only used by virt-resize.  In fact that
isn't true: it's only used (by our code) in a test script used to test
virt-resize ...

Considering that the '--yes' option is available, there is nothing more to be done.

In principal, changing CLI behavior in the middle of a release should be avoided.  This is a goal (and might have been possible in this case), but some situations that warrant it may arise.  In those cases, upstream "soak" time is recommended.

Makes me wondering if introducing  maybe  'lvm.conf' option with some sort of fine-grain 'yes'  would make this better.

i.e.   yes_pv_resize_to_bigger_size = 1/0