[libvirt] [PATCH v4 09/14] qemu: Introduce pr_helper to qemu.conf

Michal Privoznik posted 14 patches 7 years, 1 month ago
There is a newer version of this series
[libvirt] [PATCH v4 09/14] qemu: Introduce pr_helper to qemu.conf
Posted by Michal Privoznik 7 years, 1 month ago
Just like we allow users overriding path to bridge-helper
detected at compile time we can allow them to override path to
qemu-pr-helper.

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

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index b9bafdab90..80e1d3ad46 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -57,6 +57,11 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
                [/usr/libexec:/usr/lib/qemu:/usr/lib])
   AC_DEFINE_UNQUOTED([QEMU_BRIDGE_HELPER], ["$QEMU_BRIDGE_HELPER"],
                      [QEMU bridge helper])
+  AC_PATH_PROG([QEMU_PR_HELPER], [qemu-pr-helper],
+               [/usr/bin/qemu-pr-helper],
+               [/usr/bin:/usr/libexec])
+  AC_DEFINE_UNQUOTED([QEMU_PR_HELPER], ["$QEMU_PR_HELPER"],
+                     [QEMU PR helper])
 ])
 
 AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index c19bf3a43a..2dc16e91fd 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -86,6 +86,7 @@ module Libvirtd_qemu =
    let process_entry = str_entry "hugetlbfs_mount"
                  | bool_entry "clear_emulator_capabilities"
                  | str_entry "bridge_helper"
+                 | str_entry "pr_helper"
                  | bool_entry "set_process_name"
                  | int_entry "max_processes"
                  | int_entry "max_files"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 07eab7efff..30fdd54e2c 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -775,3 +775,7 @@
 # This directory is used for memoryBacking source if configured as file.
 # NOTE: big files will be stored here
 #memory_backing_dir = "/var/lib/libvirt/qemu/ram"
+
+# Path to the SCSI persistent reservations helper. This helper is
+# used whenever <reservations/> are enabled for SCSI disks.
+#pr_helper = "/usr/libexec/qemu-pr-helper"
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 36cf3a281c..8c69dbe75c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -307,7 +307,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
             goto error;
     }
 
-    if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0)
+    if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0 ||
+        VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0)
         goto error;
 
     cfg->clearEmulatorCapabilities = true;
@@ -392,6 +393,7 @@ static void virQEMUDriverConfigDispose(void *obj)
     }
     VIR_FREE(cfg->hugetlbfs);
     VIR_FREE(cfg->bridgeHelperName);
+    VIR_FREE(cfg->prHelperName);
 
     VIR_FREE(cfg->saveImageFormat);
     VIR_FREE(cfg->dumpImageFormat);
@@ -759,6 +761,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0)
         goto cleanup;
 
+    if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName) < 0)
+        goto cleanup;
+
     if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) < 0)
         goto cleanup;
 
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index e1ad5463f3..7a63780c48 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -153,6 +153,7 @@ struct _virQEMUDriverConfig {
     size_t nhugetlbfs;
 
     char *bridgeHelperName;
+    char *prHelperName;
 
     bool macFilter;
 
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 688e5b9fda..c0efae47bd 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -100,3 +100,4 @@ module Test_libvirtd_qemu =
     { "1" = "mount" }
 }
 { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
+{ "pr_helper" = "/usr/libexec/qemu-pr-helper" }
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 09/14] qemu: Introduce pr_helper to qemu.conf
Posted by John Ferlan 7 years ago

On 04/10/2018 10:58 AM, Michal Privoznik wrote:
> Just like we allow users overriding path to bridge-helper
> detected at compile time we can allow them to override path to
> qemu-pr-helper.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  m4/virt-driver-qemu.m4             | 5 +++++
>  src/qemu/libvirtd_qemu.aug         | 1 +
>  src/qemu/qemu.conf                 | 4 ++++
>  src/qemu/qemu_conf.c               | 7 ++++++-
>  src/qemu/qemu_conf.h               | 1 +
>  src/qemu/test_libvirtd_qemu.aug.in | 1 +
>  6 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
> index b9bafdab90..80e1d3ad46 100644
> --- a/m4/virt-driver-qemu.m4
> +++ b/m4/virt-driver-qemu.m4
> @@ -57,6 +57,11 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
>                 [/usr/libexec:/usr/lib/qemu:/usr/lib])
>    AC_DEFINE_UNQUOTED([QEMU_BRIDGE_HELPER], ["$QEMU_BRIDGE_HELPER"],
>                       [QEMU bridge helper])
> +  AC_PATH_PROG([QEMU_PR_HELPER], [qemu-pr-helper],
> +               [/usr/bin/qemu-pr-helper],
> +               [/usr/bin:/usr/libexec])

So the default install location of qemu-pr-helper is /usr/bin unlike
bridge-helper which is /usr/libexec?

> +  AC_DEFINE_UNQUOTED([QEMU_PR_HELPER], ["$QEMU_PR_HELPER"],
> +                     [QEMU PR helper])
>  ])
>  
>  AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index c19bf3a43a..2dc16e91fd 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -86,6 +86,7 @@ module Libvirtd_qemu =
>     let process_entry = str_entry "hugetlbfs_mount"
>                   | bool_entry "clear_emulator_capabilities"
>                   | str_entry "bridge_helper"
> +                 | str_entry "pr_helper"
>                   | bool_entry "set_process_name"
>                   | int_entry "max_processes"
>                   | int_entry "max_files"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 07eab7efff..30fdd54e2c 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -775,3 +775,7 @@
>  # This directory is used for memoryBacking source if configured as file.
>  # NOTE: big files will be stored here
>  #memory_backing_dir = "/var/lib/libvirt/qemu/ram"
> +
> +# Path to the SCSI persistent reservations helper. This helper is
> +# used whenever <reservations/> are enabled for SCSI disks.

s/disks/LUN devices/

> +#pr_helper = "/usr/libexec/qemu-pr-helper"

Going with my note above - the default path in the m4 file shows
/usr/bin first... So should this match that? Similar to how
qemu-bridge-helper matches the /usr/libexec install location?

> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 36cf3a281c..8c69dbe75c 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -307,7 +307,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>              goto error;
>      }
>  
> -    if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0)
> +    if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0 ||
> +        VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0)
>          goto error;
>  
>      cfg->clearEmulatorCapabilities = true;
> @@ -392,6 +393,7 @@ static void virQEMUDriverConfigDispose(void *obj)
>      }
>      VIR_FREE(cfg->hugetlbfs);
>      VIR_FREE(cfg->bridgeHelperName);
> +    VIR_FREE(cfg->prHelperName);
>  
>      VIR_FREE(cfg->saveImageFormat);
>      VIR_FREE(cfg->dumpImageFormat);
> @@ -759,6 +761,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>      if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0)
>          goto cleanup;
>  
> +    if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName) < 0)
> +        goto cleanup;
> +
>      if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) < 0)
>          goto cleanup;
>  
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index e1ad5463f3..7a63780c48 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -153,6 +153,7 @@ struct _virQEMUDriverConfig {
>      size_t nhugetlbfs;
>  
>      char *bridgeHelperName;
> +    char *prHelperName;
>  
>      bool macFilter;
>  
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index 688e5b9fda..c0efae47bd 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -100,3 +100,4 @@ module Test_libvirtd_qemu =
>      { "1" = "mount" }
>  }
>  { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
> +{ "pr_helper" = "/usr/libexec/qemu-pr-helper" }
> 

And don't forget this one if something does change, but I'm sure that'd
wash out of the check syntax-check

I think the questions/issues are minimal and easily fixed by a response,
so consider the concept at least

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

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 09/14] qemu: Introduce pr_helper to qemu.conf
Posted by Michal Privoznik 7 years ago
On 04/14/2018 03:35 PM, John Ferlan wrote:
> 
> 
> On 04/10/2018 10:58 AM, Michal Privoznik wrote:
>> Just like we allow users overriding path to bridge-helper
>> detected at compile time we can allow them to override path to
>> qemu-pr-helper.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  m4/virt-driver-qemu.m4             | 5 +++++
>>  src/qemu/libvirtd_qemu.aug         | 1 +
>>  src/qemu/qemu.conf                 | 4 ++++
>>  src/qemu/qemu_conf.c               | 7 ++++++-
>>  src/qemu/qemu_conf.h               | 1 +
>>  src/qemu/test_libvirtd_qemu.aug.in | 1 +
>>  6 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
>> index b9bafdab90..80e1d3ad46 100644
>> --- a/m4/virt-driver-qemu.m4
>> +++ b/m4/virt-driver-qemu.m4
>> @@ -57,6 +57,11 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
>>                 [/usr/libexec:/usr/lib/qemu:/usr/lib])
>>    AC_DEFINE_UNQUOTED([QEMU_BRIDGE_HELPER], ["$QEMU_BRIDGE_HELPER"],
>>                       [QEMU bridge helper])
>> +  AC_PATH_PROG([QEMU_PR_HELPER], [qemu-pr-helper],
>> +               [/usr/bin/qemu-pr-helper],
>> +               [/usr/bin:/usr/libexec])
> 
> So the default install location of qemu-pr-helper is /usr/bin unlike
> bridge-helper which is /usr/libexec?

Yes. I guess this has something to do with aforementioned systemd socket
activation.

> 
>> +  AC_DEFINE_UNQUOTED([QEMU_PR_HELPER], ["$QEMU_PR_HELPER"],
>> +                     [QEMU PR helper])
>>  ])
>>  
>>  AC_DEFUN([LIBVIRT_DRIVER_RESULT_QEMU], [
>> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
>> index c19bf3a43a..2dc16e91fd 100644
>> --- a/src/qemu/libvirtd_qemu.aug
>> +++ b/src/qemu/libvirtd_qemu.aug
>> @@ -86,6 +86,7 @@ module Libvirtd_qemu =
>>     let process_entry = str_entry "hugetlbfs_mount"
>>                   | bool_entry "clear_emulator_capabilities"
>>                   | str_entry "bridge_helper"
>> +                 | str_entry "pr_helper"
>>                   | bool_entry "set_process_name"
>>                   | int_entry "max_processes"
>>                   | int_entry "max_files"
>> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
>> index 07eab7efff..30fdd54e2c 100644
>> --- a/src/qemu/qemu.conf
>> +++ b/src/qemu/qemu.conf
>> @@ -775,3 +775,7 @@
>>  # This directory is used for memoryBacking source if configured as file.
>>  # NOTE: big files will be stored here
>>  #memory_backing_dir = "/var/lib/libvirt/qemu/ram"
>> +
>> +# Path to the SCSI persistent reservations helper. This helper is
>> +# used whenever <reservations/> are enabled for SCSI disks.
> 
> s/disks/LUN devices/
> 
>> +#pr_helper = "/usr/libexec/qemu-pr-helper"
> 
> Going with my note above - the default path in the m4 file shows
> /usr/bin first... So should this match that? Similar to how
> qemu-bridge-helper matches the /usr/libexec install location?

Yeah, this is a leftover from previous rounds where I had /usr/libexec/.
I'm changing this to /usr/bin/.

> 
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 36cf3a281c..8c69dbe75c 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -307,7 +307,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>              goto error;
>>      }
>>  
>> -    if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0)
>> +    if (VIR_STRDUP(cfg->bridgeHelperName, QEMU_BRIDGE_HELPER) < 0 ||
>> +        VIR_STRDUP(cfg->prHelperName, QEMU_PR_HELPER) < 0)
>>          goto error;
>>  
>>      cfg->clearEmulatorCapabilities = true;
>> @@ -392,6 +393,7 @@ static void virQEMUDriverConfigDispose(void *obj)
>>      }
>>      VIR_FREE(cfg->hugetlbfs);
>>      VIR_FREE(cfg->bridgeHelperName);
>> +    VIR_FREE(cfg->prHelperName);
>>  
>>      VIR_FREE(cfg->saveImageFormat);
>>      VIR_FREE(cfg->dumpImageFormat);
>> @@ -759,6 +761,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>>      if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0)
>>          goto cleanup;
>>  
>> +    if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName) < 0)
>> +        goto cleanup;
>> +
>>      if (virConfGetValueBool(conf, "mac_filter", &cfg->macFilter) < 0)
>>          goto cleanup;
>>  
>> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
>> index e1ad5463f3..7a63780c48 100644
>> --- a/src/qemu/qemu_conf.h
>> +++ b/src/qemu/qemu_conf.h
>> @@ -153,6 +153,7 @@ struct _virQEMUDriverConfig {
>>      size_t nhugetlbfs;
>>  
>>      char *bridgeHelperName;
>> +    char *prHelperName;
>>  
>>      bool macFilter;
>>  
>> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
>> index 688e5b9fda..c0efae47bd 100644
>> --- a/src/qemu/test_libvirtd_qemu.aug.in
>> +++ b/src/qemu/test_libvirtd_qemu.aug.in
>> @@ -100,3 +100,4 @@ module Test_libvirtd_qemu =
>>      { "1" = "mount" }
>>  }
>>  { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" }
>> +{ "pr_helper" = "/usr/libexec/qemu-pr-helper" }
>>
> 
> And don't forget this one if something does change, but I'm sure that'd
> wash out of the check syntax-check
> 
> I think the questions/issues are minimal and easily fixed by a response,
> so consider the concept at least
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>

Thanks,
Michal

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