Re: [PATCHv2 4/8] qemu: prepare secret for the graphics upfront

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/21/19 7:59 AM, Ján Tomko wrote:
> Instead of hardcoding the TLS creds alias in
> qemuBuildGraphicsVNCCommandLine, store it
> in the domain private data.
> 
> Given that we only support one VNC graphics
> and thus have only one alias per-domain,
> this is overengineered, but it will allow us
> to prepare the secret upfront when we start
> supporting encrypted server TLS keys.
> 
> Note that the alias is not formatted anywhere
> since we won't need to access it after domain
> startup.
> 
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c |  8 ++++----
>  src/qemu/qemu_domain.c  | 44 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+), 4 deletions(-)
> 

Rather than respond in v1... Irony is:

   sizeof(*gfxPriv) = 24
   sizeof("vnc-tls-creds0") = 15

and the reason for the free during GraphicsDestroy is "to not have the
temporary data allocated during the whole time the domain is running",
but the algorithm leaves privateData allocated for every graphics device
and not just VNC one's.

It's used once and doesn't need to stick around. Of course who knows
what the future brings and the algorithm essentially follows storage and
chardev source (even though they are each a bit different).

Since it's not possible to have a struct with just "virObject parent;"
alone, an alternative is to have a "bool useTLSCreds;" and then continue
with the use of "const char *alias = "vnc-tls-creds0";" in
qemuBuildGraphicsVNCCommandLine.

Then again you did say overengineered above, so even though I may have
done things differently, what you have isn't wrong, so consider it,

Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux