Re: [libvirt] [PATCH 1/4] test_driver: extract image saving code into a separate function
On Mon, Jun 3, 2019 at 10:01 AM Erik Skultety <eskultet(a)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(a)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(a)redhat.com>
> +testDomainSaveImageWrite(testDriverPtr driver,
> + virDomainDefPtr def,
> + const char *path)
> {