From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: Setting devid for emulated NICs (Xen 4.3.1 /
 libvirt 1.2.0) using libxl driver
Date: Tue, 17 Dec 2013 16:58:54 +0000
Message-ID: <1387299534.1025.19.camel@dagon.hellion.org.uk>
References: <52B07D09.5060008@canonical.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <52B07D09.5060008@canonical.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Stefan Bader <stefan.bader@canonical.com>
Cc: libvir-list@redhat.com, Jim Fehlig <jfehlig@suse.com>, Xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On Tue, 2013-12-17 at 17:34 +0100, Stefan Bader wrote:
> Using virt-manager "hypervisor default" type:
> 
>     <interface type='bridge'>
>       <mac address='00:16:3e:5e:09:9d'/>
>       <source bridge='br0'/>
>       <script path='vif-bridge'/>
>     </interface>
> 
> This causes the qemu call to have "-net none" which removes PXE boot
> abilities. A linux kernel has network through the xen pv-driver.
> 
> Changing in virt-manager to "e1000" type:
> 
>     <interface type='bridge'>
>       <mac address='00:16:3e:5e:09:9d'/>
>       <source bridge='br0'/>
>       <script path='vif-bridge'/>
>       <model type='e1000'/>
>     </interface>
> 
> This currently (Xen 4.3.1 + libvirt 1.2.0) fails and the qemu arguments
> in the log look suspicious:
>   -device e1000,id=nic-1,netdev=net-1,mac=00:16:3e:5e:09:9d
>   -netdev type=tap,id=net-1,ifname=vif1.-1-emu,script=no,downscript=no
> 
> Looking through git I found ba64b97134a6129a48684f22f31be92c3b6eef96
>   libxl: Allow libxl to set NIC devid

Is that a libvirt commit? Not seeing it in xen.git.

Might this libxl fix be relevant:
        commit 5420f26507fc5c9853eb1076401a8658d72669da
        Author: Jim Fehlig <jfehlig@suse.com>
        Date:   Fri Jan 11 12:22:26 2013 +0000
        
            libxl: Set vfb and vkb devid if not done so by the caller
            
            Other devices set a sensible devid if the caller has not done so.
            Do the same for vfb and vkb.  While at it, factor out the common code
            used to determine a sensible devid, so it can be used by other
            libxl__device_*_add functions.
            
            Signed-off-by: Jim Fehlig <jfehlig@suse.com>
            Acked-by: Ian Campbell <ian.campbell@citrix.com>
            Committed-by: Ian Campbell <ian.campbell@citrix.com>
        
and a follow up in dfeccbeaa. Although the comment implies that nic's
were already correctly assigning a devid if the caller specified -1, so
I don't know why it doesn't work for you :-(

> 
> which removes a line that sets the devid of new NICs. I assume the comment
> says (sorry I always seem to get confused reading it) that devid should
> be auto-set by the libxl library provided by Xen.

Correct.

> But I could not find where that would be done.

I expect the above commits have pointed you to the right line, but the
code should be quite near the top of libxl__device_nic_add in libxl.c

>  Even the xl command implementation sets it
> explicitly.

The caller is also allowed to do this if it cares what number is used.

>  libxl_device_nic_init sets it to -1 which would explain the
> wrong qemu arguments above.

Right, libxl_device_nic_init sets everything to "the defaults please"
and __device_nic_add will instantiate any defaults which the application
didn't supply.

> For testing I re-added the following after the libxlMakeNic call:
> 
> --- libvirt-1.2.0.orig/src/libxl/libxl_conf.c   2013-12-11 17:04:17.000000000 +0
> +++ libvirt-1.2.0/src/libxl/libxl_conf.c        2013-12-16 19:08:46.830016646 +0
> @@ -907,6 +907,8 @@ libxlMakeNicList(virDomainDefPtr def,  l
>      for (i = 0; i < nnics; i++) {
>          if (libxlMakeNic(l_nics[i], &x_nics[i]))
>              goto error;
> +       if (x_nics[i].devid < 0)
> +               x_nics[i].devid = i;
>      }
> 
>      d_config->nics = x_nics;
> 
> And with that I get a working PXE boot when I set the device type
> in the config. (Side note that I think not using a type defaults to
> rtl8139 in the old xend driver. Maybe this should be the same for the
> xl driver).
> I am not sure how the right fix for it should look like as I am unsure
> which part was supposed to set the devid. Just right now it does not
> seem to be done.
> 
> -Stefan
>