Re: [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory

[Don Slutz <dslutz@verizon.com>]

On 11/24/14 10:26, Stefano Stabellini wrote:
> On Mon, 24 Nov 2014, Konrad Rzeszutek Wilk wrote:
>> On Mon, Nov 24, 2014 at 12:17:12PM +0000, Stefano Stabellini wrote:
>>> On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
>>>> On Fri, Nov 21, 2014 at 06:48:53PM +0000, Stefano Stabellini wrote:
>>>>> On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
>>>>>> On Fri, Nov 21, 2014 at 05:11:09PM +0000, Stefano Stabellini wrote:
>>>>>>> The rom is used for pxebooting. We don't need to allow pxebooting from
>>>>>>> more than one network card.  Loading a romfile for every NIC wastes
>>>>>> Why not? Why can't we PXE boot from each network card?
>>>>> I take it back: you are right, at the moment it is actually possible to
>>>>> pxe boot from the fourth nic for example. Maybe we could just load the
>>>>> first four romfiles and skip the others (four is the limit before QEMU
>>>>> fails to allocate any more memory).
>>>> The limit is in the count of ROMs or the memory consumption?
>>> The limit is memory consumption.
>> Um, how big are those ROMs? Gigs?
> I think they are 60K each in the case of rtl8139.
> However they are accounted on top of the ram of the guest, that's why
> the allocation fails. Strictly speaking I guess it shouldn't work even
> the first time around.

My extra QEMU debug says:

xen_ram_alloc: alloc 40000 bytes (256 Kib) of ram at 42010000 
mr.name=rtl8139.rom
e1000 vhdr enabled
xen_ram_alloc: alloc 40000 bytes (256 Kib) of ram at 42050000 
mr.name=e1000.rom

And that matches the max of 4.

#define LIBXL_MAXMEM_CONSTANT 1024

is what controls this.

> Maybe the bug is in libxl memory allocation, that doesn't account for
> rom sizes.

Yes.

    -Don Slutz

>
>>>> What if you also do PCI passthrough? Does that figure in this calculation?
>>> In the case of PCI passthrough, roms are remapped, so they shouldn't
>>> affect memory consumption.
>>>
>>>   
>>>>> TBH not all the emulated nics need a romfile but I wouldn't want to go
>>>>> down at that level of details beause they become QEMU implementation
>>>>> details. I wouldn't want to tie libxl to a certain version of QEMU in
>>>>> any way.
>>>>>
>>>>> A better way would be handling memory allocation errors in QEMU but QEMU
>>>>> doesn't do that at the moment and the change there would be certainly
>>>>> more invasive that a couple of lines in libxl.
>>>>>
>>>>>
>>>>>>> memory and as a matter of fact breaks configurations with more than 4
>>>>>>> NICs as QEMU fails to allocate memory on behalf of the guest.
>>>>>> What if you have four different type of NICs? Say 1 rlt8193, 1 e1000, one eepro,
>>>>>> and ne2k ?
>>>>>>
>>>>>> Don't you want to load the ROM for each one?
>>>>> The rom cannot be reused in QEMU among multiple instances of the same
>>>>> nic, so having four different types of nics or just one type doesn't
>>>>> change anything.
>>>>>
>>>>>
>>>>>>> With this fix, it is possible to assign more than 4 rtl8139 NICs to the
>>>>>>> guest.
>>>>>>>
>>>>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>>>>>
>>>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>>>> index 3e191c3..f907ca9 100644
>>>>>>> --- a/tools/libxl/libxl_dm.c
>>>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>>>> @@ -674,9 +674,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>>>                                                   LIBXL_NIC_TYPE_VIF_IOEMU);
>>>>>>>                   flexarray_append(dm_args, "-device");
>>>>>>>                   flexarray_append(dm_args,
>>>>>>> -                   libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
>>>>>>> +                   libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s%s",
>>>>>>>                                                   nics[i].model, nics[i].devid,
>>>>>>> -                                                nics[i].devid, smac));
>>>>>>> +                                                nics[i].devid, smac,
>>>>>>> +                                                i ? ",romfile=\"\"" : ""));
>>>>>>>                   flexarray_append(dm_args, "-netdev");
>>>>>>>                   flexarray_append(dm_args, GCSPRINTF(
>>>>>>>                                             "type=tap,id=net%d,ifname=%s,"
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Xen-devel mailing list
>>>>>>> Xen-devel@lists.xen.org
>>>>>>> http://lists.xen.org/xen-devel
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel