| Message ID | 20221012153801.2604340-4-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace QERR_PERMISSION_DENIED by better error messages | expand |
On 12/10/2022 17.38, Markus Armbruster wrote: > When you try to set qtest property "log" while the qtest object is > active, the error message blames "insufficient permission": > > $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio -chardev socket,id=chrqt0,path=qtest.socket,server=on,wait=off -object qtest,id=qt0,chardev=chrqt0,log=/dev/null > QEMU 7.1.50 monitor - type 'help' for more information > (qemu) qom-set /objects/qt0 log qtest.log > Error: Insufficient permission to perform this operation > > This implies it could work with "sufficient permission". It can't. > Change the error message to: > > Error: Property 'log' can not be set now Can it be set later? ... if not, that error message is almost as confusing as the original one. Maybe it's better to tell the users *when* they can set the property? Thomas
On 12.10.22 20:05, Thomas Huth wrote: > On 12/10/2022 17.38, Markus Armbruster wrote: >> When you try to set qtest property "log" while the qtest object is >> active, the error message blames "insufficient permission": >> >> $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio -chardev socket,id=chrqt0,path=qtest.socket,server=on,wait=off -object qtest,id=qt0,chardev=chrqt0,log=/dev/null >> QEMU 7.1.50 monitor - type 'help' for more information >> (qemu) qom-set /objects/qt0 log qtest.log >> Error: Insufficient permission to perform this operation >> >> This implies it could work with "sufficient permission". It can't. >> Change the error message to: >> >> Error: Property 'log' can not be set now > > Can it be set later? ... if not, that error message is almost as confusing > as the original one. Maybe it's better to tell the users *when* they can set > the property? I assume it's mostly about "This property cannot be set." and "This property can no longer be set." ?
Thomas Huth <thuth@redhat.com> writes: > On 12/10/2022 17.38, Markus Armbruster wrote: >> When you try to set qtest property "log" while the qtest object is >> active, the error message blames "insufficient permission": >> >> $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio -chardev socket,id=chrqt0,path=qtest.socket,server=on,wait=off -object qtest,id=qt0,chardev=chrqt0,log=/dev/null >> QEMU 7.1.50 monitor - type 'help' for more information >> (qemu) qom-set /objects/qt0 log qtest.log >> Error: Insufficient permission to perform this operation >> >> This implies it could work with "sufficient permission". It can't. >> Change the error message to: >> >> Error: Property 'log' can not be set now > > Can it be set later? ... if not, that error message is almost as confusing > as the original one. Maybe it's better to tell the users *when* they can set > the property? The property cannot be set while the object is "active", i.e. global @qtest points to it. Right now, @qtest points to the object from completion with user_creatable_complete() to unparent. Completion fails when @qtest already points to another object, i.e. only one object can be complete at any time. Since Paolo took the trouble to code an unparent method, I assume unparent can happen. I can't tell offhand when. Help!
Markus Armbruster <armbru@redhat.com> writes: > Thomas Huth <thuth@redhat.com> writes: > >> On 12/10/2022 17.38, Markus Armbruster wrote: >>> When you try to set qtest property "log" while the qtest object is >>> active, the error message blames "insufficient permission": >>> >>> $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio -chardev socket,id=chrqt0,path=qtest.socket,server=on,wait=off -object qtest,id=qt0,chardev=chrqt0,log=/dev/null >>> QEMU 7.1.50 monitor - type 'help' for more information >>> (qemu) qom-set /objects/qt0 log qtest.log >>> Error: Insufficient permission to perform this operation >>> >>> This implies it could work with "sufficient permission". It can't. >>> Change the error message to: >>> >>> Error: Property 'log' can not be set now >> >> Can it be set later? ... if not, that error message is almost as confusing >> as the original one. Maybe it's better to tell the users *when* they can set >> the property? > > The property cannot be set while the object is "active", i.e. global > @qtest points to it. > > Right now, @qtest points to the object from completion with > user_creatable_complete() to unparent. > > Completion fails when @qtest already points to another object, i.e. only > one object can be complete at any time. > > Since Paolo took the trouble to code an unparent method, I assume > unparent can happen. I can't tell offhand when. > > Help! Since users are unlikely to hit this error, the new message feels good enough to me. Happy to take rewordings, or to redo the patch so that it preserves the old message while getting rid of QERR_PERMISSION_DENIED.
diff --git a/softmmu/qtest.c b/softmmu/qtest.c index f8acef2628..afea7693d0 100644 --- a/softmmu/qtest.c +++ b/softmmu/qtest.c @@ -977,7 +977,7 @@ static void qtest_set_log(Object *obj, const char *value, Error **errp) QTest *q = QTEST(obj); if (qtest == q) { - error_setg(errp, QERR_PERMISSION_DENIED); + error_setg(errp, "Property 'log' can not be set now"); } else { g_free(q->log); q->log = g_strdup(value); @@ -997,7 +997,7 @@ static void qtest_set_chardev(Object *obj, const char *value, Error **errp) Chardev *chr; if (qtest == q) { - error_setg(errp, QERR_PERMISSION_DENIED); + error_setg(errp, "Property 'chardev' can not be set now"); return; }
When you try to set qtest property "log" while the qtest object is active, the error message blames "insufficient permission": $ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio -chardev socket,id=chrqt0,path=qtest.socket,server=on,wait=off -object qtest,id=qt0,chardev=chrqt0,log=/dev/null QEMU 7.1.50 monitor - type 'help' for more information (qemu) qom-set /objects/qt0 log qtest.log Error: Insufficient permission to perform this operation This implies it could work with "sufficient permission". It can't. Change the error message to: Error: Property 'log' can not be set now Same for property "chardev". Signed-off-by: Markus Armbruster <armbru@redhat.com> --- softmmu/qtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)