[libvirt] [PATCH v4 05/14] qemu: Store pr runtime data in status XML

[John Ferlan]

On 04/16/2018 10:56 AM, Michal Privoznik wrote:
> On 04/13/2018 10:57 PM, John Ferlan wrote:
>>
>>
>> On 04/10/2018 10:58 AM, Michal Privoznik wrote:
>>> Now that we generate pr-manager alias and socket path store them
>>> in status XML so that they are preserved across daemon restarts.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/qemu/qemu_domain.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 64 insertions(+)
>>>
>>
>> So if we were to save this information (and at this point I don't think
>> we need to), then this is where we should be allocating and filling in
>> the private data (e.g. not in the previous patch).
> 
> How come? What would be left from the previous patch if private runtime
> struct would be introduced only here? Or are you just suggesting
> swapping these two patches?
> 

I hope I provided enough feedback in the prior response to answer this.

>>
>> Again as I noted previously, save the alias when printing the domain
>> active information and I think you're good.
> 
> No, I don't want to expose info on PR helper more than is necessary. The
> fact that it's a separate process should not be visible to users because
> it is an implementation detail of QEMU. Other hypervisors might do this
> differently. And even though it might not be visible from the patches,
> using unmanaged mode is discouraged. In fact, unmanaged mode is on the
> edge. If pr-helper is viewed as internal implementation the unmanaged
> mode has no place in libvirt. However, qemu devels are experimenting
> with systemd socket activation and for socket path must be configurable
> through libvirt. So the only reason for using unmanaged PRs is systemd
> socket activation.

We "expose" aliases a lot in the active domain XML. Someone that's going
to add a <reservations enabled='yes' managed='yes'/> to their <disk...>
definition I cannot believe would be surprised to see an alias printed.

How would they know from the alias that it's a separate process? The
only way to correlate the two would be to read the code and know what
QEMU did to make libvirt do a little dance in order to support.

As for systemd, oh great another area to fall flat on our faces...
Wasn't another reason to shorten the path w/ domain name because there
was some sort of bad systemd interaction?

> 
> Side note, we are not even exposing qemu's PID anywhere because not
> every hypervisor we support has VMs as separate processes.
> 

The PID though could be an unexposed domain private data, couldn't it?


John

>>
>> AFAICT the only thing printed now (@relPath) is something generated via
>> qemu_driver calls (I didn't dig deep); whereas, this data is easily
>> regeneratable from existing domain definition data.
> 
> Yes it is. Currently. But just look at the history of channelTargetDir,
> e.g. a89f05ba8df095875f. We have to have qemuDomainSetPrivatePathsOld(),
> worse we have to keep it around for the rest of libvirt's life time.
> Only because nobody thought of storing channelTargerDir in runtime XML.
> 
> Michal
>