Message ID | 1360558742-5520-1-git-send-email-david@gibson.dropbear.id.au |
---|---|
State | New |
Headers | show |
On 11.02.2013, at 05:59, David Gibson wrote: > Currently the spapr-vlan device does not supply a cleanup call for its > NetClientInfo structure. With current qemu versions, that leads to a SEGV > on exit, when net_cleanup() attempts to call the cleanup handlers on all > net clients. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> The offending patch breaks more than just spapr. We need to revert the bit that checks for the existence of a cleanup function. However, I'll still apply this to ppc-next. It's a good idea to make the cleanup function mandatory. In fact, couldn't we add a runtime check that mandatory callback functions actually exist when registering a NetClientInfo? Alex > --- > hw/spapr_llan.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > As a SEGV fix, this really should go in for 1.4. What needs to be > done to make that happen. > > diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c > index 6ef2936..0ace2eb 100644 > --- a/hw/spapr_llan.c > +++ b/hw/spapr_llan.c > @@ -175,11 +175,19 @@ static ssize_t spapr_vlan_receive(NetClientState *nc, const uint8_t *buf, > return size; > } > > +static void spapr_vlan_cleanup(NetClientState *nc) > +{ > + VIOsPAPRVLANDevice *dev = qemu_get_nic_opaque(nc); > + > + dev->nic = NULL; > +} > + > static NetClientInfo net_spapr_vlan_info = { > .type = NET_CLIENT_OPTIONS_KIND_NIC, > .size = sizeof(NICState), > .can_receive = spapr_vlan_can_receive, > .receive = spapr_vlan_receive, > + .cleanup = spapr_vlan_cleanup, > }; > > static void spapr_vlan_reset(VIOsPAPRDevice *sdev) > -- > 1.7.10.4 >
Am 11.02.2013 05:59, schrieb David Gibson: > Currently the spapr-vlan device does not supply a cleanup call for its > NetClientInfo structure. With current qemu versions, that leads to a SEGV > on exit, when net_cleanup() attempts to call the cleanup handlers on all > net clients. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Tested-by: Andreas Färber <afaerber@suse.de> On Anthony's request I posted a patch that adds back the if surrounding the cleanup callback, fixing also the other affected nics. Regards, Andreas
Andreas Färber <afaerber@suse.de> writes: > Am 11.02.2013 05:59, schrieb David Gibson: >> Currently the spapr-vlan device does not supply a cleanup call for its >> NetClientInfo structure. With current qemu versions, that leads to a SEGV >> on exit, when net_cleanup() attempts to call the cleanup handlers on all >> net clients. >> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Tested-by: Andreas Färber <afaerber@suse.de> > > On Anthony's request I posted a patch that adds back the if surrounding > the cleanup callback, fixing also the other affected nics. Since this cleanup hook doesn't do anything useful and we need to apply Andreas' patch anyway, I don't think there's any benefit to apply this one. Regards, Anthony Liguori > > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c index 6ef2936..0ace2eb 100644 --- a/hw/spapr_llan.c +++ b/hw/spapr_llan.c @@ -175,11 +175,19 @@ static ssize_t spapr_vlan_receive(NetClientState *nc, const uint8_t *buf, return size; } +static void spapr_vlan_cleanup(NetClientState *nc) +{ + VIOsPAPRVLANDevice *dev = qemu_get_nic_opaque(nc); + + dev->nic = NULL; +} + static NetClientInfo net_spapr_vlan_info = { .type = NET_CLIENT_OPTIONS_KIND_NIC, .size = sizeof(NICState), .can_receive = spapr_vlan_can_receive, .receive = spapr_vlan_receive, + .cleanup = spapr_vlan_cleanup, }; static void spapr_vlan_reset(VIOsPAPRDevice *sdev)
Currently the spapr-vlan device does not supply a cleanup call for its NetClientInfo structure. With current qemu versions, that leads to a SEGV on exit, when net_cleanup() attempts to call the cleanup handlers on all net clients. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/spapr_llan.c | 8 ++++++++ 1 file changed, 8 insertions(+) As a SEGV fix, this really should go in for 1.4. What needs to be done to make that happen.