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

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.