Re: [libvirt] [PATCH v4 05/14] qemu: Store pr runtime data in status XML
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(a)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