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@xxxxxxxxxx> >>> --- >>> 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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
