Machine type compatible properties

[PATCH v1 6/9] chardev: add appropriate getting address

Maxim Davydov posted 9 patches 3 years, 10 months ago

Download mbox

Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Yanan Wang <wangyanan55@huawei.com>, "Michael S. Tsirkin" <mst@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <ani@anisinha.ca>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, Zhang Chen <chen.zhang@intel.com>, Li Zhijian <lizhijian@fujitsu.com>, Jason Wang <jasowang@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Thomas Huth <thuth@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>, Laurent Vivier <lvivier@redhat.com>

Expand all Fold all

[PATCH v1 6/9] chardev: add appropriate getting address

Posted by Maxim Davydov 3 years, 10 months ago

Attempt to get address after initialization shouldn't fail on assert in
the qapi automatically generated code. As a possible solution, it can
return null type.

Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
---
 chardev/char-socket.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index fab2d791d4..f851e3346b 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -33,6 +33,7 @@
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-visit-sockets.h"
 #include "qemu/yank.h"
+#include "qapi/qmp/qnull.h"
 
 #include "chardev/char-io.h"
 #include "chardev/char-socket.h"
@@ -1509,6 +1510,14 @@ char_socket_get_addr(Object *obj, Visitor *v, const char *name,
 {
     SocketChardev *s = SOCKET_CHARDEV(obj);
 
+    QNull *null = NULL;
+
+    /* Return NULL type if getting addr was called after init */
+    if (!s->addr) {
+        visit_type_null(v, NULL, &null, errp);
+        return;
+    }
+
     visit_type_SocketAddress(v, name, &s->addr, errp);
 }
 
-- 
2.31.1

Re: [PATCH v1 6/9] chardev: add appropriate getting address

Posted by Vladimir Sementsov-Ogievskiy 3 years, 10 months ago

29.03.2022 00:15, Maxim Davydov wrote:
> Attempt to get address after initialization shouldn't fail on assert in
> the qapi automatically generated code. As a possible solution, it can
> return null type.

But at some point this address appears? May be we try to query it too early, or we need some more initialization steps?

Isn't it better to report failure, when we try to query things that are not yet initialized?

> 
> Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
> ---
>   chardev/char-socket.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index fab2d791d4..f851e3346b 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -33,6 +33,7 @@
>   #include "qapi/clone-visitor.h"
>   #include "qapi/qapi-visit-sockets.h"
>   #include "qemu/yank.h"
> +#include "qapi/qmp/qnull.h"
>   
>   #include "chardev/char-io.h"
>   #include "chardev/char-socket.h"
> @@ -1509,6 +1510,14 @@ char_socket_get_addr(Object *obj, Visitor *v, const char *name,
>   {
>       SocketChardev *s = SOCKET_CHARDEV(obj);
>   
> +    QNull *null = NULL;
> +
> +    /* Return NULL type if getting addr was called after init */
> +    if (!s->addr) {
> +        visit_type_null(v, NULL, &null, errp);
> +        return;
> +    }
> +
>       visit_type_SocketAddress(v, name, &s->addr, errp);
>   }
>   


-- 
Best regards,
Vladimir

Re: [PATCH v1 6/9] chardev: add appropriate getting address

Posted by Maxim Davydov 3 years, 10 months ago

On 3/30/22 14:32, Vladimir Sementsov-Ogievskiy wrote:
> 29.03.2022 00:15, Maxim Davydov wrote:
>> Attempt to get address after initialization shouldn't fail on assert in
>> the qapi automatically generated code. As a possible solution, it can
>> return null type.
>
> But at some point this address appears? May be we try to query it too 
> early, or we need some more initialization steps?
For example, query address after object_new_with_class(). Without the 
patch it triggers assert(). I tried to implement the same solution as in 
hw/ppc/spapr_drc.c:prop_get_fdt
>
> Isn't it better to report failure, when we try to query things that 
> are not yet initialized?
Yes, maybe it should set errp after visit_type_null. And it should be a 
common error for unrealized devices to fix the same problem with 
MemoryRegion, etc.
>
>>
>> Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
>> ---
>>   chardev/char-socket.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index fab2d791d4..f851e3346b 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -33,6 +33,7 @@
>>   #include "qapi/clone-visitor.h"
>>   #include "qapi/qapi-visit-sockets.h"
>>   #include "qemu/yank.h"
>> +#include "qapi/qmp/qnull.h"
>>     #include "chardev/char-io.h"
>>   #include "chardev/char-socket.h"
>> @@ -1509,6 +1510,14 @@ char_socket_get_addr(Object *obj, Visitor *v, 
>> const char *name,
>>   {
>>       SocketChardev *s = SOCKET_CHARDEV(obj);
>>   +    QNull *null = NULL;
>> +
>> +    /* Return NULL type if getting addr was called after init */
>> +    if (!s->addr) {
>> +        visit_type_null(v, NULL, &null, errp);
>> +        return;
>> +    }
>> +
>>       visit_type_SocketAddress(v, name, &s->addr, errp);
>>   }
>
>
-- 
Best regards,
Maxim Davydov