[libvirt] [RFC] test_driver: Thoughts on the implementation of some FS-related APIs

Hello, I was thinking about how to implement the following APIs in the test driver: -virDomainFSFreeze -virDomainFSThaw -virDomainFSTrim The first two are conceptually paired. They both get a mountpoints argument. The QEMU driver (which is the only driver implementing this currently) ignores this completely. However, I thought that we can allow it and pretend that the only available mountpoints are "/" and "/boot" (like in a default Fedora installation). The following questions arise though: - Should we keep some kind of temporary or permanent state on the test driver about this? Because it makes sense for virDomainFSThaw to succeed only on mountpoints for which previously virDomainFSFreeze has been called. If so, *where* exactly should we keep this state? - In case a non-existing mountpoint is passed to either of the first 2 APIs. Should we fail? If so, a new error code should be introduced for this. Also for the cases above where e.g. Thaw is called on a fs that isn't frozen probably yet another different error code must be used. Should we add all these new error codes? Or maybe all the above is more complicated than necessary and it's fine if we ignore mountpoints completely like the QEMU driver does and make all 3 APIs above to always succeed no matter what in the test driver? Thanks, Ilias

On Wed, Jun 26, 2019 at 06:36:28PM +0200, Ilias Stamatis wrote:
Hello,
I was thinking about how to implement the following APIs in the test driver: -virDomainFSFreeze -virDomainFSThaw -virDomainFSTrim
The first two are conceptually paired. They both get a mountpoints argument. The QEMU driver (which is the only driver implementing this currently) ignores this completely.
What do you mean by "ignores this completely"? From the code the mountpoints are passed to QEMU agent and that agent issues appropriate commands in the VM OS.
However, I thought that we can allow it and pretend that the only available mountpoints are "/" and "/boot" (like in a default Fedora installation). The following questions arise though:
Sounds reasonable, in general we should probably accept only mountpoints that are reported by virDomainGetFSInfo.
- Should we keep some kind of temporary or permanent state on the test driver about this? Because it makes sense for virDomainFSThaw to succeed only on mountpoints for which previously virDomainFSFreeze has been called. If so, *where* exactly should we keep this state?
I guess it makes sense to store the state of mountpoints, it should not be that difficult, the proper place would be domain private data.
- In case a non-existing mountpoint is passed to either of the first 2 APIs. Should we fail? If so, a new error code should be introduced for this. Also for the cases above where e.g. Thaw is called on a fs that isn't frozen probably yet another different error code must be used. Should we add all these new error codes?
No need to introduce new error codes, I think that reusing OPERATION_IS_INVALID or something like that is perfectly fine with reasonable error message.
Or maybe all the above is more complicated than necessary and it's fine if we ignore mountpoints completely like the QEMU driver does and make all 3 APIs above to always succeed no matter what in the test driver?
If it would require a lot of code and would be complicated it is probably not worth it, but doesn't look that way so I would say give it a try and we'll see :). Pavel

On Thu, Jul 4, 2019 at 12:58 PM Pavel Hrdina <phrdina@redhat.com> wrote:
On Wed, Jun 26, 2019 at 06:36:28PM +0200, Ilias Stamatis wrote:
Hello,
I was thinking about how to implement the following APIs in the test driver: -virDomainFSFreeze -virDomainFSThaw -virDomainFSTrim
The first two are conceptually paired. They both get a mountpoints argument. The QEMU driver (which is the only driver implementing this currently) ignores this completely.
What do you mean by "ignores this completely"? From the code the mountpoints are passed to QEMU agent and that agent issues appropriate commands in the VM OS.
I meant qemuDomainFSThaw and qemuDomainFSTrime. In the case of virDomainFSFreeze you are right.
However, I thought that we can allow it and pretend that the only available mountpoints are "/" and "/boot" (like in a default Fedora installation). The following questions arise though:
Sounds reasonable, in general we should probably accept only mountpoints that are reported by virDomainGetFSInfo.
- Should we keep some kind of temporary or permanent state on the test driver about this? Because it makes sense for virDomainFSThaw to succeed only on mountpoints for which previously virDomainFSFreeze has been called. If so, *where* exactly should we keep this state?
I guess it makes sense to store the state of mountpoints, it should not be that difficult, the proper place would be domain private data.
Ok, since we decided last time to introduce and start using domain private data in general this can be solved easily then.
- In case a non-existing mountpoint is passed to either of the first 2 APIs. Should we fail? If so, a new error code should be introduced for this. Also for the cases above where e.g. Thaw is called on a fs that isn't frozen probably yet another different error code must be used. Should we add all these new error codes?
No need to introduce new error codes, I think that reusing OPERATION_IS_INVALID or something like that is perfectly fine with reasonable error message.
Ok, I'll use this code then.
Or maybe all the above is more complicated than necessary and it's fine if we ignore mountpoints completely like the QEMU driver does and make all 3 APIs above to always succeed no matter what in the test driver?
If it would require a lot of code and would be complicated it is probably not worth it, but doesn't look that way so I would say give it a try and we'll see :).
Sure. I'll provide an implementation based on the above. Thanks! Ilias
Pavel

On Thu, Jul 04, 2019 at 01:28:36PM +0200, Ilias Stamatis wrote:
On Thu, Jul 4, 2019 at 12:58 PM Pavel Hrdina <phrdina@redhat.com> wrote:
On Wed, Jun 26, 2019 at 06:36:28PM +0200, Ilias Stamatis wrote:
Hello,
I was thinking about how to implement the following APIs in the test driver: -virDomainFSFreeze -virDomainFSThaw -virDomainFSTrim
The first two are conceptually paired. They both get a mountpoints argument. The QEMU driver (which is the only driver implementing this currently) ignores this completely.
What do you mean by "ignores this completely"? From the code the mountpoints are passed to QEMU agent and that agent issues appropriate commands in the VM OS.
I meant qemuDomainFSThaw and qemuDomainFSTrime. In the case of virDomainFSFreeze you are right.
Right, I check only FSFreeze, we don't have to copy QEMU driver behavior so if the test driver can easily support it I would go for it. Pavel
participants (2)
-
Ilias Stamatis
-
Pavel Hrdina