2:22 p.m.
Remove the odd three-phase defaults filling for TLS cert dirs
and split out the huge function into smaller ones based on
the entry groups in the augeas file.
Ján Tomko (23):
qemu: fix double space in augeas file
qemu: group swtpm entry in augeas file
qemu.conf: fill out TLS verify attributes after parsing
qemu: fill out usage-specific TLS settings after parsing
qemu_conf: split out virQEMUDriverConfigLoadSWTPMEntry
qemu_conf: split out virQEMUDriverConfigLoadMemoryEntry
qemu_conf: split out virQEMUDriverConfigLoadSecurityEntry
qemu_conf: split out virQEMUDriverConfigLoadGlusterDebugEntry
qemu_conf: split out virQEMUDriverConfigLoadNVRAMEntry
qemu_conf: split out virQEMUDriverConfigLoadLogEntry
qemu_conf: split out virQEMUDriverConfigLoadNetworkEntry
qemu_conf: split out virQEMUDriverConfigLoadRPCEntry
qemu_conf: split out virQEMUDriverConfigLoadDeviceEntry
qemu_conf: split out virQEMUDriverConfigLoadProcessEntry
qemu_conf: split out virQEMUDriverConfigLoadSaveEntry
qemu_conf: split out virQEMUDriverConfigLoadRemoteDisplayEntry
qemu_conf: split out virQEMUDriverConfigLoadSpecificTLS
qemu_conf: split out virQEMUDriverConfigLoadSPICEEntry
qemu_conf: split out virQEMUDriverConfigLoadNographicsEntry
qemu_conf: split out virQEMUDriverConfigLoadVNCEntry
qemu_conf: split out virQEMUDriverConfigLoadDefaultTLSEntry
qemu_conf: rename checkdefaultTLSx509certdir
qemu_conf: fix stray space
src/qemu/libvirtd_qemu.aug | 9 +-
src/qemu/qemu_conf.c | 751 +++++++++++++++++++++++--------------
src/qemu/qemu_conf.h | 8 +-
src/qemu/qemu_driver.c | 3 +
4 files changed, 480 insertions(+), 291 deletions(-)
--
2.20.1
2:22 p.m.
New subject: [libvirt] [PATCH 01/23] qemu: fix double space in augeas file
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index ddc4bbfd1d..434ae510a0 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -76,7 +76,7 @@ module Libvirtd_qemu =
| int_entry "seccomp_sandbox"
| str_array_entry "namespaces"
- let save_entry = str_entry "save_image_format"
+ let save_entry = str_entry "save_image_format"
| str_entry "dump_image_format"
| str_entry "snapshot_image_format"
| str_entry "auto_dump_path"
--
2.20.1
1:56 p.m.
New subject: [libvirt] [PATCH 01/23] qemu: fix double space in augeas file
On 1/15/19 8:22 AM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Trivial,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:22 p.m.
New subject: [libvirt] [PATCH 02/23] qemu: group swtpm entry in augeas file
They are meant to be together.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 434ae510a0..28bd851411 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -122,8 +122,8 @@ module Libvirtd_qemu =
let nbd_entry = bool_entry "nbd_tls"
| str_entry "nbd_tls_x509_cert_dir"
- let swtpm_user_entry = str_entry "swtpm_user"
- let swtpm_group_entry = str_entry "swtpm_group"
+ let swtpm_entry = str_entry "swtpm_user"
+ | str_entry "swtpm_group"
(* Each entry in the config is one of the following ... *)
let entry = default_tls_entry
@@ -145,8 +145,7 @@ module Libvirtd_qemu =
| memory_entry
| vxhs_entry
| nbd_entry
- | swtpm_user_entry
- | swtpm_group_entry
+ | swtpm_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store
/([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
--
2.20.1
1:56 p.m.
New subject: [libvirt] [PATCH 02/23] qemu: group swtpm entry in augeas file
On 1/15/19 8:22 AM, Ján Tomko wrote:
They are meant to be together.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
Yes, they are.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
John
2:22 p.m.
New subject: [libvirt] [PATCH 03/23] qemu.conf: fill out TLS verify attributes after parsing
Introduce a set of bool variables with the 'present' suffix
to track whether the value was actually specified.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 28 ++++++++++++++++++++++++----
src/qemu/qemu_conf.h | 6 ++++++
src/qemu/qemu_driver.c | 3 +++
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b03e38b831..a0855032e5 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -535,8 +535,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if ((rv = virConfGetValueBool(conf, "vnc_tls_x509_verify",
&cfg->vncTLSx509verify)) < 0)
goto cleanup;
- if (rv == 0)
- cfg->vncTLSx509verify = cfg->defaultTLSx509verify;
+ if (rv == 1)
+ cfg->vncTLSx509verifyPresent = true;
if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir",
&cfg->vncTLSx509certdir) < 0)
goto cleanup;
if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) <
0)
@@ -601,8 +601,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \
&cfg->val## TLSx509verify)) < 0) \
goto cleanup; \
- if (rv == 0) \
- cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \
+ if (rv == 1) \
+ cfg->val## TLSx509verifyPresent = true; \
if ((rv = virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
&cfg->val## TLSx509certdir)) < 0) \
goto cleanup; \
@@ -1055,6 +1055,26 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return 0;
}
+int
+virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
+{
+ int ret = -1;
+
+#define SET_TLS_VERIFY_DEFAULT(val) \
+ do { \
+ if (!cfg->val## TLSx509verifyPresent) \
+ cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \
+ } while (0)
+
+ SET_TLS_VERIFY_DEFAULT(vnc);
+ SET_TLS_VERIFY_DEFAULT(chardev);
+ SET_TLS_VERIFY_DEFAULT(migrate);
+
+#undef SET_TLS_VERIFY_DEFAULT
+
+ ret = 0;
+ return ret;
+}
virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
{
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 1013cfcaed..87e730058b 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -122,6 +122,7 @@ struct _virQEMUDriverConfig {
bool vncAutoUnixSocket;
bool vncTLS;
bool vncTLSx509verify;
+ bool vncTLSx509verifyPresent;
bool vncSASL;
char *vncTLSx509certdir;
char *vncListen;
@@ -139,10 +140,12 @@ struct _virQEMUDriverConfig {
bool chardevTLS;
char *chardevTLSx509certdir;
bool chardevTLSx509verify;
+ bool chardevTLSx509verifyPresent;
char *chardevTLSx509secretUUID;
char *migrateTLSx509certdir;
bool migrateTLSx509verify;
+ bool migrateTLSx509verifyPresent;
char *migrateTLSx509secretUUID;
unsigned int remotePortMin;
@@ -317,6 +320,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
int
virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg);
+int
+virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg);
+
virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver);
bool virQEMUDriverIsPrivileged(virQEMUDriverPtr driver);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d961707cc..5032edec50 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -628,6 +628,9 @@ qemuStateInitialize(bool privileged,
if (virQEMUDriverConfigValidate(cfg) < 0)
goto error;
+ if (virQEMUDriverConfigSetDefaults(cfg) < 0)
+ goto error;
+
if (virFileMakePath(cfg->stateDir) < 0) {
virReportSystemError(errno, _("Failed to create state dir %s"),
cfg->stateDir);
--
2.20.1
1:56 p.m.
New subject: [libvirt] [PATCH 03/23] qemu.conf: fill out TLS verify attributes after parsing
On 1/15/19 8:22 AM, Ján Tomko wrote:
Introduce a set of bool variables with the 'present' suffix
to track whether the value was actually specified.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 28 ++++++++++++++++++++++++----
src/qemu/qemu_conf.h | 6 ++++++
src/qemu/qemu_driver.c | 3 +++
3 files changed, 33 insertions(+), 4 deletions(-)
6 of one, half-dozen of another.
There should be two empty lines before/after the new method below even
though it's not the norm in the file.
And yes, the ret = 0; return ret; looks odd now, but I see usage in next
patch...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:23 p.m.
New subject: [libvirt] [PATCH 04/23] qemu: fill out usage-specific TLS settings after parsing
Instead of copying the default default values upfront
and then wondering whether the user has given us a new default,
leave the per-usage TLS certdirs and secrets empty during
parsing and only fill them afterwards if they weren't provided
by the user.
This means that instead of looking whether the specific certdir
paths match the default default, the Validate function (which
is called inbetween parsing and setting the defaults) can error
out for missing directories if the value is present, because
it must've come from the user.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 150 ++++++++++++++++---------------------------
1 file changed, 56 insertions(+), 94 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a0855032e5..837bff6b30 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -277,35 +277,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
if (VIR_STRDUP(cfg->spiceListen, VIR_LOOPBACK_IPV4_ADDR) < 0)
goto error;
- /*
- * If a "SYSCONFDIR" + "pki/libvirt-<val>" exists, then
assume someone
- * has created a val specific area to place service specific certificates.
- *
- * If the service specific directory doesn't exist, 'assume' that the
- * user has created and populated the "SYSCONFDIR" +
"pki/libvirt-default".
- */
-#define SET_TLS_X509_CERT_DEFAULT(val) \
- do { \
- if (virFileExists(SYSCONFDIR "/pki/libvirt-"#val)) { \
- if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
- SYSCONFDIR "/pki/libvirt-"#val) < 0) \
- goto error; \
- } else { \
- if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
- cfg->defaultTLSx509certdir) < 0) \
- goto error; \
- } \
- } while (0)
-
- SET_TLS_X509_CERT_DEFAULT(vnc);
- SET_TLS_X509_CERT_DEFAULT(spice);
- SET_TLS_X509_CERT_DEFAULT(chardev);
- SET_TLS_X509_CERT_DEFAULT(migrate);
- SET_TLS_X509_CERT_DEFAULT(vxhs);
- SET_TLS_X509_CERT_DEFAULT(nbd);
-
-#undef SET_TLS_X509_CERT_DEFAULT
-
cfg->remotePortMin = QEMU_REMOTE_PORT_MIN;
cfg->remotePortMax = QEMU_REMOTE_PORT_MAX;
@@ -452,45 +423,6 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
-/**
- * @cfg: Just read config TLS values
- *
- * If the default_tls_x509_cert_dir was uncommented or changed from
- * the default value assigned to the *_tls_x509_cert_dir values when
- * virQEMUDriverConfigNew was executed, we need to check if we need
- * to update the other defaults.
- *
- * Returns 0 on success, -1 on failure
- */
-static int
-virQEMUDriverConfigTLSDirResetDefaults(virQEMUDriverConfigPtr cfg)
-{
- /* Not changed or set to the default default, nothing to do */
- if (!cfg->checkdefaultTLSx509certdir ||
- STREQ(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu"))
- return 0;
-
-#define CHECK_RESET_CERT_DIR_DEFAULT(val) \
- do { \
- if (STREQ(cfg->val ## TLSx509certdir, SYSCONFDIR "/pki/qemu")) { \
- VIR_FREE(cfg->val ## TLSx509certdir); \
- if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
- cfg->defaultTLSx509certdir) < 0) \
- return -1; \
- } \
- } while (0)
-
- CHECK_RESET_CERT_DIR_DEFAULT(vnc);
- CHECK_RESET_CERT_DIR_DEFAULT(spice);
- CHECK_RESET_CERT_DIR_DEFAULT(chardev);
- CHECK_RESET_CERT_DIR_DEFAULT(migrate);
- CHECK_RESET_CERT_DIR_DEFAULT(vxhs);
- CHECK_RESET_CERT_DIR_DEFAULT(nbd);
-
- return 0;
-}
-
-
int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
const char *filename,
bool privileged)
@@ -603,23 +535,13 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup; \
if (rv == 1) \
cfg->val## TLSx509verifyPresent = true; \
- if ((rv = virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
- &cfg->val## TLSx509certdir)) < 0) \
+ if (virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
+ &cfg->val## TLSx509certdir) < 0) \
goto cleanup; \
if (virConfGetValueString(conf, \
#val "_tls_x509_secret_uuid", \
&cfg->val## TLSx509secretUUID) < 0) \
goto cleanup; \
- /* Only if a *tls_x509_cert_dir wasn't found (e.g. rv == 0), should \
- * we copy the defaultTLSx509secretUUID. If this environment needs \
- * a passphrase to decode the certificate, then it should provide \
- * it's own secretUUID for that. */ \
- if (rv == 0 && !cfg->val## TLSx509secretUUID && \
- cfg->defaultTLSx509secretUUID) { \
- if (VIR_STRDUP(cfg->val## TLSx509secretUUID, \
- cfg->defaultTLSx509secretUUID) < 0) \
- goto cleanup; \
- } \
} while (0)
if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) <
0)
@@ -630,9 +552,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
#undef GET_CONFIG_TLS_CERTINFO
- if (virQEMUDriverConfigTLSDirResetDefaults(cfg) < 0)
- goto cleanup;
-
if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
goto cleanup;
if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
@@ -989,7 +908,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
int
virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
{
- /* If the default entry was uncommented, then validate existence */
if (cfg->checkdefaultTLSx509certdir) {
if (!virFileExists(cfg->defaultTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
@@ -1000,11 +918,7 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
}
}
- /* For each of the others - if the value is not to the default default
- * then check if the directory exists (this may duplicate the check done
- * during virQEMUDriverConfigNew).
- */
- if (STRNEQ(cfg->vncTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+ if (cfg->vncTLSx509certdir &&
!virFileExists(cfg->vncTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("vnc_tls_x509_cert_dir directory '%s' does not
exist"),
@@ -1012,7 +926,7 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return -1;
}
- if (STRNEQ(cfg->spiceTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+ if (cfg->spiceTLSx509certdir &&
!virFileExists(cfg->spiceTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("spice_tls_x509_cert_dir directory '%s' does not
exist"),
@@ -1020,7 +934,7 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return -1;
}
- if (STRNEQ(cfg->chardevTLSx509certdir, SYSCONFDIR "/pki/qemu")
&&
+ if (cfg->chardevTLSx509certdir &&
!virFileExists(cfg->chardevTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("chardev_tls_x509_cert_dir directory '%s' does not
exist"),
@@ -1028,7 +942,7 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return -1;
}
- if (STRNEQ(cfg->migrateTLSx509certdir, SYSCONFDIR "/pki/qemu")
&&
+ if (cfg->migrateTLSx509certdir &&
!virFileExists(cfg->migrateTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("migrate_tls_x509_cert_dir directory '%s' does not
exist"),
@@ -1036,7 +950,7 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return -1;
}
- if (STRNEQ(cfg->vxhsTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+ if (cfg->vxhsTLSx509certdir &&
!virFileExists(cfg->vxhsTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("vxhs_tls_x509_cert_dir directory '%s' does not
exist"),
@@ -1044,7 +958,7 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
return -1;
}
- if (STRNEQ(cfg->nbdTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+ if (cfg->nbdTLSx509certdir &&
!virFileExists(cfg->nbdTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("nbd_tls_x509_cert_dir directory '%s' does not
exist"),
@@ -1060,6 +974,53 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
{
int ret = -1;
+#define SET_TLS_SECRET_UUID_DEFAULT(val) \
+ do { \
+ if (!cfg->val## TLSx509certdir && \
+ !cfg->val## TLSx509secretUUID && \
+ cfg->defaultTLSx509secretUUID) { \
+ if (VIR_STRDUP(cfg->val## TLSx509secretUUID, \
+ cfg->defaultTLSx509secretUUID) < 0) \
+ goto cleanup; \
+ } \
+ } while (0)
+
+ SET_TLS_SECRET_UUID_DEFAULT(chardev);
+ SET_TLS_SECRET_UUID_DEFAULT(migrate);
+
+#undef SET_TLS_SECRET_UUID_DEFAULT
+
+ /*
+ * If a "SYSCONFDIR" + "pki/libvirt-<val>" exists, then
assume someone
+ * has created a val specific area to place service specific certificates.
+ *
+ * If the service specific directory doesn't exist, 'assume' that the
+ * user has created and populated the "SYSCONFDIR" +
"pki/libvirt-default".
+ */
+#define SET_TLS_X509_CERT_DEFAULT(val) \
+ do { \
+ if (cfg->val ## TLSx509certdir) \
+ break; \
+ if (virFileExists(SYSCONFDIR "/pki/libvirt-"#val)) { \
+ if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
+ SYSCONFDIR "/pki/libvirt-"#val) < 0) \
+ goto cleanup; \
+ } else { \
+ if (VIR_STRDUP(cfg->val ## TLSx509certdir, \
+ cfg->defaultTLSx509certdir) < 0) \
+ goto cleanup; \
+ } \
+ } while (0)
+
+ SET_TLS_X509_CERT_DEFAULT(vnc);
+ SET_TLS_X509_CERT_DEFAULT(spice);
+ SET_TLS_X509_CERT_DEFAULT(chardev);
+ SET_TLS_X509_CERT_DEFAULT(migrate);
+ SET_TLS_X509_CERT_DEFAULT(vxhs);
+ SET_TLS_X509_CERT_DEFAULT(nbd);
+
+#undef SET_TLS_X509_CERT_DEFAULT
+
#define SET_TLS_VERIFY_DEFAULT(val) \
do { \
if (!cfg->val## TLSx509verifyPresent) \
@@ -1073,6 +1034,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg)
#undef SET_TLS_VERIFY_DEFAULT
ret = 0;
+ cleanup:
return ret;
}
--
2.20.1
1:57 p.m.
New subject: [libvirt] [PATCH 04/23] qemu: fill out usage-specific TLS settings after parsing
On 1/15/19 8:23 AM, Ján Tomko wrote:
Instead of copying the default default values upfront
and then wondering whether the user has given us a new default,
leave the per-usage TLS certdirs and secrets empty during
parsing and only fill them afterwards if they weren't provided
by the user.
This means that instead of looking whether the specific certdir
paths match the default default, the Validate function (which
is called inbetween parsing and setting the defaults) can error
in between
out for missing directories if the value is present, because
it must've come from the user.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 150 ++++++++++++++++---------------------------
1 file changed, 56 insertions(+), 94 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:23 p.m.
New subject: [libvirt] [PATCH 05/23] qemu_conf: split out virQEMUDriverConfigLoadSWTPMEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 837bff6b30..325e5ccfd5 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,30 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ char *swtpm_user = NULL, *swtpm_group = NULL;
+ int ret = -1;
+
+ if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0)
+ goto cleanup;
+ if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0)
+ goto cleanup;
+
+ if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0)
+ goto cleanup;
+ if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) <
0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(swtpm_user);
+ VIR_FREE(swtpm_group);
+ return ret;
+}
+
int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
const char *filename,
bool privileged)
@@ -433,7 +457,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
size_t i, j;
char *stdioHandler = NULL;
char *user = NULL, *group = NULL;
- char *swtpm_user = NULL, *swtpm_group = NULL;
char **controllers = NULL;
char **hugetlbfs = NULL;
char **nvram = NULL;
@@ -871,14 +894,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueString(conf, "memory_backing_dir",
&cfg->memoryBackingDir) < 0)
goto cleanup;
- if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0)
- goto cleanup;
- if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0)
- goto cleanup;
-
- if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0)
- goto cleanup;
- if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) <
0)
+ if (virQEMUDriverConfigLoadSWTPMEntry(cfg, conf) < 0)
goto cleanup;
ret = 0;
@@ -891,8 +907,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
VIR_FREE(corestr);
VIR_FREE(user);
VIR_FREE(group);
- VIR_FREE(swtpm_user);
- VIR_FREE(swtpm_group);
virConfFree(conf);
return ret;
}
--
2.20.1
2:03 p.m.
New subject: [libvirt] [PATCH 05/23] qemu_conf: split out virQEMUDriverConfigLoadSWTPMEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 837bff6b30..325e5ccfd5 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,30 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ char *swtpm_user = NULL, *swtpm_group = NULL;
Some would note these should be on separate lines, but I'll note that
since we have it, these should be:
VIR_AUTOPTR(char *) swtpm_user = NULL;
VIR_AUTOPTR(char *) swtpm_group = NULL;
+ int ret = -1;
+
+ if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0)
+ goto cleanup;
+ if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0)
+ goto cleanup;
+
+ if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0)
+ goto cleanup;
+ if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) <
0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(swtpm_user);
+ VIR_FREE(swtpm_group);
Thus freeing you of needing this and perhaps changing your logic above.
I "understand" the "norm" is to just purely copy and then have
another
patch, but I really believe in this case it's so obvious that a separate
patch isn't required especially since VIR_AUTOFREE is more commonly used.
+ return ret;
+}
+
one more blank line. If you feel you must create two patches out of
this one patch, then go ahead, I trust that you can do that, but I think
we really should use the VIR_AUTOPTR more now...
BTW: I have a feeling VIR_AUTOPTR and 2 empty lines will become a
recurring theme...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
12:56 p.m.
New subject: [libvirt] [PATCH 05/23] qemu_conf: split out virQEMUDriverConfigLoadSWTPMEntry
On Thu, Jan 17, 2019 at 08:03:44AM -0500, John Ferlan wrote:
On 1/15/19 8:23 AM, Ján Tomko wrote:
> Split out parts of the config parsing code to make
> the parent function easier to read.
>
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 837bff6b30..325e5ccfd5 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -423,6 +423,30 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
> }
>
>
> +static int
> +virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg,
> + virConfPtr conf)
> +{
> + char *swtpm_user = NULL, *swtpm_group = NULL;
Some would note these should be on separate lines, but I'll note that
since we have it, these should be:
VIR_AUTOPTR(char *) swtpm_user = NULL;
VIR_AUTOPTR(char *) swtpm_group = NULL;
> + int ret = -1;
> +
> + if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) <
0)
> + goto cleanup;
> + if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) <
0)
> + goto cleanup;
> +
> + if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) <
0)
> + goto cleanup;
> + if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group)
< 0)
> + goto cleanup;
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(swtpm_user);
> + VIR_FREE(swtpm_group);
Thus freeing you of needing this and perhaps changing your logic above.
I "understand" the "norm" is to just purely copy and then have
another
patch, but I really believe in this case it's so obvious that a separate
patch isn't required especially since VIR_AUTOFREE is more commonly used.
Yes, I'd rather not include any functional changes here (other than
those necessary to deal with the (lack of a) cleanup section.
Jano
1:39 p.m.
New subject: [libvirt] [PATCH 05/23] qemu_conf: split out virQEMUDriverConfigLoadSWTPMEntry
On 1/18/19 6:56 AM, Ján Tomko wrote:
On Thu, Jan 17, 2019 at 08:03:44AM -0500, John Ferlan wrote:
>
>
> On 1/15/19 8:23 AM, Ján Tomko wrote:
>> Split out parts of the config parsing code to make
>> the parent function easier to read.
>>
>> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
>> ---
>> src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++-----------
>> 1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 837bff6b30..325e5ccfd5 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -423,6 +423,30 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr
>> hugetlbfs,
>> }
>>
>>
>> +static int
>> +virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg,
>> + virConfPtr conf)
>> +{
>> + char *swtpm_user = NULL, *swtpm_group = NULL;
>
> Some would note these should be on separate lines, but I'll note that
> since we have it, these should be:
>
> VIR_AUTOPTR(char *) swtpm_user = NULL;
> VIR_AUTOPTR(char *) swtpm_group = NULL;
>
>> + int ret = -1;
>> +
>> + if (virConfGetValueString(conf, "swtpm_user", &swtpm_user)
< 0)
>> + goto cleanup;
>> + if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user)
< 0)
>> + goto cleanup;
>> +
>> + if (virConfGetValueString(conf, "swtpm_group", &swtpm_group)
< 0)
>> + goto cleanup;
>> + if (swtpm_group && virGetGroupID(swtpm_group,
&cfg->swtpm_group)
>> < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> + cleanup:
>> + VIR_FREE(swtpm_user);
>> + VIR_FREE(swtpm_group);
>
> Thus freeing you of needing this and perhaps changing your logic above.
>
> I "understand" the "norm" is to just purely copy and then have
another
> patch, but I really believe in this case it's so obvious that a separate
> patch isn't required especially since VIR_AUTOFREE is more commonly used.
>
Yes, I'd rather not include any functional changes here (other than
those necessary to deal with the (lack of a) cleanup section.
Jano
So I see you left the AUTOPTR/AUTOFREE to some future change... Is that
in your short term plan or waiting for some bite size libvirt first task
to complete?
John
2:23 p.m.
New subject: [libvirt] [PATCH 06/23] qemu_conf: split out virQEMUDriverConfigLoadMemoryEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 325e5ccfd5..7fdfed7db1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,16 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadMemoryEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueString(conf, "memory_backing_dir",
&cfg->memoryBackingDir) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -891,7 +901,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
}
}
- if (virConfGetValueString(conf, "memory_backing_dir",
&cfg->memoryBackingDir) < 0)
+ if (virQEMUDriverConfigLoadMemoryEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadSWTPMEntry(cfg, conf) < 0)
--
2.20.1
2:06 p.m.
New subject: [libvirt] [PATCH 06/23] qemu_conf: split out virQEMUDriverConfigLoadMemoryEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 325e5ccfd5..7fdfed7db1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,16 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadMemoryEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueString(conf, "memory_backing_dir",
&cfg->memoryBackingDir) < 0)
+ return -1;
+
+ return 0;
return virConfGetValueString(conf, "memory_backing_dir",
&cfg->memoryBackingDir);
+}
+
Add an extra blank line here...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Although this one's almost pointless, but for consistency, fine.
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 07/23] qemu_conf: split out virQEMUDriverConfigLoadSecurityEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 219 +++++++++++++++++++++++--------------------
1 file changed, 117 insertions(+), 102 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7fdfed7db1..135cb9e25d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,121 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf,
+ bool privileged)
+{
+ char *user = NULL, *group = NULL;
+ char **controllers = NULL;
+ char **namespaces = NULL;
+ int ret = -1;
+ size_t i, j;
+
+ if (virConfGetValueStringList(conf, "security_driver", true,
&cfg->securityDriverNames) < 0)
+ goto cleanup;
+
+ for (i = 0; cfg->securityDriverNames && cfg->securityDriverNames[i] !=
NULL; i++) {
+ for (j = i + 1; cfg->securityDriverNames[j] != NULL; j++) {
+ if (STREQ(cfg->securityDriverNames[i],
+ cfg->securityDriverNames[j])) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Duplicate security driver %s"),
+ cfg->securityDriverNames[i]);
+ goto cleanup;
+ }
+ }
+ }
+
+ if (virConfGetValueBool(conf, "security_default_confined",
&cfg->securityDefaultConfined) < 0)
+ goto cleanup;
+ if (virConfGetValueBool(conf, "security_require_confined",
&cfg->securityRequireConfined) < 0)
+ goto cleanup;
+ if (virConfGetValueString(conf, "user", &user) < 0)
+ goto cleanup;
+ if (user && virGetUserID(user, &cfg->user) < 0)
+ goto cleanup;
+
+ if (virConfGetValueString(conf, "group", &group) < 0)
+ goto cleanup;
+ if (group && virGetGroupID(group, &cfg->group) < 0)
+ goto cleanup;
+
+ if (virConfGetValueBool(conf, "dynamic_ownership",
&cfg->dynamicOwnership) < 0)
+ goto cleanup;
+
+ if (virConfGetValueStringList(conf, "cgroup_controllers", false,
+ &controllers) < 0)
+ goto cleanup;
+
+ if (controllers) {
+ cfg-> cgroupControllers = 0;
+ for (i = 0; controllers[i] != NULL; i++) {
+ int ctl;
+ if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Unknown cgroup controller '%s'"),
+ controllers[i]);
+ goto cleanup;
+ }
+ cfg->cgroupControllers |= (1 << ctl);
+ }
+ }
+
+ if (virConfGetValueStringList(conf, "cgroup_device_acl", false,
+ &cfg->cgroupDeviceACL) < 0)
+ goto cleanup;
+
+ if (virConfGetValueInt(conf, "seccomp_sandbox",
&cfg->seccompSandbox) < 0)
+ goto cleanup;
+
+ if (virConfGetValueStringList(conf, "namespaces", false, &namespaces)
< 0)
+ goto cleanup;
+
+ if (namespaces) {
+ virBitmapClearAll(cfg->namespaces);
+
+ for (i = 0; namespaces[i]; i++) {
+ int ns = qemuDomainNamespaceTypeFromString(namespaces[i]);
+
+ if (ns < 0) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Unknown namespace: %s"),
+ namespaces[i]);
+ goto cleanup;
+ }
+
+ if (!privileged) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cannot use namespaces in session mode"));
+ goto cleanup;
+ }
+
+ if (!qemuDomainNamespaceAvailable(ns)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s namespace is not available"),
+ namespaces[i]);
+ goto cleanup;
+ }
+
+ if (virBitmapSetBit(cfg->namespaces, ns) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to enable namespace: %s"),
+ namespaces[i]);
+ goto cleanup;
+ }
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(controllers);
+ virStringListFree(namespaces);
+ VIR_FREE(user);
+ VIR_FREE(group);
+ return ret;
+}
+
static int
virQEMUDriverConfigLoadMemoryEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -464,14 +579,11 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
virConfPtr conf = NULL;
int ret = -1;
int rv;
- size_t i, j;
+ size_t i;
char *stdioHandler = NULL;
- char *user = NULL, *group = NULL;
- char **controllers = NULL;
char **hugetlbfs = NULL;
char **nvram = NULL;
char *corestr = NULL;
- char **namespaces = NULL;
bool tmp;
/* Just check the file is readable before opening it, otherwise
@@ -518,26 +630,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
- if (virConfGetValueStringList(conf, "security_driver", true,
&cfg->securityDriverNames) < 0)
- goto cleanup;
-
- for (i = 0; cfg->securityDriverNames && cfg->securityDriverNames[i] !=
NULL; i++) {
- for (j = i + 1; cfg->securityDriverNames[j] != NULL; j++) {
- if (STREQ(cfg->securityDriverNames[i],
- cfg->securityDriverNames[j])) {
- virReportError(VIR_ERR_CONF_SYNTAX,
- _("Duplicate security driver %s"),
- cfg->securityDriverNames[i]);
- goto cleanup;
- }
- }
- }
-
- if (virConfGetValueBool(conf, "security_default_confined",
&cfg->securityDefaultConfined) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "security_require_confined",
&cfg->securityRequireConfined) < 0)
- goto cleanup;
-
if (virConfGetValueBool(conf, "spice_tls", &cfg->spiceTLS) < 0)
goto cleanup;
if (virConfGetValueString(conf, "spice_tls_x509_cert_dir",
&cfg->spiceTLSx509certdir) < 0)
@@ -667,41 +759,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- if (virConfGetValueString(conf, "user", &user) < 0)
- goto cleanup;
- if (user && virGetUserID(user, &cfg->user) < 0)
- goto cleanup;
-
- if (virConfGetValueString(conf, "group", &group) < 0)
- goto cleanup;
- if (group && virGetGroupID(group, &cfg->group) < 0)
- goto cleanup;
-
- if (virConfGetValueBool(conf, "dynamic_ownership",
&cfg->dynamicOwnership) < 0)
- goto cleanup;
-
- if (virConfGetValueStringList(conf, "cgroup_controllers", false,
- &controllers) < 0)
- goto cleanup;
-
- if (controllers) {
- cfg-> cgroupControllers = 0;
- for (i = 0; controllers[i] != NULL; i++) {
- int ctl;
- if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) {
- virReportError(VIR_ERR_CONF_SYNTAX,
- _("Unknown cgroup controller '%s'"),
- controllers[i]);
- goto cleanup;
- }
- cfg->cgroupControllers |= (1 << ctl);
- }
- }
-
- if (virConfGetValueStringList(conf, "cgroup_device_acl", false,
- &cfg->cgroupDeviceACL) < 0)
- goto cleanup;
-
if (virConfGetValueString(conf, "save_image_format",
&cfg->saveImageFormat) < 0)
goto cleanup;
if (virConfGetValueString(conf, "dump_image_format",
&cfg->dumpImageFormat) < 0)
@@ -812,9 +869,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueUInt(conf, "keepalive_count",
&cfg->keepAliveCount) < 0)
goto cleanup;
- if (virConfGetValueInt(conf, "seccomp_sandbox",
&cfg->seccompSandbox) < 0)
- goto cleanup;
-
if (virConfGetValueString(conf, "migration_host", &cfg->migrateHost)
< 0)
goto cleanup;
virStringStripIPv6Brackets(cfg->migrateHost);
@@ -863,44 +917,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueUInt(conf, "gluster_debug_level",
&cfg->glusterDebugLevel) < 0)
goto cleanup;
- if (virConfGetValueStringList(conf, "namespaces", false, &namespaces)
< 0)
+ if (virQEMUDriverConfigLoadSecurityEntry(cfg, conf, privileged) < 0)
goto cleanup;
- if (namespaces) {
- virBitmapClearAll(cfg->namespaces);
-
- for (i = 0; namespaces[i]; i++) {
- int ns = qemuDomainNamespaceTypeFromString(namespaces[i]);
-
- if (ns < 0) {
- virReportError(VIR_ERR_CONF_SYNTAX,
- _("Unknown namespace: %s"),
- namespaces[i]);
- goto cleanup;
- }
-
- if (!privileged) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("cannot use namespaces in session mode"));
- goto cleanup;
- }
-
- if (!qemuDomainNamespaceAvailable(ns)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("%s namespace is not available"),
- namespaces[i]);
- goto cleanup;
- }
-
- if (virBitmapSetBit(cfg->namespaces, ns) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to enable namespace: %s"),
- namespaces[i]);
- goto cleanup;
- }
- }
- }
-
if (virQEMUDriverConfigLoadMemoryEntry(cfg, conf) < 0)
goto cleanup;
@@ -910,13 +929,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
ret = 0;
cleanup:
- virStringListFree(namespaces);
- virStringListFree(controllers);
virStringListFree(hugetlbfs);
virStringListFree(nvram);
VIR_FREE(corestr);
- VIR_FREE(user);
- VIR_FREE(group);
virConfFree(conf);
return ret;
}
--
2.20.1
2:21 p.m.
New subject: [libvirt] [PATCH 07/23] qemu_conf: split out virQEMUDriverConfigLoadSecurityEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 219 +++++++++++++++++++++++--------------------
1 file changed, 117 insertions(+), 102 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7fdfed7db1..135cb9e25d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,121 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf,
+ bool privileged)
This does security, cgroups, and namespaces...
+{
+ char *user = NULL, *group = NULL;
VIR_AUTOPTR(char *)
+ char **controllers = NULL;
+ char **namespaces = NULL;
VIR_AUTOPTR(virString)
+ int ret = -1;
+ size_t i, j;
+
+ if (virConfGetValueStringList(conf, "security_driver", true,
&cfg->securityDriverNames) < 0)
+ goto cleanup;
+
+ for (i = 0; cfg->securityDriverNames && cfg->securityDriverNames[i] !=
NULL; i++) {
+ for (j = i + 1; cfg->securityDriverNames[j] != NULL; j++) {
+ if (STREQ(cfg->securityDriverNames[i],
+ cfg->securityDriverNames[j])) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Duplicate security driver %s"),
+ cfg->securityDriverNames[i]);
+ goto cleanup;
+ }
+ }
+ }
+
+ if (virConfGetValueBool(conf, "security_default_confined",
&cfg->securityDefaultConfined) < 0)
+ goto cleanup;
+ if (virConfGetValueBool(conf, "security_require_confined",
&cfg->securityRequireConfined) < 0)
+ goto cleanup;
nit: blank line for readability/separation
+ if (virConfGetValueString(conf, "user", &user)
< 0)
+ goto cleanup;
+ if (user && virGetUserID(user, &cfg->user) < 0)
+ goto cleanup;
+
+ if (virConfGetValueString(conf, "group", &group) < 0)
+ goto cleanup;
+ if (group && virGetGroupID(group, &cfg->group) < 0)
+ goto cleanup;
+
+ if (virConfGetValueBool(conf, "dynamic_ownership",
&cfg->dynamicOwnership) < 0)
+ goto cleanup;
+
The following hunk feels separable since it's not security related...
+ if (virConfGetValueStringList(conf,
"cgroup_controllers", false,
^^
Oh look an extra space (existing)... may as well fix it, but a separate
patch is not needed.
+ &controllers) < 0)
+ goto cleanup;
+
+ if (controllers) {
+ cfg-> cgroupControllers = 0;
+ for (i = 0; controllers[i] != NULL; i++) {
+ int ctl;
+ if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("Unknown cgroup controller '%s'"),
+ controllers[i]);
+ goto cleanup;
+ }
+ cfg->cgroupControllers |= (1 << ctl);
+ }
+ }
+
+ if (virConfGetValueStringList(conf, "cgroup_device_acl", false,
^^
and again copy-paste probably
+ &cfg->cgroupDeviceACL) <
0)
+ goto cleanup;
end cgroup separable hunk
+> + if (virConfGetValueInt(conf, "seccomp_sandbox",
&cfg->seccompSandbox) < 0)
+ goto cleanup;
+
And again, not security related.
Still, for the concept of splitting it's fine. I trust you can
manipulate a bit more for a final result, so
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
1:03 p.m.
New subject: [libvirt] [PATCH 07/23] qemu_conf: split out virQEMUDriverConfigLoadSecurityEntry
On Thu, Jan 17, 2019 at 08:21:00AM -0500, John Ferlan wrote:
On 1/15/19 8:23 AM, Ján Tomko wrote:
> Split out parts of the config parsing code to make
> the parent function easier to read.
>
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> src/qemu/qemu_conf.c | 219 +++++++++++++++++++++++--------------------
> 1 file changed, 117 insertions(+), 102 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 7fdfed7db1..135cb9e25d 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -423,6 +423,121 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
> }
>
>
> +static int
> +virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
> + virConfPtr conf,
> + bool privileged)
This does security, cgroups, and namespaces...
The division is based on src/qemu/libvirtd_qemu.aug
[...]
> +> + if (virConfGetValueInt(conf,
"seccomp_sandbox",
&cfg->seccompSandbox) < 0)
> + goto cleanup;
> +
And again, not security related.
How is seccomp not security related?
Jano
1:30 p.m.
New subject: [libvirt] [PATCH 07/23] qemu_conf: split out virQEMUDriverConfigLoadSecurityEntry
On 1/18/19 7:03 AM, Ján Tomko wrote:
On Thu, Jan 17, 2019 at 08:21:00AM -0500, John Ferlan wrote:
>
>
> On 1/15/19 8:23 AM, Ján Tomko wrote:
>> Split out parts of the config parsing code to make
>> the parent function easier to read.
>>
>> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
>> ---
>> src/qemu/qemu_conf.c | 219 +++++++++++++++++++++++--------------------
>> 1 file changed, 117 insertions(+), 102 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 7fdfed7db1..135cb9e25d 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -423,6 +423,121 @@
>> virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
>> }
>>
>>
>> +static int
>> +virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
>> + virConfPtr conf,
>> + bool privileged)
>
> This does security, cgroups, and namespaces...
>
The division is based on src/qemu/libvirtd_qemu.aug
[...]
>> +> + if (virConfGetValueInt(conf, "seccomp_sandbox",
> &cfg->seccompSandbox) < 0)
>> + goto cleanup;
>> +
>
> And again, not security related.
>
How is seccomp not security related?
Jano
Bad cut/snip by me - I meant after seccomp, as in the namespace stuff.
John
2:23 p.m.
New subject: [libvirt] [PATCH 08/23] qemu_conf: split out virQEMUDriverConfigLoadGlusterDebugEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 135cb9e25d..ca31a222e8 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,15 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadGlusterDebugEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueUInt(conf, "gluster_debug_level",
&cfg->glusterDebugLevel) < 0)
+ return -1;
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf,
@@ -914,7 +923,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
}
- if (virConfGetValueUInt(conf, "gluster_debug_level",
&cfg->glusterDebugLevel) < 0)
+
+ if (virQEMUDriverConfigLoadGlusterDebugEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadSecurityEntry(cfg, conf, privileged) < 0)
--
2.20.1
2:22 p.m.
New subject: [libvirt] [PATCH 08/23] qemu_conf: split out virQEMUDriverConfigLoadGlusterDebugEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 135cb9e25d..ca31a222e8 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,15 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadGlusterDebugEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueUInt(conf, "gluster_debug_level",
&cfg->glusterDebugLevel) < 0)
+ return -1;
+ return 0;
return virConfGetValueUInt(...)
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
static int
virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf,
@@ -914,7 +923,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
}
- if (virConfGetValueUInt(conf, "gluster_debug_level",
&cfg->glusterDebugLevel) < 0)
+
+ if (virQEMUDriverConfigLoadGlusterDebugEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadSecurityEntry(cfg, conf, privileged) < 0)
2:23 p.m.
New subject: [libvirt] [PATCH 09/23] qemu_conf: split out virQEMUDriverConfigLoadNVRAMEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 49 +++++++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ca31a222e8..29f948346d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,37 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ char **nvram = NULL;
+ int ret = -1;
+ size_t i;
+
+ if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0)
+ goto cleanup;
+ if (nvram) {
+ virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
+
+ cfg->nfirmwares = virStringListLength((const char *const *)nvram);
+ if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) <
0)
+ goto cleanup;
+
+ for (i = 0; nvram[i] != NULL; i++) {
+ if (VIR_ALLOC(cfg->firmwares[i]) < 0)
+ goto cleanup;
+ if (virFirmwareParse(nvram[i], cfg->firmwares[i]) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(nvram);
+ return ret;
+}
+
static int
virQEMUDriverConfigLoadGlusterDebugEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -591,7 +622,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
size_t i;
char *stdioHandler = NULL;
char **hugetlbfs = NULL;
- char **nvram = NULL;
char *corestr = NULL;
bool tmp;
@@ -907,22 +937,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueBool(conf, "log_timestamp", &cfg->logTimestamp)
< 0)
goto cleanup;
- if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0)
+ if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf) < 0)
goto cleanup;
- if (nvram) {
- virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
-
- cfg->nfirmwares = virStringListLength((const char *const *)nvram);
- if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) <
0)
- goto cleanup;
-
- for (i = 0; nvram[i] != NULL; i++) {
- if (VIR_ALLOC(cfg->firmwares[i]) < 0)
- goto cleanup;
- if (virFirmwareParse(nvram[i], cfg->firmwares[i]) < 0)
- goto cleanup;
- }
- }
if (virQEMUDriverConfigLoadGlusterDebugEntry(cfg, conf) < 0)
goto cleanup;
@@ -940,7 +956,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
cleanup:
virStringListFree(hugetlbfs);
- virStringListFree(nvram);
VIR_FREE(corestr);
virConfFree(conf);
return ret;
--
2.20.1
2:32 p.m.
New subject: [libvirt] [PATCH 09/23] qemu_conf: split out virQEMUDriverConfigLoadNVRAMEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 49 +++++++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ca31a222e8..29f948346d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,37 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ char **nvram = NULL;
VIR_AUTOPTR(virString)
+ int ret = -1;
+ size_t i;
+
+ if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0)
+ goto cleanup;
+ if (nvram) {
+ virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
+
+ cfg->nfirmwares = virStringListLength((const char *const *)nvram);
+ if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) <
0)
+ goto cleanup;
+
+ for (i = 0; nvram[i] != NULL; i++) {
+ if (VIR_ALLOC(cfg->firmwares[i]) < 0)
+ goto cleanup;
+ if (virFirmwareParse(nvram[i], cfg->firmwares[i]) < 0)
+ goto cleanup;
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(nvram);
+ return ret;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:23 p.m.
New subject: [libvirt] [PATCH 10/23] qemu_conf: split out virQEMUDriverConfigLoadLogEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 29f948346d..b7d258f17a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,16 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadLogEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueBool(conf, "log_timestamp", &cfg->logTimestamp)
< 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -934,7 +944,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- if (virConfGetValueBool(conf, "log_timestamp", &cfg->logTimestamp)
< 0)
+ if (virQEMUDriverConfigLoadLogEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadNVRAMEntry(cfg, conf) < 0)
--
2.20.1
2:33 p.m.
New subject: [libvirt] [PATCH 10/23] qemu_conf: split out virQEMUDriverConfigLoadLogEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 29f948346d..b7d258f17a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,16 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadLogEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueBool(conf, "log_timestamp", &cfg->logTimestamp)
< 0)
+ return -1;
+
+ return 0;
return virConfGetValueBool(...);
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 11/23] qemu_conf: split out virQEMUDriverConfigLoadNetworkEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 99 +++++++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b7d258f17a..8aa5157cd1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,60 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf,
+ const char *filename)
+{
+ if (virConfGetValueUInt(conf, "migration_port_min",
&cfg->migrationPortMin) < 0)
+ return -1;
+ if (cfg->migrationPortMin <= 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: migration_port_min: port must be greater than
0"),
+ filename);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "migration_port_max",
&cfg->migrationPortMax) < 0)
+ return -1;
+ if (cfg->migrationPortMax > 65535 ||
+ cfg->migrationPortMax < cfg->migrationPortMin) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: migration_port_max: port must be between "
+ "the minimal port %d and 65535"),
+ filename, cfg->migrationPortMin);
+ return -1;
+ }
+
+ if (virConfGetValueString(conf, "migration_host", &cfg->migrateHost)
< 0)
+ return -1;
+ virStringStripIPv6Brackets(cfg->migrateHost);
+ if (cfg->migrateHost &&
+ (STRPREFIX(cfg->migrateHost, "localhost") ||
+ virSocketAddrIsNumericLocalhost(cfg->migrateHost))) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("migration_host must not be the address of"
+ " the local machine: %s"),
+ cfg->migrateHost);
+ return -1;
+ }
+
+ if (virConfGetValueString(conf, "migration_address",
&cfg->migrationAddress) < 0)
+ return -1;
+ virStringStripIPv6Brackets(cfg->migrationAddress);
+ if (cfg->migrationAddress &&
+ (STRPREFIX(cfg->migrationAddress, "localhost") ||
+ virSocketAddrIsNumericLocalhost(cfg->migrationAddress))) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("migration_address must not be the address of"
+ " the local machine: %s"),
+ cfg->migrationAddress);
+ return -1;
+ }
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadLogEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -788,25 +842,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
- if (virConfGetValueUInt(conf, "migration_port_min",
&cfg->migrationPortMin) < 0)
- goto cleanup;
- if (cfg->migrationPortMin <= 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: migration_port_min: port must be greater than
0"),
- filename);
- goto cleanup;
- }
-
- if (virConfGetValueUInt(conf, "migration_port_max",
&cfg->migrationPortMax) < 0)
- goto cleanup;
- if (cfg->migrationPortMax > 65535 ||
- cfg->migrationPortMax < cfg->migrationPortMin) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: migration_port_max: port must be between "
- "the minimal port %d and 65535"),
- filename, cfg->migrationPortMin);
- goto cleanup;
- }
if (virConfGetValueString(conf, "save_image_format",
&cfg->saveImageFormat) < 0)
goto cleanup;
@@ -858,6 +893,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "clear_emulator_capabilities",
&cfg->clearEmulatorCapabilities) < 0)
goto cleanup;
+
if ((rv = virConfGetValueBool(conf, "allow_disk_format_probing", &tmp))
< 0)
goto cleanup;
if (rv == 1 && tmp) {
@@ -918,31 +954,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueUInt(conf, "keepalive_count",
&cfg->keepAliveCount) < 0)
goto cleanup;
- if (virConfGetValueString(conf, "migration_host", &cfg->migrateHost)
< 0)
+ if (virQEMUDriverConfigLoadNetworkEntry(cfg, conf, filename) < 0)
goto cleanup;
- virStringStripIPv6Brackets(cfg->migrateHost);
- if (cfg->migrateHost &&
- (STRPREFIX(cfg->migrateHost, "localhost") ||
- virSocketAddrIsNumericLocalhost(cfg->migrateHost))) {
- virReportError(VIR_ERR_CONF_SYNTAX,
- _("migration_host must not be the address of"
- " the local machine: %s"),
- cfg->migrateHost);
- goto cleanup;
- }
-
- if (virConfGetValueString(conf, "migration_address",
&cfg->migrationAddress) < 0)
- goto cleanup;
- virStringStripIPv6Brackets(cfg->migrationAddress);
- if (cfg->migrationAddress &&
- (STRPREFIX(cfg->migrationAddress, "localhost") ||
- virSocketAddrIsNumericLocalhost(cfg->migrationAddress))) {
- virReportError(VIR_ERR_CONF_SYNTAX,
- _("migration_address must not be the address of"
- " the local machine: %s"),
- cfg->migrationAddress);
- goto cleanup;
- }
if (virQEMUDriverConfigLoadLogEntry(cfg, conf) < 0)
goto cleanup;
--
2.20.1
2:34 p.m.
New subject: [libvirt] [PATCH 11/23] qemu_conf: split out virQEMUDriverConfigLoadNetworkEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 99 +++++++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b7d258f17a..8aa5157cd1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,60 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf,
+ const char *filename)
+{
+ if (virConfGetValueUInt(conf, "migration_port_min",
&cfg->migrationPortMin) < 0)
+ return -1;
+ if (cfg->migrationPortMin <= 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: migration_port_min: port must be greater than
0"),
+ filename);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "migration_port_max",
&cfg->migrationPortMax) < 0)
+ return -1;
+ if (cfg->migrationPortMax > 65535 ||
+ cfg->migrationPortMax < cfg->migrationPortMin) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: migration_port_max: port must be between "
+ "the minimal port %d and 65535"),
+ filename, cfg->migrationPortMin);
+ return -1;
+ }
+
+ if (virConfGetValueString(conf, "migration_host",
&cfg->migrateHost) < 0)
+ return -1;
+ virStringStripIPv6Brackets(cfg->migrateHost);
+ if (cfg->migrateHost &&
+ (STRPREFIX(cfg->migrateHost, "localhost") ||
+ virSocketAddrIsNumericLocalhost(cfg->migrateHost))) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("migration_host must not be the address of"
+ " the local machine: %s"),
+ cfg->migrateHost);
+ return -1;
+ }
+
+ if (virConfGetValueString(conf, "migration_address",
&cfg->migrationAddress) < 0)
+ return -1;
+ virStringStripIPv6Brackets(cfg->migrationAddress);
+ if (cfg->migrationAddress &&
+ (STRPREFIX(cfg->migrationAddress, "localhost") ||
+ virSocketAddrIsNumericLocalhost(cfg->migrationAddress))) {
+ virReportError(VIR_ERR_CONF_SYNTAX,
+ _("migration_address must not be the address of"
+ " the local machine: %s"),
+ cfg->migrationAddress);
+ return -1;
+ }
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 12/23] qemu_conf: split out virQEMUDriverConfigLoadRPCEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8aa5157cd1..b555e33e7b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,20 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadRPCEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueUInt(conf, "max_queued", &cfg->maxQueuedJobs)
< 0)
+ return -1;
+ if (virConfGetValueInt(conf, "keepalive_interval",
&cfg->keepAliveInterval) < 0)
+ return -1;
+ if (virConfGetValueUInt(conf, "keepalive_count",
&cfg->keepAliveCount) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf,
@@ -946,12 +960,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
VIR_FREE(stdioHandler);
}
- if (virConfGetValueUInt(conf, "max_queued", &cfg->maxQueuedJobs)
< 0)
- goto cleanup;
-
- if (virConfGetValueInt(conf, "keepalive_interval",
&cfg->keepAliveInterval) < 0)
- goto cleanup;
- if (virConfGetValueUInt(conf, "keepalive_count",
&cfg->keepAliveCount) < 0)
+ if (virQEMUDriverConfigLoadRPCEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadNetworkEntry(cfg, conf, filename) < 0)
--
2.20.1
2:36 p.m.
New subject: [libvirt] [PATCH 12/23] qemu_conf: split out virQEMUDriverConfigLoadRPCEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8aa5157cd1..b555e33e7b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,20 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadRPCEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueUInt(conf, "max_queued", &cfg->maxQueuedJobs)
< 0)
+ return -1;
+ if (virConfGetValueInt(conf, "keepalive_interval",
&cfg->keepAliveInterval) < 0)
+ return -1;
+ if (virConfGetValueUInt(conf, "keepalive_count",
&cfg->keepAliveCount) < 0)
+ return -1;
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 13/23] qemu_conf: split out virQEMUDriverConfigLoadDeviceEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 45 +++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b555e33e7b..776ba3f3ad 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,31 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadDeviceEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ bool tmp;
+ int rv;
+
+ if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) <
0)
+ return -1;
+
+ if (virConfGetValueBool(conf, "relaxed_acs_check", &cfg->relaxedACS)
< 0)
+ return -1;
+ if (virConfGetValueString(conf, "lock_manager",
&cfg->lockManagerName) < 0)
+ return -1;
+ if ((rv = virConfGetValueBool(conf, "allow_disk_format_probing", &tmp))
< 0)
+ return -1;
+ if (rv == 1 && tmp) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("allow_disk_format_probing is no longer supported"));
+ return -1;
+ }
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadRPCEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -701,7 +726,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
char *stdioHandler = NULL;
char **hugetlbfs = NULL;
char *corestr = NULL;
- bool tmp;
/* Just check the file is readable before opening it, otherwise
* libvirt emits an error.
@@ -900,21 +924,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName)
< 0)
goto cleanup;
- if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) <
0)
- goto cleanup;
-
- if (virConfGetValueBool(conf, "relaxed_acs_check", &cfg->relaxedACS)
< 0)
- goto cleanup;
if (virConfGetValueBool(conf, "clear_emulator_capabilities",
&cfg->clearEmulatorCapabilities) < 0)
goto cleanup;
-
- if ((rv = virConfGetValueBool(conf, "allow_disk_format_probing", &tmp))
< 0)
- goto cleanup;
- if (rv == 1 && tmp) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("allow_disk_format_probing is no longer supported"));
- goto cleanup;
- }
if (virConfGetValueBool(conf, "set_process_name",
&cfg->setProcessName) < 0)
goto cleanup;
if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses)
< 0)
@@ -940,9 +951,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueBool(conf, "dump_guest_core",
&cfg->dumpGuestCore) < 0)
goto cleanup;
- if (virConfGetValueString(conf, "lock_manager",
&cfg->lockManagerName) < 0)
- goto cleanup;
-
if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) <
0)
goto cleanup;
if (stdioHandler) {
@@ -960,6 +968,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
VIR_FREE(stdioHandler);
}
+ if (virQEMUDriverConfigLoadDeviceEntry(cfg, conf) < 0)
+ goto cleanup;
+
if (virQEMUDriverConfigLoadRPCEntry(cfg, conf) < 0)
goto cleanup;
--
2.20.1
2:37 p.m.
New subject: [libvirt] [PATCH 13/23] qemu_conf: split out virQEMUDriverConfigLoadDeviceEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 45 +++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b555e33e7b..776ba3f3ad 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,31 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadDeviceEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ bool tmp;
+ int rv;
+
+ if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) <
0)
+ return -1;
+
+ if (virConfGetValueBool(conf, "relaxed_acs_check",
&cfg->relaxedACS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "lock_manager",
&cfg->lockManagerName) < 0)
+ return -1;
+ if ((rv = virConfGetValueBool(conf, "allow_disk_format_probing",
&tmp)) < 0)
+ return -1;
+ if (rv == 1 && tmp) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("allow_disk_format_probing is no longer
supported"));
+ return -1;
+ }
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 14/23] qemu_conf: split out virQEMUDriverConfigLoadProcessEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 162 ++++++++++++++++++++++++-------------------
1 file changed, 90 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 776ba3f3ad..8bc653fa9e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,95 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ char *stdioHandler = NULL;
+ char **hugetlbfs = NULL;
+ char *corestr = NULL;
+ int ret = -1;
+ size_t i;
+
+ if (virConfGetValueStringList(conf, "hugetlbfs_mount", true,
+ &hugetlbfs) < 0)
+ goto cleanup;
+ if (hugetlbfs) {
+ /* There already might be something autodetected. Avoid leaking it. */
+ while (cfg->nhugetlbfs) {
+ cfg->nhugetlbfs--;
+ VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
+ }
+ VIR_FREE(cfg->hugetlbfs);
+
+ cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs);
+ if (hugetlbfs[0] &&
+ VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0)
+ goto cleanup;
+
+ for (i = 0; hugetlbfs[i] != NULL; i++) {
+ if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i],
+ hugetlbfs[i], i != 0) < 0)
+ goto cleanup;
+ }
+ }
+
+ if (virConfGetValueBool(conf, "clear_emulator_capabilities",
&cfg->clearEmulatorCapabilities) < 0)
+ goto cleanup;
+ if (virConfGetValueString(conf, "bridge_helper",
&cfg->bridgeHelperName) < 0)
+ goto cleanup;
+
+ if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName)
< 0)
+ goto cleanup;
+
+ if (virConfGetValueBool(conf, "set_process_name",
&cfg->setProcessName) < 0)
+ goto cleanup;
+ if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses)
< 0)
+ goto cleanup;
+ if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0)
+ goto cleanup;
+
+ if (virConfGetValueType(conf, "max_core") == VIR_CONF_STRING) {
+ if (virConfGetValueString(conf, "max_core", &corestr) < 0)
+ goto cleanup;
+ if (STREQ(corestr, "unlimited")) {
+ cfg->maxCore = ULLONG_MAX;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown core size '%s'"),
+ corestr);
+ goto cleanup;
+ }
+ } else if (virConfGetValueULLong(conf, "max_core", &cfg->maxCore)
< 0) {
+ goto cleanup;
+ }
+
+ if (virConfGetValueBool(conf, "dump_guest_core",
&cfg->dumpGuestCore) < 0)
+ goto cleanup;
+ if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) <
0)
+ goto cleanup;
+ if (stdioHandler) {
+ if (STREQ(stdioHandler, "logd")) {
+ cfg->stdioLogD = true;
+ } else if (STREQ(stdioHandler, "file")) {
+ cfg->stdioLogD = false;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown stdio handler %s"),
+ stdioHandler);
+ VIR_FREE(stdioHandler);
+ goto cleanup;
+ }
+ VIR_FREE(stdioHandler);
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(hugetlbfs);
+ VIR_FREE(corestr);
+ return ret;
+}
+
static int
virQEMUDriverConfigLoadDeviceEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -722,8 +811,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
virConfPtr conf = NULL;
int ret = -1;
int rv;
- size_t i;
- char *stdioHandler = NULL;
char **hugetlbfs = NULL;
char *corestr = NULL;
@@ -894,79 +981,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "auto_start_bypass_cache",
&cfg->autoStartBypassCache) < 0)
goto cleanup;
-
- if (virConfGetValueStringList(conf, "hugetlbfs_mount", true,
- &hugetlbfs) < 0)
- goto cleanup;
- if (hugetlbfs) {
- /* There already might be something autodetected. Avoid leaking it. */
- while (cfg->nhugetlbfs) {
- cfg->nhugetlbfs--;
- VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
- }
- VIR_FREE(cfg->hugetlbfs);
-
- cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs);
- if (hugetlbfs[0] &&
- VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0)
- goto cleanup;
-
- for (i = 0; hugetlbfs[i] != NULL; i++) {
- if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i],
- hugetlbfs[i], i != 0) < 0)
- goto cleanup;
- }
- }
-
- if (virConfGetValueString(conf, "bridge_helper",
&cfg->bridgeHelperName) < 0)
- goto cleanup;
-
- if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName)
< 0)
goto cleanup;
- if (virConfGetValueBool(conf, "clear_emulator_capabilities",
&cfg->clearEmulatorCapabilities) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "set_process_name",
&cfg->setProcessName) < 0)
+ if (virQEMUDriverConfigLoadProcessEntry(cfg, conf) < 0)
goto cleanup;
- if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses)
< 0)
- goto cleanup;
- if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0)
- goto cleanup;
-
- if (virConfGetValueType(conf, "max_core") == VIR_CONF_STRING) {
- if (virConfGetValueString(conf, "max_core", &corestr) < 0)
- goto cleanup;
- if (STREQ(corestr, "unlimited")) {
- cfg->maxCore = ULLONG_MAX;
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown core size '%s'"),
- corestr);
- goto cleanup;
- }
- } else if (virConfGetValueULLong(conf, "max_core", &cfg->maxCore)
< 0) {
- goto cleanup;
- }
-
- if (virConfGetValueBool(conf, "dump_guest_core",
&cfg->dumpGuestCore) < 0)
- goto cleanup;
-
- if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) <
0)
- goto cleanup;
- if (stdioHandler) {
- if (STREQ(stdioHandler, "logd")) {
- cfg->stdioLogD = true;
- } else if (STREQ(stdioHandler, "file")) {
- cfg->stdioLogD = false;
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown stdio handler %s"),
- stdioHandler);
- VIR_FREE(stdioHandler);
- goto cleanup;
- }
- VIR_FREE(stdioHandler);
- }
if (virQEMUDriverConfigLoadDeviceEntry(cfg, conf) < 0)
goto cleanup;
--
2.20.1
2:48 p.m.
New subject: [libvirt] [PATCH 14/23] qemu_conf: split out virQEMUDriverConfigLoadProcessEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 162 ++++++++++++++++++++++++-------------------
1 file changed, 90 insertions(+), 72 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 776ba3f3ad..8bc653fa9e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,95 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ char *stdioHandler = NULL;
+ char **hugetlbfs = NULL;
+ char *corestr = NULL;
VIR_AUTOPTR for each
+ int ret = -1;
+ size_t i;
+
+ if (virConfGetValueStringList(conf, "hugetlbfs_mount", true,
+ &hugetlbfs) < 0)
+ goto cleanup;
+ if (hugetlbfs) {
+ /* There already might be something autodetected. Avoid leaking it. */
+ while (cfg->nhugetlbfs) {
+ cfg->nhugetlbfs--;
+ VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
+ }
+ VIR_FREE(cfg->hugetlbfs);
+
+ cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs);
+ if (hugetlbfs[0] &&
+ VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0)
+ goto cleanup;
+
+ for (i = 0; hugetlbfs[i] != NULL; i++) {
+ if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i],
+ hugetlbfs[i], i != 0) < 0)
+ goto cleanup;
+ }
+ }
+
+ if (virConfGetValueBool(conf, "clear_emulator_capabilities",
&cfg->clearEmulatorCapabilities) < 0)
+ goto cleanup;
+ if (virConfGetValueString(conf, "bridge_helper",
&cfg->bridgeHelperName) < 0)
+ goto cleanup;
+
+ if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName)
< 0)
+ goto cleanup;
+
+ if (virConfGetValueBool(conf, "set_process_name",
&cfg->setProcessName) < 0)
+ goto cleanup;
+ if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses)
< 0)
+ goto cleanup;
+ if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0)
+ goto cleanup;
+
+ if (virConfGetValueType(conf, "max_core") == VIR_CONF_STRING) {
+ if (virConfGetValueString(conf, "max_core", &corestr) < 0)
+ goto cleanup;
+ if (STREQ(corestr, "unlimited")) {
+ cfg->maxCore = ULLONG_MAX;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown core size '%s'"),
+ corestr);
+ goto cleanup;
+ }
+ } else if (virConfGetValueULLong(conf, "max_core", &cfg->maxCore)
< 0) {
+ goto cleanup;
+ }
+
+ if (virConfGetValueBool(conf, "dump_guest_core",
&cfg->dumpGuestCore) < 0)
+ goto cleanup;
+ if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) <
0)
+ goto cleanup;
+ if (stdioHandler) {
+ if (STREQ(stdioHandler, "logd")) {
+ cfg->stdioLogD = true;
+ } else if (STREQ(stdioHandler, "file")) {
+ cfg->stdioLogD = false;
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown stdio handler %s"),
+ stdioHandler);
+ VIR_FREE(stdioHandler);
+ goto cleanup;
+ }
+ VIR_FREE(stdioHandler);
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(hugetlbfs);
+ VIR_FREE(corestr);
+ return ret;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
3:10 p.m.
New subject: [libvirt] [PATCH 14/23] qemu_conf: split out virQEMUDriverConfigLoadProcessEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 162 ++++++++++++++++++++++++-------------------
1 file changed, 90 insertions(+), 72 deletions(-)
I was too quick with the send push...
[...]
static int
virQEMUDriverConfigLoadDeviceEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -722,8 +811,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
virConfPtr conf = NULL;
int ret = -1;
int rv;
- size_t i;
- char *stdioHandler = NULL;
char **hugetlbfs = NULL;
This is now unused and don't forget the StringListFree at the bottom.
char *corestr = NULL;
@@ -894,79 +981,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "auto_start_bypass_cache",
&cfg->autoStartBypassCache) < 0)
goto cleanup;
-
- if (virConfGetValueStringList(conf, "hugetlbfs_mount", true,
- &hugetlbfs) < 0)
- goto cleanup;
- if (hugetlbfs) {
- /* There already might be something autodetected. Avoid leaking it. */
- while (cfg->nhugetlbfs) {
- cfg->nhugetlbfs--;
- VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
- }
- VIR_FREE(cfg->hugetlbfs);
-
- cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs);
- if (hugetlbfs[0] &&
- VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0)
- goto cleanup;
-
- for (i = 0; hugetlbfs[i] != NULL; i++) {
- if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i],
- hugetlbfs[i], i != 0) < 0)
- goto cleanup;
- }
- }
-
- if (virConfGetValueString(conf, "bridge_helper",
&cfg->bridgeHelperName) < 0)
- goto cleanup;
-
- if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName)
< 0)
goto cleanup;
This makes a double "goto cleanup;" which you fix in the next patch, but
causes this patch to fail to compile...
With all that this patch is good.
John
[...]
1:05 p.m.
New subject: [libvirt] [PATCH 14/23] qemu_conf: split out virQEMUDriverConfigLoadProcessEntry
On Thu, Jan 17, 2019 at 09:10:11AM -0500, John Ferlan wrote:
On 1/15/19 8:23 AM, Ján Tomko wrote:
> @@ -894,79 +981,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> goto cleanup;
> if (virConfGetValueBool(conf, "auto_start_bypass_cache",
&cfg->autoStartBypassCache) < 0)
> goto cleanup;
> -
> - if (virConfGetValueStringList(conf, "hugetlbfs_mount", true,
> - &hugetlbfs) < 0)
> - goto cleanup;
> - if (hugetlbfs) {
> - /* There already might be something autodetected. Avoid leaking it. */
> - while (cfg->nhugetlbfs) {
> - cfg->nhugetlbfs--;
> - VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
> - }
> - VIR_FREE(cfg->hugetlbfs);
> -
> - cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs);
> - if (hugetlbfs[0] &&
> - VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0)
> - goto cleanup;
> -
> - for (i = 0; hugetlbfs[i] != NULL; i++) {
> - if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i],
> - hugetlbfs[i], i != 0) < 0)
> - goto cleanup;
> - }
> - }
> -
> - if (virConfGetValueString(conf, "bridge_helper",
&cfg->bridgeHelperName) < 0)
> - goto cleanup;
> -
> - if (virConfGetValueString(conf, "pr_helper",
&cfg->prHelperName) < 0)
> goto cleanup;
This makes a double "goto cleanup;" which you fix in the next patch, but
causes this patch to fail to compile...
Fun, clang does not mind at all.
I found another occurrence in a later patch.
Jano
With all that this patch is good.
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 15/23] qemu_conf: split out virQEMUDriverConfigLoadSaveEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8bc653fa9e..e03c0b29e3 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,26 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueString(conf, "save_image_format",
&cfg->saveImageFormat) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "dump_image_format",
&cfg->dumpImageFormat) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "snapshot_image_format",
&cfg->snapshotImageFormat) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "auto_dump_path",
&cfg->autoDumpPath) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "auto_dump_bypass_cache",
&cfg->autoDumpBypassCache) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "auto_start_bypass_cache",
&cfg->autoStartBypassCache) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -967,20 +987,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
}
-
- if (virConfGetValueString(conf, "save_image_format",
&cfg->saveImageFormat) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "dump_image_format",
&cfg->dumpImageFormat) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "snapshot_image_format",
&cfg->snapshotImageFormat) < 0)
- goto cleanup;
-
- if (virConfGetValueString(conf, "auto_dump_path",
&cfg->autoDumpPath) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "auto_dump_bypass_cache",
&cfg->autoDumpBypassCache) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "auto_start_bypass_cache",
&cfg->autoStartBypassCache) < 0)
- goto cleanup;
+ if (virQEMUDriverConfigLoadSaveEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadProcessEntry(cfg, conf) < 0)
--
2.20.1
3:10 p.m.
New subject: [libvirt] [PATCH 15/23] qemu_conf: split out virQEMUDriverConfigLoadSaveEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8bc653fa9e..e03c0b29e3 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,26 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueString(conf, "save_image_format",
&cfg->saveImageFormat) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "dump_image_format",
&cfg->dumpImageFormat) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "snapshot_image_format",
&cfg->snapshotImageFormat) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "auto_dump_path",
&cfg->autoDumpPath) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "auto_dump_bypass_cache",
&cfg->autoDumpBypassCache) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "auto_start_bypass_cache",
&cfg->autoStartBypassCache) < 0)
+ return -1;
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 16/23] qemu_conf: split out virQEMUDriverConfigLoadRemoteDisplayEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 131 +++++++++++++++++++++++--------------------
1 file changed, 71 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e03c0b29e3..317756d2a0 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,76 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf,
+ const char *filename)
+{
+ if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
+ return -1;
+ if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
+ /* if the port is too low, we can't get the display name
+ * to tell to vnc (usually subtract 5700, e.g. localhost:1
+ * for port 5701) */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_websocket_port_min: port must be greater
"
+ "than or equal to %d"),
+ filename, QEMU_WEBSOCKET_PORT_MIN);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "remote_websocket_port_max",
&cfg->webSocketPortMax) < 0)
+ return -1;
+ if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX ||
+ cfg->webSocketPortMax < cfg->webSocketPortMin) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_websocket_port_max: port must be between
"
+ "the minimal port and %d"),
+ filename, QEMU_WEBSOCKET_PORT_MAX);
+ return -1;
+ }
+
+ if (cfg->webSocketPortMin > cfg->webSocketPortMax) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_websocket_port_min: min port must not be
"
+ "greater than max port"), filename);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "remote_display_port_min",
&cfg->remotePortMin) < 0)
+ return -1;
+ if (cfg->remotePortMin < QEMU_REMOTE_PORT_MIN) {
+ /* if the port is too low, we can't get the display name
+ * to tell to vnc (usually subtract 5900, e.g. localhost:1
+ * for port 5901) */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_display_port_min: port must be greater "
+ "than or equal to %d"),
+ filename, QEMU_REMOTE_PORT_MIN);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "remote_display_port_max",
&cfg->remotePortMax) < 0)
+ return -1;
+ if (cfg->remotePortMax > QEMU_REMOTE_PORT_MAX ||
+ cfg->remotePortMax < cfg->remotePortMin) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_display_port_max: port must be between "
+ "the minimal port and %d"),
+ filename, QEMU_REMOTE_PORT_MAX);
+ return -1;
+ }
+
+ if (cfg->remotePortMin > cfg->remotePortMax) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_display_port_min: min port must not be "
+ "greater than max port"), filename);
+ return -1;
+ }
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -925,67 +995,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
#undef GET_CONFIG_TLS_CERTINFO
- if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
- goto cleanup;
- if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
- /* if the port is too low, we can't get the display name
- * to tell to vnc (usually subtract 5700, e.g. localhost:1
- * for port 5701) */
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: remote_websocket_port_min: port must be greater
"
- "than or equal to %d"),
- filename, QEMU_WEBSOCKET_PORT_MIN);
- goto cleanup;
- }
-
- if (virConfGetValueUInt(conf, "remote_websocket_port_max",
&cfg->webSocketPortMax) < 0)
- goto cleanup;
- if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX ||
- cfg->webSocketPortMax < cfg->webSocketPortMin) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: remote_websocket_port_max: port must be between
"
- "the minimal port and %d"),
- filename, QEMU_WEBSOCKET_PORT_MAX);
+ if (virQEMUDriverConfigLoadRemoteDisplayEntry(cfg, conf, filename) < 0)
goto cleanup;
- }
-
- if (cfg->webSocketPortMin > cfg->webSocketPortMax) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: remote_websocket_port_min: min port must not be
"
- "greater than max port"), filename);
- goto cleanup;
- }
-
- if (virConfGetValueUInt(conf, "remote_display_port_min",
&cfg->remotePortMin) < 0)
- goto cleanup;
- if (cfg->remotePortMin < QEMU_REMOTE_PORT_MIN) {
- /* if the port is too low, we can't get the display name
- * to tell to vnc (usually subtract 5900, e.g. localhost:1
- * for port 5901) */
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: remote_display_port_min: port must be greater "
- "than or equal to %d"),
- filename, QEMU_REMOTE_PORT_MIN);
- goto cleanup;
- }
-
- if (virConfGetValueUInt(conf, "remote_display_port_max",
&cfg->remotePortMax) < 0)
- goto cleanup;
- if (cfg->remotePortMax > QEMU_REMOTE_PORT_MAX ||
- cfg->remotePortMax < cfg->remotePortMin) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: remote_display_port_max: port must be between "
- "the minimal port and %d"),
- filename, QEMU_REMOTE_PORT_MAX);
- goto cleanup;
- }
-
- if (cfg->remotePortMin > cfg->remotePortMax) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s: remote_display_port_min: min port must not be "
- "greater than max port"), filename);
- goto cleanup;
- }
if (virQEMUDriverConfigLoadSaveEntry(cfg, conf) < 0)
goto cleanup;
--
2.20.1
3:12 p.m.
New subject: [libvirt] [PATCH 16/23] qemu_conf: split out virQEMUDriverConfigLoadRemoteDisplayEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 131 +++++++++++++++++++++++--------------------
1 file changed, 71 insertions(+), 60 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e03c0b29e3..317756d2a0 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,76 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf,
+ const char *filename)
+{
+ if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
+ return -1;
+ if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
+ /* if the port is too low, we can't get the display name
+ * to tell to vnc (usually subtract 5700, e.g. localhost:1
+ * for port 5701) */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_websocket_port_min: port must be greater
"
+ "than or equal to %d"),
+ filename, QEMU_WEBSOCKET_PORT_MIN);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "remote_websocket_port_max",
&cfg->webSocketPortMax) < 0)
+ return -1;
+ if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX ||
+ cfg->webSocketPortMax < cfg->webSocketPortMin) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_websocket_port_max: port must be between
"
+ "the minimal port and %d"),
+ filename, QEMU_WEBSOCKET_PORT_MAX);
+ return -1;
+ }
+
+ if (cfg->webSocketPortMin > cfg->webSocketPortMax) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_websocket_port_min: min port must not be
"
+ "greater than max port"), filename);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "remote_display_port_min",
&cfg->remotePortMin) < 0)
+ return -1;
+ if (cfg->remotePortMin < QEMU_REMOTE_PORT_MIN) {
+ /* if the port is too low, we can't get the display name
+ * to tell to vnc (usually subtract 5900, e.g. localhost:1
+ * for port 5901) */
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_display_port_min: port must be greater "
+ "than or equal to %d"),
+ filename, QEMU_REMOTE_PORT_MIN);
+ return -1;
+ }
+
+ if (virConfGetValueUInt(conf, "remote_display_port_max",
&cfg->remotePortMax) < 0)
+ return -1;
+ if (cfg->remotePortMax > QEMU_REMOTE_PORT_MAX ||
+ cfg->remotePortMax < cfg->remotePortMin) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_display_port_max: port must be between
"
+ "the minimal port and %d"),
+ filename, QEMU_REMOTE_PORT_MAX);
+ return -1;
+ }
+
+ if (cfg->remotePortMin > cfg->remotePortMax) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: remote_display_port_min: min port must not be
"
+ "greater than max port"), filename);
+ return -1;
+ }
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:23 p.m.
New subject: [libvirt] [PATCH 17/23] qemu_conf: split out virQEMUDriverConfigLoadSpecificTLS
Split out parts of the config parsing code to make
the parent function easier to read.
This is the only patch that mixes various augeas entry
groups in one function.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 71 ++++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 317756d2a0..b5024705d8 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,47 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ int rv;
+
+ if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir",
&cfg->vxhsTLSx509certdir) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "nbd_tls", &cfg->nbdTLS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "nbd_tls_x509_cert_dir",
&cfg->nbdTLSx509certdir) < 0)
+ return -1;
+
+#define GET_CONFIG_TLS_CERTINFO(val) \
+ do { \
+ if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \
+ &cfg->val## TLSx509verify)) < 0) \
+ return -1; \
+ if (rv == 1) \
+ cfg->val## TLSx509verifyPresent = true; \
+ if (virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
+ &cfg->val## TLSx509certdir) < 0) \
+ return -1; \
+ if (virConfGetValueString(conf, \
+ #val "_tls_x509_secret_uuid", \
+ &cfg->val## TLSx509secretUUID) < 0) \
+ return -1; \
+ } while (0)
+
+ if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) <
0)
+ return -1;
+ GET_CONFIG_TLS_CERTINFO(chardev);
+
+ GET_CONFIG_TLS_CERTINFO(migrate);
+
+#undef GET_CONFIG_TLS_CERTINFO
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf,
@@ -962,38 +1003,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "spice_auto_unix_socket",
&cfg->spiceAutoUnixSocket) < 0)
goto cleanup;
- if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir",
&cfg->vxhsTLSx509certdir) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "nbd_tls", &cfg->nbdTLS) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "nbd_tls_x509_cert_dir",
&cfg->nbdTLSx509certdir) < 0)
goto cleanup;
-#define GET_CONFIG_TLS_CERTINFO(val) \
- do { \
- if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \
- &cfg->val## TLSx509verify)) < 0) \
- goto cleanup; \
- if (rv == 1) \
- cfg->val## TLSx509verifyPresent = true; \
- if (virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
- &cfg->val## TLSx509certdir) < 0) \
- goto cleanup; \
- if (virConfGetValueString(conf, \
- #val "_tls_x509_secret_uuid", \
- &cfg->val## TLSx509secretUUID) < 0) \
- goto cleanup; \
- } while (0)
-
- if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) <
0)
+ if (virQEMUDriverConfigLoadSpecificTLSEntry(cfg, conf) < 0)
goto cleanup;
- GET_CONFIG_TLS_CERTINFO(chardev);
-
- GET_CONFIG_TLS_CERTINFO(migrate);
-
-#undef GET_CONFIG_TLS_CERTINFO
if (virQEMUDriverConfigLoadRemoteDisplayEntry(cfg, conf, filename) < 0)
goto cleanup;
--
2.20.1
3:18 p.m.
New subject: [libvirt] [PATCH 17/23] qemu_conf: split out virQEMUDriverConfigLoadSpecificTLS
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
This is the only patch that mixes various augeas entry
groups in one function.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 71 ++++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 317756d2a0..b5024705d8 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,47 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ int rv;
+
+ if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir",
&cfg->vxhsTLSx509certdir) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "nbd_tls", &cfg->nbdTLS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "nbd_tls_x509_cert_dir",
&cfg->nbdTLSx509certdir) < 0)
+ return -1;
+
+#define GET_CONFIG_TLS_CERTINFO(val) \
+ do { \
+ if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \
+ &cfg->val## TLSx509verify)) < 0) \
+ return -1; \
+ if (rv == 1) \
+ cfg->val## TLSx509verifyPresent = true; \
+ if (virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
+ &cfg->val## TLSx509certdir) < 0) \
+ return -1; \
+ if (virConfGetValueString(conf, \
+ #val "_tls_x509_secret_uuid", \
+ &cfg->val## TLSx509secretUUID) < 0) \
+ return -1; \
+ } while (0)
+
+ if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) <
0)
+ return -1;
+ GET_CONFIG_TLS_CERTINFO(chardev);
+
+ GET_CONFIG_TLS_CERTINFO(migrate);
+
+#undef GET_CONFIG_TLS_CERTINFO
+ return 0;
+}
+
blank line
static int
virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf,
@@ -962,38 +1003,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "spice_auto_unix_socket",
&cfg->spiceAutoUnixSocket) < 0)
goto cleanup;
- if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir",
&cfg->vxhsTLSx509certdir) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "nbd_tls", &cfg->nbdTLS) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "nbd_tls_x509_cert_dir",
&cfg->nbdTLSx509certdir) < 0)
goto cleanup;
This results in a double goto cleanup; ...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 18/23] qemu_conf: split out virQEMUDriverConfigLoadSPICEEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b5024705d8..d3f266e338 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,28 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSPICEEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueBool(conf, "spice_tls", &cfg->spiceTLS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_tls_x509_cert_dir",
&cfg->spiceTLSx509certdir) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "spice_sasl", &cfg->spiceSASL) <
0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_sasl_dir",
&cfg->spiceSASLdir) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_listen", &cfg->spiceListen)
< 0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_password",
&cfg->spicePassword) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "spice_auto_unix_socket",
&cfg->spiceAutoUnixSocket) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -988,21 +1010,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueBool(conf, "nographics_allow_host_audio",
&cfg->nogfxAllowHostAudio) < 0)
goto cleanup;
-
- if (virConfGetValueBool(conf, "spice_tls", &cfg->spiceTLS) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "spice_tls_x509_cert_dir",
&cfg->spiceTLSx509certdir) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "spice_sasl", &cfg->spiceSASL) <
0)
- goto cleanup;
- if (virConfGetValueString(conf, "spice_sasl_dir",
&cfg->spiceSASLdir) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "spice_listen", &cfg->spiceListen)
< 0)
- goto cleanup;
- if (virConfGetValueString(conf, "spice_password",
&cfg->spicePassword) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "spice_auto_unix_socket",
&cfg->spiceAutoUnixSocket) < 0)
- goto cleanup;
+ if (virQEMUDriverConfigLoadSPICEEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadSpecificTLSEntry(cfg, conf) < 0)
--
2.20.1
3:20 p.m.
New subject: [libvirt] [PATCH 18/23] qemu_conf: split out virQEMUDriverConfigLoadSPICEEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b5024705d8..d3f266e338 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,28 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadSPICEEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueBool(conf, "spice_tls", &cfg->spiceTLS) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_tls_x509_cert_dir",
&cfg->spiceTLSx509certdir) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "spice_sasl", &cfg->spiceSASL) <
0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_sasl_dir",
&cfg->spiceSASLdir) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_listen", &cfg->spiceListen)
< 0)
+ return -1;
+ if (virConfGetValueString(conf, "spice_password",
&cfg->spicePassword) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "spice_auto_unix_socket",
&cfg->spiceAutoUnixSocket) < 0)
+ return -1;
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:23 p.m.
New subject: [libvirt] [PATCH 19/23] qemu_conf: split out virQEMUDriverConfigLoadNographicsEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d3f266e338..62c49c600c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,16 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadNographicsEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueBool(conf, "nographics_allow_host_audio",
&cfg->nogfxAllowHostAudio) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadSPICEEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -1007,7 +1017,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "vnc_allow_host_audio",
&cfg->vncAllowHostAudio) < 0)
goto cleanup;
- if (virConfGetValueBool(conf, "nographics_allow_host_audio",
&cfg->nogfxAllowHostAudio) < 0)
+
+ if (virQEMUDriverConfigLoadNographicsEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadSPICEEntry(cfg, conf) < 0)
--
2.20.1
3:21 p.m.
New subject: [libvirt] [PATCH 19/23] qemu_conf: split out virQEMUDriverConfigLoadNographicsEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index d3f266e338..62c49c600c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,16 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadNographicsEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ if (virConfGetValueBool(conf, "nographics_allow_host_audio",
&cfg->nogfxAllowHostAudio) < 0)
+ return -1;
+
+ return 0;
return virConfGetValueBool(...);
+}
+
blank line.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 20/23] qemu_conf: split out virQEMUDriverConfigLoadVNCEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 50 +++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 62c49c600c..7eecb3ef91 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,36 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadVNCEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ int rv;
+
+ if (virConfGetValueBool(conf, "vnc_auto_unix_socket",
&cfg->vncAutoUnixSocket) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0)
+ return -1;
+ if ((rv = virConfGetValueBool(conf, "vnc_tls_x509_verify",
&cfg->vncTLSx509verify)) < 0)
+ return -1;
+ if (rv == 1)
+ cfg->vncTLSx509verifyPresent = true;
+ if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir",
&cfg->vncTLSx509certdir) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) <
0)
+ return -1;
+ if (virConfGetValueString(conf, "vnc_password", &cfg->vncPassword)
< 0)
+ return -1;
+ if (virConfGetValueBool(conf, "vnc_sasl", &cfg->vncSASL) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "vnc_sasl_dir", &cfg->vncSASLdir)
< 0)
+ return -1;
+ if (virConfGetValueBool(conf, "vnc_allow_host_audio",
&cfg->vncAllowHostAudio) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadNographicsEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -997,25 +1027,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
&cfg->defaultTLSx509secretUUID) < 0)
goto cleanup;
- if (virConfGetValueBool(conf, "vnc_auto_unix_socket",
&cfg->vncAutoUnixSocket) < 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0)
- goto cleanup;
- if ((rv = virConfGetValueBool(conf, "vnc_tls_x509_verify",
&cfg->vncTLSx509verify)) < 0)
- goto cleanup;
- if (rv == 1)
- cfg->vncTLSx509verifyPresent = true;
- if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir",
&cfg->vncTLSx509certdir) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) <
0)
- goto cleanup;
- if (virConfGetValueString(conf, "vnc_password", &cfg->vncPassword)
< 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "vnc_sasl", &cfg->vncSASL) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "vnc_sasl_dir", &cfg->vncSASLdir)
< 0)
- goto cleanup;
- if (virConfGetValueBool(conf, "vnc_allow_host_audio",
&cfg->vncAllowHostAudio) < 0)
+ if (virQEMUDriverConfigLoadVNCEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadNographicsEntry(cfg, conf) < 0)
--
2.20.1
3:22 p.m.
New subject: [libvirt] [PATCH 20/23] qemu_conf: split out virQEMUDriverConfigLoadVNCEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 50 +++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 62c49c600c..7eecb3ef91 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,36 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadVNCEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ int rv;
+
+ if (virConfGetValueBool(conf, "vnc_auto_unix_socket",
&cfg->vncAutoUnixSocket) < 0)
+ return -1;
+ if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0)
+ return -1;
+ if ((rv = virConfGetValueBool(conf, "vnc_tls_x509_verify",
&cfg->vncTLSx509verify)) < 0)
+ return -1;
+ if (rv == 1)
+ cfg->vncTLSx509verifyPresent = true;
+ if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir",
&cfg->vncTLSx509certdir) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) <
0)
+ return -1;
+ if (virConfGetValueString(conf, "vnc_password", &cfg->vncPassword)
< 0)
+ return -1;
+ if (virConfGetValueBool(conf, "vnc_sasl", &cfg->vncSASL) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "vnc_sasl_dir", &cfg->vncSASLdir)
< 0)
+ return -1;
+ if (virConfGetValueBool(conf, "vnc_allow_host_audio",
&cfg->vncAllowHostAudio) < 0)
+ return -1;
+
+ return 0;
+}
+
blank line
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 21/23] qemu_conf: split out virQEMUDriverConfigLoadDefaultTLSEntry
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7eecb3ef91..0877a5b7c1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,24 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadDefaultTLSEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ int rv;
+
+ if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir",
&cfg->defaultTLSx509certdir)) < 0)
+ return -1;
+ cfg->checkdefaultTLSx509certdir = (rv == 1);
+ if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
+ &cfg->defaultTLSx509secretUUID) < 0)
+ return -1;
+
+ return 0;
+}
+
static int
virQEMUDriverConfigLoadVNCEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -485,6 +503,7 @@ virQEMUDriverConfigLoadSPICEEntry(virQEMUDriverConfigPtr cfg,
return 0;
}
+
static int
virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -1003,7 +1022,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
{
virConfPtr conf = NULL;
int ret = -1;
- int rv;
char **hugetlbfs = NULL;
char *corestr = NULL;
@@ -1018,13 +1036,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (!(conf = virConfReadFile(filename, 0)))
goto cleanup;
- if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir",
&cfg->defaultTLSx509certdir)) < 0)
- goto cleanup;
- cfg->checkdefaultTLSx509certdir = (rv == 1);
- if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
- goto cleanup;
- if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
- &cfg->defaultTLSx509secretUUID) < 0)
+ if (virQEMUDriverConfigLoadDefaultTLSEntry(cfg, conf) < 0)
goto cleanup;
if (virQEMUDriverConfigLoadVNCEntry(cfg, conf) < 0)
--
2.20.1
3:27 p.m.
New subject: [libvirt] [PATCH 21/23] qemu_conf: split out virQEMUDriverConfigLoadDefaultTLSEntry
On 1/15/19 8:23 AM, Ján Tomko wrote:
Split out parts of the config parsing code to make
the parent function easier to read.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 7eecb3ef91..0877a5b7c1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -423,6 +423,24 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}
+static int
+virQEMUDriverConfigLoadDefaultTLSEntry(virQEMUDriverConfigPtr cfg,
+ virConfPtr conf)
+{
+ int rv;
+
+ if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir",
&cfg->defaultTLSx509certdir)) < 0)
+ return -1;
+ cfg->checkdefaultTLSx509certdir = (rv == 1);
+ if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
+ return -1;
+ if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
+ &cfg->defaultTLSx509secretUUID) < 0)
+ return -1;
+
+ return 0;
+}
+
blank line
static int
virQEMUDriverConfigLoadVNCEntry(virQEMUDriverConfigPtr cfg,
virConfPtr conf)
@@ -485,6 +503,7 @@ virQEMUDriverConfigLoadSPICEEntry(virQEMUDriverConfigPtr cfg,
return 0;
}
+
Ironically unrelated, but fine IDC.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[...]
2:23 p.m.
New subject: [libvirt] [PATCH 22/23] qemu_conf: rename checkdefaultTLSx509certdir
Use defaultTLSx509certdirPresent for consistencty.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 4 ++--
src/qemu/qemu_conf.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0877a5b7c1..0aa428b87f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -431,7 +431,7 @@ virQEMUDriverConfigLoadDefaultTLSEntry(virQEMUDriverConfigPtr cfg,
if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir",
&cfg->defaultTLSx509certdir)) < 0)
return -1;
- cfg->checkdefaultTLSx509certdir = (rv == 1);
+ cfg->defaultTLSx509certdirPresent = (rv == 1);
if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
return -1;
if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
@@ -1107,7 +1107,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
int
virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
{
- if (cfg->checkdefaultTLSx509certdir) {
+ if (cfg->defaultTLSx509certdirPresent) {
if (!virFileExists(cfg->defaultTLSx509certdir)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("default_tls_x509_cert_dir directory '%s'
"
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 87e730058b..bce8364c5a 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -115,7 +115,7 @@ struct _virQEMUDriverConfig {
char *swtpmStorageDir;
char *defaultTLSx509certdir;
- bool checkdefaultTLSx509certdir;
+ bool defaultTLSx509certdirPresent;
bool defaultTLSx509verify;
char *defaultTLSx509secretUUID;
--
2.20.1
3:29 p.m.
New subject: [libvirt] [PATCH 22/23] qemu_conf: rename checkdefaultTLSx509certdir
On 1/15/19 8:23 AM, Ján Tomko wrote:
Use defaultTLSx509certdirPresent for consistencty.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 4 ++--
src/qemu/qemu_conf.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2:23 p.m.
New subject: [libvirt] [PATCH 23/23] qemu_conf: fix stray space
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0aa428b87f..18ad99c173 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -915,7 +915,7 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (controllers) {
- cfg-> cgroupControllers = 0;
+ cfg->cgroupControllers = 0;
for (i = 0; controllers[i] != NULL; i++) {
int ctl;
if ((ctl = virCgroupControllerTypeFromString(controllers[i])) < 0) {
--
2.20.1
3:29 p.m.
New subject: [libvirt] [PATCH 23/23] qemu_conf: fix stray space
On 1/15/19 8:23 AM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Trivial
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2073
days inactive
2076
days old
52 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko