On 04/17/2018 02:00 PM, John Ferlan wrote:
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@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.
We already don't expose some aliases. For instance, if a domain is configured to use hugepages and we use memory-backend-file we don't report generated aliases anywhere. Why? Because the fact we are using memory-backend-file to tell qemu to use hugepages is internal implementation. And users should not be concerned with that. It is the same story with pr-manager and its alias. It is internal implementation deatail and as such we should not expose it.
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.
You probably misunderstood what I meant. My idea is to expose as little info back to user as possible in this case. I don't see any compelling reason for user to learn the pr-manager's alias.
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?
Don't recall. It's not relevant.
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?
Why should we track PID of pr-helper? What do we need it for? As Peter pointed out in review to my previous patches, PIDs change therefore if we start pr-helper process with PID X, later when shutting down domain we could find a different process under the same PID. Because pr-helper might have died, released the PID and another process could have been started with the same PID. Michal