Re: [Qemu-devel] [PATCH for-2.1 v2 2/2] char: report frontend open/closed state in 'query-chardev'

[Laszlo Ersek]

On 06/26/14 14:09, Eric Blake wrote:
> On 06/26/2014 05:11 AM, 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 <address@hidden>
>> ---
>>
> 
>>  ##
>> -{ 'type': 'ChardevInfo', 'data': {'label': 'str', 'filename': 'str'} }
>> +{ 'type': 'ChardevInfo', 'data': {'label': 'str',
>> +                                  'filename': 'str',
>> +                                  'frontend-open': 'bool'} }
> 
> Hmm; I wonder if this should instead be
> 'frontend-status':'VirtioSerialPortStatus', to reuse the type from patch
> 1/2.  That way, if we ever add a third state, then both the event and
> the poll will be reusing the same enum values to report that state.

I expected this remark :)

The difference is rooted in the fact that the event approaches the
virtio-serial port status from the frontend (ie. guest) side, while the
query approaches the same from the backend (ie. host, chardev) side.

If I wanted to bring those in future-proof sync, then I would have to
change the underlying, generic chardev machinery -- namely, the fe_open
field, and everything that operates on it.

I actually considered the other direction too: rather than introducing
status:VirtioSerialPortStatus, just add open:bool (which was your
original suggestion in your v1 review). I decided against it because the
current list of enum constants (connected, disconnected) expresses just
the same, and it'll be a *tiny* bit easier to extend, should that
necessity arise.

Sounds acceptible? :) If not, then I'm OK to replace
status:VirtioSerialPortStatus with open:bool in the first patch.

Thanks,
Laszlo