Re: [LTP] [PATCH 1/2] KVM test infrastructure

[Richard Palethorpe <rpalethorpe@suse.de>]

Hi,

Martin Doucha <mdoucha@suse.cz> writes:

> On 15. 03. 22 16:00, Richard Palethorpe wrote:
>> Hi Martin,
>> 
>> Martin Doucha <mdoucha@suse.cz> writes:
>>> +void tst_kvm_print_result(const struct tst_kvm_result *result)
>>> +{
>>> +	int ttype;
>>> +
>>> +	tst_kvm_validate_result(result->result);
>>> +	ttype = TTYPE_RESULT(result->result);
>>> +
>>> +	if (ttype == TBROK)
>>> +		tst_brk(ttype, "%s", result->message);
>>> +	else
>>> +		tst_res(ttype, "%s", result->message);
>>> +}
>> 
>> Could you please pass the file and lineno from the test?
>
> I've skipped that mainly because passing the filename would require an
> extra string buffer in the result structure. But I guess I can pass just
> a 64bit string address since the filename is a string constant and then
> read the text from the VM memory buffer.

Either way sounds good to me.

>
>>> +void tst_kvm_create_instance(struct tst_kvm_instance *inst, size_t ram_size)
>>> +{
>>> +	int sys_fd;
>>> +	size_t pagesize, result_pageaddr = KVM_RESULT_BASEADDR;
>>> +	char *vm_result, *reset_ptr;
>>> +	struct kvm_cpuid2 *cpuid_data;
>>> +	const size_t payload_size = kvm_payload_end - kvm_payload_start;
>>> +
>>> +	memset(inst, 0, sizeof(struct tst_kvm_instance));
>>> +	inst->vm_fd = -1;
>>> +	inst->vcpu_fd = -1;
>>> +	inst->vcpu_info = MAP_FAILED;
>>> +
>>> +	pagesize = SAFE_SYSCONF(_SC_PAGESIZE);
>>> +	result_pageaddr -= result_pageaddr % pagesize;
>>> +
>>> +	if (payload_size + MIN_FREE_RAM > ram_size - VM_KERNEL_BASEADDR) {
>>> +		ram_size = payload_size + MIN_FREE_RAM + VM_KERNEL_BASEADDR;
>>> +		ram_size += 1024 * 1024 - 1;
>>> +		ram_size -= ram_size % (1024 * 1024);
>>> +		tst_res(TWARN, "RAM size increased to %zu bytes", ram_size);
>>> +	}
>>> +
>>> +	if (ram_size > result_pageaddr) {
>>> +		ram_size = result_pageaddr;
>>> +		tst_res(TWARN, "RAM size truncated to %zu bytes", ram_size);
>>> +	}
>>> +
>>> +	/* Create VM */
>> 
>> These comments are pretty redundant when we have ioctl's like
>> KVM_CREATE_VM and KVM_CREATE_VCPU. There are much harder things to
>> understand in this patchset.
>
> I know that the code is straightforward. But it's quite dense so I've
> added the comments as section headers for easier navigation when you
> need to change something.

Makes sense. However the style guide forbids commenting the obvious.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp