Bug 1070531

Still an issue upstream.

Issue isn't specific to just raw block devices: for example qcow2_truncate (the function that block_resize maps to) has several error_report calls with fine grained error messages, but those never hit the qmp monitor and instead you'll see a similarly generic error.

I've posted a patch upstream now which ensures that error_report messages will at least hit stderr, since right now if the user is interacting with qmp, the messages are just dropped. Minor improvement, but not what's requested here.

I believe the proper solution is to change bdrv_truncate to take an Error **errp from callers... however changing that in all the drivers kinda sucks and sets an annoying precedent for whenever we want finer grained error info from deep down the call stack.

I commented in another bug[2] that perhaps we could do away with most of the explicit Error passing and instead use a global 'last reported error' tracked with thread local storage, similar to what libvirt uses. That would potentially allow us to just replace error_report calls with proper qerror_report invocations when we want a message propagated up to qmp. I'm not too familiar with the qemu code though so maybe there are reasons that won't work, but I'm going to see if I can get a POC working.

[1] http://marc.info/?l=qemu-devel&m=139457994925605&w=2
[2] https://bugzilla.redhat.com/show_bug.cgi?id=885099#c10

As mentioned in that other bug, qerror_report more or less does what I suggested, but it's not the preferred API these days. Sounds like passing Error up the call stack is the ideal solution

Not upstream and not urgent, so 7.2 material at the least.

But this is a fairly minor error reporting issue IMO. Certainly worth fixing upstream at some point, but I don't think it's worth a backport, figuring that every bdrv_truncate implementation will need to be touched which is probably 10 files or so.

I'm not really keyed into the RHEL process, so resetting assignee

This bug was filed against qemu-kvm, but comment#0 gives a qemu-kvm-rhev version.  Does the bug reproduce with current qemu-kvm?  What about current qemu-kvm-rhev?

Hi Xiangchun,

Could you reply comment10?

Best Regards,
Junyi

1)For scsi_debug disk as data disk.

Both qemu-kvm-rhev-2.6.0-15 and qemu-kvm-1.5.3-118.el7 still can be reproduced.

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 10737418240 }}
{"error": {"class": "GenericError", "desc": "Could not resize: Invalid argument"}}

2)For local file as data disk

Both qemu-kvm-rhev-2.6.0-15 and qemu-kvm-1.5.3-118.el7 work.

set 100T size

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 107374182400000}}
{"return": {}}

set 200T size
{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 207374182400000}}
{"return": {}}

result: can get 100T and 200T size disk inside guest.

QMP command block_resize runs qmp_block_resize().  Like any QMP
command handler, it returns an Error through its errp parameter on
error.

qmp_block_resize() first checks a number of error conditions, and if
everything's okay, it runs bdrv_truncate().

bdrv_truncate() checks a few more error conditions, then runs the
block driver's .bdrv_truncate() method.  Both bdrv_truncate() and the
.bdrv_truncate() method return negative errno on error.  Failure modes
and their errno values aren't specified.

bdrv_truncate() itself returns -ENOMEDIUM when the block device has no
medium (can only happen for devices with removable media, obviously),
-ENOTSUP when the block driver doesn't provide a .bdrv_truncate()
method, and -EACCES when the block device was opened read-only.  The
.bdrv_truncate() methods can of course return whatever they want.  In
particular, block driver "host_device"'s method raw_truncate() returns
-EINVAL when asked to grow a block or character device special file.

qmp_block_resize() needs to create an Error object when
bdrv_truncate() fails.  It creates specific error messages for the
errno values it recognizes: -ENOMEDIUM, -ENOTSUP, -EACCES and -EBUSY.
For any other errno value, it creates a generic "Could not resize: S"
error message, where S is strerror(errno).  This is what the reporter
observed.

Two possible solutions:

1. Create specific and suitable error messages for more errno values.
Only possible for errno values that are used consistently by all block
drivers.  Their meaning should be specified in the method contract
then.

2. As Cole observed in comment#2, some block drivers use
error_report().  This is wrong.  They probably do it because just
returning -errno produces inferior error messages on the command line
and in HMP.  "Fixing" the wrongness by dropping the error_report()
would regress error message quality there.  We instead need to fix it
by converting the bdrv_truncate() to take an Error *errp parameter and
use error_setg() instead of error_report().

The more complete solution is 2.  It's a lot of tedious work.
Upstream first.

Does the bug still reproduce with current qemu-kvm?

I'm asking because bdrv_truncate() takes an Error ** argument since upstream commit ed3d2ec98a3, v2.10.0.

(In reply to Markus Armbruster from comment #16)
> Does the bug still reproduce with current qemu-kvm?
> 
> I'm asking because bdrv_truncate() takes an Error ** argument since upstream
> commit ed3d2ec98a3, v2.10.0.



1) For scsi_debug disk as data disk.

qemu-kvm-rhev-2.12.0-7.el7 hit error as below:

set 1G size

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 1073741824 }}
{"return": {}}

set 10G size

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 10737418240 }}
{"error": {"class": "GenericError", "desc": "Cannot grow device files"}}


qemu-kvm-1.5.3-156.el7_5.4 still can be reproduced.

set 1G size

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 1073741824 }}
{"return": {}}

set 10G size

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 10737418240 }}
{"error": {"class": "GenericError", "desc": "Could not resize: Invalid argument"}}


2)For local file as data disk

Both qemu-kvm-rhev-2.12.0-7.el7 and qemu-kvm-1.5.3-156.el7_5.4 work.

set 100T size

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 107374182400000}}
{"return": {}}

{ "execute": "block_resize", "arguments": { "device": "drive-data-disk", "size": 107374182400000}}
{"return": {}}

result: can get 100T size disk inside guest.

Output for qemu-kvm-rhev-2.12.0-7.el7 now matches expectations (I think).  If you disagree, please provide expected output.

Output for qemu-kvm-1.5.3-156.el7_5.4 doesn't match expectations.  However, this bug is against qemu-kvm-rhev.  If you think we should fix qemu-kvm, clone this bug.

According to Comment 17 and Comment 18, verify this bug. And I think it is unnecessary to clone this bug, we seldom test this scenario.