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&gt; >>> --- >>> 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?