On 02/25/2016 09:25 AM, Daniel P. Berrange wrote:
On Fri, Feb 12, 2016 at 12:12:32PM -0500, John Ferlan wrote:
> Introduce virPolkitAgentCreate and virPolkitAgentDestroy
>
> virPolkitAgentCreate will run the polkit pkttyagent image as an asynchronous
> command in order to handle the local agent authentication via stdin/stdout.
> The code makes use of the pkttyagent --notify-fd mechanism to let it know
> when the agent is successfully registered.
>
> virPolkitAgentDestroy will close the command effectively reaping our
> child process
>
> Needed to move around or add the "#include vircommand.h" since,
> virpolkit.h now uses it.
This part of the commit message was removed with Martin's request/change.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/libvirt_private.syms | 2 ++
> src/util/virpolkit.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++-
> src/util/virpolkit.h | 5 ++++
> tests/virpolkittest.c | 1 +
> 4 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 4cfaed5..8f2358f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2029,6 +2029,8 @@ virPidFileWritePath;
>
>
> # util/virpolkit.h
> +virPolkitAgentCreate;
> +virPolkitAgentDestroy;
> virPolkitCheckAuth;
>
>
> diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
> index df707f1..6941d74 100644
> --- a/src/util/virpolkit.c
> +++ b/src/util/virpolkit.c
> @@ -20,20 +20,22 @@
> */
>
> #include <config.h>
> +#include <poll.h>
>
> #if WITH_POLKIT0
> # include <polkit/polkit.h>
> # include <polkit-dbus/polkit-dbus.h>
> #endif
>
> -#include "virpolkit.h"
> #include "vircommand.h"
> +#include "virpolkit.h"
> #include "virerror.h"
> #include "virlog.h"
> #include "virstring.h"
> #include "virprocess.h"
> #include "viralloc.h"
> #include "virdbus.h"
> +#include "virfile.h"
>
> #define VIR_FROM_THIS VIR_FROM_POLKIT
>
> @@ -136,6 +138,65 @@ int virPolkitCheckAuth(const char *actionid,
> }
>
>
> +/* virPolkitAgentDestroy:
> + * @cmd: Pointer to the virCommandPtr created during virPolkitAgentCreate
> + *
> + * Destroy resources used by Polkit Agent
> + */
> +void
> +virPolkitAgentDestroy(virCommandPtr cmd)
> +{
> + virCommandFree(cmd);
> +}
> +
> +/* virPolkitAgentCreate:
> + *
> + * Allocate and setup a polkit agent
> + *
> + * Returns a virCommandPtr on success and NULL on failure
> + */
> +virCommandPtr
> +virPolkitAgentCreate(void)
> +{
> + virCommandPtr cmd = virCommandNewArgList(PKTTYAGENT, "--process",
NULL);
> + int pipe_fd[2] = {-1, -1};
> + struct pollfd pollfd;
> + int outfd = STDOUT_FILENO;
> + int errfd = STDERR_FILENO;
> +
> + if (!isatty(STDIN_FILENO))
> + goto error;
> +
> + if (pipe2(pipe_fd, 0) < 0)
> + goto error;
> +
> + virCommandAddArgFormat(cmd, "%lld", (long long int) getpid());
> + virCommandAddArg(cmd, "--notify-fd");
> + virCommandAddArgFormat(cmd, "%d", pipe_fd[1]);
> + virCommandAddArg(cmd, "--fallback");
> + virCommandSetInputFD(cmd, STDIN_FILENO);
> + virCommandSetOutputFD(cmd, &outfd);
> + virCommandSetErrorFD(cmd, &errfd);
> + virCommandPassFD(cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> + if (virCommandRunAsync(cmd, NULL) < 0)
> + goto error;
> +
> + pollfd.fd = pipe_fd[0];
> + pollfd.events = POLLHUP;
> +
> + if (poll(&pollfd, 1, -1) < 0)
> + goto error;
> +
> + return cmd;
> +
> + error:
> + VIR_FORCE_CLOSE(pipe_fd[0]);
> + VIR_FORCE_CLOSE(pipe_fd[1]);
> + virCommandFree(cmd);
> + return NULL;
> +}
> +
> +
> #elif WITH_POLKIT0
> int virPolkitCheckAuth(const char *actionid,
> pid_t pid,
> @@ -254,4 +315,18 @@ int virPolkitCheckAuth(const char *actionid ATTRIBUTE_UNUSED,
> }
>
>
> +void
> +virPolkitAgentDestroy(virCommandPtr cmd ATTRIBUTE_UNUSED)
> +{
> + return; /* do nothing */
> +}
> +
> +
> +virCommandPtr
> +virPolkitAgentCreate(void)
> +{
> + virReportError(VIR_ERR_AUTH_FAILED, "%s",
> + _("polkit text authentication agent unavailable"));
> + return NULL;
> +}
> #endif /* WITH_POLKIT1 */
> diff --git a/src/util/virpolkit.h b/src/util/virpolkit.h
> index 36122d0..f0aea37 100644
> --- a/src/util/virpolkit.h
> +++ b/src/util/virpolkit.h
> @@ -24,6 +24,8 @@
>
> # include "internal.h"
Based on Martin's comments - I included "vircommand.h" here
>
> +# define PKTTYAGENT "/usr/bin/pkttyagent"
> +
> int virPolkitCheckAuth(const char *actionid,
> pid_t pid,
> unsigned long long startTime,
> @@ -31,4 +33,7 @@ int virPolkitCheckAuth(const char *actionid,
> const char **details,
> bool allowInteraction);
>
> +void virPolkitAgentDestroy(virCommandPtr cmd);
> +virCommandPtr virPolkitAgentCreate(void);
Rather than exposing use of virCommand in the API, I'd
suggest you create a
typedef struct virPolkitAgent virPolkitAgent;
typedef virPolkitAgent *virPolkitAgentPtr;
okidoke... Funny I had done it this way at some point, but when
virCommandPtr was the only thing in the structure, I just opted to use
virCommandPtr directly. Anyway, the following is now defined:
typedef struct _virPolkitAgent virPolkitAgent;
typedef virPolkitAgent *virPolkitAgentPtr;
void virPolkitAgentDestroy(virPolkitAgentPtr cmd);
virPolkitAgentPtr virPolkitAgentCreate(void);
in the header file here
and in the .c do
struct virPolitAgent {
virCommandPtr;
};
Inside the "#if WITH_POLKIT1" there is now a :
struct _virPolkitAgent {
virCommandPtr cmd;
};
results in mods to virPolkitAgentDestroy:
virPolkitAgentDestroy(virPolkitAgentPtr agent)
{
if (!agent)
return;
virCommandFree(agent->cmd);
VIR_FREE(agent);
}
and the VIR_ALLOC(agent) in virPolkitAgentCreate as well as calling
virPolkitAgentDestroy(agent) instead of virCommandFree in error:
so we hide use of virCommand
> diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c
> index 73f001b..b46e65f 100644
> --- a/tests/virpolkittest.c
> +++ b/tests/virpolkittest.c
> @@ -27,6 +27,7 @@
> # include <stdlib.h>
> # include <dbus/dbus.h>
>
> +# include "vircommand.h"
Forgot to note in my response to Martin that virpolkittest.c doesn't
need a change here since virpolkit.h now includes vircommand.h. Same of
course for virsh.h.
Should I post a v4 of patches 2 & 3?
I'll still wait for the release to push the patches though.
Tks -
John