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.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Despite their names, the following APIs:

    virNodeDeviceDettach
    virNodeDeviceDetachFlags
    virNodeDeviceReAttach
    virNodeDeviceReset

are all handled by the virt drivers, not the node device driver.
A bug in the RPC generator meant that these APIs were sent to
the nodedev driver for handling. This caused breakage with the
split daemons, since nothing was available to process them.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This lets it generate the remote dispatch for StorageVolGetInfoFlags.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

After conversion to g_strdup, the helpers now always return success.
Remove the return value to simplify the callers.

Note that many occurrences of these is in the code generated by
gendispatch.pl. Since gendispatch aggregates many cases together an
incremental conversion would require more invasive changes to
gendispatch for the time of conversion which doesn't make sense.

Also in many cases the helper was the last place where the 'error:'
label was used and thus also those conversions must be included in this
patch.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
ACKed-by: NEric Blake <eblake@redhat.com>

Replace all occurrences of
  if (VIR_STRDUP(a, b) < 0)
     /* effectively dead code */
with:
  a = g_strdup(b);
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Prefer the GLib version of the macro.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

The same way we check for limits when decoding typed parameters
(virTypedParamsDeserialize()) we should do the same check when
serializing them so that we don't put onto the wire more than our
limits allow. Surprisingly, we were doing so explicitly in some
places but not all of them.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NJonathon Jongsma <jjongsma@redhat.com>

The driver dispatch methods access the priv->conn variables directly.
In future we want to dynamically open the connections for the secondary
driver. Thus we want the methods to call a method to get the connection
handle instead of assuming the private variable is non-NULL.
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The remote code generator had to be taught about the new
virDomainCheckpointPtr type, at which point the remote driver code for
checkpoints can be generated.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Define the wire protocol for the virNetworkPort APIs and enable the
client/server RPC dispatch.
Reviewed-by: NLaine Stump <laine@laine.org>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

If we call virStreamFinish and virStreamAbort from 2 distinct
threads for example we can have access to freed memory.
Because when virStreamFinish finishes for example virStreamAbort
yet to be finished and it access virNetClientStreamPtr object
in stream->privateData.

Also it does not make sense to clear @driver field. After
stream is finished/aborted it is better to have appropriate
error message instead of "unsupported error".

This commit reverts [1] or virNetClientStreamPtr and
virStreamPtr will never be unrefed due to cyclic dependency.
Before this patch we don't have leaks because all execution
paths we call virStreamFinish or virStreamAbort.

[1] 8b6ffe40 : virNetClientStreamNew: Track origin stream
Signed-off-by: NNikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

The make_nonnull_XXX methods can all fail due to OOM but this was being
silently ignored and thus also not checked by callers. Make the methods
propagate errors and use ATTRIBUTE_RETURN_CHECK to force callers to deal
with it.
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1631606

Changes made to manage and utilize a secondary connection
driver to APIs outside the scope of the primary connection
driver have resulted in some confusion processing polkit rules
since the simple "access denied" error message doesn't provide
enough of a clue when combined with the "authentication failed:
access denied by policy" as to which connection driver refused
or failed the ACL check.

In order to provide some context, let's modify the existing
"access denied" error returned from the various vir*EnsureACL
API's to provide the connection driver name that is causing
the failure. This should provide the context for writing the
polkit rules that would allow access via the driver, but yet
still adhere to the virAccessManagerSanitizeError commentary
regarding not telling the user why access was denied.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

This reverts commit ccc72d5c.

Based on upstream comment to a follow-up patch, this didn't take the
right approach and the right thing to do is revert and rework.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

https://bugzilla.redhat.com/show_bug.cgi?id=1631606

Changes made to manage and utilize a secondary connection
driver to APIs outside the scope of the primary connection
driver have resulted in some confusion processing polkit rules
since the simple "access denied" error message doesn't provide
enough of a clue when combined with the "authentication failed:
access denied by policy" as to which connection driver refused
or failed the ACL check.

In order to provide some context, let's modify the existing
"access denied" error returne from the various vir*EnsureACL
API's to provide the connection driver name that is causing
the failure. This should provide the context for writing the
polkit rules that would allow access via the driver.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
ACKed-by: NMichal Privoznik <mprivozn@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Calling a push_privconn method to directly push the connection object
name into the arg list is inconvenient. Refactor so that we acquire
the connection variable name upfront, and push it to the arg list
separately. This allows various hardcoded usage of "priv->conn" to
be parameterized.
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

This is particularly useful on operating systems that don't ship
Perl as part of the base system (eg. FreeBSD) while still working
just as well as it did before on Linux.

In one case (src/rpc/genprotocol.pl) the interpreter path was
missing altogether.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

If we exceed a fixed limit in RPC code we get a horrible message
like this, if the parameter type is a 'string', because we forgot
to initialize the error message type field:

  $ virsh snapshot-list ostack1
  error: too many remote undefineds: 1329 > 1024

It would also be useful to know which RPC call and field was
exceeded. So this patch makes us report:

  $ virsh snapshot-list ostack1
  error: too many remote undefineds: 1329 > 1024,
  in parameter 'names' for 'virDomainSnapshotListNames'
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Now, not all APIs are going to support sparse streams. To some it
makes no sense at all, e.g. virDomainOpenConsole() or
virDomainOpenChannel(). To others, we will need a special flag to
indicate that client wants to enable sparse streams. Instead of
having to write RPC dispatchers by hand we can just annotate in
our .x files that a certain flag to certain RPC call enables this
feature. For instance:

     /**
      * @generate: both
      * @ReadStream: 1
      * @sparseflag: VIR_SPARSE_STREAM
      * @acl: storage_vol:data_read
      */
     REMOTE_PROC_DOMAIN_SOME_API = XXX,

Therefore, whenever client calls virDomainSomeAPI(..,
VIR_SPARSE_STREAM); daemon will mark that down and send stream
skips when possible.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Add a new argument to daemonCreateClientStream in order to allow for
future expansion to mark that a specific stream can be used to skip
data, such as the case with sparsely populated files. The new flag will
be the eventual decision point between client/server to decide whether
both ends can support and want to use sparse streams.

A new bool 'allowSkip' is added to both _virNetClientStream and
daemonClientStream in order to perform the tracking.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Add a virStreamPtr pointer to the _virNetClientStream
in order to reverse track the parent stream.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Since it's rather tedious to write the dispatchers for functions that
return an array of typed parameters (which are rather common) let's add
some rpcgen code to generate them.

Now that libvirt-admin supports another client-side object and provided that
we want to generate as many both client-side and server-side RPC dispatchers,
support for this needs to be added to gendispatch.
Signed-off-by: NErik Skultety <eskultet@redhat.com>

The adminDispatchConnectListServers() function is generated by
our great perl script. However, it has a tiny flaw: if
adminConnectListServers() it calls fails, the control jumps onto
cleanup label where we try to free any list of servers built so
far. However, in the loop @i is unsigned (size_t) while @nresults
is signed (int). Currently, it does no harm because of the check
for @result being non-NULL. But if that ever changes in the
future, this bug will be hard to chase.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

All other places use VIR_ERR_RPC except this one, let's be consistent,
shall we?
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Until now, the script assumed that snapshot name is 'snap', but that's
going to change.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Let's call it modern_ret_as_list as opposed to single_ret_as_list.  The
latter was able to return list of things.  However the new, more modern,
version came and it is used since listAllDomains till nowadays in
ListServers.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

We were using parentheses for grouping admin|remote even though we didn't
need to capture what's in it.  That caused some changes to be greater
than needed and, to be honest, some confusion as well.  Let's use it as
it should be used.  It'll also make future changes more consistent.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>