1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH for-2.12 0/3] block: Handle null backing link
  4. View patch

[Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()

Max Reitz posted 3 patches 8 years, 3 months ago
There is a newer version of this series
[Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Max Reitz 8 years, 3 months ago 
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/qapi/qmp/qdict.h |  1 +
 qobject/qdict.c          | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index fc218e7be6..c65ebfc748 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
                           int64_t def_value);
 bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
+bool qdict_is_qnull(const QDict *qdict, const char *key);
 
 void qdict_copy_default(QDict *dst, QDict *src, const char *key);
 void qdict_set_default_str(QDict *dst, const char *key, const char *val);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index e8f15f1132..a032ea629a 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -294,6 +294,16 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_is_qnull(): Return true if the value for 'key' is QNull
+ */
+bool qdict_is_qnull(const QDict *qdict, const char *key)
+{
+    QObject *value = qdict_get(qdict, key);
+
+    return value && value->type == QTYPE_QNULL;
+}
+
+/**
  * qdict_iter(): Iterate over all the dictionary's stored values.
  *
  * This function allows the user to provide an iterator, which will be
-- 
2.13.6
Re: [Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Eric Blake 8 years, 3 months ago 
On 11/10/2017 04:13 PM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  include/qapi/qmp/qdict.h |  1 +
>  qobject/qdict.c          | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Alberto Garcia 8 years, 2 months ago 
On Fri 10 Nov 2017 11:13:27 PM CET, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  include/qapi/qmp/qdict.h |  1 +
>  qobject/qdict.c          | 10 ++++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index fc218e7be6..c65ebfc748 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>                            int64_t def_value);
>  bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
> +bool qdict_is_qnull(const QDict *qdict, const char *key);

I found this name a bit confusing, how about something like
qdict_entry_is_qnull() ?

I'm fine if you want to keep it like this though,

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Max Reitz 8 years, 2 months ago 
On 2017-11-14 15:07, Alberto Garcia wrote:
> On Fri 10 Nov 2017 11:13:27 PM CET, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  include/qapi/qmp/qdict.h |  1 +
>>  qobject/qdict.c          | 10 ++++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
>> index fc218e7be6..c65ebfc748 100644
>> --- a/include/qapi/qmp/qdict.h
>> +++ b/include/qapi/qmp/qdict.h
>> @@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>>                            int64_t def_value);
>>  bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
>>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
>> +bool qdict_is_qnull(const QDict *qdict, const char *key);
> 
> I found this name a bit confusing, how about something like
> qdict_entry_is_qnull() ?

Yes, that sounds better.  Thanks!

Max

> I'm fine if you want to keep it like this though,
> 
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> 
> Berto
>
Re: [Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Markus Armbruster 8 years, 2 months ago 
Max Reitz <mreitz@redhat.com> writes:

> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  include/qapi/qmp/qdict.h |  1 +
>  qobject/qdict.c          | 10 ++++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index fc218e7be6..c65ebfc748 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>                            int64_t def_value);
>  bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
> +bool qdict_is_qnull(const QDict *qdict, const char *key);
>  
>  void qdict_copy_default(QDict *dst, QDict *src, const char *key);
>  void qdict_set_default_str(QDict *dst, const char *key, const char *val);
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index e8f15f1132..a032ea629a 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -294,6 +294,16 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key)
>  }
>  
>  /**
> + * qdict_is_qnull(): Return true if the value for 'key' is QNull
> + */
> +bool qdict_is_qnull(const QDict *qdict, const char *key)
> +{
> +    QObject *value = qdict_get(qdict, key);
> +
> +    return value && value->type == QTYPE_QNULL;
> +}
> +
> +/**
>   * qdict_iter(): Iterate over all the dictionary's stored values.
>   *
>   * This function allows the user to provide an iterator, which will be

As far as I can tell, the new helper function is going to be used just
once, by bdrv_open_inherit() in PATCH 2:

    qdict_is_qnull(options, "backing")

I dislike abstracting from just one concrete instance.

Perhaps a more general helper could be more generally useful.  Something
like:

    qobject_is(qdict_get(options, "backing", QTYPE_QNULL))

There are numerous instances of

    !obj || qobject_type(obj) == T

in the tree, which could then be replaced by

    qobject_is(obj, T)

An alternative helper: macro qobject_dynamic_cast(obj, T) that returns
(T *)obj if obj is a T, else null.  Leads to something like

    qobject_dynamic_cast(qdict_get(options, "backing", QNull))
Re: [Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Max Reitz 8 years, 2 months ago 
On 2017-11-14 15:57, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
> 
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  include/qapi/qmp/qdict.h |  1 +
>>  qobject/qdict.c          | 10 ++++++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
>> index fc218e7be6..c65ebfc748 100644
>> --- a/include/qapi/qmp/qdict.h
>> +++ b/include/qapi/qmp/qdict.h
>> @@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>>                            int64_t def_value);
>>  bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
>>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
>> +bool qdict_is_qnull(const QDict *qdict, const char *key);
>>  
>>  void qdict_copy_default(QDict *dst, QDict *src, const char *key);
>>  void qdict_set_default_str(QDict *dst, const char *key, const char *val);
>> diff --git a/qobject/qdict.c b/qobject/qdict.c
>> index e8f15f1132..a032ea629a 100644
>> --- a/qobject/qdict.c
>> +++ b/qobject/qdict.c
>> @@ -294,6 +294,16 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key)
>>  }
>>  
>>  /**
>> + * qdict_is_qnull(): Return true if the value for 'key' is QNull
>> + */
>> +bool qdict_is_qnull(const QDict *qdict, const char *key)
>> +{
>> +    QObject *value = qdict_get(qdict, key);
>> +
>> +    return value && value->type == QTYPE_QNULL;
>> +}
>> +
>> +/**
>>   * qdict_iter(): Iterate over all the dictionary's stored values.
>>   *
>>   * This function allows the user to provide an iterator, which will be
> 
> As far as I can tell, the new helper function is going to be used just
> once, by bdrv_open_inherit() in PATCH 2:
> 
>     qdict_is_qnull(options, "backing")
> 
> I dislike abstracting from just one concrete instance.
> 
> Perhaps a more general helper could be more generally useful.  Something
> like:
> 
>     qobject_is(qdict_get(options, "backing", QTYPE_QNULL))
> 
> There are numerous instances of
> 
>     !obj || qobject_type(obj) == T
> 
> in the tree, which could then be replaced by
> 
>     qobject_is(obj, T)
> 
> An alternative helper: macro qobject_dynamic_cast(obj, T) that returns
> (T *)obj if obj is a T, else null.  Leads to something like
> 
>     qobject_dynamic_cast(qdict_get(options, "backing", QNull))

If you think that's good, then that's good -- you know the QAPI code
better then me, after all.

To explain myself: I thought it would be the natural extension of the
qdict_get_try_*() functions for the QNull type.

Max
Re: [Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()
Posted by Markus Armbruster 8 years, 2 months ago 
Max Reitz <mreitz@redhat.com> writes:

> On 2017-11-14 15:57, Markus Armbruster wrote:
>> Max Reitz <mreitz@redhat.com> writes:
>> 
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>>>  include/qapi/qmp/qdict.h |  1 +
>>>  qobject/qdict.c          | 10 ++++++++++
>>>  2 files changed, 11 insertions(+)
>>>
>>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
>>> index fc218e7be6..c65ebfc748 100644
>>> --- a/include/qapi/qmp/qdict.h
>>> +++ b/include/qapi/qmp/qdict.h
>>> @@ -76,6 +76,7 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>>>                            int64_t def_value);
>>>  bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value);
>>>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
>>> +bool qdict_is_qnull(const QDict *qdict, const char *key);
>>>  
>>>  void qdict_copy_default(QDict *dst, QDict *src, const char *key);
>>>  void qdict_set_default_str(QDict *dst, const char *key, const char *val);
>>> diff --git a/qobject/qdict.c b/qobject/qdict.c
>>> index e8f15f1132..a032ea629a 100644
>>> --- a/qobject/qdict.c
>>> +++ b/qobject/qdict.c
>>> @@ -294,6 +294,16 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key)
>>>  }
>>>  
>>>  /**
>>> + * qdict_is_qnull(): Return true if the value for 'key' is QNull
>>> + */
>>> +bool qdict_is_qnull(const QDict *qdict, const char *key)
>>> +{
>>> +    QObject *value = qdict_get(qdict, key);
>>> +
>>> +    return value && value->type == QTYPE_QNULL;
>>> +}
>>> +
>>> +/**
>>>   * qdict_iter(): Iterate over all the dictionary's stored values.
>>>   *
>>>   * This function allows the user to provide an iterator, which will be
>> 
>> As far as I can tell, the new helper function is going to be used just
>> once, by bdrv_open_inherit() in PATCH 2:
>> 
>>     qdict_is_qnull(options, "backing")
>> 
>> I dislike abstracting from just one concrete instance.
>> 
>> Perhaps a more general helper could be more generally useful.  Something
>> like:
>> 
>>     qobject_is(qdict_get(options, "backing", QTYPE_QNULL))
>> 
>> There are numerous instances of
>> 
>>     !obj || qobject_type(obj) == T
>> 
>> in the tree, which could then be replaced by
>> 
>>     qobject_is(obj, T)
>> 
>> An alternative helper: macro qobject_dynamic_cast(obj, T) that returns
>> (T *)obj if obj is a T, else null.  Leads to something like
>> 
>>     qobject_dynamic_cast(qdict_get(options, "backing", QNull))
>
> If you think that's good, then that's good -- you know the QAPI code
> better then me, after all.

I'll play with it today.

> To explain myself: I thought it would be the natural extension of the
> qdict_get_try_*() functions for the QNull type.

I see now.  The name qdict_get_try_null() would make it obvious, but
what to return on success...

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.