Skip to content

Commit 31a4189

Browse files
Ian Campbelldavem330
Ian Campbell
authored andcommitted
xen: netback: read hotplug script once at start of day.
When we come to tear things down in netback_remove() and generate the uevent it is possible that the xenstore directory has already been removed (details below). In such cases netback_uevent() won't be able to read the hotplug script and will write a xenstore error node. A recent change to the hypervisor exposed this race such that we now sometimes lose it (where apparently we didn't ever before). Instead read the hotplug script configuration during setup and use it for the lifetime of the backend device. The apparently more obvious fix of moving the transition to state=Closed in netback_remove() to after the uevent does not work because it is possible that we are already in state=Closed (in reaction to the guest having disconnected as it shutdown). Being already in Closed means the toolstack is at liberty to start tearing down the xenstore directories. In principal it might be possible to arrange to unregister the device sooner (e.g on transition to Closing) such that xenstore would still be there but this state machine is fragile and prone to anger... A modern Xen system only relies on the hotplug uevent for driver domains, when the backend is in the same domain as the toolstack it will run the necessary setup/teardown directly in the correct sequence wrt xenstore changes. Signed-off-by: Ian Campbell <[email protected]> Acked-by: Wei Liu <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent dc5e7a8 commit 31a4189

File tree

1 file changed

+19
-14
lines changed

1 file changed

+19
-14
lines changed

drivers/net/xen-netback/xenbus.c

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ struct backend_info {
3434
enum xenbus_state frontend_state;
3535
struct xenbus_watch hotplug_status_watch;
3636
u8 have_hotplug_status_watch:1;
37+
38+
const char *hotplug_script;
3739
};
3840

3941
static int connect_rings(struct backend_info *be, struct xenvif_queue *queue);
@@ -238,6 +240,7 @@ static int netback_remove(struct xenbus_device *dev)
238240
xenvif_free(be->vif);
239241
be->vif = NULL;
240242
}
243+
kfree(be->hotplug_script);
241244
kfree(be);
242245
dev_set_drvdata(&dev->dev, NULL);
243246
return 0;
@@ -255,6 +258,7 @@ static int netback_probe(struct xenbus_device *dev,
255258
struct xenbus_transaction xbt;
256259
int err;
257260
int sg;
261+
const char *script;
258262
struct backend_info *be = kzalloc(sizeof(struct backend_info),
259263
GFP_KERNEL);
260264
if (!be) {
@@ -347,6 +351,15 @@ static int netback_probe(struct xenbus_device *dev,
347351
if (err)
348352
pr_debug("Error writing multi-queue-max-queues\n");
349353

354+
script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL);
355+
if (IS_ERR(script)) {
356+
err = PTR_ERR(script);
357+
xenbus_dev_fatal(dev, err, "reading script");
358+
goto fail;
359+
}
360+
361+
be->hotplug_script = script;
362+
350363
err = xenbus_switch_state(dev, XenbusStateInitWait);
351364
if (err)
352365
goto fail;
@@ -379,22 +392,14 @@ static int netback_uevent(struct xenbus_device *xdev,
379392
struct kobj_uevent_env *env)
380393
{
381394
struct backend_info *be = dev_get_drvdata(&xdev->dev);
382-
char *val;
383395

384-
val = xenbus_read(XBT_NIL, xdev->nodename, "script", NULL);
385-
if (IS_ERR(val)) {
386-
int err = PTR_ERR(val);
387-
xenbus_dev_fatal(xdev, err, "reading script");
388-
return err;
389-
} else {
390-
if (add_uevent_var(env, "script=%s", val)) {
391-
kfree(val);
392-
return -ENOMEM;
393-
}
394-
kfree(val);
395-
}
396+
if (!be)
397+
return 0;
398+
399+
if (add_uevent_var(env, "script=%s", be->hotplug_script))
400+
return -ENOMEM;
396401

397-
if (!be || !be->vif)
402+
if (!be->vif)
398403
return 0;
399404

400405
return add_uevent_var(env, "vif=%s", be->vif->dev->name);

0 commit comments

Comments
 (0)