Ok, with #1 I was hung up on your 'bridge with no interfaces' example,
which isn't the issue. As an aside, n my CentOS 6.3 and Ubuntu 12.04
test sytems, the default libvirt virbr0 does have an auto-configured
interface, 'vnet0'. And if there are no interfaces on a bridge, then
getPifs should treat it as null, since it's goal is to get interfaces.
So I was failing to see how the new code was different in that regard,
however it just creates a side effect, and is not your point.
Looking at getPifs(), if we change:
if(_publicBridgeName != null && bridge.equals(_publicBridgeName)){
_pifs.put("public", pif);
} else if (_guestBridgeName != null) {
_pifs.put("private", pif);
}
to
if(_publicBridgeName != null && bridge.equals(_publicBridgeName)){
_pifs.put("public", pif);
}
if (_guestBridgeName != null && bridge.equals(_guestBridgeName)) {
_pifs.put("private", pif);
}
does that solve the issue?
On Mon, Oct 29, 2012 at 7:30 AM, Marcus Sorensen <[email protected]> wrote:
> I'll take a look and address the concerns later today. #2 I can see
> immediately as its an if-else, but I don't see how the change affected #1
> from the previous behavior, just looking on my phone.
>
> On Oct 29, 2012 3:21 AM, "Dave Cahill" <[email protected]> wrote:
>>
>> Hi all,
>>
>> From LibvirtComputingResource.java, we can see that the CloudStack agent
>> fails to start if there is no "private" NIC name set.
>>
>>
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
>>
>> 698 getPifs();
>> 699 if (_pifs.get("private") == null) {
>> 700 s_logger.debug("Failed to get private nic name");
>> 701 throw new ConfigurationException("Failed to get private
>> nic name");
>> 702 }
>>
>> In other words, the agent fails to start if the entry for "private" in the
>> HashMap returned by getPifs() is null.
>>
>> A recent commit [1] seems to have changed the behavior of getPifs() so
>> that
>> this happens more often. The new code has two behaviors which seem strange
>> to me:
>>
>> 1. The value of the "private" entry is effectively set to the interface of
>> the *last* bridge in the list returned by "brctl show".
>> 2. A single bridge can only be used to set either the "public" or the
>> "private" value of the HashMap.
>>
>> This means that the getPifs() HashMap entry for "private" is null (causing
>> the agent to fail to start) in two situations which seem normal:
>>
>> 1. A bridge with no interfaces defined exists on the system.
>> For example, libvirt creates a bridge called virbr0 by default, which has
>> no interfaces. When getPifs() sees this entry, it counts it as null. If
>> this entry is the last one in the list, "private" is set to null.
>>
>> 2. The private and public bridge are the same.
>> From my reading of the previous code, if _guestBridgeName and
>> _publicBridgeName were set to the same value (only one bridge defined),
>> the
>> entries in _pifs for "public" and "private" would be the same, and the
>> agent would start correctly. However, the new code loops through the
>> bridges, setting them as the value of *either* public or private (or
>> neither), so if there is only one bridge available, the "private" entry in
>> pifs will be null.
>>
>> I'm just coming up to speed with the code base, so there may easily be a
>> good reason for this behavior change. Does anyone have an explanation?
>>
>> Thanks,
>> Dave Cahill.
>>
>>
>> [1]
>>
>> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=commit;h=915babd9
