Hi
On Fri, Nov 26, 2021 at 10:49 AM Priyankar Jain <priyankar.jain@nutanix.com>
wrote:
> The purpose of dbus_get_proxies to construct the proxies corresponding to
> the
> IDs registered to dbus-vmstate.
>
> Currenty, this function returns an error in case there is any failure
> while instantiating proxy for "all" the names on dbus.
>
> Ideally this function should error out only if it is not able to find and
> validate the proxies registered to the backend otherwise any offending
> process(for eg: the process purposefully may not export its Id property on
> the dbus) may connect to the dbus and can lead to migration failures.
>
ok
> This commit ensures that dbus_get_proxies returns an error if it is not
> able to find and validate the proxies of interest(the IDs registered
> during the dbus-vmstate instantiation).
>
> Signed-off-by: Priyankar Jain <priyankar.jain@nutanix.com>
> ---
> backends/dbus-vmstate.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> index 9cfd758c42..ec86b5bac2 100644
> --- a/backends/dbus-vmstate.c
> +++ b/backends/dbus-vmstate.c
> @@ -114,14 +114,13 @@ dbus_get_proxies(DBusVMState *self, GError **err)
> "org.qemu.VMState1",
> NULL, err);
> if (!proxy) {
> - return NULL;
>
This would leak "err", you would need to pass NULL instead. Imho we need to
report a warning anyway with "err".
> + continue;
> }
>
> result = g_dbus_proxy_get_cached_property(proxy, "Id");
> if (!result) {
> - g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> - "VMState Id property is missing.");
> - return NULL;
>
Similarly, report a warning.
> + g_clear_object(&proxy);
> + continue;
> }
>
> id = g_variant_dup_string(result, &size);
> --
> 2.30.1 (Apple Git-130)
>
>
>
thanks
--
Marc-André Lureau