[libvirt] [dbus PATCH 14/25] Implement FSFreeze method for Domain Interface

Katerina Koukiou posted 25 patches 7 years, 8 months ago
There is a newer version of this series
[libvirt] [dbus PATCH 14/25] Implement FSFreeze method for Domain Interface
Posted by Katerina Koukiou 7 years, 8 months ago
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
---
 data/org.libvirt.Domain.xml |  7 +++++++
 src/domain.c                | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index fb2bcce..66cf5e2 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -135,6 +135,13 @@
       <arg name="xml" type="s" direction="in"/>
       <arg name="flags" type="u" direction="in"/>
     </method>
+    <method name="FSFreeze">
+      <annotation name="org.gtk.GDBus.DocString"
+        value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSFreeze"/>
+      <arg name="mountpoints" type="as" direction="in"/>
+      <arg name="flags" type="u" direction="in"/>
+      <arg name="frozenFilesystems" type="u" direction="out"/>
+    </method>
     <method name="GetBlkioParameters">
       <annotation name="org.gtk.GDBus.DocString"
         value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetBlkioParameters"/>
diff --git a/src/domain.c b/src/domain.c
index 2fd8343..04115d1 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -693,6 +693,45 @@ virtDBusDomainDetachDevice(GVariant *inArgs,
         virtDBusUtilSetLastVirtError(error);
 }
 
+static void
+virtDBusDomainFSFreeze(GVariant *inArgs,
+                       GUnixFDList *inFDs G_GNUC_UNUSED,
+                       const gchar *objectPath,
+                       gpointer userData,
+                       GVariant **outArgs,
+                       GUnixFDList **outFDs G_GNUC_UNUSED,
+                       GError **error)
+{
+    virtDBusConnect *connect = userData;
+    g_autoptr(virDomain) domain = NULL;
+    g_autofree const gchar **mountpoints = NULL;
+    GVariantIter *iter;
+    gsize nmountpoints = 0;
+    guint flags;
+    gint ret;
+
+    g_variant_get(inArgs, "(asu)", &iter, &flags);
+
+    nmountpoints = g_variant_iter_n_children(iter);
+    if (nmountpoints) {
+        mountpoints = g_new0(const gchar*, nmountpoints + 1);
+        while (g_variant_iter_loop(iter, "s", &mountpoints[nmountpoints-1]))
+            nmountpoints++;
+        nmountpoints--;
+        g_variant_iter_free(iter);
+    }
+
+    domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
+    if (!domain)
+        return;
+
+    ret = virDomainFSFreeze(domain, (const gchar **)mountpoints, nmountpoints, flags);
+    if (ret < 0)
+        virtDBusUtilSetLastVirtError(error);
+
+    *outArgs = g_variant_new("(u)", ret);
+}
+
 static void
 virtDBusDomainGetBlkioParameters(GVariant *inArgs,
                                  GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1492,6 +1531,7 @@ static virtDBusGDBusMethodTable virtDBusDomainMethodTable[] = {
     { "DelIOThread", virtDBusDomainDelIOThread },
     { "Destroy", virtDBusDomainDestroy },
     { "DetachDevice", virtDBusDomainDetachDevice },
+    { "FSFreeze", virtDBusDomainFSFreeze },
     { "GetBlkioParameters", virtDBusDomainGetBlkioParameters },
     { "GetControlInfo", virtDBusDomainGetControlInfo },
     { "GetJobInfo", virtDBusDomainGetJobInfo },
-- 
2.15.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 14/25] Implement FSFreeze method for Domain Interface
Posted by Pavel Hrdina 7 years, 8 months ago
On Tue, Apr 17, 2018 at 02:04:33PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> ---
>  data/org.libvirt.Domain.xml |  7 +++++++
>  src/domain.c                | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index fb2bcce..66cf5e2 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -135,6 +135,13 @@
>        <arg name="xml" type="s" direction="in"/>
>        <arg name="flags" type="u" direction="in"/>
>      </method>
> +    <method name="FSFreeze">
> +      <annotation name="org.gtk.GDBus.DocString"
> +        value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainFSFreeze"/>
> +      <arg name="mountpoints" type="as" direction="in"/>
> +      <arg name="flags" type="u" direction="in"/>
> +      <arg name="frozenFilesystems" type="u" direction="out"/>
> +    </method>
>      <method name="GetBlkioParameters">
>        <annotation name="org.gtk.GDBus.DocString"
>          value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetBlkioParameters"/>
> diff --git a/src/domain.c b/src/domain.c
> index 2fd8343..04115d1 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -693,6 +693,45 @@ virtDBusDomainDetachDevice(GVariant *inArgs,
>          virtDBusUtilSetLastVirtError(error);
>  }
>  
> +static void
> +virtDBusDomainFSFreeze(GVariant *inArgs,
> +                       GUnixFDList *inFDs G_GNUC_UNUSED,
> +                       const gchar *objectPath,
> +                       gpointer userData,
> +                       GVariant **outArgs,
> +                       GUnixFDList **outFDs G_GNUC_UNUSED,
> +                       GError **error)
> +{
> +    virtDBusConnect *connect = userData;
> +    g_autoptr(virDomain) domain = NULL;
> +    g_autofree const gchar **mountpoints = NULL;
> +    GVariantIter *iter;
> +    gsize nmountpoints = 0;
> +    guint flags;
> +    gint ret;
> +
> +    g_variant_get(inArgs, "(asu)", &iter, &flags);
> +
> +    nmountpoints = g_variant_iter_n_children(iter);
> +    if (nmountpoints) {
> +        mountpoints = g_new0(const gchar*, nmountpoints + 1);
> +        while (g_variant_iter_loop(iter, "s", &mountpoints[nmountpoints-1]))
> +            nmountpoints++;
> +        nmountpoints--;
> +        g_variant_iter_free(iter);
> +    }

This code will not work for nmountpoints >= 2.

At the first line you get a number of mountpoints, let's say it's
2 ("/root", "/home").

Then you allocate nmountpoints + 1 array of strings, the array should
have only nmountpoints elements.

The while loop is the place where it actually fails:

1 cycle: g_variant_iter_loop(iter, "s", &mountpoints[3-1])

    - this will not fail because mountpoints[2] is still allocated

2 cycle: g_variant_iter_loop(iter, "s", &mountpoints[4-1])

    - there the program crashes because mountpoints[3] is not allocated

Another issue here is that using "s" as signature makes a copy of the
string and it needs to be freed, in this case "&s" would be better.


One possible way how the code can look like:

    
    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_loop(iter, "&s", tmp))
            tmp++;
        g_variant_iter_free(iter);
    }

> +
> +    domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
> +    if (!domain)
> +        return;
> +
> +    ret = virDomainFSFreeze(domain, (const gchar **)mountpoints, nmountpoints, flags);

There is no need to typecast mountpoints and the line is too long.

> +    if (ret < 0)
> +        virtDBusUtilSetLastVirtError(error);

Missing return.

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 14/25] Implement FSFreeze method for Domain Interface
Posted by Katerina Koukiou 7 years, 8 months ago
On Wed, 2018-04-18 at 09:42 +0200, Pavel Hrdina wrote:
> On Tue, Apr 17, 2018 at 02:04:33PM +0200, Katerina Koukiou wrote:
> > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> > ---
> >  data/org.libvirt.Domain.xml |  7 +++++++
> >  src/domain.c                | 40
> > ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/data/org.libvirt.Domain.xml
> > b/data/org.libvirt.Domain.xml
> > index fb2bcce..66cf5e2 100644
> > --- a/data/org.libvirt.Domain.xml
> > +++ b/data/org.libvirt.Domain.xml
> > @@ -135,6 +135,13 @@
> >        <arg name="xml" type="s" direction="in"/>
> >        <arg name="flags" type="u" direction="in"/>
> >      </method>
> > +    <method name="FSFreeze">
> > +      <annotation name="org.gtk.GDBus.DocString"
> > +        value="See https://libvirt.org/html/libvirt-libvirt-domain
> > .html#virDomainFSFreeze"/>;
> > +      <arg name="mountpoints" type="as" direction="in"/>
> > +      <arg name="flags" type="u" direction="in"/>
> > +      <arg name="frozenFilesystems" type="u" direction="out"/>
> > +    </method>
> >      <method name="GetBlkioParameters">
> >        <annotation name="org.gtk.GDBus.DocString"
> >          value="See https://libvirt.org/html/libvirt-libvirt-domain
> > .html#virDomainGetBlkioParameters"/>;
> > diff --git a/src/domain.c b/src/domain.c
> > index 2fd8343..04115d1 100644
> > --- a/src/domain.c
> > +++ b/src/domain.c
> > @@ -693,6 +693,45 @@ virtDBusDomainDetachDevice(GVariant *inArgs,
> >          virtDBusUtilSetLastVirtError(error);
> >  }
> >  
> > +static void
> > +virtDBusDomainFSFreeze(GVariant *inArgs,
> > +                       GUnixFDList *inFDs G_GNUC_UNUSED,
> > +                       const gchar *objectPath,
> > +                       gpointer userData,
> > +                       GVariant **outArgs,
> > +                       GUnixFDList **outFDs G_GNUC_UNUSED,
> > +                       GError **error)
> > +{
> > +    virtDBusConnect *connect = userData;
> > +    g_autoptr(virDomain) domain = NULL;
> > +    g_autofree const gchar **mountpoints = NULL;
> > +    GVariantIter *iter;
> > +    gsize nmountpoints = 0;
> > +    guint flags;
> > +    gint ret;
> > +
> > +    g_variant_get(inArgs, "(asu)", &iter, &flags);
> > +
> > +    nmountpoints = g_variant_iter_n_children(iter);
> > +    if (nmountpoints) {
> > +        mountpoints = g_new0(const gchar*, nmountpoints + 1);
> > +        while (g_variant_iter_loop(iter, "s",
> > &mountpoints[nmountpoints-1]))
> > +            nmountpoints++;
> > +        nmountpoints--;
> > +        g_variant_iter_free(iter);
> > +    }
> 
> This code will not work for nmountpoints >= 2.
> 
> At the first line you get a number of mountpoints, let's say it's
> 2 ("/root", "/home").
> 
> Then you allocate nmountpoints + 1 array of strings, the array should
> have only nmountpoints elements.
> 
> The while loop is the place where it actually fails:
> 
> 1 cycle: g_variant_iter_loop(iter, "s", &mountpoints[3-1])
> 
>     - this will not fail because mountpoints[2] is still allocated
> 
> 2 cycle: g_variant_iter_loop(iter, "s", &mountpoints[4-1])
> 
>     - there the program crashes because mountpoints[3] is not
> allocated
> 
> Another issue here is that using "s" as signature makes a copy of the
> string and it needs to be freed, in this case "&s" would be better.
> 
> 
> One possible way how the code can look like:
> 
>     
>     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_loop(iter, "&s", tmp))
>             tmp++;
>         g_variant_iter_free(iter);
>     }
> 

I 'll need slight modification here: I 'll have to allocate
nmountpoints + 1 because else I am getting the following Error
after the last iteration.

free(): invalid next size (fast)

Thanks,
Katerina

> > +
> > +    domain = virtDBusDomainGetVirDomain(connect, objectPath,
> > error);
> > +    if (!domain)
> > +        return;
> > +
> > +    ret = virDomainFSFreeze(domain, (const gchar **)mountpoints,
> > nmountpoints, flags);
> 
> There is no need to typecast mountpoints and the line is too long.
> 
> > +    if (ret < 0)
> > +        virtDBusUtilSetLastVirtError(error);
> 
> Missing return.
> 
> Pavel

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