Re: [libvirt] [PATCH v3 2/3] util: Introduce API's for Polkit text authentication
On 02/22/2016 10:41 AM, Martin Kletzander 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.
>
I have literally no idea how PolicyKit is used, so I don't feel like the
right person to review more than the first patch (which looks fine).
Understood - part of the first patch did get an ACK in v2; however,
since I updated it to use VIR_ERR_AUTH_UNAVAILABLE (based on other
review comments), I figured it was best to send it out for review.
But this is something I can't let go unnoticed, sorry. If you
are using
virCommandPtr in virpolkit.h, then that file should also include
vircommand.h. Then you can remove it from virpolkit.c (if it is
included there) and you don't need to reorganize the includes or touch
them in other files. And anyone can include virpolkit.h without needing
to care about any dependencies.
Oh - right... I moved the include of vircommand.h into virpolkit.h.
That means removing it from virpolkit.c on this patch and virsh.h on the
next patch.
Tks -
John