On Sat, Jul 27, 2019 at 05:04:37PM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass@xxxxxxxxx>
> ---
> src/test/test_driver.c | 90 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index ab0f8b06d6..9f4e255e35 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3500,6 +3500,95 @@ testDomainGetInterfaceParameters(virDomainPtr dom,
> virDomainObjEndAPI(&vm);
> return ret;
> }
> +
> +
> +static int
> +testDomainGetBlockIoTune(virDomainPtr dom,
> + const char *path,
> + virTypedParameterPtr params,
> + int *nparams,
> + unsigned int flags)
> +{
> + virDomainObjPtr vm = NULL;
> + virDomainDefPtr def = NULL;
> + virDomainDiskDefPtr disk;
> + virDomainBlockIoTuneInfo reply = {0};
> + int ret = -1;
> +
> + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> + VIR_DOMAIN_AFFECT_CONFIG |
> + VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> + flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
> +
> + if (*nparams == 0) {
> + *nparams = 20;
> + return 0;
> + }
> +
> + if (!(vm = testDomObjFromDomain(dom)))
> + return -1;
> +
> + if (!(def = virDomainObjGetOneDef(vm, flags)))
> + goto cleanup;
> +
> + if (!(disk = virDomainDiskByName(def, path, true))) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("disk '%s' was not found in the domain config"),
> + path);
> + goto cleanup;
> + }
> +
> + reply = disk->blkdeviotune;
> + if (VIR_STRDUP(reply.group_name, disk->blkdeviotune.group_name) < 0)
> + goto cleanup;
> +
> +#define ULL VIR_TYPED_PARAM_ULLONG
I don't see a point in doing ^this just to save a few characters, we're way
over the 80 columns already anyway, so wrap the long lines and span the macro
call over multiple lines.
Funny that I remember us having a discussion about macros doing string
concatenation (which I don't really mind that much) but prevents you from jumping
directly to the symbol definition - that's exactly what the QEMU alternative
does, I guess just to save some common prefixes :D.
Looking at the functions that use the typed parameters, an improvement we could
definitely do (in a standalone patch) is to ditch the index from the macro
definition since it's quite fragile, by doing:
int maxparams = X;
if (*nparams == 0) {
*nparams = maxparams;
return 0;
}
*nparams = 0;
TEST_SET_PARAM(¶m[*nparams++],...)
> + TEST_SET_PARAM(0, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, ULL, reply.total_bytes_sec);
> + TEST_SET_PARAM(1, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, ULL, reply.read_bytes_sec);
> + TEST_SET_PARAM(2, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, ULL, reply.write_bytes_sec);
> +
> + TEST_SET_PARAM(3, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, ULL, reply.total_iops_sec);
> + TEST_SET_PARAM(4, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, ULL, reply.read_iops_sec);
> + TEST_SET_PARAM(5, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, ULL, reply.write_iops_sec);
> +
> + TEST_SET_PARAM(6, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, ULL, reply.total_bytes_sec_max);
> + TEST_SET_PARAM(7, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, ULL, reply.read_bytes_sec_max);
> + TEST_SET_PARAM(8, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, ULL, reply.write_bytes_sec_max);
> +
> + TEST_SET_PARAM(9, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, ULL, reply.total_iops_sec_max);
> + TEST_SET_PARAM(10, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, ULL, reply.read_iops_sec_max);
> + TEST_SET_PARAM(11, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, ULL, reply.write_iops_sec_max);
> +
> + TEST_SET_PARAM(12, VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, ULL, reply.size_iops_sec);
> +
> + TEST_SET_PARAM(13, VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, VIR_TYPED_PARAM_STRING, reply.group_name);
> + reply.group_name = NULL;
> +
> + TEST_SET_PARAM(14, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, ULL,
> + reply.total_bytes_sec_max_length);
> + TEST_SET_PARAM(15, VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, ULL,
> + reply.read_bytes_sec_max_length);
> + TEST_SET_PARAM(16, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, ULL,
> + reply.write_bytes_sec_max_length);
> +
> + TEST_SET_PARAM(17, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, ULL,
> + reply.total_iops_sec_max_length);
> + TEST_SET_PARAM(18, VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, ULL,
> + reply.read_iops_sec_max_length);
> + TEST_SET_PARAM(19, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, ULL,
> + reply.write_iops_sec_max_length);
> +#undef ULL
> +
> + if (*nparams > 20)
> + *nparams = 20;
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(reply.group_name);
> + virDomainObjEndAPI(&vm);
> + return ret;
> +}
> #undef TEST_SET_PARAM
>
>
> @@ -8512,6 +8601,7 @@ static virHypervisorDriver testHypervisorDriver = {
> .domainGetNumaParameters = testDomainGetNumaParameters, /* 5.6.0 */
> .domainSetInterfaceParameters = testDomainSetInterfaceParameters, /* 5.6.0 */
> .domainGetInterfaceParameters = testDomainGetInterfaceParameters, /* 5.6.0 */
> + .domainGetBlockIoTune = testDomainGetBlockIoTune, /* 5.6.0 */
5.7.0 (so that we/I don't forget about to change it ;))
With breaking the long lines:
Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
