Re: [PATCH v3 2/3] util: Introduce API's for Polkit text authentication

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]