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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list