Patchwork kvm: add detail error message when fail to add ioeventfd

login
register
mail settings
Submitter Amos Kong
Date May 22, 2013, 4:57 a.m.
Message ID <1369198655-25156-1-git-send-email-akong@redhat.com>
Download mbox | patch
Permalink /patch/245505/
State New
Headers show

Comments

Amos Kong - May 22, 2013, 4:57 a.m.
I try to hotplug 28 * 8 multiple-function devices to guest with
old host kernel, ioeventfds in host kernel will be exhausted, then
qemu fails to allocate ioeventfds for blk/nic devices.

It's better to add detail error here.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 kvm-all.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Stefan Hajnoczi - May 22, 2013, 9:32 a.m.
On Wed, May 22, 2013 at 12:57:35PM +0800, Amos Kong wrote:
> I try to hotplug 28 * 8 multiple-function devices to guest with
> old host kernel, ioeventfds in host kernel will be exhausted, then
> qemu fails to allocate ioeventfds for blk/nic devices.
> 
> It's better to add detail error here.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  kvm-all.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)

It would be nice to make kvm bus scalable so that the hardcoded
in-kernel I/O device limit can be lifted.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Amos Kong - May 22, 2013, 1:48 p.m.
On Wed, May 22, 2013 at 11:32:27AM +0200, Stefan Hajnoczi wrote:
> On Wed, May 22, 2013 at 12:57:35PM +0800, Amos Kong wrote:
> > I try to hotplug 28 * 8 multiple-function devices to guest with
> > old host kernel, ioeventfds in host kernel will be exhausted, then
> > qemu fails to allocate ioeventfds for blk/nic devices.
> > 
> > It's better to add detail error here.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  kvm-all.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> It would be nice to make kvm bus scalable so that the hardcoded
> in-kernel I/O device limit can be lifted.

I had increased kernel NR_IOBUS_DEVS to 1000 (a limitation is needed for
security) in last Mar, and make resizing kvm_io_range array dynamical.

> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi - May 23, 2013, 7:46 a.m.
On Wed, May 22, 2013 at 09:48:21PM +0800, Amos Kong wrote:
> On Wed, May 22, 2013 at 11:32:27AM +0200, Stefan Hajnoczi wrote:
> > On Wed, May 22, 2013 at 12:57:35PM +0800, Amos Kong wrote:
> > > I try to hotplug 28 * 8 multiple-function devices to guest with
> > > old host kernel, ioeventfds in host kernel will be exhausted, then
> > > qemu fails to allocate ioeventfds for blk/nic devices.
> > > 
> > > It's better to add detail error here.
> > > 
> > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > ---
> > >  kvm-all.c |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > It would be nice to make kvm bus scalable so that the hardcoded
> > in-kernel I/O device limit can be lifted.
> 
> I had increased kernel NR_IOBUS_DEVS to 1000 (a limitation is needed for
> security) in last Mar, and make resizing kvm_io_range array dynamical.

The maximum should not be hardcoded.  File descriptor, maximum memory,
etc are all controlled by rlimits.  And since ioeventfds are file
descriptors they are already limited by the maximum number of file
descriptors.

Why is there a need to impose a hardcoded limit?

Stefan
Amos Kong - May 24, 2013, 9:16 a.m.
On Thu, May 23, 2013 at 09:46:07AM +0200, Stefan Hajnoczi wrote:
> On Wed, May 22, 2013 at 09:48:21PM +0800, Amos Kong wrote:
> > On Wed, May 22, 2013 at 11:32:27AM +0200, Stefan Hajnoczi wrote:
> > > On Wed, May 22, 2013 at 12:57:35PM +0800, Amos Kong wrote:
> > > > I try to hotplug 28 * 8 multiple-function devices to guest with
> > > > old host kernel, ioeventfds in host kernel will be exhausted, then
> > > > qemu fails to allocate ioeventfds for blk/nic devices.
> > > > 
> > > > It's better to add detail error here.
> > > > 
> > > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > > ---
> > > >  kvm-all.c |    4 ++++
> > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > 
> > > It would be nice to make kvm bus scalable so that the hardcoded
> > > in-kernel I/O device limit can be lifted.
> > 
> > I had increased kernel NR_IOBUS_DEVS to 1000 (a limitation is needed for
> > security) in last Mar, and make resizing kvm_io_range array dynamical.
> 
> The maximum should not be hardcoded.  File descriptor, maximum memory,
> etc are all controlled by rlimits.  And since ioeventfds are file
> descriptors they are already limited by the maximum number of file
> descriptors.

For implement the dynamically resize the kvm_io_range array,
I re-allocate new array (with new size) and free old array
when the array flexes. The array is only resized when
add/remove ioeventfds. It will not effect the perf.
 
> Why is there a need to impose a hardcoded limit?

I will send a patch to fix it.
 
> Stefan
Gleb Natapov - June 3, 2013, 9:27 a.m.
On Wed, May 22, 2013 at 12:57:35PM +0800, Amos Kong wrote:
> I try to hotplug 28 * 8 multiple-function devices to guest with
> old host kernel, ioeventfds in host kernel will be exhausted, then
> qemu fails to allocate ioeventfds for blk/nic devices.
> 
> It's better to add detail error here.
> 
Applied, thanks.

> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  kvm-all.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8222729..3d5f7b7 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -813,6 +813,8 @@ static void kvm_mem_ioeventfd_add(MemoryListener *listener,
>      r = kvm_set_ioeventfd_mmio(fd, section->offset_within_address_space,
>                                 data, true, section->size, match_data);
>      if (r < 0) {
> +        fprintf(stderr, "%s: error adding ioeventfd: %s\n",
> +                __func__, strerror(-r));
>          abort();
>      }
>  }
> @@ -843,6 +845,8 @@ static void kvm_io_ioeventfd_add(MemoryListener *listener,
>      r = kvm_set_ioeventfd_pio(fd, section->offset_within_address_space,
>                                data, true, section->size, match_data);
>      if (r < 0) {
> +        fprintf(stderr, "%s: error adding ioeventfd: %s\n",
> +                __func__, strerror(-r));
>          abort();
>      }
>  }
> -- 
> 1.7.1

--
			Gleb.

Patch

diff --git a/kvm-all.c b/kvm-all.c
index 8222729..3d5f7b7 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -813,6 +813,8 @@  static void kvm_mem_ioeventfd_add(MemoryListener *listener,
     r = kvm_set_ioeventfd_mmio(fd, section->offset_within_address_space,
                                data, true, section->size, match_data);
     if (r < 0) {
+        fprintf(stderr, "%s: error adding ioeventfd: %s\n",
+                __func__, strerror(-r));
         abort();
     }
 }
@@ -843,6 +845,8 @@  static void kvm_io_ioeventfd_add(MemoryListener *listener,
     r = kvm_set_ioeventfd_pio(fd, section->offset_within_address_space,
                               data, true, section->size, match_data);
     if (r < 0) {
+        fprintf(stderr, "%s: error adding ioeventfd: %s\n",
+                __func__, strerror(-r));
         abort();
     }
 }