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

[Stefan Bader <stefan.bader@canonical.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5106 bytes --]

On 19.12.2013 18:57, Ian Campbell wrote:
> 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. 

Yeah, probably rather rare. This just sounded like a less intrusive change which
would only address the problem that no devid is no option when starting the dm.

> 
>> 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.

Right, that kind of was what I tried to say in many words. :) Oh, hm probably
with the addition that xl would save configs with one key and libvirt with
another. So relying on that at least results in xl not being able to show
details of libvirt created domains (and the other way round).
But as you said, I seem to have missed some discussion about it (which I saw Jim
mentioning a link but have not followed, yet, this morning.

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

Yes, that sounds good. It would feel like a more intuitive behavior to me (not
that I think I could call myself an expert on "intuitivity").


> 
> Ian.,
> 
> 



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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