Re: [libvirt] [PATCH v8 1/5] domain: Add optional 'tls' attribute for TCP chardev

[...] >> >> UGH... mea culpa - in my mind the TLS hadn't been added to the chardev >> yet and that was the rest of this series... Of course the rest of this >> series is adding the passphrase for the environment (<sigh>). >> >> If I could have got that review earlier, then this situation wouldn't be >> a problem (see in my mind it wasn't). >> >> If a domain is already running, then encryption is occurring and this >> setting has no effect except for hotplug. >> >> If we were using encryption in 2.3.0... stopped our domain... installed >> 2.4.0... started our domain, then yes, I see/agree.... Frustrating >> since there's really no "simple" way to determine this. >> >>> >>> The correct solution is: >>> >>> - if chardev_tls is set to 1 and tls attribute is not configured in the XML >>> the default after starting the domain should be tls=yes >> >> So IOW: >> >> + if (cfg->chardevTLS && >> + dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) { >> >> changes to >> >> + if (cfg->chardevTLS && >> + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) { >> >> or (haveTLS == YES || haveTLS == ABSENT) >> >> This of course is essentially the logic from v6 which said disableTLS on >> a case by case basis.... All we have now is the positive form... >> >> So for both qemuBuildChrChardevStr and qemuDomainAttachChrDevice I'd >> have a similar change. Does that suffice and do you need to see the change? > > I think that we should reflect the state in live XML if the attribute exists. > > Add a code to qemuProcessPrepareDomain() that will set dev->data.tcp.haveTLS > to config value if dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_ABSENT and in > qemuBuildChrChardevStr() you can just check if > dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES. This will also ensure that > the live XML contains the 'tls' attribute. But make sure, that migration works > properly for all combinations. > I don't see the advantage... Altering the running domain would involve

messing in qemuProcessAttach and setting the attribute for the domain if chardevTLS is set *BUT* how do we know without a bit of introspection if that domain was started with TLS and is using TLS? (IOW: looking at the running domain for tls-creds-x509). Additionally, someone could have

changed the chardevTLS setting in qemu.conf after the domain was initially started and thus it wouldn't have it, so setting it blindly

would probably have disastrous results. Altering a domain definition during qemuProcessPrepareDomain (e.g. domain startup time) still has no way of determining if that domain had ever been started using TLS if it's not in the XML.

Finally, yeah migration is the final nail in the coffin for this. How does one migrate from 2.4 to 2.3 if 2.3 doesn't have this attribute we just quietly set to YES for them?

I think the != NO is the safest solution (plus some adjustment to the formatdomain.html.in in order to describe this situation). John > I guess a another version would be better if you agree with exposing current > state into live XML. > > Pavel > >>> - if chardev_tls is not set and tls attribute is not configured in the XML >>> the default after starting the domain should be tls=no >>> >> >> We're not getting anywhere if chardev_tls is not set. >> >> John >> >> [...] >> >> -- >> libvir-list mailing list >> libvir-list(a)redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list