On Wed, May 02, 2018 at 04:51:42PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 7 +++++ src/domain.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index db43b1c..eae6d97 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -463,6 +463,13 @@ <arg name="params" type="a{sv}" direction="in"/> <arg name="flags" type="u" direction="in"/> </method> + <method name="SetGuestVcpus"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/> + <arg name="vcpumap" type="ab" direction="in"/> + <arg name="state" type="i" direction="in"/> + <arg name="flags" type="u" direction="in"/> + </method> <method name="SetInterfaceParameters"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetInterfaceParameters"/> diff --git a/src/domain.c b/src/domain.c index e305fa3..bbc4ead 100644 --- a/src/domain.c +++ b/src/domain.c @@ -72,6 +72,39 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata, "title", "element")

+static gchar * +virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter) +{ + g_autoptr(GVariantIter) tmpIter = NULL;

tmpIter is not used

+ gboolean set; + gint intervalCnt = 0; + guint intervalStart = 0; + gboolean setPrev = 0; + g_autofree GString *ret = NULL; + + ret = g_string_new(""); + for (guint i = 0; ; i++) { + gboolean next = g_variant_iter_loop(iter, "b", &set);

I would rename this variable to stop and invert the logic, the return value tells you whether there was some value to unpack or not.

+ + if (set && !setPrev) + intervalStart = i; + else if ((!set && setPrev) || next) {

Here you should remove the '|| next' part, otherwise the 'else if' section is called for every unset CPU in the array of boolean mask.

+ if (intervalCnt > 0) + g_string_append_printf(ret, ",");

I would change it to 'boolean first' instead of intervalCnt to make it clear what's the purpose of this variable.

+ if (intervalStart != i - 1) + g_string_append_printf(ret, "%d-%d", intervalStart, i - 1); + else + g_string_append_printf(ret, "%d", intervalStart); + intervalCnt++; + } + setPrev = set; + if (!next) + break; + } + + return ret->str; +} + struct _virtDBusDomainFSInfoList { virDomainFSInfoPtr *info; gint count; @@ -2490,6 +2523,34 @@ virtDBusDomainSetBlockIOTune(GVariant *inArgs, } }

+static void +virtDBusDomainSetGuestVcpus(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) +{ + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + g_autoptr(GVariantIter) iter = NULL; + gint state; + guint flags; + g_autofree gchar *ret = NULL;

s/ret/cpumap/ Reviewed-by: Pavel Hrdina <phrdina@redhat.com> with the following diff applied, to make the suggested changes clear: diff --git a/src/domain.c b/src/domain.c index bbc4ead..8210a04 100644 --- a/src/domain.c +++ b/src/domain.c @@ -75,30 +75,32 @@ VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata, static gchar * virtDBusDomainConvertBoolArrayToGuestVcpumap(GVariantIter *iter) { - g_autoptr(GVariantIter) tmpIter = NULL; gboolean set; - gint intervalCnt = 0; + gboolean first = TRUE; guint intervalStart = 0; gboolean setPrev = 0; g_autofree GString *ret = NULL; ret = g_string_new(""); for (guint i = 0; ; i++) { - gboolean next = g_variant_iter_loop(iter, "b", &set); + gboolean stop = !g_variant_iter_loop(iter, "b", &set); - if (set && !setPrev) + if (set && !setPrev) { intervalStart = i; - else if ((!set && setPrev) || next) { - if (intervalCnt > 0) + } else if (!set && setPrev) { + if (!first) g_string_append_printf(ret, ","); + else + first = FALSE; + if (intervalStart != i - 1) g_string_append_printf(ret, "%d-%d", intervalStart, i - 1); else g_string_append_printf(ret, "%d", intervalStart); - intervalCnt++; } setPrev = set; - if (!next) + + if (stop) break; } @@ -2537,7 +2539,7 @@ virtDBusDomainSetGuestVcpus(GVariant *inArgs, g_autoptr(GVariantIter) iter = NULL; gint state; guint flags; - g_autofree gchar *ret = NULL; + g_autofree gchar *cpumap = NULL; g_variant_get(inArgs, "(abiu)", &iter, &state, &flags); @@ -2545,9 +2547,9 @@ virtDBusDomainSetGuestVcpus(GVariant *inArgs, if (!domain) return; - ret = virtDBusDomainConvertBoolArrayToGuestVcpumap(iter); + cpumap = virtDBusDomainConvertBoolArrayToGuestVcpumap(iter); - if (virDomainSetGuestVcpus(domain, ret, state, flags) < 0) + if (virDomainSetGuestVcpus(domain, cpumap, state, flags) < 0) virtDBusUtilSetLastVirtError(error); }