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@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
