Re: [libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
On 03/01/2012 11:32 AM, Roopa Prabhu wrote:
On 3/1/12 7:52 AM, "Stefan Berger"<stefanb(a)linux.vnet.ibm.com> wrote:
> On 03/01/2012 10:32 AM, Roopa Prabhu wrote:
>>
>> On 3/1/12 4:39 AM, "Stefan Berger"<stefanb(a)linux.vnet.ibm.com>
wrote:
>>
>>> On 03/01/2012 04:02 AM, Roopa Prabhu wrote:
>>>> From: Roopa Prabhu<roprabhu(a)cisco.com>
>>>>
>>>> This patch includes the following changes
>>>> - removes some netlink functions which are now available in virnetdev.c
>>>> - Adds a vf argument to all port profile functions
>>>>
>>>> For 802.1Qbh devices, the port profile calls can use a vf argument if
>>>> passed by the caller. If the vf argument is -1 it will try to derive the
vf
>>>> if the device passed is a virtual function.
>>>>
>>>> For 802.1Qbg devices, This patch introduces a null check for the device
>>>> argument because during port profile assignment on a hostdev, this
argument
>>>> can be null. Stefan CC'ed for comments
>>> I agree to the change per se since entering the function with a NULL
>>> parameter would be fatal, though I am wondering under what condition
>>> this can occur. I don't see this happening in the Associate call path
>>> for example.
>> It happens in patch 4/4 where we call associate for a hostdev and there is
>> no macvtap available there.
>>
> So this hostdev related code can only be used by 802.1Qbh because the
> type of device does not exist for 802.1Qbg?
Not really. The hostdev device is just a pci device. But looking at 802.1Qbg
port profile related code..i was not sure how it can be done when the device
is a hostdev.
My guess is that in the 802.1Qbg case we would have to build a macvtap
device on top of the linkdev to have the macvtap_ifname parameter
available. Well, maybe the point of intercept would be to check the type
of profile in 4/4 and make sure it's a 802.1Qbh type and only then
invoke the function -- like a switch() statement where 802.1Qbh type of
profile is supported and 802.1Qbg is not (yet).
> I think that at least there
> should be an error message logging the fact that the function was called
> without an interface name. Also the return error code should probably
> be -1 and not be overloaded with -E*.
Ok This can be done.
> If possible (unless already done)
> the combination of hostdev and 802.1Qbg should probably even be
> prevented on the XML level.
This is not done yet. I think I will do it. And then if someone knows how to
do it for 802.1Qbg they can open this up.
Stefan