7:49 a.m.
On Tue, Jul 2, 2019 at 2:41 PM Erik Skultety <eskultet(a)redhat.com> wrote:
On Tue, Jun 25, 2019 at 11:58:07PM +0200, Ilias Stamatis wrote:
> Always return / and /boot as the mount points imitating a default
> Fedora installation. Use the first disk found, otherwise if no disk
> device of type VIR_DOMAIN_DISK_DEVICE_DISK is present, return 0 mount
> points.
>
> Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com>
> ---
> src/test/test_driver.c | 66 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 1b1ff3003e..626449501f 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3298,6 +3298,71 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
> return ret;
> }
>
> +
> +
> +static int
> +testDomainGetFSInfo(virDomainPtr dom,
> + virDomainFSInfoPtr **info,
> + unsigned int flags)
> +{
> + size_t i;
> + char *name;
> + virDomainObjPtr vm;
> + virDomainFSInfoPtr *info_ret = NULL;
> + int ret = -1;
> +
> + virCheckFlags(0, -1);
> +
> + if (!(vm = testDomObjFromDomain(dom)))
> + return -1;
> +
> + if (virDomainObjCheckActive(vm) < 0)
> + goto cleanup;
> +
> + *info = NULL;
> +
> + for (i = 0; i < vm->def->ndisks; i++) {
> + if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> + if (VIR_ALLOC_N(info_ret, 2) < 0 || VIR_ALLOC(info_ret[0]) < 0
||
> + VIR_ALLOC(info_ret[1]) < 0 ||
VIR_ALLOC(info_ret[0]->devAlias) < 0 ||
> + VIR_ALLOC(info_ret[1]->devAlias) < 0)
^these should be on separate lines to enhance readability.
Sure. I'm not sure if there are certain guidelines for that, but I
think nothing is mentioned in the "Contributor guidelines".
Many times I'm unsure how to split long lines, especially on if statements.
> + goto cleanup;
> +
> + name = vm->def->disks[i]->dst;
> +
> + if (VIR_STRDUP(info_ret[0]->mountpoint, "/") < 0 ||
> + VIR_STRDUP(info_ret[1]->mountpoint, "/boot") < 0
||
> + VIR_STRDUP(info_ret[0]->fstype, "ext4") < 0 ||
> + VIR_STRDUP(info_ret[1]->fstype, "ext4") < 0 ||
> + VIR_STRDUP(info_ret[0]->devAlias[0], name) < 0 ||
> + VIR_STRDUP(info_ret[1]->devAlias[0], name) < 0 ||
> + virAsprintf(&info_ret[0]->name, "%s1", name) <
0 ||
> + virAsprintf(&info_ret[1]->name, "%s2", name) <
0)
> + goto cleanup;
Okay, alternatively we could have 2 'if' clauses for each mountpoint, this is
strictly a personal preference, I think it could read even better if the
mountpoints were logically separated, but I won't insist on that.
That makes sense and is perfectly fine with me.
Let me know whether you're okay with the proposed tweaks.
Will you edit these before merging or do you want me to re-send it?
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
> +
> + info_ret[0]->ndevAlias = info_ret[1]->ndevAlias = 1;
> +
> + VIR_STEAL_PTR(*info, info_ret);
> +
> + ret = 2;
> + goto cleanup;
> + }
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + if (info_ret) {
> + virDomainFSInfoFree(info_ret[0]);
> + virDomainFSInfoFree(info_ret[1]);
> + VIR_FREE(info_ret);
> + }
> +
> + virDomainObjEndAPI(&vm);
> + return ret;
> +}
> +
> +
> static char *testDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED,
> int *nparams)
> {
> @@ -7292,6 +7357,7 @@ static virHypervisorDriver testHypervisorDriver = {
> .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
> .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
> .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */
> + .domainGetFSInfo = testDomainGetFSInfo, /* 5.5.0 */
> .domainGetSchedulerType = testDomainGetSchedulerType, /* 0.3.2 */
> .domainGetSchedulerParameters = testDomainGetSchedulerParameters, /* 0.3.2 */
> .domainGetSchedulerParametersFlags = testDomainGetSchedulerParametersFlags, /*
0.9.2 */
> --
> 2.22.0
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list