Re: [libvirt] [dbus PATCH] Implement SetGuestVcpus method for Domain Interface

[Pavel Hrdina]

On Wed, May 02, 2018 at 04:51:42PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  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 @@
>
>
>  
> +
> +   +  value="See 
> https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/>
> +  
> +  
> +  
> +
>  
>  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", );

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 

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", );
+gboolean stop = !g_variant_iter_loop(iter, "b", );

-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)", , 

[libvirt] [dbus PATCH] Implement SetGuestVcpus method for Domain Interface

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
 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 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetGuestVcpus"/>
+  
+  
+  
+
 
   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;
+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", );
+
+if (set && !setPrev)
+intervalStart = i;
+else if ((!set && setPrev) || next) {
+if (intervalCnt > 0)
+g_string_append_printf(ret, ",");
+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;
+
+g_variant_get(inArgs, "(abiu)", , , );
+
+domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
+if (!domain)
+return;
+
+ret = virtDBusDomainConvertBoolArrayToGuestVcpumap(iter);
+
+if (virDomainSetGuestVcpus(domain, ret, state, flags) < 0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static void
 virtDBusDomainSetInterfaceParameters(GVariant *inArgs,
  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -2988,6 +3049,7 @@ static virtDBusGDBusMethodTable 
virtDBusDomainMethodTable[] = {
 { "SendProcessSignal", virtDBusDomainSendProcessSignal },
 { "SetBlkioParameters", virtDBusDomainSetBlkioParameters },
 { "SetBlockIOTune", virtDBusDomainSetBlockIOTune },
+{ "SetGuestVcpus", virtDBusDomainSetGuestVcpus },
 { "SetInterfaceParameters", virtDBusDomainSetInterfaceParameters },
 { "SetVcpus", virtDBusDomainSetVcpus },
 { "SetMemory", virtDBusDomainSetMemory },
-- 
2.15.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list