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

Re: [libvirt] [PATCH 5/5] qemu: launch bridge helper from libvirtd



On 04/18/2013 11:32 AM, Laine Stump wrote:
> On 03/25/2013 10:25 AM, Paolo Bonzini wrote:
>> <source type='bridge'> uses a helper application to do the necessary
>> TUN/TAP setup to use an existing network bridge, thus letting
>> unprivileged users use TUN/TAP interfaces.
>>

>> @@ -3746,7 +3828,6 @@ error:
>>  char *
>>  qemuBuildHostNetStr(virDomainNetDefPtr net,
>>                      virQEMUDriverPtr driver,
>> -                    virQEMUCapsPtr qemuCaps,
> 
> 
> qemuCaps might again become useful for this function in the future, so
> you may want to leave it here (marked as ATTRIBUTE_UNUSED) to reduce
> code churn.

For that matter, do we need a qemu capability bit that says whether the
bridge helper even exists, or do you give a nice error message to
someone using qemu:///session on a machine with too-old qemu without the
need for a capability bit??
> 
> I still don't like using qemu-bridge-helper, but this is better than the
> alternative of having qemu call it (although, due to the way that
> process capabilities works, we are unable to prevent a rogue qemu
> started by unprivileged libvirtd from calling it :-(
> 
> ACK to this patch (I think I would prefer you left the qemuCaps arg in,
> but others may disagree with me.)

I haven't pushed 4 or 5 (4 needed an OOM fix, and there's the question
of whether a capability bit is still useful).  I don't have a strong
preference whether to leave qemuCaps in; we have version control to
inspect if we need to figure out how to add it back in later.  I prefer
to limit ATTRIBUTE_UNUSED markers to functions that have a required
signature (generally, callbacks to be called from some other generic
code site); but this doesn't feel like we have any reason to be locked
into leaving qemuCaps in the interface.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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