On 05/29/14 22:12, Eric Blake wrote:
> On 05/29/2014 01:36 PM, Laszlo Ersek wrote:
>> In addition to the on-line reporting added in the previous patch, allow
>> libvirt to query frontend state independently of events.
>>
>> Libvirt's path to identify the guest agent channel it cares about differs
>> between the event added in the previous patch and the QMP response field
>> added here. The event identifies the frontend device, by "id". The
>> 'query-chardev' QMP command identifies the backend device (again by "id").
>> The association is under libvirt's control.
>>
>> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1080376
>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>> ---
>> qapi-schema.json | 8 +++++++-
>> qemu-char.c | 2 ++
>> qmp-commands.hx | 19 ++++++++++++++-----
>> 3 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 7bc33ea..7692f9f 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -476,12 +476,18 @@
>> #
>> # @filename: the filename of the character device
>> #
>> +# @frontend_open: #optional shows whether the frontend device attached to
>> this
>
> s/frontend_open/frontend-open/ - as a new interface, we should be
> preferring '-' in the name.
Aaaargh, you're right of course. Shame I forgot that.
>
>> +# backend (eg. with the chardev=... option) is in open or
>> +# closed state (since 2.2)
>
> Why 2.2? Are you saying it is too late to make the 2.1 soft freeze?
I thought that reviewers would immediately question the direction of the
patchset (ie. monitor events + new query field), and not just suggest
tweaks; so 2.2 seemed safer. Perhaps I can make it till the 2.1 soft
freeze (June 17th), but that depends (as I've learned now) on Wenchao's
series too.
> Why optional? It is always emitted by your code, so it's simpler to
> just state that it is now a permanent part of the output struct (no
> back-compat considerations, since it is an output-only struct).
I wasn't sure how libvirt would react to an earlier qemu's output struct
if its parser code expected the new field as mandatory. I didn't
consider that you could infer this field from the presence of the
events. So yeah I can make it mandatory.
>
>> +#
>> # Notes: @filename is encoded using the QEMU command line character device
>> # encoding. See the QEMU man page for details.
>> #
>> # Since: 0.14.0
>> ##
>> -{ 'type': 'ChardevInfo', 'data': {'label': 'str', 'filename': 'str'} }
>> +{ 'type': 'ChardevInfo', 'data': {'label': 'str',
>> + 'filename': 'str',
>> + '*frontend_open': 'bool'} }
>
> Thus, s/*frontend_open/frontend-open/
Yep.
>
>>
>> ##
>> # @query-chardev:
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 17b476e..021f86c 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -3439,6 +3439,8 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
>> info->value = g_malloc0(sizeof(*info->value));
>> info->value->label = g_strdup(chr->label);
>> info->value->filename = g_strdup(chr->filename);
>> + info->value->has_frontend_open = true;
>
> and drop this line
>
>> + info->value->frontend_open = chr->fe_open;
>>
>> info->next = chr_list;
>> chr_list = info;
>
>
Thanks!
Laszlo
