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

[Joao Martins]

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,
>>>
>>> Channels have been on xen toolstack since Xen 4.5 and this small series
>>> adds support for it, including xenconfig conversion and appropriate tests.
>>
>> Cool! Thanks.
>>
>>> After this series it's possible to do this:
>>> (assuming correct configuration of qemu agent in the guest)
>>>
>>>  $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>>>  <channel type='unix'>
>>>    <source mode='bind' path='/tmp/channel'/>
>>>    <target type='xen' name='org.qemu.guest_agent.0'/>
>>>  </channel>
>>>
>>>  $ virsh create domain.xml
>>>  $ echo '{"execute":"guest-network-get-interfaces"}' | socat
>>>  stdio,ignoreeof  unix-connect:/tmp/channel
>>>
>>>  {"execute":"guest-network-get-interfaces"}
>>>  {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
>>>  "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
>>>  "ip-address": "::1", "prefix": 128}], "hardware-address":
>>>  "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
>>>  [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
>>>  {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
>>>  "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
>>>  "sit0"}]}
> 
> BTW: libvirt has an API for this too - virDomainInterfaceAddresses in
> case you'd like to take the extra step :-).
> 
Indeed, I have that implemented already for guest agent integration series
mentioned further below :)

>>>
>>> There is just one thing unclear: is source "path" meant to be auto-generated
>>> if it's not specified in the channel config?
>>
>> I think so. According to docs/formatdomain.html
>>
>> Moreover, since 1.0.6 it is possible to have source path auto generated for
>> virtio unix channels. This is very useful in case of a qemu guest agent, where
>> users don't usually care about the source path since it's libvirt who talks to
>> the guest agent. In case users want to utilize this feature, they should leave
>> <source> element out.
I see.

> 
> Correct. Historically, we required users to provide us a path that host
> side of the channel is supposed to be listening at. But it wasn't very
> user-friendly feature lets say. So we wrote some code that generates the
> path automatically when needed. qemuDomainPrepareChannel() and
> qemuProcessPrepareMonitorChr().
OK. I will update it to auto-generate the path if not specified.

>>>  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.

>>> I also have one other series that lets us share the qemu agent across both qemu
>>> and libxl drivers, with qemu-agent-command implemented. But it's still in an
>>> early stage.
> 
> Ah, that would be very nice indeed. We don't like copying code around.
Yeap :)

Cheers,
Joao

> 
>>>
>>> Any comments or feedback is appreciated :)
>>
>> Thanks again. I'll start reviewing the individual patches.
>>
>> Regards,
>> Jim
>>
> 
> Michal
>