[libvirt] [PATCH 1/4] test_driver: extract image saving code into a separate function

[Ilias Stamatis]

On Mon, Jun 3, 2019 at 10:01 AM Erik Skultety <eskultet at redhat.com> wrote:
>
> On Wed, May 29, 2019 at 02:22:56PM +0200, Ilias Stamatis wrote:
> > Extracting the code logic for writing a test image to disk from
> > testDomainSaveFlags into a separate function, allows us to reuse this
> > code in other functions such as testDomainSaveImageDefineXML.
> >
> > Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> > ---
> >  src/test/test_driver.c | 114 +++++++++++++++++++++++++----------------
> >  1 file changed, 69 insertions(+), 45 deletions(-)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 2f58a1da95..e71b931790 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -1974,75 +1974,106 @@ testDomainGetTime(virDomainPtr dom ATTRIBUTE_UNUSED,
> >
> >  #define TEST_SAVE_MAGIC "TestGuestMagic"
> >
> > -static int
> > -testDomainSaveFlags(virDomainPtr domain, const char *path,
> > -                    const char *dxml, unsigned int flags)
> > +
> > +/**
> > + * testDomainSaveImageWrite:
> > + * @driver: test driver data
> > + * @def: domain definition whose XML will be stored in the image
> > + * @path: path of the save image
> > + *
> > + * Returns true on success, else false.
> > + */
> > +static bool
>
> A minor nitpick, I'd probably prefer 'int' rather than 'bool', feels more
> natural for the given function.

I was skeptical about the return value type tbh. Initially I had it as
an int as well because I wasn't sure if the bool type is used in
general.

But probably you're right, int makes more sense since this function
could potentially return different error codes for different error
cases.

>
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
>
> > +testDomainSaveImageWrite(testDriverPtr driver,
> > +                         virDomainDefPtr def,
> > +                         const char *path)
> >  {