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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx>
Thanks,
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
