"-device foobar" is short for "-device driver=foobar".  The error message makes sense then.  Once again, syntactic sugar leads to suboptimal error messages.

Several other options provide similar sugar (QemuOptsList member implied_opt_name).

Not sure this is fixable with reasonable effort.

An upstream fix would probably mess with three subsystems: qdev, QemuOpts and QMP.  All three are going through major change.  Anthony promises the change will lead to better error handling.  Now, I'll believe that when I see it, but regardless, I'd rather not interfere without a really compelling reason.  Which makes me want to WONTFIX this bug.

But: "A user can easily encounter this via libvirt" makes me hesistate.  What's the user experience exactly?

If improving the user experience turns out to be important, maybe improving it in libvirt is more practical at this time.

Would it be possible for libvirt to catch the suboptimal error and report it in a friendlier way?

Alternatively, would it be possible for libvirt to query available devices with -device \?, and simply not ask for devices that aren't available?

(In reply to comment #4)
> An upstream fix would probably mess with three subsystems: qdev, QemuOpts and
> QMP.  All three are going through major change.  Anthony promises the change
> will lead to better error handling.  Now, I'll believe that when I see it, but
> regardless, I'd rather not interfere without a really compelling reason.  Which
> makes me want to WONTFIX this bug.
> 
> But: "A user can easily encounter this via libvirt" makes me hesistate.  What's
> the user experience exactly?
> 

If someone uses a new libvirt with an older qemu, and specifies some new device in the guest XML and libvirt just passes it through to qemu, guest startup fails with the reported error message.

> If improving the user experience turns out to be important, maybe improving it
> in libvirt is more practical at this time.
> 
> Would it be possible for libvirt to catch the suboptimal error and report it in
> a friendlier way?
> 

Sure, but that sounds like a hack and not future proof at all. And if libvirt can catch the error and scrape it to report a more accurate meaning, QEMU should be able to as well.

> Alternatively, would it be possible for libvirt to query available devices with
> -device \?, and simply not ask for devices that aren't available?

Sure, but that is more moving parts and could cause false positives, not specifying a device that could work if our parsing failed (or regressed between qemu versions).

I know there is QOM stuff going on upstream, and since this is targeted for RHEL7 and not RHEL6 it sounds like now is a good opportunity to get this sorted out upstream. But if it's still programmaticly infeasible to address qemu for RHEL7 I'd say just close this bug WONTFIX.

Bug has been punted from 6.1 to 7.0 since my NEEDINFO.  Your reply suggests you're fine with that.  Let's try to get decent error messages out of upstream QOM in time for RHEL-7.  Thanks!

*** Bug 892286 has been marked as a duplicate of this bug. ***

Current upstream's error message:
-device foobar: Parameter 'driver' expects device type

Two years after comment#4, much has changed in the code, yet my
assessment remains unchanged: we'd still have to touch several
subsystems, which are still very much in flux, and the effort required
to fix the bug seems very hard to justify by the bug's impact on
users.

Comment#6 made the case against leaving the bug alone, and against
libvirt working around it, in four steps:

1. The bug impacts users: a scenario exists where users can trip this
error with libvirt, and the message they then get is confusing.

2. Catching the error in libvirt so we can emit a better message would
require an unpleasant and fragile hack.

3. Avoiding the error in libvirt would require libvirt gaining precise
knowledge on available devices, which is complex and possibly fragile.

4. Therefore, the only option is to improve the message at its root,
in qemu-kvm.

Since then, we've acquired means to reliably and robustly query
available device types in QMP:

{ "execute": "qom-list-types", "arguments": { "implements": "device" } }

I think we should reassess 3 now.  Cole?

How about this?

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c30c2f6..ded5b3e 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -145,7 +145,7 @@ void assert_no_error(Error *err);
     ERROR_CLASS_GENERIC_ERROR, "Invalid parameter type for '%s', expected: %s"
 
 #define QERR_INVALID_PARAMETER_VALUE \
-    ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' expects %s"
+    ERROR_CLASS_GENERIC_ERROR, "Invalid '%s' value '%s'"
 
 #define QERR_INVALID_PASSWORD \
     ERROR_CLASS_GENERIC_ERROR, "Password incorrect"
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 410cdcb..85756f7 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -468,7 +468,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     }
 
     if (!obj) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", driver);
         return NULL;
     }
 


That would require fixing up the rest of the usages of QERR_INVALID_PARAMATER_VALUE but there aren't many.

The string error message of 'Paramater FOO requires HUMAN STRING DESCRIBING WHAT FOO IS' can be actively misleading anyways. AC97 is a valid device type, but if it's compiled out, we give a confusing error 'paramater 'driver' expects device type'. 

Your suggestion of handling this in libvirt sounds reasonable and could be useful for other libvirt enhancements as well, but you'll need to ask the libvirt guys about that.

I originally filed this bug with the assumption that the fix was trivial-ish, if my above suggestion isn't sufficient and there isn't an equally simple fix, then I'd just recommend closing it as WONTFIX rather than have it linger forever.

QERR_INVALID_PARAMETER_VALUE is a product of upstream's attempt at a
rich error reporting API.  Errors were classified into classes such as
InvalidParameterValue, and a single human-readable error message was
associated with each class.  Unsurprisingly, that led to rather
unhelpful error messages when the same class of error was reported
from many different contexts.  QERR_INVALID_PARAMETER_VALUE is such a
case.

Upstream eventually recognized that the rich error reporting API was a
mistake, and switched to a simpler and saner one.  We still have the
old error macros like QERR_INVALID_PARAMETER_VALUE spread all over the
code, printing the same, old, unhelpful error messages.

Your proposed change to the wording of the error message associated
with QERR_INVALID_PARAMETER_VALUE is problematic.  It is not obvious
that the new text is an improvement in all the contexts
QERR_INVALID_PARAMETER_VALUE is used.  In particular since old message
gives a hint at what's expected, while the new one does not.

Perhaps we can change just the single use, like this:

     if (!obj) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
+	 qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                      "%s is not a valid device model name", driver);
         return NULL;
     }

The message text may need more polish; I'm just illustrating how to
have another message here without changing messages elsewhere.

Proposed upstream patch: https://lists.nongnu.org/archive/html/qemu-devel/2013-12/msg03670.html

Fix included in qemu-kvm-1.5.3-39.el7

Fix also fixes bug 1026712 and bug 1052085.  QE, please add their reproducers to your testing.  They are:

From bug 1026712:

Steps to Reproduce:
1. Start with -device virtio-pci

Actual results:
Assertion failure

Expected results:
Fatal error "qemu-kvm: -device virtio-pci: Parameter 'driver' expects non-abstract device type"

From bug 1052085:

Steps to Reproduce:
1. qemu-kvm -nodefaults -S -display none -monitor stdio
2. device_add rng-egd

Actual results:
Assertion failure

Expected results:
Command fails with error message "'rng-egd' is not a valid device model name"

See these bugs for additional detail.

*** Bug 1026712 has been marked as a duplicate of this bug. ***

*** Bug 1052085 has been marked as a duplicate of this bug. ***

*** Bug 1055025 has been marked as a duplicate of this bug. ***

Reproduced this issue with the same steps as comment #0 and comment #18 on qemu-kvm-1.5.3-38.el7.x86_64.
host info:
# uname -r && rpm -q qemu-kvm
3.10.0-76.el7.x86_64
qemu-kvm-1.5.3-38.el7.x86_64

Steps and Results:
From bug 669524:
# /usr/libexec/qemu-kvm -device foobar
Segmentation fault (core dumped)

From bug 1026712:
# /usr/libexec/qemu-kvm -device virtio-pci
qemu-kvm: -device virtio-pci: Parameter 'driver' expects non-abstract device type

From bug 1052085:
# /usr/libexec/qemu-kvm -nodefaults -S -display none -monitor stdio
QEMU 1.5.3 monitor - type 'help' for more information
(qemu) device_add rng-egd
qdev-monitor.c:486:qdev_device_add: Object 0x7f76d76f31e0 is not an instance of type device
Aborted (core dumped)

Verified this issue on qemu-kvm-1.5.3-45.el7.x86_64.
host info:
# uname -r && rpm -q qemu-kvm
3.10.0-76.el7.x86_64
qemu-kvm-1.5.3-45.el7.x86_64

Steps and Results:
From bug 669524:
# /usr/libexec/qemu-kvm -device foobar
qemu-kvm: -device foobar: 'foobar' is not a valid device model name

From bug 1026712:
# /usr/libexec/qemu-kvm -device virtio-pci
qemu-kvm: -device virtio-pci: Parameter 'driver' expects non-abstract device type

From bug 1052085:
# /usr/libexec/qemu-kvm -nodefaults -S -display none -monitor stdio
QEMU 1.5.3 monitor - type 'help' for more information
(qemu) device_add rng-egd
'rng-egd' is not a valid device model name
(qemu) q

Base on above, this issue has been fixed correctly, move to VERIFIED status.

Best Regards,
sluo

This request was resolved in Red Hat Enterprise Linux 7.0.

Contact your manager or support representative in case you have further questions about the request.