[libvirt] [PATCH v1 0/4] libxl: channels support

[Joao Martins]

On 09/20/2016 01:14 PM, Michal Privoznik wrote:
> On 20.09.2016 13:57, Joao Martins wrote:
>> On 09/20/2016 11:54 AM, Michal Privoznik wrote:
>>> On 20.09.2016 12:43, Joao Martins wrote:
>>>> On 09/20/2016 05:14 AM, Michal Privoznik wrote:
>>>>> On 20.09.2016 00:04, Jim Fehlig wrote:
>>>>>> On 09/16/2016 05:43 PM, Joao Martins wrote:
>>>>>>> Hey,
>>>>>>>
>>>>>>>  Additionally what does "state"
>>>>>>> signify for virtio case: is it that the guest agent is connected, or that the
>>>>>>> virtio serial is connected? Looking at the qemu driver on
>>>>>>> processSerialChangedEvent it sounds to me like it's about the serial state.
>>>>>>
>>>>>> AFAICT you are right, but perhaps Michal is kind enough to confirm. I think he
>>>>>> is familiar with this code.
>>>>>
>>>>> That attribute is put by libvirt into live XML so that mgmt apps can
>>>>> query for it. The attribute says whether the guest part of channel is
>>>>> opened by a process running within guest. In case of the qemu-ga whether
>>>>> we have the guest agent up & running. Whenever the guest end of a
>>>>> channel is opened/closed, qemu sends us an event so we can update our
>>>>> internal state and put the correct value when formatting live XML.
>>>>> Therefore, it makes no sense to make this attribute RW (meaning users
>>>>> could set it), obviously.
>>>> Hmm if it refers to qemu agent actually (dis)connected from guest side it might
>>>> be difficult to provide the same "state" semantics in libxl driver at least
>>>> without changes on the toolstack. If it referring to virtio-serial state we
>>>> could easily do that by periodically calling libxl_channel_getinfo until state
>>>> is 4 (connected). But it's the actual agent process state as connected in which
>>>> case to have "state" attribute we would need to add similar to libxl event
>>>> DOMAIN_CREATE_CONSOLE_AVAILABLE but instead referring to the guest channel.
>>>
>>> Well, in qemu it really just means that somebody is listening. Note that
>>> I say "somebody". That is if you kill qemu-ga in the guest and just 'cat
>>> /dev/virtio-ports/org.qemu-ga.' we will report state='connected' (or
>>> what's the path, and yes you can't use cat, but you get my point).
>> Yeap I understand. Looking at both libvirt, qemu  it looks like the event
>> allowing this is QMP event VSERPORT_CONNECTED|VSERPORT_DISCONNECTED on libvirt
>> commit 15bbaaf01 ("qemu: Add handling for VSERPORT_CHANGE event") and qemu
>> commit e2ae6159d ("virtio-serial: report frontend connection state via
>> monitor"). Looking at the frontend driver it has the granularity of sending port
>> open/close events (quite nice!), which I assume it's what qemu is using as info
>> to propagate these events to libvirt. Although this looks very specific to
>> virtio workings, which might not be possible to get the equivalent with the Xen
>> console. So perhaps having a periodical (with a timeout) guest-ping would be the
>> best course of action I guess?
> 
> Well, what we have used in the past (until having this fancy system of
> events) was that we just did not set the attribute at all (in fact it
> was introduced only after we've done our part of implementation).
> Moreover, we've sent 'guest-ping' to the guest agent just before trying
> to execute a real command. If ping timed out, we've error-ed out and did
> not execute the real command. This was to keep libvirt and qemu state in
> sync - the guest agent command might change the guest state.
> 
> I think xen can reuse this logic until future time when events are
> introduced to it too. And also, we don't need to expose the attribute
> for xen domains until then.
Cool, sounds good. BTW the first patch in this series, I think it goes like
you're suggesting: defining a new "xenTarget" in the schema whereas this
attribute is not included, plus avoiding the "state" attribute updates.

Joao