Re: [PATCH 3/4] qtest: Improve error messages when property can not be set right now

2022-10-27 Thread Markus Armbruster
Markus Armbruster  writes:

> Thomas Huth  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.




Re: [PATCH 3/4] qtest: Improve error messages when property can not be set right now

2022-10-12 Thread Markus Armbruster
Thomas Huth  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!




Re: [PATCH 3/4] qtest: Improve error messages when property can not be set right now

2022-10-12 Thread David Hildenbrand

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." ?


--
Thanks,

David / dhildenb




Re: [PATCH 3/4] qtest: Improve error messages when property can not be set right now

2022-10-12 Thread Thomas Huth

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





[PATCH 3/4] qtest: Improve error messages when property can not be set right now

2022-10-12 Thread Markus Armbruster
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 
---
 softmmu/qtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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;
 }
 
-- 
2.37.2