Re: [libvirt] [PATCH 2/4] test_driver: implement virDomainFSFreeze
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(a)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