[libvirt] [dbus PATCH 0/9] random fixes
*** BLURB HERE *** Pavel Hrdina (9): Process input arguments before getting libvirt object Use ++ operator instead of += in for loop connect: Fix List* methods to return empty array for empty list connect: Pass error into virtDBusConnectOpen function domain: There is no need to check ret again domain: Return error if libvirt API fails domain: Introduce virtDBusDomainGVariantToMountpoints helper domain: Introduce virtDBusDomainGVariantToCpumap helper util: Use spaces around operators src/connect.c | 28 ++++++--------- src/domain.c | 110 ++++++++++++++++++++++++++++------------------------------ src/main.c | 2 +- src/network.c | 2 +- src/util.c | 6 ++-- 5 files changed, 70 insertions(+), 78 deletions(-) -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/connect.c | 6 +++--- src/domain.c | 24 ++++++++++++------------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/connect.c b/src/connect.c index 0025023..3d2be96 100644 --- a/src/connect.c +++ b/src/connect.c @@ -1046,14 +1046,14 @@ virtDBusConnectNodeSetMemoryParameters(GVariant *inArgs, g_variant_get(inArgs, "(a{sv}u)", &iter, &flags); - if (!virtDBusConnectOpen(connect, error)) - return; - if (!virtDBusUtilGVariantToTypedParams(iter, ¶ms.params, ¶ms.nparams, error)) { return; } + if (!virtDBusConnectOpen(connect, error)) + return; + if (virNodeSetMemoryParameters(connect->connection, params.params, params.nparams, flags) < 0) { virtDBusUtilSetLastVirtError(error); diff --git a/src/domain.c b/src/domain.c index 262ebe2..17231bd 100644 --- a/src/domain.c +++ b/src/domain.c @@ -1995,10 +1995,6 @@ virtDBusDomainPinEmulator(GVariant *inArgs, g_variant_get(inArgs, "(abu)", &iter, &flags); - domain = virtDBusDomainGetVirDomain(connect, objectPath, error); - if (!domain) - return; - cpus = g_variant_iter_n_children(iter); cpumaplen = VIR_CPU_MAPLEN(cpus); cpumap = g_new0(guchar, cpumaplen); @@ -2008,6 +2004,10 @@ virtDBusDomainPinEmulator(GVariant *inArgs, cnt++; } + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + if (virDomainPinEmulator(domain, cpumap, cpumaplen, flags) < 0) virtDBusUtilSetLastVirtError(error); } @@ -2035,10 +2035,6 @@ virtDBusDomainPinIOThread(GVariant *inArgs, g_variant_get(inArgs, "(uabu)", &iothreadId, &iter, &flags); - domain = virtDBusDomainGetVirDomain(connect, objectPath, error); - if (!domain) - return; - cpus = g_variant_iter_n_children(iter); cpumaplen = VIR_CPU_MAPLEN(cpus); cpumap = g_new0(guchar, cpumaplen); @@ -2048,6 +2044,10 @@ virtDBusDomainPinIOThread(GVariant *inArgs, cnt++; } + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + if (virDomainPinIOThread(domain, iothreadId, cpumap, cpumaplen, flags) < 0) virtDBusUtilSetLastVirtError(error); } @@ -2075,10 +2075,6 @@ virtDBusDomainPinVcpu(GVariant *inArgs, g_variant_get(inArgs, "(uabu)", &vcpu, &iter, &flags); - domain = virtDBusDomainGetVirDomain(connect, objectPath, error); - if (!domain) - return; - cpus = g_variant_iter_n_children(iter); cpumaplen = VIR_CPU_MAPLEN(cpus); cpumap = g_new0(guchar, cpumaplen); @@ -2088,6 +2084,10 @@ virtDBusDomainPinVcpu(GVariant *inArgs, cnt++; } + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + if (virDomainPinVcpuFlags(domain, vcpu, cpumap, cpumaplen, flags) < 0) virtDBusUtilSetLastVirtError(error); } -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/connect.c | 10 +++++----- src/main.c | 2 +- src/network.c | 2 +- src/util.c | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/connect.c b/src/connect.c index 3d2be96..e96294d 100644 --- a/src/connect.c +++ b/src/connect.c @@ -39,7 +39,7 @@ virtDBusConnectClose(virtDBusConnect *connect, gboolean deregisterEvents) { - for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) { + for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) { if (connect->domainCallbackIds[i] >= 0) { if (deregisterEvents) { virConnectDomainEventDeregisterAny(connect->connection, @@ -49,7 +49,7 @@ virtDBusConnectClose(virtDBusConnect *connect, } } - for (gint i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i += 1) { + for (gint i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) { if (connect->networkCallbackIds[i] >= 0) { if (deregisterEvents) { virConnectNetworkEventDeregisterAny(connect->connection, @@ -1135,10 +1135,10 @@ virtDBusConnectNew(virtDBusConnect **connectp, g_mutex_init(&connect->lock); - for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) + for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) connect->domainCallbackIds[i] = -1; - for (gint i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i += 1) + for (gint i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) connect->networkCallbackIds[i] = -1; connect->bus = bus; @@ -1170,7 +1170,7 @@ virtDBusConnectListFree(virtDBusConnect **connectList) if (!connectList) return; - for (gint i = 0; connectList[i]; i += 1) + for (gint i = 0; connectList[i]; i++) virtDBusConnectFree(connectList[i]); g_free(connectList); diff --git a/src/main.c b/src/main.c index eb30ef8..c953726 100644 --- a/src/main.c +++ b/src/main.c @@ -54,7 +54,7 @@ virtDBusAcquired(GDBusConnection *connection, virtDBusRegisterData *data = opaque; g_autoptr(GError) error = NULL; - for (gsize i = 0; i < data->ndrivers; i += 1) { + for (gsize i = 0; i < data->ndrivers; i++) { virtDBusConnectNew(&data->connectList[i], connection, data->drivers[i].uri, data->drivers[i].object, &error); diff --git a/src/network.c b/src/network.c index 2fb5ec6..1763eca 100644 --- a/src/network.c +++ b/src/network.c @@ -6,7 +6,7 @@ static void virtDBusNetworkDHCPLeaseListFree(virNetworkDHCPLeasePtr *leases) { - for (gint i = 0; leases[i] != NULL; i += 1) + for (gint i = 0; leases[i] != NULL; i++) virNetworkDHCPLeaseFree(leases[i]); g_free(leases); diff --git a/src/util.c b/src/util.c index 4efa3ec..8b28496 100644 --- a/src/util.c +++ b/src/util.c @@ -208,7 +208,7 @@ virtDBusUtilVirDomainFromBusPath(virConnectPtr connection, void virtDBusUtilVirDomainListFree(virDomainPtr *domains) { - for (gint i = 0; domains[i] != NULL; i += 1) + for (gint i = 0; domains[i] != NULL; i++) virDomainFree(domains[i]); g_free(domains); @@ -241,7 +241,7 @@ virtDBusUtilBusPathForVirNetwork(virNetworkPtr network, void virtDBusUtilVirNetworkListFree(virNetworkPtr *networks) { - for (gint i = 0; networks[i] != NULL; i += 1) + for (gint i = 0; networks[i] != NULL; i++) virNetworkFree(networks[i]); g_free(networks); -- 2.14.3
On Mon, May 07, 2018 at 12:06:29PM +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/connect.c | 10 +++++----- src/main.c | 2 +- src/network.c | 2 +- src/util.c | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/connect.c b/src/connect.c index 3d2be96..e96294d 100644 --- a/src/connect.c +++ b/src/connect.c @@ -39,7 +39,7 @@ virtDBusConnectClose(virtDBusConnect *connect, gboolean deregisterEvents) {
- for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) { + for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) { if (connect->domainCallbackIds[i] >= 0) { if (deregisterEvents) { virConnectDomainEventDeregisterAny(connect->connection,
Thank you! Jano
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/connect.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/connect.c b/src/connect.c index e96294d..cd90939 100644 --- a/src/connect.c +++ b/src/connect.c @@ -657,9 +657,6 @@ virtDBusConnectListDomains(GVariant *inArgs, if (virConnectListAllDomains(connect->connection, &domains, flags) < 0) return virtDBusUtilSetLastVirtError(error); - if (!*domains) - return; - g_variant_builder_init(&builder, G_VARIANT_TYPE("ao")); for (gint i = 0; domains[i]; i++) { @@ -697,9 +694,6 @@ virtDBusConnectListNetworks(GVariant *inArgs, if (virConnectListAllNetworks(connect->connection, &networks, flags) < 0) return virtDBusUtilSetLastVirtError(error); - if (!*networks) - return; - g_variant_builder_init(&builder, G_VARIANT_TYPE("ao")); for (gint i = 0; networks[i]; i++) { -- 2.14.3
We need to pass the error pointer in order to report proper error. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/connect.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/connect.c b/src/connect.c index cd90939..fd335e3 100644 --- a/src/connect.c +++ b/src/connect.c @@ -342,7 +342,7 @@ virtDBusConnectDomainLookupByID(GVariant *inArgs, g_variant_get(inArgs, "(u)", &id); - if (!virtDBusConnectOpen(connect, NULL)) + if (!virtDBusConnectOpen(connect, error)) return; domain = virDomainLookupByID(connect->connection, id); @@ -370,7 +370,7 @@ virtDBusConnectDomainLookupByName(GVariant *inArgs, g_variant_get(inArgs, "(s)", &name); - if (!virtDBusConnectOpen(connect, NULL)) + if (!virtDBusConnectOpen(connect, error)) return; domain = virDomainLookupByName(connect->connection, name); @@ -398,7 +398,7 @@ virtDBusConnectDomainLookupByUUID(GVariant *inArgs, g_variant_get(inArgs, "(s)", &uuidstr); - if (!virtDBusConnectOpen(connect, NULL)) + if (!virtDBusConnectOpen(connect, error)) return; domain = virDomainLookupByUUIDString(connect->connection, uuidstr); -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/domain.c b/src/domain.c index 17231bd..20d41fe 100644 --- a/src/domain.c +++ b/src/domain.c @@ -938,7 +938,8 @@ virtDBusDomainGetBlockIOTune(GVariant *inArgs, ret = virDomainGetBlockIoTune(domain, disk, NULL, ¶ms.nparams, flags); if (ret < 0) return virtDBusUtilSetLastVirtError(error); - if (ret == 0 && params.nparams != 0) { + + if (params.nparams != 0) { params.params = g_new0(virTypedParameter, params.nparams); if (virDomainGetBlockIoTune(domain, disk, params.params, ¶ms.nparams, flags) < 0) { -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/domain.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/domain.c b/src/domain.c index 20d41fe..b361aa6 100644 --- a/src/domain.c +++ b/src/domain.c @@ -1339,7 +1339,10 @@ virtDBusDomainGetMemoryParameters(GVariant *inArgs, return; ret = virDomainGetMemoryParameters(domain, NULL, ¶ms.nparams, flags); - if (ret == 0 && params.nparams != 0) { + if (ret < 0) + return virtDBusUtilSetLastVirtError(error); + + if (params.nparams != 0) { params.params = g_new0(virTypedParameter, params.nparams); if (virDomainGetMemoryParameters(domain, params.params, ¶ms.nparams, flags) < 0) { @@ -1475,7 +1478,10 @@ virtDBusDomainGetSchedulerParameters(GVariant *inArgs, return; ret = virDomainGetSchedulerType(domain, ¶ms.nparams); - if (ret && params.nparams != 0) { + if (!ret) + return virtDBusUtilSetLastVirtError(error); + + if (params.nparams != 0) { params.params = g_new0(virTypedParameter, params.nparams); if (virDomainGetSchedulerParametersFlags(domain, params.params, ¶ms.nparams, 0)) -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/domain.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/domain.c b/src/domain.c index b361aa6..c6dc744 100644 --- a/src/domain.c +++ b/src/domain.c @@ -109,6 +109,22 @@ virtDBusDomainMemoryStatsToGVariant(virDomainMemoryStatPtr stats, return g_variant_builder_end(&builder); } +static void +virtDBusDomainGVariantToMountpoints(GVariantIter *iter, + const gchar ***mountpoints, + guint *nmountpoints) +{ + const gchar **tmp; + + *nmountpoints = g_variant_iter_n_children(iter); + if (*nmountpoints > 0) { + *mountpoints = g_new0(const gchar*, *nmountpoints); + tmp = *mountpoints; + while (g_variant_iter_next(iter, "&s", tmp)) + tmp++; + } +} + static virDomainPtr virtDBusDomainGetVirDomain(virtDBusConnect *connect, const gchar *objectPath, @@ -783,21 +799,14 @@ virtDBusDomainFSFreeze(GVariant *inArgs, virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; g_autofree const gchar **mountpoints = NULL; - const gchar **tmp; g_autoptr(GVariantIter) iter; - gsize nmountpoints = 0; + guint nmountpoints; guint flags; gint ret; g_variant_get(inArgs, "(asu)", &iter, &flags); - nmountpoints = g_variant_iter_n_children(iter); - if (nmountpoints > 0) { - mountpoints = g_new0(const gchar*, nmountpoints); - tmp = mountpoints; - while (g_variant_iter_next(iter, "&s", tmp)) - tmp++; - } + virtDBusDomainGVariantToMountpoints(iter, &mountpoints, &nmountpoints); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) @@ -822,7 +831,6 @@ virtDBusDomainFSThaw(GVariant *inArgs, virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; g_autofree const gchar **mountpoints = NULL; - const gchar **tmp; g_autoptr(GVariantIter) iter; guint nmountpoints; guint flags; @@ -830,13 +838,7 @@ virtDBusDomainFSThaw(GVariant *inArgs, g_variant_get(inArgs, "(asu)", &iter, &flags); - nmountpoints = g_variant_iter_n_children(iter); - if (nmountpoints > 0) { - mountpoints = g_new0(const gchar*, nmountpoints); - tmp = mountpoints; - while (g_variant_iter_next(iter, "&s", tmp)) - tmp++; - } + virtDBusDomainGVariantToMountpoints(iter, &mountpoints, &nmountpoints); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/domain.c | 55 ++++++++++++++++++++++--------------------------------- 1 file changed, 22 insertions(+), 33 deletions(-) diff --git a/src/domain.c b/src/domain.c index c6dc744..2ba58e0 100644 --- a/src/domain.c +++ b/src/domain.c @@ -125,6 +125,25 @@ virtDBusDomainGVariantToMountpoints(GVariantIter *iter, } } +static void +virtDBusDomainGVariantToCpumap(GVariantIter *iter, + guchar **cpumap, + guint *cpumaplen) +{ + gboolean usable; + guint cpus = g_variant_iter_n_children(iter); + guint cnt = 0; + + *cpumaplen = VIR_CPU_MAPLEN(cpus); + *cpumap = g_new0(guchar, cpumaplen); + + while (g_variant_iter_loop(iter, "b", &usable)) { + if (usable) + VIR_USE_CPU(*cpumap, cnt); + cnt++; + } +} + static virDomainPtr virtDBusDomainGetVirDomain(virtDBusConnect *connect, const gchar *objectPath, @@ -1996,22 +2015,12 @@ virtDBusDomainPinEmulator(GVariant *inArgs, g_autoptr(virDomain) domain = NULL; g_autoptr(GVariantIter) iter = NULL; guint flags; - guint cpus; guint cpumaplen; g_autofree guchar *cpumap = NULL; - gboolean usable; - guint cnt = 0; g_variant_get(inArgs, "(abu)", &iter, &flags); - cpus = g_variant_iter_n_children(iter); - cpumaplen = VIR_CPU_MAPLEN(cpus); - cpumap = g_new0(guchar, cpumaplen); - while (g_variant_iter_loop(iter, "b", &usable)) { - if (usable) - VIR_USE_CPU(cpumap, cnt); - cnt++; - } + virtDBusDomainGVariantToCpumap(iter, &cpumap, &cpumaplen); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) @@ -2036,22 +2045,12 @@ virtDBusDomainPinIOThread(GVariant *inArgs, guint iothreadId; g_autoptr(GVariantIter) iter = NULL; guint flags; - guint cpus; guint cpumaplen; g_autofree guchar *cpumap = NULL; - gboolean usable; - guint cnt = 0; g_variant_get(inArgs, "(uabu)", &iothreadId, &iter, &flags); - cpus = g_variant_iter_n_children(iter); - cpumaplen = VIR_CPU_MAPLEN(cpus); - cpumap = g_new0(guchar, cpumaplen); - while (g_variant_iter_loop(iter, "b", &usable)) { - if (usable) - VIR_USE_CPU(cpumap, cnt); - cnt++; - } + virtDBusDomainGVariantToCpumap(iter, &cpumap, &cpumaplen); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) @@ -2076,22 +2075,12 @@ virtDBusDomainPinVcpu(GVariant *inArgs, guint vcpu; g_autoptr(GVariantIter) iter = NULL; guint flags; - guint cpus; guint cpumaplen; g_autofree guchar *cpumap = NULL; - gboolean usable; - guint cnt = 0; g_variant_get(inArgs, "(uabu)", &vcpu, &iter, &flags); - cpus = g_variant_iter_n_children(iter); - cpumaplen = VIR_CPU_MAPLEN(cpus); - cpumap = g_new0(guchar, cpumaplen); - while (g_variant_iter_loop(iter, "b", &usable)) { - if (usable) - VIR_USE_CPU(cpumap, cnt); - cnt++; - } + virtDBusDomainGVariantToCpumap(iter, &cpumap, &cpumaplen); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) -- 2.14.3
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.c b/src/util.c index 8b28496..548fac8 100644 --- a/src/util.c +++ b/src/util.c @@ -177,7 +177,7 @@ virtDBusUtilEncodeUUID(const gchar *uuid) static gchar * virtDBusUtilDecodeUUID(const gchar *uuid) { - gchar *ret = g_strdup(uuid+1); + gchar *ret = g_strdup(uuid + 1); return g_strdelimit(ret, "_", '-'); } -- 2.14.3
On Mon, 2018-05-07 at 12:06 +0200, Pavel Hrdina wrote:
*** BLURB HERE ***
Pavel Hrdina (9): Process input arguments before getting libvirt object Use ++ operator instead of += in for loop connect: Fix List* methods to return empty array for empty list connect: Pass error into virtDBusConnectOpen function domain: There is no need to check ret again domain: Return error if libvirt API fails domain: Introduce virtDBusDomainGVariantToMountpoints helper domain: Introduce virtDBusDomainGVariantToCpumap helper util: Use spaces around operators
src/connect.c | 28 ++++++--------- src/domain.c | 110 ++++++++++++++++++++++++++++------------------ ------------ src/main.c | 2 +- src/network.c | 2 +- src/util.c | 6 ++-- 5 files changed, 70 insertions(+), 78 deletions(-)
Thanks for the fixes. ACK whole patchset. Katerina
participants (3)
-
Ján Tomko -
Katerina Koukiou -
Pavel Hrdina