Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Ian Campbell]

On Mon, 2014-11-17 at 13:00 +0000, Stefano Stabellini wrote:
> On Mon, 17 Nov 2014, Ian Campbell wrote:
> > On Sat, 2014-11-15 at 10:16 +0800, hanyandong wrote:
> > > By the way, how many NICs can I apply to a VM?
> > > 
> > > On xen-4.4.0, Using qemu-dm, I can apply 8 NIC to a VM, but using
> > > qemu-system-i386, I can only apply 4 NICs to an VM?
> > > is it normal?
> > 
> > I've no idea, CCing the qemu maintainers.
> > 
> > I'd have expected the number of PV nics to be completely independent of
> > the device mode, so I suppose you mean emulated NICs?
> 
> I can pass 4 emulated NICs maximum, but you can easily reach 8 if you
> use PV NICs instead. Just pass 'type=pv' like this:
> 
> vif=['', '', '', '', 'type=pv', 'type=pv', 'type=pv', 'type=pv']
> 
> it is going to create 4 emulated nics and 4 pv nics. The 4 emulated nics
> also have 4 corresponding pv nics. The emulated nics get disconnected
> soon after boot by the guest operating system (if it has pv drivers
> installed, such as Linux). So overall once the boot sequence is fully
> completed you'll end up with the 8 pv nics that you want.

I wonder if we should do something in (lib)xl such that by default the
first 4 NICs have type LIBXL_NIC_TYPE_VIF_IOEMU (i.e. emulated+PV path)
and the rest have LIBXL_NIC_TYPE_VIF (i.e. PV only).

> BTW the reason for the failure seems to be that QEMU runs out of ram
> (xen: failed to populate ram at 80110000, so
> xc_domain_populate_physmap_exact failed) allocating roms for the rtl8139
> (40000 bytes each). Maybe qemu-trad wasn't loading any roms for rtl8139.
> Interestingly e1000 doesn't need any roms either, so another way around
> this would be to set 'type=e1000' for all the vifs.

Or to use the new option in 4.5 to increase the MMIO space (or is that
not where ROMs end up?)

Do we need to plumb through qemu's optionrom parameter to allow a)
limiting the number of NICs which will try to do PXE and b) allow custom
roms etc?

Ian.

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Stefano Stabellini]

On Thu, 20 Nov 2014, Ian Campbell wrote:
> On Mon, 2014-11-17 at 13:00 +0000, Stefano Stabellini wrote:
> > On Mon, 17 Nov 2014, Ian Campbell wrote:
> > > On Sat, 2014-11-15 at 10:16 +0800, hanyandong wrote:
> > > > By the way, how many NICs can I apply to a VM?
> > > > 
> > > > On xen-4.4.0, Using qemu-dm, I can apply 8 NIC to a VM, but using
> > > > qemu-system-i386, I can only apply 4 NICs to an VM?
> > > > is it normal?
> > > 
> > > I've no idea, CCing the qemu maintainers.
> > > 
> > > I'd have expected the number of PV nics to be completely independent of
> > > the device mode, so I suppose you mean emulated NICs?
> > 
> > I can pass 4 emulated NICs maximum, but you can easily reach 8 if you
> > use PV NICs instead. Just pass 'type=pv' like this:
> > 
> > vif=['', '', '', '', 'type=pv', 'type=pv', 'type=pv', 'type=pv']
> > 
> > it is going to create 4 emulated nics and 4 pv nics. The 4 emulated nics
> > also have 4 corresponding pv nics. The emulated nics get disconnected
> > soon after boot by the guest operating system (if it has pv drivers
> > installed, such as Linux). So overall once the boot sequence is fully
> > completed you'll end up with the 8 pv nics that you want.
> 
> I wonder if we should do something in (lib)xl such that by default the
> first 4 NICs have type LIBXL_NIC_TYPE_VIF_IOEMU (i.e. emulated+PV path)
> and the rest have LIBXL_NIC_TYPE_VIF (i.e. PV only).

That looks like a simple and reasonable idea.


> > BTW the reason for the failure seems to be that QEMU runs out of ram
> > (xen: failed to populate ram at 80110000, so
> > xc_domain_populate_physmap_exact failed) allocating roms for the rtl8139
> > (40000 bytes each). Maybe qemu-trad wasn't loading any roms for rtl8139.
> > Interestingly e1000 doesn't need any roms either, so another way around
> > this would be to set 'type=e1000' for all the vifs.
> 
> Or to use the new option in 4.5 to increase the MMIO space (or is that
> not where ROMs end up?)
> 
> Do we need to plumb through qemu's optionrom parameter to allow a)
> limiting the number of NICs which will try to do PXE and b) allow custom
> roms etc?

The libxl solution is the best one for simplicity, besides I don't think
there is such an option for QEMU.

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Ian Campbell]

On Thu, 2014-11-20 at 11:39 +0000, Stefano Stabellini wrote:
> On Thu, 20 Nov 2014, Ian Campbell wrote:
> > On Mon, 2014-11-17 at 13:00 +0000, Stefano Stabellini wrote:
> > > On Mon, 17 Nov 2014, Ian Campbell wrote:
> > > > On Sat, 2014-11-15 at 10:16 +0800, hanyandong wrote:
> > > > > By the way, how many NICs can I apply to a VM?
> > > > > 
> > > > > On xen-4.4.0, Using qemu-dm, I can apply 8 NIC to a VM, but using
> > > > > qemu-system-i386, I can only apply 4 NICs to an VM?
> > > > > is it normal?
> > > > 
> > > > I've no idea, CCing the qemu maintainers.
> > > > 
> > > > I'd have expected the number of PV nics to be completely independent of
> > > > the device mode, so I suppose you mean emulated NICs?
> > > 
> > > I can pass 4 emulated NICs maximum, but you can easily reach 8 if you
> > > use PV NICs instead. Just pass 'type=pv' like this:
> > > 
> > > vif=['', '', '', '', 'type=pv', 'type=pv', 'type=pv', 'type=pv']
> > > 
> > > it is going to create 4 emulated nics and 4 pv nics. The 4 emulated nics
> > > also have 4 corresponding pv nics. The emulated nics get disconnected
> > > soon after boot by the guest operating system (if it has pv drivers
> > > installed, such as Linux). So overall once the boot sequence is fully
> > > completed you'll end up with the 8 pv nics that you want.
> > 
> > I wonder if we should do something in (lib)xl such that by default the
> > first 4 NICs have type LIBXL_NIC_TYPE_VIF_IOEMU (i.e. emulated+PV path)
> > and the rest have LIBXL_NIC_TYPE_VIF (i.e. PV only).
> 
> That looks like a simple and reasonable idea.
> 
> 
> > > BTW the reason for the failure seems to be that QEMU runs out of ram
> > > (xen: failed to populate ram at 80110000, so
> > > xc_domain_populate_physmap_exact failed) allocating roms for the rtl8139
> > > (40000 bytes each). Maybe qemu-trad wasn't loading any roms for rtl8139.
> > > Interestingly e1000 doesn't need any roms either, so another way around
> > > this would be to set 'type=e1000' for all the vifs.
> > 
> > Or to use the new option in 4.5 to increase the MMIO space (or is that
> > not where ROMs end up?)
> > 
> > Do we need to plumb through qemu's optionrom parameter to allow a)
> > limiting the number of NICs which will try to do PXE and b) allow custom
> > roms etc?
> 
> The libxl solution is the best one for simplicity, besides I don't think
> there is such an option for QEMU.

There is, it's the romfile option to -device e.g.
         -device $NICMODEL,vlan=0,romfile=$ROMFILE
        
where NICMODEL is e100, rtl8139, virtio-blah
and ROMFILE is e.g. an ipxe binary.

Ian.

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Stefano Stabellini]

On Thu, 20 Nov 2014, Ian Campbell wrote:
> On Thu, 2014-11-20 at 11:39 +0000, Stefano Stabellini wrote:
> > On Thu, 20 Nov 2014, Ian Campbell wrote:
> > > On Mon, 2014-11-17 at 13:00 +0000, Stefano Stabellini wrote:
> > > > On Mon, 17 Nov 2014, Ian Campbell wrote:
> > > > > On Sat, 2014-11-15 at 10:16 +0800, hanyandong wrote:
> > > > > > By the way, how many NICs can I apply to a VM?
> > > > > > 
> > > > > > On xen-4.4.0, Using qemu-dm, I can apply 8 NIC to a VM, but using
> > > > > > qemu-system-i386, I can only apply 4 NICs to an VM?
> > > > > > is it normal?
> > > > > 
> > > > > I've no idea, CCing the qemu maintainers.
> > > > > 
> > > > > I'd have expected the number of PV nics to be completely independent of
> > > > > the device mode, so I suppose you mean emulated NICs?
> > > > 
> > > > I can pass 4 emulated NICs maximum, but you can easily reach 8 if you
> > > > use PV NICs instead. Just pass 'type=pv' like this:
> > > > 
> > > > vif=['', '', '', '', 'type=pv', 'type=pv', 'type=pv', 'type=pv']
> > > > 
> > > > it is going to create 4 emulated nics and 4 pv nics. The 4 emulated nics
> > > > also have 4 corresponding pv nics. The emulated nics get disconnected
> > > > soon after boot by the guest operating system (if it has pv drivers
> > > > installed, such as Linux). So overall once the boot sequence is fully
> > > > completed you'll end up with the 8 pv nics that you want.
> > > 
> > > I wonder if we should do something in (lib)xl such that by default the
> > > first 4 NICs have type LIBXL_NIC_TYPE_VIF_IOEMU (i.e. emulated+PV path)
> > > and the rest have LIBXL_NIC_TYPE_VIF (i.e. PV only).
> > 
> > That looks like a simple and reasonable idea.
> > 
> > 
> > > > BTW the reason for the failure seems to be that QEMU runs out of ram
> > > > (xen: failed to populate ram at 80110000, so
> > > > xc_domain_populate_physmap_exact failed) allocating roms for the rtl8139
> > > > (40000 bytes each). Maybe qemu-trad wasn't loading any roms for rtl8139.
> > > > Interestingly e1000 doesn't need any roms either, so another way around
> > > > this would be to set 'type=e1000' for all the vifs.
> > > 
> > > Or to use the new option in 4.5 to increase the MMIO space (or is that
> > > not where ROMs end up?)
> > > 
> > > Do we need to plumb through qemu's optionrom parameter to allow a)
> > > limiting the number of NICs which will try to do PXE and b) allow custom
> > > roms etc?
> > 
> > The libxl solution is the best one for simplicity, besides I don't think
> > there is such an option for QEMU.
> 
> There is, it's the romfile option to -device e.g.
>          -device $NICMODEL,vlan=0,romfile=$ROMFILE
>         
> where NICMODEL is e100, rtl8139, virtio-blah
> and ROMFILE is e.g. an ipxe binary.

I think that option was intended to point QEMU to a different file, not
to disable the romfile.

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Ian Campbell]

On Thu, 2014-11-20 at 14:46 +0000, Stefano Stabellini wrote:
> On Thu, 20 Nov 2014, Ian Campbell wrote:
> > There is, it's the romfile option to -device e.g.
> >          -device $NICMODEL,vlan=0,romfile=$ROMFILE
> >         
> > where NICMODEL is e100, rtl8139, virtio-blah
> > and ROMFILE is e.g. an ipxe binary.
> 
> I think that option was intended to point QEMU to a different file, not
> to disable the romfile.

romfile="" does that, I think. Or use /dev/null etc etc.

Ian.

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Ian Campbell]

create ^
title it qemu-upstream: limitation on 4 emulated NICs prevents guest from starting unless PV override is used.
thanks

Processed: Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[xen]

Processing commands for xen@bugs.xenproject.org:

> create ^
Created new bug #46 rooted at `<1416474814.29243.59.camel@citrix.com>'
Title: `Re: [Xen-devel] Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)'
> title it qemu-upstream: limitation on 4 emulated NICs prevents guest from starting unless PV override is used.
Set title for #46 to `qemu-upstream: limitation on 4 emulated NICs prevents guest from starting unless PV override is used.'
> thanks
Finished processing.

Modified/created Bugs:
 - 46: http://bugs.xenproject.org/xen/bug/46 (new)

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

Re: Number of NICs per VM with qemu-upstream (Was: Re: Re: [Xen-users] libvirt <emulator> /usr/local/lib/xen/bin/qemu-dm <emulator> did not work on xen-4.4)

[Stefano Stabellini]

On Thu, 20 Nov 2014, Ian Campbell wrote:
> On Thu, 2014-11-20 at 14:46 +0000, Stefano Stabellini wrote:
> > On Thu, 20 Nov 2014, Ian Campbell wrote:
> > > There is, it's the romfile option to -device e.g.
> > >          -device $NICMODEL,vlan=0,romfile=$ROMFILE
> > >         
> > > where NICMODEL is e100, rtl8139, virtio-blah
> > > and ROMFILE is e.g. an ipxe binary.
> > 
> > I think that option was intended to point QEMU to a different file, not
> > to disable the romfile.
> 
> romfile="" does that, I think. Or use /dev/null etc etc.

Confirmed working.

---

libxl: do not load roms for any NICs except the first to avoid wasting memory

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,"

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

[Stefano Stabellini]

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

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,"

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

[Konrad Rzeszutek Wilk]

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?

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

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

[Stefano Stabellini]

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

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
> 

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

[Konrad Rzeszutek Wilk]

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? What if you
also do PCI passthrough? Does that figure in this calculation?
> 
> 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
> > 

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

[Stefano Stabellini]

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.


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

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

[Konrad Rzeszutek Wilk]

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?

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

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

[Stefano Stabellini]

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.

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


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

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

[Konrad Rzeszutek Wilk]

On Mon, Nov 24, 2014 at 03:26:04PM +0000, 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.

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

Or maybe the VGA hole size calculation is used for this. Anyhow, why aren't
we using the guest memory as instead of increasing the memory. Or is
this just an accounting issue in QEMU?
> 
> 
> > > > 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
> > > > > > 
> > > > 
> > 

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

[Don Slutz]

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

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

[Stefano Stabellini]

On Mon, 24 Nov 2014, Don Slutz wrote:
> 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.

Here is a better patch.

---

libxl: account for romfile memory

Account for memory needed for emulated network card rom files.
Assume 256K for each romfile.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index de23fec..2edb194 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4527,6 +4527,33 @@ out:
 
 /******************************************************************************/
 
+int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config)
+{
+    int i, count_rom, rc;
+    libxl_domain_config local_d_config;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+
+    if (d_config == NULL) {
+        libxl_domain_config_init(&local_d_config);
+        rc = libxl_retrieve_domain_configuration(ctx, domid, &local_d_config);
+        if (rc < 0)
+            return rc;
+        d_config = &local_d_config;
+    }
+
+    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV)
+        return 0;
+
+    for (i = 0, count_rom = 0; i < d_config->num_nics; i++) {
+        if (d_config->nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU)
+            count_rom++;
+    }
+
+    return count_rom*LIBXL_ROMSIZE_KB;
+
+    return 0;
+}
+
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
 {
     GC_INIT(ctx);
@@ -4550,11 +4577,14 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be greater than or or equal to memory_dynamic_max\n");
         goto out;
     }
-    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + LIBXL_MAXMEM_CONSTANT);
+    rc = xc_domain_setmaxmem(ctx->xch, domid,
+                             max_memkb + LIBXL_MAXMEM_CONSTANT
+                             + libxl__get_rom_memory_kb(gc, domid, NULL));
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                 "xc_domain_setmaxmem domid=%d memkb=%d failed "
-                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
+                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT +
+                libxl__get_rom_memory_kb(gc, domid, NULL), rc);
         goto out;
     }
 
@@ -4770,11 +4800,12 @@ retry_transaction:
     if (enforce) {
         memorykb = new_target_memkb;
         rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
-                LIBXL_MAXMEM_CONSTANT);
+                LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, NULL));
         if (rc != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                     "xc_domain_setmaxmem domid=%d memkb=%d failed "
-                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
+                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT +
+                    libxl__get_rom_memory_kb(gc, domid, NULL), rc);
             abort_transaction = 1;
             goto out;
         }
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 74ea84b..ca254f9 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -406,7 +406,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     }
 
     if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
-        LIBXL_MAXMEM_CONSTANT) < 0) {
+        LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, d_config)) < 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
         return ERROR_FAIL;
     }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4361421..33826ea 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -90,6 +90,7 @@
 #define LIBXL_XENCONSOLE_LIMIT 1048576
 #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
 #define LIBXL_MAXMEM_CONSTANT 1024
+#define LIBXL_ROMSIZE_KB 256
 #define LIBXL_PV_EXTRA_MEMORY 1024
 #define LIBXL_HVM_EXTRA_MEMORY 2048
 #define LIBXL_MIN_DOM0_MEM (128*1024)
@@ -1023,6 +1024,12 @@ _hidden char * libxl__domain_pvcontrol_read(libxl__gc *gc,
 _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
                                           uint32_t domid, const char *cmd);
 
+/* Returns the amount of extra mem required to allocate roms or an libxl
+ * error code on error.
+ * The *d_config parameter is optional.
+ */
+_hidden int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config);
+
 /* from xl_device */
 _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);

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

[Don Slutz]

On 11/24/14 12:07, Stefano Stabellini wrote:
> On Mon, 24 Nov 2014, Don Slutz wrote:
>> 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.
> Here is a better patch.
>
> ---
>
> libxl: account for romfile memory
>
> Account for memory needed for emulated network card rom files.
> Assume 256K for each romfile.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

This looks to be a good idea, just not fully done.

> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index de23fec..2edb194 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4527,6 +4527,33 @@ out:
>   
>   /******************************************************************************/
>   
> +int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config)
> +{
> +    int i, count_rom, rc;
> +    libxl_domain_config local_d_config;
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +
> +    if (d_config == NULL) {
> +        libxl_domain_config_init(&local_d_config);
> +        rc = libxl_retrieve_domain_configuration(ctx, domid, &local_d_config);
> +        if (rc < 0)
> +            return rc;
> +        d_config = &local_d_config;
> +    }
> +
> +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV)
> +        return 0;
> +
> +    for (i = 0, count_rom = 0; i < d_config->num_nics; i++) {
> +        if (d_config->nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU)
> +            count_rom++;
> +    }
> +
> +    return count_rom*LIBXL_ROMSIZE_KB;
> +
> +    return 0;

2 return statements?

> +}
> +
>   int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
>   {
>       GC_INIT(ctx);
> @@ -4550,11 +4577,14 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be greater than or or equal to memory_dynamic_max\n");
>           goto out;
>       }
> -    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + LIBXL_MAXMEM_CONSTANT);
> +    rc = xc_domain_setmaxmem(ctx->xch, domid,
> +                             max_memkb + LIBXL_MAXMEM_CONSTANT
> +                             + libxl__get_rom_memory_kb(gc, domid, NULL));

Here (and the rest) need to check for error's.  Also I think that
LIBXL_MAXMEM_CONSTANT can be dropped here.  I found out that

#define LIBXL_HVM_EXTRA_MEMORY 2048

Should be

#define LIBXL_HVM_EXTRA_MEMORY (LIBXL_MAXMEM_CONSTANT + 1024)

if you change the size of the ROM memory for QEMU.  I have only done the 
static
change.

    -Don Slutz


>       if (rc != 0) {
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                   "xc_domain_setmaxmem domid=%d memkb=%d failed "
> -                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
> +                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT +
> +                libxl__get_rom_memory_kb(gc, domid, NULL), rc);
>           goto out;
>       }
>   
> @@ -4770,11 +4800,12 @@ retry_transaction:
>       if (enforce) {
>           memorykb = new_target_memkb;
>           rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
> -                LIBXL_MAXMEM_CONSTANT);
> +                LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, NULL));
>           if (rc != 0) {
>               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
>                       "xc_domain_setmaxmem domid=%d memkb=%d failed "
> -                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
> +                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT +
> +                    libxl__get_rom_memory_kb(gc, domid, NULL), rc);
>               abort_transaction = 1;
>               goto out;
>           }
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 74ea84b..ca254f9 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -406,7 +406,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>       }
>   
>       if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> -        LIBXL_MAXMEM_CONSTANT) < 0) {
> +        LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, d_config)) < 0) {
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
>           return ERROR_FAIL;
>       }
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 4361421..33826ea 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -90,6 +90,7 @@
>   #define LIBXL_XENCONSOLE_LIMIT 1048576
>   #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
>   #define LIBXL_MAXMEM_CONSTANT 1024
> +#define LIBXL_ROMSIZE_KB 256
>   #define LIBXL_PV_EXTRA_MEMORY 1024
>   #define LIBXL_HVM_EXTRA_MEMORY 2048
>   #define LIBXL_MIN_DOM0_MEM (128*1024)
> @@ -1023,6 +1024,12 @@ _hidden char * libxl__domain_pvcontrol_read(libxl__gc *gc,
>   _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
>                                             uint32_t domid, const char *cmd);
>   
> +/* Returns the amount of extra mem required to allocate roms or an libxl
> + * error code on error.
> + * The *d_config parameter is optional.
> + */
> +_hidden int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config);
> +
>   /* from xl_device */
>   _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
>   _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);

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

[Stefano Stabellini]

On Mon, 24 Nov 2014, Don Slutz wrote:
> On 11/24/14 12:07, Stefano Stabellini wrote:
> > On Mon, 24 Nov 2014, Don Slutz wrote:
> > > 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.
> > Here is a better patch.
> > 
> > ---
> > 
> > libxl: account for romfile memory
> > 
> > Account for memory needed for emulated network card rom files.
> > Assume 256K for each romfile.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> This looks to be a good idea, just not fully done.
> 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index de23fec..2edb194 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -4527,6 +4527,33 @@ out:
> >     /******************************************************************************/
> >   +int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid,
> > libxl_domain_config *d_config)
> > +{
> > +    int i, count_rom, rc;
> > +    libxl_domain_config local_d_config;
> > +    libxl_ctx *ctx = libxl__gc_owner(gc);
> > +
> > +    if (d_config == NULL) {
> > +        libxl_domain_config_init(&local_d_config);
> > +        rc = libxl_retrieve_domain_configuration(ctx, domid,
> > &local_d_config);
> > +        if (rc < 0)
> > +            return rc;
> > +        d_config = &local_d_config;
> > +    }
> > +
> > +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV)
> > +        return 0;
> > +
> > +    for (i = 0, count_rom = 0; i < d_config->num_nics; i++) {
> > +        if (d_config->nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU)
> > +            count_rom++;
> > +    }
> > +
> > +    return count_rom*LIBXL_ROMSIZE_KB;
> > +
> > +    return 0;
> 
> 2 return statements?
> 
> > +}
> > +
> >   int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t
> > max_memkb)
> >   {
> >       GC_INIT(ctx);
> > @@ -4550,11 +4577,14 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t
> > domid, uint32_t max_memkb)
> >           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be
> > greater than or or equal to memory_dynamic_max\n");
> >           goto out;
> >       }
> > -    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb +
> > LIBXL_MAXMEM_CONSTANT);
> > +    rc = xc_domain_setmaxmem(ctx->xch, domid,
> > +                             max_memkb + LIBXL_MAXMEM_CONSTANT
> > +                             + libxl__get_rom_memory_kb(gc, domid, NULL));
> 
> Here (and the rest) need to check for error's.

Good suggestion.


> Also I think that LIBXL_MAXMEM_CONSTANT can be dropped here.

Probably, but I don't know whether there are any other "hidden"
allocations that would begin to fail if I removed LIBXL_MAXMEM_CONSTANT.
I think it is best to leave it as is for the moment and maybe remove it
at the beginning of the next release cycle?


> I found out that
> 
> #define LIBXL_HVM_EXTRA_MEMORY 2048
> 
> Should be
> 
> #define LIBXL_HVM_EXTRA_MEMORY (LIBXL_MAXMEM_CONSTANT + 1024)

OK


> if you change the size of the ROM memory for QEMU.  I have only done the
> static
> change.

I don't understand what you mean here. Yes, if the ROM size changes in
QEMU we'll have an issue, specifically if it increases. However I don't
know how to solve that. I guess it would be another reason to keep
LIBXL_MAXMEM_CONSTANT: have an extra little buffer.


>    -Don Slutz
> 
> 
> >       if (rc != 0) {
> >           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >                   "xc_domain_setmaxmem domid=%d memkb=%d failed "
> > -                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
> > +                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT +
> > +                libxl__get_rom_memory_kb(gc, domid, NULL), rc);
> >           goto out;
> >       }
> >   @@ -4770,11 +4800,12 @@ retry_transaction:
> >       if (enforce) {
> >           memorykb = new_target_memkb;
> >           rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
> > -                LIBXL_MAXMEM_CONSTANT);
> > +                LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid,
> > NULL));
> >           if (rc != 0) {
> >               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> >                       "xc_domain_setmaxmem domid=%d memkb=%d failed "
> > -                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT,
> > rc);
> > +                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT +
> > +                    libxl__get_rom_memory_kb(gc, domid, NULL), rc);
> >               abort_transaction = 1;
> >               goto out;
> >           }
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index 74ea84b..ca254f9 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -406,7 +406,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> >       }
> >         if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> > -        LIBXL_MAXMEM_CONSTANT) < 0) {
> > +        LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid,
> > d_config)) < 0) {
> >           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max
> > memory");
> >           return ERROR_FAIL;
> >       }
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index 4361421..33826ea 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -90,6 +90,7 @@
> >   #define LIBXL_XENCONSOLE_LIMIT 1048576
> >   #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
> >   #define LIBXL_MAXMEM_CONSTANT 1024
> > +#define LIBXL_ROMSIZE_KB 256
> >   #define LIBXL_PV_EXTRA_MEMORY 1024
> >   #define LIBXL_HVM_EXTRA_MEMORY 2048
> >   #define LIBXL_MIN_DOM0_MEM (128*1024)
> > @@ -1023,6 +1024,12 @@ _hidden char * libxl__domain_pvcontrol_read(libxl__gc
> > *gc,
> >   _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t
> > t,
> >                                             uint32_t domid, const char
> > *cmd);
> >   +/* Returns the amount of extra mem required to allocate roms or an libxl
> > + * error code on error.
> > + * The *d_config parameter is optional.
> > + */
> > +_hidden int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid,
> > libxl_domain_config *d_config);
> > +
> >   /* from xl_device */
> >   _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend
> > backend);
> >   _hidden char *libxl__device_disk_string_of_format(libxl_disk_format
> > format);
>