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

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2013-12-19 at 18:06 +0100, Stefan Bader wrote:
> > How about we:
> >       * move the init to setdefault to catch the single NIC added via
> >         hotplug case
> 
> Init of devid?

Yes, sorry for not being clear.

>  Hm, would that work as I am not sure there is a simple way of
> differentiating between a NIC config for a single hotplug and one that is part
> of a create-time array...

The create time array would be initialised with devid's != -1 as part of
the steps outlined in the following bullets, so setdefault woudln't
touch it again.

> 
> >       * we add somewhere early in the domain create path a call to a
> >         function which assigns devids to an entire array of devices (and
> >         do it for all the different device types). Perhaps in
> >         initiate_domain_create() after the calls to
> >         libxl__domain_create_info_setdefault and
> >         libxl__domain_build_info_setdefault but before the loop calling
> >         libxl__device_disk_setdefault for the disks.
> >       * perhaps that same function should call setdefault too, after
> >         having assigned the device, rather than it being done later in
> >         an adhoc way?
> > 
> > Does that sound at all plausible?
> 
> I wonder, well this won't help for any other device types (maybe not really
> needed), what about just adding the following to the existing loop in
> domcreate_launch_dm (just a brain dump, not even tried to compile):
> 
>     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;
> +	if (d_config->nics[i].devid < 0)
> +		d_config->nics[i].devid = i;
>     }
> 
> Of course this a gain won't work well if the caller had assigned some devids but
> not other.

Indeed.

> Ok, maybe do the loop twice, first round sets default and picks the
> highest pre-assigned devid and second round makes sure any still unassigned ones
> are set to ++that.

That would potentially leave holes, I don't know if that matters. 

> Oh, just while talking about setdefault. Jim, this is one of the odd things when
> moving from xm to xl stack from libvirt: libvirt defaults to the netfront NIC
> when no model is specified and sets the type. The libxl setdefault function sets
> the model to rtl8139 but leaves the type untouched.

This sounds like a bug in libxl to me -- it should do something
consistent I think.

>  So setting no model in the
> xml config creates a domain with no emulated NIC (this does not matter after
> Linux is up because the emulated devices get unplugged). Just that PXE boot will
> not work. This gets odd because with the old xen (xm) driver, no model meant
> rtl8139.
> 
> Sigh, and to hijack this thread even further I noticed a quite unexpected
> behaviour when starting a domain trhough libvirt and then try to use "xl list
> -l" to get config details. "xl list" shows all running domains but "xl list -l"
> produces something like "you have to specify a domain name". I found the origin
> of this to be libxl_userdata_retrieve which takes a userdate_userid as an
> argument. Libvirt uses "libvirt-xml" for that, while xl uses "xl". This might be
> intentional and the bug is just that we need a better check for not finding the
> userdata and then skipping those domains. On the other hand ... its after all in
> both cases a domain created and started through libxl...

I think this was discussed a few weeks ago on the list, and there were
one or two separate bugs and short comings. I'm not sure which subset
were actually fixed.

One issue is that xl stored the guest config and then retrieves it for
use in xl list -l, but libvirt != xl and therefore has no config file to
save.

The solution is probably for the list stuff to be based on dynamically
gathering the domain info, instead of reparsing the config.

Ian.,