From: Chen Hanxiao <chenhanxiao@gmail.com>
As the description of daemon/libvirtd.conf, setting
key_file, cert_file or key_file will override the default value.
But if we set any one of them, we need to set all the rest of them.
This patch set default value to them as daemon/libvirtd.conf
described.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
daemon/libvirtd.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 6d3b83355..93983f63b 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -493,19 +493,28 @@ daemonSetupNetworking(virNetServerPtr srv,
config->cert_file ||
config->key_file) {
if (!config->ca_file) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("No CA certificate path set to match server key/cert"));
- goto cleanup;
+ VIR_WARN("Using default path for ca_file");
+ if (VIR_STRDUP(config->ca_file, LIBVIRT_CACERT) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("No CA certificate path set to match server key/cert"));
+ goto cleanup;
+ }
}
if (!config->cert_file) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("No server certificate path set to match server key"));
- goto cleanup;
+ VIR_WARN("Using default path for cert_file");
+ if (VIR_STRDUP(config->cert_file, LIBVIRT_SERVERCERT) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("No server certificate path set to match server key"));
+ goto cleanup;
+ }
}
if (!config->key_file) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("No server key path set to match server cert"));
- goto cleanup;
+ VIR_WARN("Using default path for key_file");
+ if (VIR_STRDUP(config->key_file, LIBVIRT_SERVERKEY) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("No server key path set to match server cert"));
+ goto cleanup;
+ }
}
VIR_DEBUG("Using CA='%s' cert='%s' key='%s'",
config->ca_file, config->cert_file, config->key_file);
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jan 12, 2018 at 05:09:10PM +0800, Chen Hanxiao wrote: > From: Chen Hanxiao <chenhanxiao@gmail.com> > > As the description of daemon/libvirtd.conf, setting > key_file, cert_file or key_file will override the default value. > But if we set any one of them, we need to set all the rest of them. > > This patch set default value to them as daemon/libvirtd.conf > described. NACK, I don't thing this is a good idea. I would rather fix the description in the config file. If you change one of these values you definitely need to change all of them. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jan 12, 2018 at 17:09:10 +0800, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao@gmail.com>
>
> As the description of daemon/libvirtd.conf, setting
> key_file, cert_file or key_file will override the default value.
> But if we set any one of them, we need to set all the rest of them.
I think this is a reasonable behavior. If a default value is not usable
for one of them, the other will likely need to be changed too.
Although ca_file could be separated. In other words, I can imagine
someone wants to change ca_file but keep default values for
cert_file/key_file or keep default ca_file and override
cert_file/key_file. Overriding cert_file or key_file only without also
changing the other one doesn't make a lot of sense.
Anyway, the patch is incorrect...
> This patch set default value to them as daemon/libvirtd.conf
> described.
>
> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
> ---
> daemon/libvirtd.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 6d3b83355..93983f63b 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -493,19 +493,28 @@ daemonSetupNetworking(virNetServerPtr srv,
> config->cert_file ||
> config->key_file) {
> if (!config->ca_file) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("No CA certificate path set to match server key/cert"));
> - goto cleanup;
> + VIR_WARN("Using default path for ca_file");
> + if (VIR_STRDUP(config->ca_file, LIBVIRT_CACERT) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("No CA certificate path set to match server key/cert"));
This error message doesn't make any sense now. Not to mention you're
overriding the error which was already set by VIR_STRDUP.
> + goto cleanup;
> + }
> }
> if (!config->cert_file) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("No server certificate path set to match server key"));
> - goto cleanup;
> + VIR_WARN("Using default path for cert_file");
> + if (VIR_STRDUP(config->cert_file, LIBVIRT_SERVERCERT) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("No server certificate path set to match server key"));
Dtto.
> + goto cleanup;
> + }
> }
> if (!config->key_file) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("No server key path set to match server cert"));
> - goto cleanup;
> + VIR_WARN("Using default path for key_file");
> + if (VIR_STRDUP(config->key_file, LIBVIRT_SERVERKEY) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("No server key path set to match server cert"));
Dtto.
> + goto cleanup;
> + }
> }
> VIR_DEBUG("Using CA='%s' cert='%s' key='%s'",
> config->ca_file, config->cert_file, config->key_file);
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
At 2018-01-12 17:44:38, "Jiri Denemark" <jdenemar@redhat.com> wrote: >On Fri, Jan 12, 2018 at 17:09:10 +0800, Chen Hanxiao wrote: >> From: Chen Hanxiao <chenhanxiao@gmail.com> >> >> As the description of daemon/libvirtd.conf, setting >> key_file, cert_file or key_file will override the default value. >> But if we set any one of them, we need to set all the rest of them. > >I think this is a reasonable behavior. If a default value is not usable >for one of them, the other will likely need to be changed too. > >Although ca_file could be separated. In other words, I can imagine >someone wants to change ca_file but keep default values for >cert_file/key_file or keep default ca_file and override >cert_file/key_file. Overriding cert_file or key_file only without also >changing the other one doesn't make a lot of sense. > >Anyway, the patch is incorrect... > Thanks for the review. I'll post another patch for the description of the conf. Regards, - Chen -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.