Re: [Xen-devel] [libvirt] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver

[Stefan Bader <stefan.bader@xxxxxxxxxxxxx>]

On 18.12.2013 14:28, Ian Campbell wrote:
> On Wed, 2013-12-18 at 14:12 +0100, Stefan Bader wrote:
>> On 18.12.2013 13:27, Ian Campbell wrote:
>>> On Tue, 2013-12-17 at 18:32 +0100, Stefan Bader wrote:
>>>>>
>>>>> Might this libxl fix be relevant:
>>>>>         commit 5420f26507fc5c9853eb1076401a8658d72669da
>>>>>         Author: Jim Fehlig <jfehlig@xxxxxxxx>
>>>>>         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@xxxxxxxx>
>>>>>             Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>>>>>             Committed-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>>>>>         
>>>>> 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 :-(
>>>>
>>>> Ok, yes, the commit above indeed changes libxl__device_nic_add to call
>>>> libxl__device_nextid for the devid... Just how is this actually called.
>>>> Maybe not sufficient but "git grep libxl__device_nic_add" in the xen code 
>>>> only
>>>> shows the definition and a declaration in libxl_internal.h to me...
>>>
>>> I have a feeling a macro might be involved...
>>>
>>> Here we go, look for DEFINE_DEVICE_REMOVE in libxl.c. We should really
>>> add the eventual function names in comments to provide grep fodder....
>>
>> Oh duh, yeah. So in DEFINE_DEVICE_ADD a libxl_device_nic_add is created which
>> calls to libxl__device_nic_add. When I look for the single _ version I find a
>> call from xl_cmdimpl.c and its public declaration in libxl.h.
>> So I guess the bug is that libvirt in the libxl driver never seems to do so
> 
> The macro creates libxl__add_nics which adds the nics from the
> libxl_domain_config->nics array. I don't think libvirt needs to call
> libxl_device_nic_add manually unless it is hotplugging a new nic at
> runtime.
> 

Hm, so I think this is the path:

libxl_domain_create_new
-> do_domain_create
   -> initiate_domain_create
      -> libxl__bootloader_run (HVM domain, skipping bootloader)
      <- domcreate_bootloader_done
         -> domcreate_rebuild_done
            <- domcreate_launch_dm
               -> libxl__spawn_local_dm
         <- domcreate_devmodel_started

In libxl__spawn_local_dm, there is the following loop:

    for (i = 0; i < d_config->num_nics; i++) {
        /* We have to init the nic here, because we still haven't
         * called libxl_device_nic_add at this point, but qemu needs
         * the nic information to be complete.
         */
        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
        if (ret)
            goto error_out;
    }

So I think when starting the dm, the devid just is not set as setdefault does
not seem to do so. I would be done in the later domcreate_devmodel_started
callback but that is too late for the generated qemu arguments.

-Stefan

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel