[Qemu-devel] [for-2.10 PATCH 0/3] spapr: fix PCI hotplug issue when PHBs don't have index

[Qemu-devel] [for-2.10 PATCH 0/3] spapr: fix PCI hotplug issue when PHBs don't have index

[Greg Kurz]

While working on PHB hotplug for 2.11, a bug was discovered in the PCI DR
logic in the PHB code: it relies on the PHB index property to be set but
it doesn't enforce it. It is hence possible to create two PHBs with the
same index (ie, the default value -1), even though this isn't expected
by the rest of the PHB code. The most visible consequence, is that PCI
hotplug doesn't work anymore.

It was agreed that the right fix would be to make the index property
mandatory. This is too an intrusive change to do during soft/hard
freeze though. It is postponed for 2.11.

In the meantime, we can at least have QEMU to detect the error early
and to exit with an error message, instead of silently going on with
half-broken PHBs.

David,

These patches were made with the future work on PHB hotplug in mind. If
the series is too long, a similar result can be achieved with this single
change in spapr_dr_connector_new():

-    object_property_set_bool(OBJECT(drc), true, "realized", NULL);
+    object_property_set_bool(OBJECT(drc), true, "realized", &error_fatal);
 
Just tell me, if you prefer the shorter version.

--
Greg

---

Greg Kurz (3):
      spapr_drc: abort if object_property_add_child() fails
      spapr_drc: add Error ** argument to spapr_dr_connector_new()
      spapr: error out if PHB fails to setup PCI DRCs


 hw/ppc/spapr.c             |    4 ++--
 hw/ppc/spapr_drc.c         |    6 +++---
 hw/ppc/spapr_pci.c         |    8 +++++++-
 include/hw/ppc/spapr_drc.h |    2 +-
 4 files changed, 13 insertions(+), 7 deletions(-)

[Qemu-devel] [for-2.10 PATCH 1/3] spapr_drc: abort if object_property_add_child() fails

[Greg Kurz]

object_property_add_child() can only fail in two cases:
- the child already has a parent, which shouldn't happen since the DRC was
  allocated a few lines above
- the parent already has a child with the same name, which would mean the
  caller tries to create a DRC that already exists

In both case, this is a QEMU bug and we should abort.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_drc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 47d94e782ac2..5260b5d363a0 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -541,7 +541,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
     drc->owner = owner;
     prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
                                 spapr_drc_index(drc));
-    object_property_add_child(owner, prop_name, OBJECT(drc), NULL);
+    object_property_add_child(owner, prop_name, OBJECT(drc), &error_abort);
     object_property_set_bool(OBJECT(drc), true, "realized", NULL);
     g_free(prop_name);
 

Re: [Qemu-devel] [for-2.10 PATCH 1/3] spapr_drc: abort if object_property_add_child() fails

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On Mon, Aug 07, 2017 at 07:24:39PM +0200, Greg Kurz wrote:
> object_property_add_child() can only fail in two cases:
> - the child already has a parent, which shouldn't happen since the DRC was
>   allocated a few lines above
> - the parent already has a child with the same name, which would mean the
>   caller tries to create a DRC that already exists
> 
> In both case, this is a QEMU bug and we should abort.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

This is definitely a fix, so applied for 2.10.

> ---
>  hw/ppc/spapr_drc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 47d94e782ac2..5260b5d363a0 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -541,7 +541,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
>      drc->owner = owner;
>      prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
>                                  spapr_drc_index(drc));
> -    object_property_add_child(owner, prop_name, OBJECT(drc), NULL);
> +    object_property_add_child(owner, prop_name, OBJECT(drc), &error_abort);
>      object_property_set_bool(OBJECT(drc), true, "realized", NULL);
>      g_free(prop_name);
>  
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[Qemu-devel] [for-2.10 PATCH 2/3] spapr_drc: add Error ** argument to spapr_dr_connector_new()

[Greg Kurz]

This just allow spapr_dr_connector_new() to propagate errors to its
callers. It doesn't change any functionality.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c             |    4 ++--
 hw/ppc/spapr_drc.c         |    4 ++--
 hw/ppc/spapr_pci.c         |    2 +-
 include/hw/ppc/spapr_drc.h |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f7a19720dcdf..2bc3dbc25653 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2023,7 +2023,7 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
 
         addr = i * lmb_size + spapr->hotplug_memory.base;
         spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
-                               addr / lmb_size);
+                               addr / lmb_size, NULL);
     }
 }
 
@@ -2118,7 +2118,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
 
         if (mc->has_hotpluggable_cpus) {
             spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
-                                   (core_id / smp_threads) * smt);
+                                   (core_id / smp_threads) * smt, NULL);
         }
 
         if (i < boot_cores_nr) {
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 5260b5d363a0..061fff41654a 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -532,7 +532,7 @@ static void unrealize(DeviceState *d, Error **errp)
 }
 
 sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
-                                         uint32_t id)
+                                         uint32_t id, Error **errp)
 {
     sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type));
     char *prop_name;
@@ -542,7 +542,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
     prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
                                 spapr_drc_index(drc));
     object_property_add_child(owner, prop_name, OBJECT(drc), &error_abort);
-    object_property_set_bool(OBJECT(drc), true, "realized", NULL);
+    object_property_set_bool(OBJECT(drc), true, "realized", errp);
     g_free(prop_name);
 
     return drc;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index d84abf1070a0..4b7882e3613d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1733,7 +1733,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     if (sphb->dr_enabled) {
         for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
             spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
-                                   (sphb->index << 16) | i);
+                                   (sphb->index << 16) | i, NULL);
         }
     }
 
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index a7958d0a8d14..8651af2ffae0 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -248,7 +248,7 @@ uint32_t spapr_drc_index(sPAPRDRConnector *drc);
 sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
 
 sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
-                                         uint32_t id);
+                                         uint32_t id, Error **errp);
 sPAPRDRConnector *spapr_drc_by_index(uint32_t index);
 sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id);
 int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,

[Qemu-devel] [for-2.10 PATCH 3/3] spapr: error out if PHB fails to setup PCI DRCs

[Greg Kurz]

It is currently possible to start QEMU with two PHBs without using the
index property:

-device spapr-pci-host-bridge,id=pci1,\
                              buid=0x800000020000001,\
                              liobn=0x80000100,\
                              liobn64=0x80000101,\
                              mem_win_addr=0x200100000000,\
                              mem64_win_addr=0x220000000000,\
                              io_win_addr=0x200000010000 \
-device spapr-pci-host-bridge,id=pci2,\
                              buid=0x800000020000002,\
                              liobn=0x80000200,\
                              liobn64=0x80000201,\
                              mem_win_addr=0x200180000000,\
                              mem64_win_addr=0x230000000000,\
                              io_win_addr=0x200000020000 \

Each PHB has its index property equal to -1. As a consequence, each PHB
will want to create PCI DRCs with the same ids:

    /* allocate connectors for child PCI devices */
    if (sphb->dr_enabled) {
        for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
                                   (sphb->index << 16) | i);
        }
    }

When DRC objects are added to the composition tree, an alias using the
DRC id is created in the "/dr-connector" path. But DRC ids are supposed
to be globally unique and the alias creation fails, leaving the DRC
object unrealized, which isn't expected by the rest of the DR logic.

This has the effect of silently turning-off PCI hotplug support (ie, PCI
hotplug no longer works on any PHB and no error message is printed).

This bug always existed and would even cause a non-fatal error to be
reported on the console (until recent commit bf26ae32a92a). This patch
causes the error message to be printed again and QEMU to exit.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 4b7882e3613d..4a1e6c5f697c 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1731,9 +1731,16 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
 
     /* allocate connectors for child PCI devices */
     if (sphb->dr_enabled) {
+        Error *local_err = NULL;
+
         for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
             spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
-                                   (sphb->index << 16) | i, NULL);
+                                   (sphb->index << 16) | i, &local_err);
+            if (local_err) {
+                error_propagate(errp, local_err);
+                error_prepend(errp, "Failed to create DRC: ");
+                return;
+            }
         }
     }
 

Re: [Qemu-devel] [for-2.10 PATCH 3/3] spapr: error out if PHB fails to setup PCI DRCs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3613 bytes --]

On Mon, Aug 07, 2017 at 07:25:03PM +0200, Greg Kurz wrote:
> It is currently possible to start QEMU with two PHBs without using the
> index property:
> 
> -device spapr-pci-host-bridge,id=pci1,\
>                               buid=0x800000020000001,\
>                               liobn=0x80000100,\
>                               liobn64=0x80000101,\
>                               mem_win_addr=0x200100000000,\
>                               mem64_win_addr=0x220000000000,\
>                               io_win_addr=0x200000010000 \
> -device spapr-pci-host-bridge,id=pci2,\
>                               buid=0x800000020000002,\
>                               liobn=0x80000200,\
>                               liobn64=0x80000201,\
>                               mem_win_addr=0x200180000000,\
>                               mem64_win_addr=0x230000000000,\
>                               io_win_addr=0x200000020000 \
> 
> Each PHB has its index property equal to -1. As a consequence, each PHB
> will want to create PCI DRCs with the same ids:
> 
>     /* allocate connectors for child PCI devices */
>     if (sphb->dr_enabled) {
>         for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
>             spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
>                                    (sphb->index << 16) | i);
>         }
>     }
> 
> When DRC objects are added to the composition tree, an alias using the
> DRC id is created in the "/dr-connector" path. But DRC ids are supposed
> to be globally unique and the alias creation fails, leaving the DRC
> object unrealized, which isn't expected by the rest of the DR logic.
> 
> This has the effect of silently turning-off PCI hotplug support (ie, PCI
> hotplug no longer works on any PHB and no error message is printed).
> 
> This bug always existed and would even cause a non-fatal error to be
> reported on the console (until recent commit bf26ae32a92a). This patch
> causes the error message to be printed again and QEMU to exit.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Given that the bug isn't a regression, I'm a bit disinclined to merge
patches 2&3 this late in 2.10.

It just seems bogus to have all this code to (supposedly) allow
bridges without an index, but then have it error out if there's more
than one of them.

I think skip straight to the real fix of making index madatory in 2.11.

> ---
>  hw/ppc/spapr_pci.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 4b7882e3613d..4a1e6c5f697c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1731,9 +1731,16 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>  
>      /* allocate connectors for child PCI devices */
>      if (sphb->dr_enabled) {
> +        Error *local_err = NULL;
> +
>          for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
>              spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> -                                   (sphb->index << 16) | i, NULL);
> +                                   (sphb->index << 16) | i, &local_err);
> +            if (local_err) {
> +                error_propagate(errp, local_err);
> +                error_prepend(errp, "Failed to create DRC: ");
> +                return;
> +            }
>          }
>      }
>  
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [for-2.10 PATCH 3/3] spapr: error out if PHB fails to setup PCI DRCs

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 3816 bytes --]

On Tue, 8 Aug 2017 16:16:36 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Aug 07, 2017 at 07:25:03PM +0200, Greg Kurz wrote:
> > It is currently possible to start QEMU with two PHBs without using the
> > index property:
> > 
> > -device spapr-pci-host-bridge,id=pci1,\
> >                               buid=0x800000020000001,\
> >                               liobn=0x80000100,\
> >                               liobn64=0x80000101,\
> >                               mem_win_addr=0x200100000000,\
> >                               mem64_win_addr=0x220000000000,\
> >                               io_win_addr=0x200000010000 \
> > -device spapr-pci-host-bridge,id=pci2,\
> >                               buid=0x800000020000002,\
> >                               liobn=0x80000200,\
> >                               liobn64=0x80000201,\
> >                               mem_win_addr=0x200180000000,\
> >                               mem64_win_addr=0x230000000000,\
> >                               io_win_addr=0x200000020000 \
> > 
> > Each PHB has its index property equal to -1. As a consequence, each PHB
> > will want to create PCI DRCs with the same ids:
> > 
> >     /* allocate connectors for child PCI devices */
> >     if (sphb->dr_enabled) {
> >         for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> >             spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> >                                    (sphb->index << 16) | i);
> >         }
> >     }
> > 
> > When DRC objects are added to the composition tree, an alias using the
> > DRC id is created in the "/dr-connector" path. But DRC ids are supposed
> > to be globally unique and the alias creation fails, leaving the DRC
> > object unrealized, which isn't expected by the rest of the DR logic.
> > 
> > This has the effect of silently turning-off PCI hotplug support (ie, PCI
> > hotplug no longer works on any PHB and no error message is printed).
> > 
> > This bug always existed and would even cause a non-fatal error to be
> > reported on the console (until recent commit bf26ae32a92a). This patch
> > causes the error message to be printed again and QEMU to exit.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>  
> 
> Given that the bug isn't a regression, I'm a bit disinclined to merge

FWIW, 2.9 would at least print an error message, but 2.10 doesn't because
of commit bf26ae32a92a.

> patches 2&3 this late in 2.10.
> 
> It just seems bogus to have all this code to (supposedly) allow
> bridges without an index, but then have it error out if there's more
> than one of them.
> 

I agree this is weird.

> I think skip straight to the real fix of making index madatory in 2.11.
> 

Ok.

> > ---
> >  hw/ppc/spapr_pci.c |    9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 4b7882e3613d..4a1e6c5f697c 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1731,9 +1731,16 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> >  
> >      /* allocate connectors for child PCI devices */
> >      if (sphb->dr_enabled) {
> > +        Error *local_err = NULL;
> > +
> >          for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> >              spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> > -                                   (sphb->index << 16) | i, NULL);
> > +                                   (sphb->index << 16) | i, &local_err);
> > +            if (local_err) {
> > +                error_propagate(errp, local_err);
> > +                error_prepend(errp, "Failed to create DRC: ");
> > +                return;
> > +            }
> >          }
> >      }
> >  
> >   
> 


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

Re: [Qemu-devel] [for-2.10 PATCH 3/3] spapr: error out if PHB fails to setup PCI DRCs

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

On Tue, Aug 08, 2017 at 11:18:35AM +0200, Greg Kurz wrote:
> On Tue, 8 Aug 2017 16:16:36 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Aug 07, 2017 at 07:25:03PM +0200, Greg Kurz wrote:
> > > It is currently possible to start QEMU with two PHBs without using the
> > > index property:
> > > 
> > > -device spapr-pci-host-bridge,id=pci1,\
> > >                               buid=0x800000020000001,\
> > >                               liobn=0x80000100,\
> > >                               liobn64=0x80000101,\
> > >                               mem_win_addr=0x200100000000,\
> > >                               mem64_win_addr=0x220000000000,\
> > >                               io_win_addr=0x200000010000 \
> > > -device spapr-pci-host-bridge,id=pci2,\
> > >                               buid=0x800000020000002,\
> > >                               liobn=0x80000200,\
> > >                               liobn64=0x80000201,\
> > >                               mem_win_addr=0x200180000000,\
> > >                               mem64_win_addr=0x230000000000,\
> > >                               io_win_addr=0x200000020000 \
> > > 
> > > Each PHB has its index property equal to -1. As a consequence, each PHB
> > > will want to create PCI DRCs with the same ids:
> > > 
> > >     /* allocate connectors for child PCI devices */
> > >     if (sphb->dr_enabled) {
> > >         for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> > >             spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> > >                                    (sphb->index << 16) | i);
> > >         }
> > >     }
> > > 
> > > When DRC objects are added to the composition tree, an alias using the
> > > DRC id is created in the "/dr-connector" path. But DRC ids are supposed
> > > to be globally unique and the alias creation fails, leaving the DRC
> > > object unrealized, which isn't expected by the rest of the DR logic.
> > > 
> > > This has the effect of silently turning-off PCI hotplug support (ie, PCI
> > > hotplug no longer works on any PHB and no error message is printed).
> > > 
> > > This bug always existed and would even cause a non-fatal error to be
> > > reported on the console (until recent commit bf26ae32a92a). This patch
> > > causes the error message to be printed again and QEMU to exit.
> > > 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>  
> > 
> > Given that the bug isn't a regression, I'm a bit disinclined to merge
> 
> FWIW, 2.9 would at least print an error message, but 2.10 doesn't because
> of commit bf26ae32a92a.

Ah, yeah that is a bit nasty.  Still, I'm not comfortable with the
complexity of patch required to fix it this late in 2.10.

> 
> > patches 2&3 this late in 2.10.
> > 
> > It just seems bogus to have all this code to (supposedly) allow
> > bridges without an index, but then have it error out if there's more
> > than one of them.
> > 
> 
> I agree this is weird.
> 
> > I think skip straight to the real fix of making index madatory in 2.11.
> > 
> 
> Ok.
> 
> > > ---
> > >  hw/ppc/spapr_pci.c |    9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > index 4b7882e3613d..4a1e6c5f697c 100644
> > > --- a/hw/ppc/spapr_pci.c
> > > +++ b/hw/ppc/spapr_pci.c
> > > @@ -1731,9 +1731,16 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > >  
> > >      /* allocate connectors for child PCI devices */
> > >      if (sphb->dr_enabled) {
> > > +        Error *local_err = NULL;
> > > +
> > >          for (i = 0; i < PCI_SLOT_MAX * 8; i++) {
> > >              spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> > > -                                   (sphb->index << 16) | i, NULL);
> > > +                                   (sphb->index << 16) | i, &local_err);
> > > +            if (local_err) {
> > > +                error_propagate(errp, local_err);
> > > +                error_prepend(errp, "Failed to create DRC: ");
> > > +                return;
> > > +            }
> > >          }
> > >      }
> > >  
> > >   
> > 
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]