[libvirt] [PATCH v3 19/28] qemu_conf: Introduce metadata_lock_manager

Michal Privoznik posted 28 patches 7 years, 5 months ago
There is a newer version of this series
[libvirt] [PATCH v3 19/28] qemu_conf: Introduce metadata_lock_manager
Posted by Michal Privoznik 7 years, 5 months ago
This config option allows users to set and enable lock manager
for domain metadata. The lock manager is going to be used by
security drivers to serialize each other when changing a file
ownership or changing the SELinux label. The only supported lock
manager is 'lockd' for now.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/libvirtd_qemu.aug         |  1 +
 src/qemu/qemu.conf                 |  6 ++++++
 src/qemu/qemu_conf.c               | 13 +++++++++++++
 src/qemu/qemu_conf.h               |  1 +
 src/qemu/test_libvirtd_qemu.aug.in |  1 +
 5 files changed, 22 insertions(+)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index ddc4bbfd1d..42e325d4fb 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -98,6 +98,7 @@ module Libvirtd_qemu =
                  | bool_entry "relaxed_acs_check"
                  | bool_entry "allow_disk_format_probing"
                  | str_entry "lock_manager"
+                 | str_entry "metadata_lock_manager"
 
    let rpc_entry = int_entry "max_queued"
                  | int_entry "keepalive_interval"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index cd57b3cc69..06caa39232 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -659,6 +659,12 @@
 #lock_manager = "lockd"
 
 
+# To serialize two daemons trying to change metadata on a file,
+# libvirt offers a locking mechanism. Currently, only "lockd" is
+# supported (or no locking at all if unset).
+#
+#metadata_lock_manager = "lockd"
+
 
 # Set limit of maximum APIs queued on one domain. All other APIs
 # over this threshold will fail on acquiring job lock. Specially,
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a4f545ef92..46318b7b2a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -428,6 +428,7 @@ static void virQEMUDriverConfigDispose(void *obj)
     virStringListFree(cfg->securityDriverNames);
 
     VIR_FREE(cfg->lockManagerName);
+    VIR_FREE(cfg->metadataLockManagerName);
 
     virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
 
@@ -838,6 +839,18 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 
     if (virConfGetValueString(conf, "lock_manager", &cfg->lockManagerName) < 0)
         goto cleanup;
+
+    if (virConfGetValueString(conf, "metadata_lock_manager",
+                              &cfg->metadataLockManagerName) < 0)
+        goto cleanup;
+    if (cfg->metadataLockManagerName &&
+        STRNEQ(cfg->metadataLockManagerName, "lockd")) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("unknown metadata lock manager name %s"),
+                       cfg->metadataLockManagerName);
+        goto cleanup;
+    }
+
     if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) < 0)
         goto cleanup;
     if (stdioHandler) {
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index a8d84efea2..c227ac72cc 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -186,6 +186,7 @@ struct _virQEMUDriverConfig {
     bool autoStartBypassCache;
 
     char *lockManagerName;
+    char *metadataLockManagerName;
 
     int keepAliveInterval;
     unsigned int keepAliveCount;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index f1e8806ad2..451e73126e 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -81,6 +81,7 @@ module Test_libvirtd_qemu =
 { "mac_filter" = "1" }
 { "relaxed_acs_check" = "1" }
 { "lock_manager" = "lockd" }
+{ "metadata_lock_manager" = "lockd" }
 { "max_queued" = "0" }
 { "keepalive_interval" = "5" }
 { "keepalive_count" = "5" }
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 19/28] qemu_conf: Introduce metadata_lock_manager
Posted by John Ferlan 7 years, 5 months ago

On 08/27/2018 04:08 AM, Michal Privoznik wrote:
> This config option allows users to set and enable lock manager
> for domain metadata. The lock manager is going to be used by
> security drivers to serialize each other when changing a file
> ownership or changing the SELinux label. The only supported lock
> manager is 'lockd' for now.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/libvirtd_qemu.aug         |  1 +
>  src/qemu/qemu.conf                 |  6 ++++++
>  src/qemu/qemu_conf.c               | 13 +++++++++++++
>  src/qemu/qemu_conf.h               |  1 +
>  src/qemu/test_libvirtd_qemu.aug.in |  1 +
>  5 files changed, 22 insertions(+)
> 
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index ddc4bbfd1d..42e325d4fb 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -98,6 +98,7 @@ module Libvirtd_qemu =
>                   | bool_entry "relaxed_acs_check"
>                   | bool_entry "allow_disk_format_probing"
>                   | str_entry "lock_manager"
> +                 | str_entry "metadata_lock_manager"
>  
>     let rpc_entry = int_entry "max_queued"
>                   | int_entry "keepalive_interval"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index cd57b3cc69..06caa39232 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -659,6 +659,12 @@
>  #lock_manager = "lockd"
>  
>  
> +# To serialize two daemons trying to change metadata on a file,

Just two? ;-)

> +# libvirt offers a locking mechanism. Currently, only "lockd" is
> +# supported (or no locking at all if unset).
> +#
> +#metadata_lock_manager = "lockd"
> +

Should we state that the domain locking is independent of the metadata
daemon locking? I know it's obvious to the author (and now the
reviewer), but for the first time reader of the config file.

And of course that leaves the question on the table for the consumer
about what is meant by multiple daemons. As in, which daemons...  That
little factoid would be lost in the commit message.

Assuming some beefed up text,

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

[...]

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