On Tue, Jul 09, 2019 at 09:23:22PM +0200, Ilias Stamatis wrote:
On success update the domain-private data. Consider / and /boot to be the only mountpoints avaiable in order to be consistent with the other FS-related calls.
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com> --- src/test/test_driver.c | 58 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index af3503c523..8c25c679a5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -388,6 +388,8 @@ typedef struct _testDomainObjPrivate testDomainObjPrivate; typedef testDomainObjPrivate *testDomainObjPrivatePtr; struct _testDomainObjPrivate { testDriverPtr driver; + + bool frozen[2]; /* used by file system related calls */ };
@@ -400,6 +402,7 @@ testDomainObjPrivateAlloc(void *opaque) return NULL;
priv->driver = opaque; + priv->frozen[0] = priv->frozen[1] = false;
return priv; } @@ -3439,6 +3442,60 @@ static int testDomainUndefine(virDomainPtr domain) return testDomainUndefineFlags(domain, 0); }
+ +static int +testDomainFSFreeze(virDomainPtr dom, + const char **mountpoints, + unsigned int nmountpoints, + unsigned int flags) +{ + virDomainObjPtr vm; + testDomainObjPrivatePtr priv; + size_t i; + int slash = 0, slash_boot = 0;
One declaration per line please. Also, we should define these explicitly as booleans, the standard states these would return 0 and 1 respectively.
+ int ret = -1; + + virCheckFlags(0, -1); + + if (!(vm = testDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + priv = vm->privateData; + + if (nmountpoints == 0) { + priv->frozen[0] = priv->frozen[1] = true; + ret = 2;
Well, this is not always true, e.g. if '/' was frozen before, ret should be 1. Similarly, if both were frozen prior to calling this API, we should return 0.
+ } else { + for (i = 0; i < nmountpoints; i++) { + if (STREQ(mountpoints[i], "/")) { + slash = 1; + } else if (STREQ(mountpoints[i], "/boot")) { + slash_boot = 1;
^here too, if the filesystems were already frozen, we should not account for them in @ret.
+ } else { + virReportError(VIR_ERR_OPERATION_INVALID, + _("mount point not found: %s"), + mountpoints[i]); + goto cleanup; + } + } + + if (slash) + priv->frozen[0] = true; + if (slash_boot) + priv->frozen[1] = true; + + ret = slash + slash_boot; + }
We could do ^this or alternatively, we could introduce another iterator @nfreeze, use it within the loop. We could also declare: bool freeze[2]; //backup the original values: memcpy(&freeze, priv->frozen, 2); for (mountpoints) { //set the helper array if (mount[i] == / && !freeze[i]) { freeze[i] = true; nfreeze++; } else if (mount[i] == /boot && !freeze[i]) { freeze[i] = true; nfreeze++; } } ret = nfreeze; //steal the helper copy memcpy(priv->frozen, &freeze, 2); return ret; Erik