[PATCH v2] qemu: fix memory leak about virDomainEventTunableNew
by luzhipeng
From: lu zhipeng <luzhipeng(a)cestc.cn>
For prevent memory leak and easier to use, So change
virDomainEventTunableNew to get virTypedParameterPtr *params
and set it = NULL.
Signed-off-by: lu zhipeng <luzhipeng(a)cestc.cn>
---
v1: https://patchew.org/Libvirt/20220922130038.1616-1-luzhipeng@cestc.cn/
src/conf/domain_event.c | 12 ++++++------
src/conf/domain_event.h | 4 ++--
src/qemu/qemu_driver.c | 22 ++++++++++------------
src/remote/remote_driver.c | 2 +-
4 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index ff8ea2c389..97d58c2521 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1495,7 +1495,7 @@ static virObjectEvent *
virDomainEventTunableNew(int id,
const char *name,
unsigned char *uuid,
- virTypedParameterPtr params,
+ virTypedParameterPtr *params,
int nparams)
{
virDomainEventTunable *ev;
@@ -1508,19 +1508,19 @@ virDomainEventTunableNew(int id,
id, name, uuid)))
goto error;
- ev->params = params;
+ ev->params = *params;
ev->nparams = nparams;
-
+ *params = NULL;
return (virObjectEvent *)ev;
error:
- virTypedParamsFree(params, nparams);
+ virTypedParamsFree(*params, nparams);
return NULL;
}
virObjectEvent *
virDomainEventTunableNewFromObj(virDomainObj *obj,
- virTypedParameterPtr params,
+ virTypedParameterPtr *params,
A
int nparams)
{
return virDomainEventTunableNew(obj->def->id,
@@ -1532,7 +1532,7 @@ virDomainEventTunableNewFromObj(virDomainObj *obj,
virObjectEvent *
virDomainEventTunableNewFromDom(virDomainPtr dom,
- virTypedParameterPtr params,
+ virTypedParameterPtr *params,
int nparams)
{
return virDomainEventTunableNew(dom->id,
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 4a9f6b988b..f4016dc1e9 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -195,11 +195,11 @@ virDomainEventDeviceRemovalFailedNewFromDom(virDomainPtr dom,
virObjectEvent *
virDomainEventTunableNewFromObj(virDomainObj *obj,
- virTypedParameterPtr params,
+ virTypedParameterPtr *params,
int nparams);
virObjectEvent *
virDomainEventTunableNewFromDom(virDomainPtr dom,
- virTypedParameterPtr params,
+ virTypedParameterPtr *params,
int nparams);
virObjectEvent *
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 94b70872d4..3db4592945 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4472,12 +4472,12 @@ qemuDomainPinVcpuLive(virDomainObj *vm,
&eventMaxparams, paramField, str) < 0)
goto cleanup;
- event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
A
-
+ event = virDomainEventTunableNewFromObj(vm, &eventParams, eventNparams);
ret = 0;
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
@@ -4681,7 +4681,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
str) < 0)
goto endjob;
- event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
+ event = virDomainEventTunableNewFromDom(dom, &eventParams, eventNparams);
}
if (persistentDef) {
B
A
@@ -4700,6 +4700,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
virDomainObjEndAPI(&vm);
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
@@ -5078,7 +5079,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
&eventMaxparams, paramField, str) < 0)
goto endjob;
- event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
+ event = virDomainEventTunableNewFromDom(dom, &eventParams, eventNparams);
}
if (persistentDef) {
@@ -5106,6 +5107,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
virDomainObjEndAPI(&vm);
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
@@ -9633,8 +9635,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
qemuDomainSaveStatus(vm);
if (eventNparams) {
- event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
- eventNparams = 0;
+ event = virDomainEventTunableNewFromDom(dom, &eventParams, eventNparams);
virObjectEventStateQueue(driver->domainEventState, event);
}
@@ -9654,8 +9655,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- if (eventNparams)
- virTypedParamsFree(eventParams, eventNparams);
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
#undef SCHED_RANGE_CHECK
@@ -16159,8 +16159,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
qemuDomainSaveStatus(vm);
if (eventNparams) {
- event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
- eventNparams = 0;
+ event = virDomainEventTunableNewFromDom(dom, &eventParams, eventNparams);
virObjectEventStateQueue(driver->domainEventState, event);
}
}
@@ -16202,8 +16201,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
VIR_FREE(info.group_name);
VIR_FREE(conf_info.group_name);
virDomainObjEndAPI(&vm);
- if (eventNparams)
- virTypedParamsFree(eventParams, eventNparams);
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index a4efe101a3..b0dba9057b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5078,7 +5078,7 @@ remoteDomainBuildEventCallbackTunable(virNetClientProgram *prog G_GNUC_UNUSED,
return;
}
- event = virDomainEventTunableNewFromDom(dom, params, nparams);
+ event = virDomainEventTunableNewFromDom(dom, ¶ms, nparams);
virObjectUnref(dom);
--
2.31.1
1 year, 11 months
[PATCH] qemu: fix memory leak on some paths
by luzhipeng
From: lu zhipeng <luzhipeng(a)cestc.cn>
virTypedParamsAddString may return -1 but alloc params,
so free it.
Signed-off-by: lu zhipeng <luzhipeng(a)cestc.cn>
---
src/qemu/qemu_driver.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 94b70872d4..c4501cd705 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4473,11 +4473,13 @@ qemuDomainPinVcpuLive(virDomainObj *vm,
goto cleanup;
event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
-
+ eventNparams = 0;
ret = 0;
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
+ if (eventNparams)
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
@@ -4682,6 +4684,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
goto endjob;
event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
+ eventNparams = 0;
}
if (persistentDef) {
@@ -4700,6 +4703,8 @@ qemuDomainPinEmulator(virDomainPtr dom,
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
virDomainObjEndAPI(&vm);
+ if (eventNparams)
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
@@ -5079,6 +5084,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
goto endjob;
event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
+ eventNparams = 0;
}
if (persistentDef) {
@@ -5106,6 +5112,8 @@ qemuDomainPinIOThread(virDomainPtr dom,
cleanup:
virObjectEventStateQueue(driver->domainEventState, event);
virDomainObjEndAPI(&vm);
+ if (eventNparams)
+ virTypedParamsFree(eventParams, eventNparams);
return ret;
}
--
2.31.1
1 year, 11 months
[PATCH 0/2] security_selinux: Don't ignore NVMe disks when setting image label
by Michal Privoznik
The first patch is the important one. But since I could not decide which
code looks better I've posted 2/2 too which moves a check.
Michal Prívozník (2):
security_selinux: Don't ignore NVMe disks when setting image label
security_selinux: Move shortcut in
virSecuritySELinuxSetImageLabelInternal() later
src/security/security_selinux.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--
2.35.1
1 year, 11 months
Re: [PATCH] watchdog: remove -watchdog option
by Thomas Huth
On 22/09/2022 11.29, Paolo Bonzini wrote:
> This was deprecated in 6.2 and is ready to go. It removes quite a bit
> of code that handled the registration of watchdog models.
>
> Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com>
> ---
> docs/about/deprecated.rst | 5 ----
> docs/about/removed-features.rst | 5 ++++
> hw/watchdog/sbsa_gwdt.c | 6 -----
> hw/watchdog/watchdog.c | 45 ---------------------------------
> hw/watchdog/wdt_aspeed.c | 6 -----
> hw/watchdog/wdt_diag288.c | 6 -----
> hw/watchdog/wdt_i6300esb.c | 6 -----
> hw/watchdog/wdt_ib700.c | 6 -----
> hw/watchdog/wdt_imx2.c | 6 -----
> include/sysemu/watchdog.h | 12 ---------
> qemu-options.hx | 33 ++----------------------
> softmmu/vl.c | 16 ------------
> 12 files changed, 7 insertions(+), 145 deletions(-)
Nice clean-up!
Reviewed-by: Thomas Huth <thuth(a)redhat.com>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index a72fedba5f..93affe3669 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -81,11 +81,6 @@ the process listing. This is replaced by the new ``password-secret``
> option which lets the password be securely provided on the command
> line using a ``secret`` object instance.
>
> -``-watchdog`` (since 6.2)
> -'''''''''''''''''''''''''
> -
> -Use ``-device`` instead.
> -
> ``-smp`` ("parameter=0" SMP configurations) (since 6.2)
> '''''''''''''''''''''''''''''''''''''''''''''''''''''''
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index a4aa3dca69..63df9848fd 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -403,6 +403,11 @@ Sound card devices should be created using ``-device`` or ``-audio``.
> The exception is ``pcspk`` which can be activated using ``-machine
> pcspk-audiodev=<name>``.
>
> +``-watchdog`` (since 7.2)
> +'''''''''''''''''''''''''
> +
> +Use ``-device`` instead.
> +
>
> QEMU Machine Protocol (QMP) commands
> ------------------------------------
> diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
> index e49cacd0e2..7aa57a8c51 100644
> --- a/hw/watchdog/sbsa_gwdt.c
> +++ b/hw/watchdog/sbsa_gwdt.c
> @@ -24,11 +24,6 @@
> #include "qemu/log.h"
> #include "qemu/module.h"
>
> -static WatchdogTimerModel model = {
> - .wdt_name = TYPE_WDT_SBSA,
> - .wdt_description = "SBSA-compliant generic watchdog device",
> -};
> -
> static const VMStateDescription vmstate_sbsa_gwdt = {
> .name = "sbsa-gwdt",
> .version_id = 1,
> @@ -287,7 +282,6 @@ static const TypeInfo wdt_sbsa_gwdt_info = {
>
> static void wdt_sbsa_gwdt_register_types(void)
> {
> - watchdog_add_model(&model);
> type_register_static(&wdt_sbsa_gwdt_info);
> }
>
> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
> index 1437e6c5b6..b132546516 100644
> --- a/hw/watchdog/watchdog.c
> +++ b/hw/watchdog/watchdog.c
> @@ -31,51 +31,6 @@
> #include "hw/nmi.h"
> #include "qemu/help_option.h"
>
> -static WatchdogAction watchdog_action = WATCHDOG_ACTION_RESET;
> -static QLIST_HEAD(, WatchdogTimerModel) watchdog_list;
> -
> -void watchdog_add_model(WatchdogTimerModel *model)
> -{
> - QLIST_INSERT_HEAD(&watchdog_list, model, entry);
> -}
> -
> -/* Returns:
> - * 0 = continue
> - * 1 = exit program with error
> - * 2 = exit program without error
> - */
> -int select_watchdog(const char *p)
> -{
> - WatchdogTimerModel *model;
> - QemuOpts *opts;
> -
> - /* -watchdog ? lists available devices and exits cleanly. */
> - if (is_help_option(p)) {
> - QLIST_FOREACH(model, &watchdog_list, entry) {
> - fprintf(stderr, "\t%s\t%s\n",
> - model->wdt_name, model->wdt_description);
> - }
> - return 2;
> - }
> -
> - QLIST_FOREACH(model, &watchdog_list, entry) {
> - if (strcasecmp(model->wdt_name, p) == 0) {
> - /* add the device */
> - opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
> - &error_abort);
> - qemu_opt_set(opts, "driver", p, &error_abort);
> - return 0;
> - }
> - }
> -
> - fprintf(stderr, "Unknown -watchdog device. Supported devices are:\n");
> - QLIST_FOREACH(model, &watchdog_list, entry) {
> - fprintf(stderr, "\t%s\t%s\n",
> - model->wdt_name, model->wdt_description);
> - }
> - return 1;
> -}
> -
> WatchdogAction get_watchdog_action(void)
> {
> return watchdog_action;
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index 31855afdf4..d753693a2e 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -202,11 +202,6 @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data,
> return;
> }
>
> -static WatchdogTimerModel model = {
> - .wdt_name = TYPE_ASPEED_WDT,
> - .wdt_description = "Aspeed watchdog device",
> -};
> -
> static const VMStateDescription vmstate_aspeed_wdt = {
> .name = "vmstate_aspeed_wdt",
> .version_id = 0,
> @@ -416,7 +411,6 @@ static const TypeInfo aspeed_1030_wdt_info = {
>
> static void wdt_aspeed_register_types(void)
> {
> - watchdog_add_model(&model);
> type_register_static(&aspeed_wdt_info);
> type_register_static(&aspeed_2400_wdt_info);
> type_register_static(&aspeed_2500_wdt_info);
> diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c
> index 9e8882a11c..76d89fbf78 100644
> --- a/hw/watchdog/wdt_diag288.c
> +++ b/hw/watchdog/wdt_diag288.c
> @@ -19,11 +19,6 @@
> #include "migration/vmstate.h"
> #include "qemu/log.h"
>
> -static WatchdogTimerModel model = {
> - .wdt_name = TYPE_WDT_DIAG288,
> - .wdt_description = "diag288 device for s390x platform",
> -};
> -
> static const VMStateDescription vmstate_diag288 = {
> .name = "vmstate_diag288",
> .version_id = 0,
> @@ -138,7 +133,6 @@ static const TypeInfo wdt_diag288_info = {
>
> static void wdt_diag288_register_types(void)
> {
> - watchdog_add_model(&model);
> type_register_static(&wdt_diag288_info);
> }
>
> diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
> index f99a1c9d29..5693ec6a09 100644
> --- a/hw/watchdog/wdt_i6300esb.c
> +++ b/hw/watchdog/wdt_i6300esb.c
> @@ -457,11 +457,6 @@ static void i6300esb_exit(PCIDevice *dev)
> timer_free(d->timer);
> }
>
> -static WatchdogTimerModel model = {
> - .wdt_name = "i6300esb",
> - .wdt_description = "Intel 6300ESB",
> -};
> -
> static void i6300esb_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -493,7 +488,6 @@ static const TypeInfo i6300esb_info = {
>
> static void i6300esb_register_types(void)
> {
> - watchdog_add_model(&model);
> type_register_static(&i6300esb_info);
> }
>
> diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c
> index 91d1bdc0da..b116c3a3aa 100644
> --- a/hw/watchdog/wdt_ib700.c
> +++ b/hw/watchdog/wdt_ib700.c
> @@ -128,11 +128,6 @@ static void wdt_ib700_reset(DeviceState *dev)
> timer_del(s->timer);
> }
>
> -static WatchdogTimerModel model = {
> - .wdt_name = "ib700",
> - .wdt_description = "iBASE 700",
> -};
> -
> static void wdt_ib700_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -153,7 +148,6 @@ static const TypeInfo wdt_ib700_info = {
>
> static void wdt_ib700_register_types(void)
> {
> - watchdog_add_model(&model);
> type_register_static(&wdt_ib700_info);
> }
>
> diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c
> index c3128370b5..e776a2fbd4 100644
> --- a/hw/watchdog/wdt_imx2.c
> +++ b/hw/watchdog/wdt_imx2.c
> @@ -291,14 +291,8 @@ static const TypeInfo imx2_wdt_info = {
> .class_init = imx2_wdt_class_init,
> };
>
> -static WatchdogTimerModel model = {
> - .wdt_name = "imx2-watchdog",
> - .wdt_description = "i.MX2 Watchdog",
> -};
> -
> static void imx2_wdt_register_type(void)
> {
> - watchdog_add_model(&model);
> type_register_static(&imx2_wdt_info);
> }
> type_init(imx2_wdt_register_type)
> diff --git a/include/sysemu/watchdog.h b/include/sysemu/watchdog.h
> index d2d4901dbb..745c89b02b 100644
> --- a/include/sysemu/watchdog.h
> +++ b/include/sysemu/watchdog.h
> @@ -25,20 +25,8 @@
> #include "qemu/queue.h"
> #include "qapi/qapi-types-run-state.h"
>
> -struct WatchdogTimerModel {
> - QLIST_ENTRY(WatchdogTimerModel) entry;
> -
> - /* Short name of the device - used to select it on the command line. */
> - const char *wdt_name;
> - /* Longer description (eg. manufacturer and full model number). */
> - const char *wdt_description;
> -};
> -typedef struct WatchdogTimerModel WatchdogTimerModel;
> -
> /* in hw/watchdog.c */
> -int select_watchdog(const char *p);
> WatchdogAction get_watchdog_action(void);
> -void watchdog_add_model(WatchdogTimerModel *model);
> void watchdog_perform_action(void);
>
> #endif /* QEMU_WATCHDOG_H */
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d8b5ce5b43..df4b8c8f1a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4330,7 +4330,7 @@ SRST
>
> ``-action panic=none``
> ``-action reboot=shutdown,shutdown=pause``
> - ``-watchdog i6300esb -action watchdog=pause``
> + ``-device i6300esb -action watchdog=pause``
>
> ERST
>
> @@ -4448,35 +4448,6 @@ SRST
> specifies the snapshot name used to load the initial VM state.
> ERST
>
> -DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
> - "-watchdog model\n" \
> - " enable virtual hardware watchdog [default=none]\n",
> - QEMU_ARCH_ALL)
> -SRST
> -``-watchdog model``
> - Create a virtual hardware watchdog device. Once enabled (by a guest
> - action), the watchdog must be periodically polled by an agent inside
> - the guest or else the guest will be restarted. Choose a model for
> - which your guest has drivers.
> -
> - The model is the model of hardware watchdog to emulate. Use
> - ``-watchdog help`` to list available hardware models. Only one
> - watchdog can be enabled for a guest.
> -
> - The following models may be available:
> -
> - ``ib700``
> - iBASE 700 is a very simple ISA watchdog with a single timer.
> -
> - ``i6300esb``
> - Intel 6300ESB I/O controller hub is a much more featureful
> - PCI-based dual-timer watchdog.
> -
> - ``diag288``
> - A virtual watchdog for s390x backed by the diagnose 288
> - hypercall (currently KVM only).
> -ERST
> -
> DEF("watchdog-action", HAS_ARG, QEMU_OPTION_watchdog_action, \
> "-watchdog-action reset|shutdown|poweroff|inject-nmi|pause|debug|none\n" \
> " action when watchdog fires [default=reset]\n",
> @@ -4498,7 +4469,7 @@ SRST
>
> Examples:
>
> - ``-watchdog i6300esb -watchdog-action pause``; \ ``-watchdog ib700``
> + ``-device i6300esb -watchdog-action pause``
>
> ERST
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index e62b9cc35d..b8788e765a 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -53,7 +53,6 @@
> #include "hw/isa/isa.h"
> #include "hw/scsi/scsi.h"
> #include "hw/display/vga.h"
> -#include "sysemu/watchdog.h"
> #include "hw/firmware/smbios.h"
> #include "hw/acpi/acpi.h"
> #include "hw/xen/xen.h"
> @@ -181,7 +180,6 @@ static Chardev **serial_hds;
> static const char *log_mask;
> static const char *log_file;
> static bool list_data_dirs;
> -static const char *watchdog;
> static const char *qtest_chrdev;
> static const char *qtest_log;
>
> @@ -2329,12 +2327,6 @@ static void qemu_process_sugar_options(void)
> }
> object_register_sugar_prop("memory-backend", "prealloc", "on", false);
> }
> -
> - if (watchdog) {
> - int i = select_watchdog(watchdog);
> - if (i > 0)
> - exit (i == 1 ? 1 : 0);
> - }
> }
>
> /* -action processing */
> @@ -3107,14 +3099,6 @@ void qemu_init(int argc, char **argv, char **envp)
> default_monitor = 0;
> }
> break;
> - case QEMU_OPTION_watchdog:
> - if (watchdog) {
> - error_report("only one watchdog option may be given");
> - exit(1);
> - }
> - warn_report("-watchdog is deprecated; use -device instead.");
> - watchdog = optarg;
> - break;
> case QEMU_OPTION_action:
> olist = qemu_find_opts("action");
> if (!qemu_opts_parse_noisily(olist, optarg, false)) {
1 year, 11 months
[RFC PATCH 0/5] add support for histograms as a virTypedParameterType
by Amneesh Singh
This series extends
https://listman.redhat.com/archives/libvir-list/2022-September/234197.html
to support histogram statistics. This requires adding it as a
virTypedParameterType.
[1/5]:
- add virHistogram as a virTypedParameterType
- add utility functions for virHistogram
[2/5]
- add a global feature for histograms similar to strings
[3/5]
- add a flag for histograms similar to the one for strings to check
compatibility for older clients.
[4/5]
- add histogram statistics which were previously ignored in the series
linked above
[5/5]
- add RPC support for virHistogram
Amneesh Singh (5):
virtypedparam: add virHistogram as a virTypedParameterType
add a global feature for supporting virHistogram
virtypedparams: add VIR_TYPED_PARAM_HISTOGRAM_OKAY
qemu_driver: add histograms to the stats
remote: add virHistogram support for RPC as a virTypedParameterType
include/libvirt/libvirt-common.h.in | 59 +++++++++--
src/admin/admin_server_dispatch.c | 6 +-
src/ch/ch_driver.c | 1 +
src/driver.c | 1 +
src/esx/esx_driver.c | 1 +
src/libvirt.c | 46 ++++++---
src/libvirt_internal.h | 5 +
src/libvirt_private.syms | 7 ++
src/libvirt_public.syms | 6 ++
src/libxl/libxl_driver.c | 1 +
src/lxc/lxc_driver.c | 1 +
src/network/bridge_driver.c | 1 +
src/openvz/openvz_driver.c | 1 +
src/qemu/qemu_driver.c | 42 +++++++-
src/remote/remote_daemon_dispatch.c | 15 ++-
src/remote/remote_driver.c | 16 +--
src/remote/remote_protocol.x | 16 +++
src/remote_protocol-structs | 13 +++
src/rpc/gendispatch.pl | 4 +-
src/test/test_driver.c | 1 +
src/util/virtypedparam-public.c | 101 +++++++++++++++++++
src/util/virtypedparam.c | 145 ++++++++++++++++++++++++++++
src/util/virtypedparam.h | 22 +++++
src/vz/vz_driver.c | 1 +
tools/vsh.c | 5 +
25 files changed, 479 insertions(+), 38 deletions(-)
--
2.37.1
1 year, 11 months
[libvirt PATCH] src: warn if client hits the max requests limit
by Daniel P. Berrangé
Since they are simply normal RPC messages, the keep alive packets are
subject to the "max_client_requests" limit just like any API calls.
Thus, if a client hits the 'max_client_requests' limit and all the
pending API calls take a long time to complete, it may result in
keep-alives firing and dropping the client connection.
This has been seen by a number of users with the default value of
max_client_requests=5, by issuing 5 concurrent live migration
operations.
By printing a warning message when this happens, admins will be alerted
to the fact that their active clients are exceeding the default client
requests limit.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
I'm a little wary of this change. If we use anything less than VIR_WARN
it is not going to be useful, as we need it visible by default. At the
same time though I'm concerned that this might expose very many
deployments using an unreasonably low max_client_requests value for
their workload. For example OpenStack deployment tools have often left
this on the default setting and have been known to exceed it with live
migration running concurrently.
One possible optimization would be to only issue this warning once per
connected client, so we don't spam repeatedly ?
src/rpc/virnetserverclient.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index d57ca07167..0d82726194 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1259,6 +1259,10 @@ static virNetMessage *virNetServerClientDispatchRead(virNetServerClient *client)
client->rx->buffer = g_new0(char, client->rx->bufferLength);
client->nrequests++;
}
+ } else {
+ VIR_WARN("Client hit max requests limit %zd. This may result "
+ "in keep-alive timeouts. Consider tuning the "
+ "max_client_requests server parameter", client->nrequests);
}
virNetServerClientUpdateEvent(client);
--
2.37.2
1 year, 11 months
[PULL 21/30] Deprecate 32 bit big-endian MIPS
by Alex Bennée
It's becoming harder to maintain a cross-compiler to test this host
architecture as the old stable Debian 10 ("Buster") moved into LTS
which supports fewer architectures. For now:
- mark it's deprecation in the docs
- downgrade the containers to build TCG tests only
- drop the cross builds from our CI
Users with an appropriate toolchain and user-space can still take
their chances building it.
Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug(a)amsat.org>
Reviewed-by: Huacai Chen <chenhuacai(a)kernel.org>
Reviewed-by: Richard Henderson <richard.henderson(a)linaro.org>
Message-Id: <20220914155950.804707-22-alex.bennee(a)linaro.org>
diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index a2fee53248..1c1e7b9e11 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -41,7 +41,7 @@ Those hosts are officially supported, with various accelerators:
- Accelerators
* - Arm
- kvm (64 bit only), tcg, xen
- * - MIPS
+ * - MIPS (little endian only)
- kvm, tcg
* - PPC
- kvm, tcg
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index c75a25daad..0d1fd4469b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -213,6 +213,19 @@ MIPS ``Trap-and-Emul`` KVM support (since 6.0)
The MIPS ``Trap-and-Emul`` KVM host and guest support has been removed
from Linux upstream kernel, declare it deprecated.
+Host Architectures
+------------------
+
+BE MIPS (since 7.2)
+'''''''''''''''''''
+
+As Debian 10 ("Buster") moved into LTS the big endian 32 bit version of
+MIPS moved out of support making it hard to maintain our
+cross-compilation CI tests of the architecture. As we no longer have
+CI coverage support may bitrot away before the deprecation process
+completes. The little endian variants of MIPS (both 32 and 64 bit) are
+still a supported host architecture.
+
QEMU API (QAPI) events
----------------------
diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
index 611c6c0b39..95d57e1c5d 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -89,7 +89,6 @@ mips64el-debian-cross-container:
mips-debian-cross-container:
extends: .container_job_template
stage: containers
- needs: ['amd64-debian10-container']
variables:
NAME: debian-mips-cross
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 4a5fb6ea2a..c4cd96433d 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -70,20 +70,6 @@ cross-i386-tci:
EXTRA_CONFIGURE_OPTS: --target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
MAKE_CHECK_ARGS: check check-tcg
-cross-mips-system:
- extends: .cross_system_build_job
- needs:
- job: mips-debian-cross-container
- variables:
- IMAGE: debian-mips-cross
-
-cross-mips-user:
- extends: .cross_user_build_job
- needs:
- job: mips-debian-cross-container
- variables:
- IMAGE: debian-mips-cross
-
cross-mipsel-system:
extends: .cross_system_build_job
needs:
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index c3375f89c5..b1bf56434f 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -81,14 +81,12 @@ endif
# For non-x86 hosts not all cross-compilers have been packaged
ifneq ($(HOST_ARCH),x86_64)
-DOCKER_PARTIAL_IMAGES += debian-mips-cross debian-mipsel-cross debian-mips64el-cross
+DOCKER_PARTIAL_IMAGES += debian-mipsel-cross debian-mips64el-cross
DOCKER_PARTIAL_IMAGES += debian-ppc64el-cross
DOCKER_PARTIAL_IMAGES += debian-s390x-cross
DOCKER_PARTIAL_IMAGES += fedora
endif
-docker-image-debian-mips-cross: docker-image-debian10
-
# The native build should never use the registry
docker-image-debian-native: DOCKER_REGISTRY=
@@ -144,6 +142,7 @@ DOCKER_PARTIAL_IMAGES += debian-hppa-cross
DOCKER_PARTIAL_IMAGES += debian-loongarch-cross
DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
DOCKER_PARTIAL_IMAGES += debian-microblaze-cross
+DOCKER_PARTIAL_IMAGES += debian-mips-cross
DOCKER_PARTIAL_IMAGES += debian-nios2-cross
DOCKER_PARTIAL_IMAGES += debian-riscv64-test-cross
DOCKER_PARTIAL_IMAGES += debian-sh4-cross debian-sparc64-cross
diff --git a/tests/docker/dockerfiles/debian-mips-cross.docker b/tests/docker/dockerfiles/debian-mips-cross.docker
index 26c154014d..7b55f0f3b2 100644
--- a/tests/docker/dockerfiles/debian-mips-cross.docker
+++ b/tests/docker/dockerfiles/debian-mips-cross.docker
@@ -1,32 +1,14 @@
#
# Docker mips cross-compiler target
#
-# This docker target builds on the debian Buster base image.
+# This docker target builds on the Debian Bullseye base image.
#
-FROM qemu/debian10
-
-MAINTAINER Philippe Mathieu-Daudé <f4bug(a)amsat.org>
-
-# Add the foreign architecture we want and install dependencies
-RUN dpkg --add-architecture mips
-RUN apt update && \
- DEBIAN_FRONTEND=noninteractive eatmydata \
- apt install -y --no-install-recommends \
- gcc-mips-linux-gnu
-
-RUN apt update && \
- DEBIAN_FRONTEND=noninteractive eatmydata \
- apt build-dep -yy -a mips --arch-only qemu
-
-# Specify the cross prefix for this image (see tests/docker/common.rc)
-ENV QEMU_CONFIGURE_OPTS --cross-prefix=mips-linux-gnu-
-ENV DEF_TARGET_LIST mips-softmmu,mipsel-linux-user
-
-# Install extra libraries to increase code coverage
-RUN apt update && \
- DEBIAN_FRONTEND=noninteractive eatmydata \
- apt install -y --no-install-recommends \
- libbz2-dev:mips \
- liblzo2-dev:mips \
- librdmacm-dev:mips \
- libsnappy-dev:mips
+FROM docker.io/library/debian:11-slim
+
+RUN export DEBIAN_FRONTEND=noninteractive && \
+ apt-get update && \
+ apt-get install -y eatmydata && \
+ eatmydata apt-get dist-upgrade -y && \
+ eatmydata apt-get install --no-install-recommends -y \
+ gcc-mips-linux-gnu \
+ libc6-dev-mips-cross
--
2.34.1
1 year, 11 months
[PATCH v2] virdomainjob: virDomainObjInitJob: Avoid borrowing memory from 'virDomainXMLOption'
by Peter Krempa
The 'cb' and 'jobDataPrivateCb' pointers are stored in the job object
but made point to the memory owned by the virDomainXMLOption struct in
the callers.
Since the 'virdomainjob' module isn't in control the lifetime of the
virDomainXMLOption, which in some cases is freed before the domain job
data, freed memory would be dereferenced in some cases.
Copy the structs from virDomainXMLOption to ensure the lifetime. This is
possible since the callback functions are immutable.
Fixes: 84e9fd068ccad6e19e037cd6680df437617e2de5
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
v2:
- copy both pointers
- don't bother with creating copy functions, use g_memdup
src/conf/virdomainjob.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/virdomainjob.c b/src/conf/virdomainjob.c
index 7915faa125..aca801af38 100644
--- a/src/conf/virdomainjob.c
+++ b/src/conf/virdomainjob.c
@@ -128,8 +128,8 @@ virDomainObjInitJob(virDomainJobObj *job,
virDomainJobDataPrivateDataCallbacks *jobDataPrivateCb)
{
memset(job, 0, sizeof(*job));
- job->cb = cb;
- job->jobDataPrivateCb = jobDataPrivateCb;
+ job->cb = g_memdup(cb, sizeof(*cb));
+ job->jobDataPrivateCb = g_memdup(jobDataPrivateCb, sizeof(*jobDataPrivateCb));
if (virCondInit(&job->cond) < 0)
return -1;
@@ -229,6 +229,9 @@ virDomainObjClearJob(virDomainJobObj *job)
if (job->cb && job->cb->freeJobPrivate)
g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
+
+ g_clear_pointer(&job->cb, g_free);
+ g_clear_pointer(&job->jobDataPrivateCb, g_free);
}
void
--
2.37.1
1 year, 11 months
[PATCH] virdomainjob: virDomainObjInitJob: Store a copy of virDomainObjPrivateJobCallbacks
by Peter Krempa
'virDomainObjPrivateJobCallbacks' is passed into the job object by
copying a pointer from the 'virDomainXMLOption' struct passed in from
the caller. Unfortunately the 'virdomainjob' module can't control the
lifetime of the virDomainXMLOption, which in some cases is freed before
the domain job data.
To avoid dereferencing freed memory create a copy of the struct holding
the private job callbacks.
Fixes: 84e9fd068ccad6e19e037cd6680df437617e2de5
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
This is a naive attempt to fix a crash reported on the upstream mailing
list
https://listman.redhat.com/archives/libvir-list/2022-September/234310.html
Note that I didn't analyze the code in that series in detail, I just
debugged the visible misbehaviour.
CI pipeline now passes even on OpenSUSE:
https://gitlab.com/pipo.sk/libvirt/-/pipelines/643828256
src/conf/virdomainjob.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/conf/virdomainjob.c b/src/conf/virdomainjob.c
index 7915faa125..16fb0c2177 100644
--- a/src/conf/virdomainjob.c
+++ b/src/conf/virdomainjob.c
@@ -122,13 +122,25 @@ virDomainJobStatusToType(virDomainJobStatus status)
return VIR_DOMAIN_JOB_NONE;
}
+
+static virDomainObjPrivateJobCallbacks *
+virDomainObjPrivateJobCallbacksCopy(virDomainObjPrivateJobCallbacks *cb)
+{
+ virDomainObjPrivateJobCallbacks *ret = g_new0(virDomainObjPrivateJobCallbacks, 1);
+
+ memcpy(ret, cb, sizeof(virDomainObjPrivateJobCallbacks));
+
+ return ret;
+}
+
+
int
virDomainObjInitJob(virDomainJobObj *job,
virDomainObjPrivateJobCallbacks *cb,
virDomainJobDataPrivateDataCallbacks *jobDataPrivateCb)
{
memset(job, 0, sizeof(*job));
- job->cb = cb;
+ job->cb = virDomainObjPrivateJobCallbacksCopy(cb);
job->jobDataPrivateCb = jobDataPrivateCb;
if (virCondInit(&job->cond) < 0)
@@ -229,6 +241,8 @@ virDomainObjClearJob(virDomainJobObj *job)
if (job->cb && job->cb->freeJobPrivate)
g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
+
+ g_clear_pointer(&job->cb, g_free);
}
void
--
2.37.1
1 year, 11 months
[PATCH v2] meson: Require libssh-0.8.1 or newer
by Michal Privoznik
According to repology.org:
RHEL-8: 0.9.4
RHEL-9: 0.9.6
Debian 11: 0.9.5
openSUSE Leap 15.3: 0.8.7
Ubuntu 20.04: 0.9.3
And the rest of distros has something newer anyways. Requiring
0.8.1 or newer allows us to drop the terrible hack where we
rename functions at meson level using #define. Note, 0.8.0 is
the version of libssh where the rename happened. It also allows
us to stick with SHA-256 hash algorithm for public keys.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
v2 of:
https://listman.redhat.com/archives/libvir-list/2022-September/234296.html
diff to v1:
- Require even newer version so that more code can be dropped.
libvirt.spec.in | 2 +-
meson.build | 15 +--------------
src/rpc/virnetlibsshsession.c | 8 +-------
3 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b199c624b8..654057bf57 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -378,7 +378,7 @@ BuildRequires: wireshark-devel
%endif
%if %{with_libssh}
-BuildRequires: libssh-devel >= 0.7.0
+BuildRequires: libssh-devel >= 0.8.1
%endif
BuildRequires: rpcgen
diff --git a/meson.build b/meson.build
index ed9f4b3f70..24b12515c2 100644
--- a/meson.build
+++ b/meson.build
@@ -1025,24 +1025,11 @@ else
libpcap_dep = dependency('', required: false)
endif
-libssh_version = '0.7'
+libssh_version = '0.8.1'
if conf.has('WITH_REMOTE')
libssh_dep = dependency('libssh', version: '>=' + libssh_version, required: get_option('libssh'))
if libssh_dep.found()
conf.set('WITH_LIBSSH', 1)
-
- # Check if new functions exists, if not redefine them with old deprecated ones.
- # List of [ new_function, deprecated_function ].
- functions = [
- [ 'ssh_get_server_publickey', 'ssh_get_publickey' ],
- [ 'ssh_session_is_known_server', 'ssh_is_server_known' ],
- [ 'ssh_session_update_known_hosts', 'ssh_write_knownhost' ],
- ]
- foreach name : functions
- if not cc.has_function(name[0], dependencies: libssh_dep)
- conf.set(name[0], name[1])
- endif
- endforeach
endif
else
libssh_dep = dependency('', required: false)
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index a3adc85728..b1420bea2c 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -39,12 +39,6 @@ VIR_LOG_INIT("rpc.netlibsshsession");
#define VIR_NET_LIBSSH_BUFFER_SIZE 1024
-#if LIBSSH_VERSION_INT < SSH_VERSION_INT(0, 8, 1)
-# define VIR_SSH_HOSTKEY_HASH SSH_PUBLICKEY_HASH_SHA1
-#else
-# define VIR_SSH_HOSTKEY_HASH SSH_PUBLICKEY_HASH_SHA256
-#endif
-
/* TRACE_LIBSSH=<level> enables tracing in libssh itself.
* The meaning of <level> is described here:
* https://api.libssh.org/master/group__libssh__log.html
@@ -212,7 +206,7 @@ virLibsshServerKeyAsString(virNetLibsshSession *sess)
/* calculate remote key hash, using SHA256 algorithm that is
* the default in modern OpenSSH, fallback to SHA1 for older
* libssh. The returned value must be freed */
- ret = ssh_get_publickey_hash(key, VIR_SSH_HOSTKEY_HASH,
+ ret = ssh_get_publickey_hash(key, SSH_PUBLICKEY_HASH_SHA256,
&keyhash, &keyhashlen);
ssh_key_free(key);
if (ret < 0) {
--
2.35.1
1 year, 11 months