On 4/28/22 11:34 AM, Daniel P. Berrangé wrote:
> On Wed, Apr 27, 2022 at 10:56:22PM +0200, Claudio Fontana wrote:
>> On 4/27/22 1:00 AM, Jim Fehlig wrote:
>>> On 4/26/22 10:47, Claudio Fontana wrote:
>>>> Signed-off-by: Claudio Fontana <cfontana@xxxxxxx>
>>>> ---
>>>> src/remote/remote_driver.c | 1 +
>>>> src/remote/remote_protocol.x | 14 +++++++++++++-
>>>> src/remote_protocol-structs | 8 ++++++++
>>>> 3 files changed, 22 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
>>>> index 1fc5d41971..c5b644ce49 100644
>>>> --- a/src/remote/remote_driver.c
>>>> +++ b/src/remote/remote_driver.c
>>>> @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = {
>>>> .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */
>>>> .domainRestore = remoteDomainRestore, /* 0.3.0 */
>>>> .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */
>>>> + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */
>>>> .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */
>>>> .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */
>>>> .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */
>>>> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
>>>> index c2ae5c5748..7b919ef375 100644
>>>> --- a/src/remote/remote_protocol.x
>>>> +++ b/src/remote/remote_protocol.x
>>>> @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args {
>>>> unsigned int flags;
>>>> };
>>>>
>>>> +struct remote_domain_restore_parameters_flags_args {
>>>> + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>;
>>>> + unsigned int flags;
>>>> +};
>>>> +
>>>> /* The device removed event is the last event where we have to support
>>>> * dual forms for back-compat to older clients; all future events can
>>>> * use just the modern form with callbackID. */
>>>> @@ -6935,5 +6940,12 @@ enum remote_procedure {
>>>> * @generate: both
>>>> * @acl: domain:hibernate
>>>> */
>>>> - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440
>>>> + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440,
>>>> +
>>>> + /**
>>>> + * @generate: both
>>>> + * @acl: domain:start
>>>> + * @acl: domain:write
>>>> + */
>>>> + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
>>>
>>> I've stared at this for quite a while but can't figure out why the dispatch stub
>>> does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting the
>>> following build failure
>>>
>>> In file included from ../src/remote/remote_daemon_dispatch.c:133:
>>>
>>> src/remote/remote_daemon_dispatch_stubs.h: In function
>>> ‘remoteDispatchDomainRestoreParametersFlags’:
>>>
>>> src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1 of
>>> ‘virDomainRestoreParametersFlags’ from incompatible pointer type
>>> [-Werror=incompatible-pointer-types]
>>>
>>> 10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0)
>>>
>>> | ^~~~~~
>>>
>>> | |
>>>
>>> | virTypedParameterPtr {aka
>>> struct _virTypedParameter *}
>>>
>>> In file included from ../include/libvirt/libvirt.h:36,
>>>
>>> from ../src/internal.h:65,
>>>
>>> from ../src/util/virerror.h:24,
>>>
>>> from ../src/remote/remote_daemon_dispatch.c:23:
>>>
>>> ../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’ {aka
>>> ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka
>>> ‘struct _virTypedParameter *’}
>>>
>>> 1576 | int virDomainRestoreParametersFlags (virConnectPtr
>>> conn,
>>>
>>>
>>> Perhaps a bug in gendispatch.pl. I'm not familiar with the script or debugging
>>> it, but others here can likely provide help.
>>>
>>> Jim
>>>
>>
>> Still fighting this one, could not defeat the beast yet..
>
> Don't bother. gendispatch.pl is not capable of correctly handling all
> API designs we have and it isn't worth trying to fix it unless the
> problem is obvious or you enjoy reading & writing obtuse perl code ;-P
>
> In the .xdr file, switch 'generate: both' to exclude either the client
> or server code generation, or both, depending on which bit is broken.
> Then just write the code by hand. You can start with the broken code
> and just fix it up and add to remote_daemon_dispatch.c directly. You
> will see several examples not using 'generate: both' you can follow
>
>
> With regards,
> Daniel
I actually think I found the problem. It needs to be put explicitly into a list
of methods that require conn, like NodeSetMemoryParameters.
Here is the change:
commit 192d8e56e88c00ede94768f6f73c6be64f31c789 (HEAD -> api_draft)
Author: Claudio Fontana <cfontana@xxxxxxx>
Date: Thu Apr 28 08:16:27 2022 -0600
gendispatch: add DomainRestoreParametersFlags as requiring conn argument
Signed-off-by: Claudio Fontana <cfontana@xxxxxxx>
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 9f5bf0e316..bce88cfc52 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -637,7 +637,10 @@ elsif ($mode eq "server") {
} elsif ($args_member =~ m/^remote_typed_param (\S+)<(\S+)>;/) {
push(@vars_list, "virTypedParameterPtr $1 = NULL");
push(@vars_list, "int n$1 = 0");
- if ($call->{ProcName} eq "NodeSetMemoryParameters") {
+
+ # NB: if your new API starts with remote_typed_params, enter it here if you need
+ # the conn arg to be passed first!
+ if ($call->{ProcName} eq "NodeSetMemoryParameters" || $call->{ProcName} eq "DomainRestoreParametersFlags") {
push(@args_list, $conn_var);
}
push(@args_list, "$1");
