Re: [PATCH v4 1/2] Add VIR_TYPED_PARAM_STRING

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/25/2011 03:31 PM, Eric Blake wrote:
Here's what I'm squashing in:

Good thing I haven't pushed yet - I just realized another problem.


But on Get interfaces, like virDomainGetMemoryParamaters, params is an
output-only interface; it can be uninitialized memory on entry, so we
must not base our behavior on the client's contents. Rather, the typed
parameter check has to either be done by each the callback (where we
write each hypervisor driver to not output strings unless they first
check flags), or can be factored into libvirt.c (hypervisors can blindly
write back all parameters, then libvirt.c does a post-process pass that
shrinks the array size to skip past all string entries).

Shoot, we have another problem. The current Get interfaces are documented as returning 0 on success, rather than the number of successfully returned elements. virDomainGetSchedulerParameters is hard-wired in the RPC protocol to not pass an array length, and we repeated that mistake in virDomainGetSchedulerParamatersFlags. That means we _can't_ reduce *nparams by doing libvirt.c filtering of the results. And if that is the case, then each hypervisor driver _has_ to do filtering manually, to return the right *nparams value in the first place - that is, if they ever want to pass back strings.

But virDomainGetBlkioParameters, virDomainGetMemoryParameters, and virDomainBlockStatsFlags could be "fixed" to return the number of elements populated, rather than 0. Are we okay making that sort of API change? I'll ask again when I post this as a v5.

--
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]