[libvirt PATCH 2/3] rpc: avoid name lookup when dispatching node device APIs

[Michal Prívozník]

On 10. 3. 2020 19:13, Daniel P. Berrangé wrote:
> The node device APIs are a little unusual because we don't use a
> "remote_nonnull_node_device" object on the wire, instead we just
> have a "remote_string" for the device name. This meant dispatcher
> code generation needed special cases. In doing so we mistakenly
> used the virNodeDeviceLookupByName() API which gets dispatched
> into the driver, instead of get_nonnull_node_device() which
> directly populates a virNodeDevicePtr object.
> 
> This wasn't a problem with monolithic libvirtd, as the
> virNodeDeviceLookupByName() API call was trivially satisfied
> by the registered driver, albeit with an extra (undesirable)
> authentication check. With the split daemons, the call to
> virNodeDeviceLookupByName() fails in virtqemud, because the
> node device driver obviously doesn't exist in that daemon.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/remote/remote_daemon_dispatch.c | 7 +++++++
>  src/rpc/gendispatch.pl              | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
> index 2741a32f63..226049fed6 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -95,6 +95,7 @@ static virNWFilterBindingPtr get_nonnull_nwfilter_binding(virConnectPtr conn, re
>  static virDomainCheckpointPtr get_nonnull_domain_checkpoint(virDomainPtr dom, remote_nonnull_domain_checkpoint checkpoint);
>  static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr dom, remote_nonnull_domain_snapshot snapshot);
>  static virNodeDevicePtr get_nonnull_node_device(virConnectPtr conn, remote_nonnull_node_device dev);
> +static virNodeDevicePtr get_nonnull_node_device_name(virConnectPtr conn, remote_nonnull_string devdev);
>  static void make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src);
>  static void make_nonnull_network(remote_nonnull_network *net_dst, virNetworkPtr net_src);
>  static void make_nonnull_network_port(remote_nonnull_network_port *port_dst, virNetworkPortPtr port_src);
> @@ -7291,6 +7292,12 @@ get_nonnull_node_device(virConnectPtr conn, remote_nonnull_node_device dev)
>      return virGetNodeDevice(conn, dev.name);
>  }
>  
> +static virNodeDevicePtr
> +get_nonnull_node_device_name(virConnectPtr conn, remote_nonnull_string devname)

s/devname/dev/ because syntax-check suggests that @devname is a function
on FreeBSD.

> +{
> +    return virGetNodeDevice(conn, devname);
> +}
> +
>  static void
>  make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src)
>  {
> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
> index c140ed712c..0b2ae59910 100755
> --- a/src/rpc/gendispatch.pl
> +++ b/src/rpc/gendispatch.pl
> @@ -571,7 +571,7 @@ elsif ($mode eq "server") {
>                  $has_node_device = 1;
>                  push(@vars_list, "virNodeDevicePtr dev = NULL");
>                  push(@getters_list,
> -                     "    if (!(dev = virNodeDeviceLookupByName($conn_var, args->name)))\n" .
> +                     "    if (!(dev = get_nonnull_node_device_name($conn_var, args->name)))\n" .
>                       "        goto cleanup;\n");
>                  push(@args_list, "dev");
>                  push(@free_list,
> 

Michal