Re: [libvirt] [PATCH v11] add 802.1Qbh and 802.1Qbg handling
Stefan Berger wrote:
On Fri, 2010-05-28 at 23:17 +0200, Jim Meyering wrote:
> Stefan Berger wrote:
> > This is now hopefully the final version of this patch that adds support
> > for configuring 802.1Qbg and 802.1Qbh switches. The 802.1Qbh part has
> > been successfully tested with real hardware. The 802.1Qbg part has only
> > been tested with a (dummy) server that 'behaves' similarly to how we
> > expect lldpad to 'behave'.
> >
> > V11:
> > - determining pid of lldpad daemon by reading it from /var/run/libvirt.pid
> > (hardcode as is hardcode alson in lldpad sources)
> > - merging netlink send code for kernel target and user space target
> > (lldpad) using one function nlComm() to send the messages
> > - adding a select() after the sending and before the reading of the
> > netlink response in case lldpad doesn't respond and so we don't hang
> > - when reading the port status, in case of 802.1Qbg, no status may be
> > received while things are 'in progress' and only at the end a status
> > will be there.
> > - when reading the port status, use the given instanceId and vf to pick
> > the right IFLA_VF_PORT among those nested under IFLA_VF_PORTS.
>
> Hi Stefan,
>
> There are a few nits, but nothing serious.
>
> If this will be pushed in your name, you should add
> your name/email to the AUTHORS file.
Already there...
Oh, I see what happened.
The problem I saw was when using git-am to apply that, it used
stefanb-at-linux.vnet.ibm.com (with "@", of course) as your address, and
that address is not in AUTHORS (there, it's @ibm...), so the test failed.
Assuming you push with the email address that is already in AUTHORS,
there will be no failure.
> There are two cpp-indentation nits:
> cppi: src/storage/storage_backend.h: line 96: not properly indented
> cppi: src/util/macvtap.c: line 75: not properly indented
>
> And one unmarked diagnostic:
> src/util/macvtap.c-766- "error parsing pid of
lldpad");
>
> Those three things were exposed by running "make syntax-check".
Fixed the two related to this patch. The storage one didn't appear.
It's independent of your change.
I've fixed it.