On Wed, Feb 15, 2012 at 10:08:24AM +0100, Christophe Fergeau wrote: > On Tue, Feb 14, 2012 at 02:10:37PM -0700, Eric Blake wrote: > > Meta-question - if the XML requests secure, but TLS is disabled, should > > we instead be failing to start the domain with a complaint that we can't > > honor the XML? > > Meta-non-answer, when a TLS port is set but TLS is disabled in the config > file, it's silently ignored: What value does allowing TLS configuration in qemu.conf add? That seems wrong to me because it creates the possibility of the kind of ambiguity discovered here. Shouldn't the domain XML be the only required statement of the user's intent? Dave > if (driver->spiceTLS && def->graphics[0]->data.spice.tlsPort != -1) > virBufferAsprintf(&opt, ",tls-port=%u", def->graphics[0]->data.spice.tlsPort); > > so I just did the same for the secure channels. If we decide an error > should be returned, this can be done in a follow-up patch changing both > locations. > > > > > If the answer is that we should start the domain anyways, and there is > > just no security, because whoever disabled tls in qemu.conf knows what > > they are doing, then ACK. If the answer is that we should error out on > > an impossible configuration, > > My initial feeling was that we should error out. However, I don't think > it's realistic at this point: > * oVirt is already disabling TLS in qemu.conf if the admin says he does not > want it during engine-setup, but oVirt still adds the TLS port/secure > channels to its libvirt VMs even when TLS is disabled. They probably > won't be that happy if libvirt starts erroring out here. Though we can > also argue that it's a bug on their side, and that the VMs they start this > way (with TLS XML bits but TLS disabled in the conf file) are not usable > anyway because of these extra tls-channel options > * this will also be annoying if someone has setup a bunch of VMs with TLS > parameters and then decides to change the TLS setting in qemu.conf, these > VMs will suddenly fail to start until their conf is modified > > Because of that, I think we should unfortunately just ignore these TLS bits > when TLS is disabled. We can log a warning though when we detect this. Once > again, regardless of what we decide to do, it's better done in a separate > patch. > > Christophe > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list