* Re: [PATCH] libxl/netbsd: check num_exec in hotplug function
[not found] <201607040811.u648B4MN011551@server.cornerstoneservice.ca>
@ 2016-07-04 11:09 ` Wei Liu
2016-07-04 11:23 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-07-04 11:09 UTC (permalink / raw)
To: John Nemeth; +Cc: Xen-devel, Wei Liu
Add back xen-devel. Please reply to all recipients in the future.
On Mon, Jul 04, 2016 at 01:11:04AM -0700, John Nemeth wrote:
> On Jul 2, 12:35pm, Wei Liu wrote:
> }
> } This basically replicates the same logic in libxl_linux.c. Without this
> } libxl will loop indefinitely trying to execute hotplug script.
>
> One minor change required (see below).
>
> } Reported-by: John Nemeth <jnemeth@cue.bc.ca>
> } Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> } ---
> } tools/libxl/libxl_netbsd.c | 18 ++++++++++++++++++
> } 1 file changed, 18 insertions(+)
> }
> } diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
> } index 096c057..92d3c89 100644
> } --- a/tools/libxl/libxl_netbsd.c
> } +++ b/tools/libxl/libxl_netbsd.c
> } @@ -68,7 +68,25 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
> }
> } switch (dev->backend_kind) {
> } case LIBXL__DEVICE_KIND_VBD:
> } + if (num_exec != 0) {
> } + LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
> } + rc = 0;
> } + goto out;
> } + }
> } + rc = libxl__hotplug(gc, dev, args, action);
> } + if (!rc) rc = 1;
> } + break;
> } case LIBXL__DEVICE_KIND_VIF:
> } + /*
> } + * If domain has a stubdom we don't have to execute hotplug scripts
> } + * for emulated interfaces
> } + */
> } + if ((num_exec > 1) ||
>
> The function is called with num_exec set to 0 and 1, so this
> should be:
>
> if ((num_exec != 0) ||
>
AIUI this is related to how network is setup because we would need to
hotplug both the emulated nic in QEMU and the PV nic. Is this line
causing problem for you?
Wei.
> } + (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
> } + LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
> } + rc = 0;
> } + goto out;
> } + }
> } rc = libxl__hotplug(gc, dev, args, action);
> } if (!rc) rc = 1;
> } break;
> } --
> } 2.1.4
> }
> }-- End of excerpt from Wei Liu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/netbsd: check num_exec in hotplug function
2016-07-04 11:09 ` Wei Liu
@ 2016-07-04 11:23 ` Wei Liu
2016-07-04 11:56 ` John Nemeth
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-07-04 11:23 UTC (permalink / raw)
To: John Nemeth; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
Also CC Roger since he authored the original code.
Feel free to correct me misunderstanding on this issue.
On Mon, Jul 04, 2016 at 12:09:30PM +0100, Wei Liu wrote:
> Add back xen-devel. Please reply to all recipients in the future.
>
> On Mon, Jul 04, 2016 at 01:11:04AM -0700, John Nemeth wrote:
> > On Jul 2, 12:35pm, Wei Liu wrote:
> > }
> > } This basically replicates the same logic in libxl_linux.c. Without this
> > } libxl will loop indefinitely trying to execute hotplug script.
> >
> > One minor change required (see below).
> >
> > } Reported-by: John Nemeth <jnemeth@cue.bc.ca>
> > } Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > } ---
> > } tools/libxl/libxl_netbsd.c | 18 ++++++++++++++++++
> > } 1 file changed, 18 insertions(+)
> > }
> > } diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
> > } index 096c057..92d3c89 100644
> > } --- a/tools/libxl/libxl_netbsd.c
> > } +++ b/tools/libxl/libxl_netbsd.c
> > } @@ -68,7 +68,25 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
> > }
> > } switch (dev->backend_kind) {
> > } case LIBXL__DEVICE_KIND_VBD:
> > } + if (num_exec != 0) {
> > } + LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
> > } + rc = 0;
> > } + goto out;
> > } + }
> > } + rc = libxl__hotplug(gc, dev, args, action);
> > } + if (!rc) rc = 1;
> > } + break;
> > } case LIBXL__DEVICE_KIND_VIF:
> > } + /*
> > } + * If domain has a stubdom we don't have to execute hotplug scripts
> > } + * for emulated interfaces
> > } + */
> > } + if ((num_exec > 1) ||
> >
> > The function is called with num_exec set to 0 and 1, so this
> > should be:
> >
> > if ((num_exec != 0) ||
> >
>
> AIUI this is related to how network is setup because we would need to
> hotplug both the emulated nic in QEMU and the PV nic. Is this line
> causing problem for you?
>
> Wei.
>
> > } + (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
> > } + LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
> > } + rc = 0;
> > } + goto out;
> > } + }
> > } rc = libxl__hotplug(gc, dev, args, action);
> > } if (!rc) rc = 1;
> > } break;
> > } --
> > } 2.1.4
> > }
> > }-- End of excerpt from Wei Liu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/netbsd: check num_exec in hotplug function
2016-07-04 11:23 ` Wei Liu
@ 2016-07-04 11:56 ` John Nemeth
2016-07-04 12:02 ` Roger Pau Monné
0 siblings, 1 reply; 6+ messages in thread
From: John Nemeth @ 2016-07-04 11:56 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel, Roger Pau Monné
On Jul 4, 12:23pm, Wei Liu wrote:
}
} Also CC Roger since he authored the original code.
}
} Feel free to correct me misunderstanding on this issue.
}
} On Mon, Jul 04, 2016 at 12:09:30PM +0100, Wei Liu wrote:
} > Add back xen-devel. Please reply to all recipients in the future.
Oops, I usually do (*checks header*; yep, good this time).
} > On Mon, Jul 04, 2016 at 01:11:04AM -0700, John Nemeth wrote:
} > > On Jul 2, 12:35pm, Wei Liu wrote:
} > > }
} > > } This basically replicates the same logic in libxl_linux.c. Without this
} > > } libxl will loop indefinitely trying to execute hotplug script.
} > >
} > > One minor change required (see below).
} > >
} > > } Reported-by: John Nemeth <jnemeth@cue.bc.ca>
} > > } Signed-off-by: Wei Liu <wei.liu2@citrix.com>
} > > } ---
} > > } tools/libxl/libxl_netbsd.c | 18 ++++++++++++++++++
} > > } 1 file changed, 18 insertions(+)
} > > }
} > > } diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
} > > } index 096c057..92d3c89 100644
} > > } --- a/tools/libxl/libxl_netbsd.c
} > > } +++ b/tools/libxl/libxl_netbsd.c
} > > } @@ -68,7 +68,25 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
} > > }
} > > } switch (dev->backend_kind) {
} > > } case LIBXL__DEVICE_KIND_VBD:
} > > } + if (num_exec != 0) {
} > > } + LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
} > > } + rc = 0;
} > > } + goto out;
} > > } + }
} > > } + rc = libxl__hotplug(gc, dev, args, action);
} > > } + if (!rc) rc = 1;
} > > } + break;
} > > } case LIBXL__DEVICE_KIND_VIF:
} > > } + /*
} > > } + * If domain has a stubdom we don't have to execute hotplug scripts
} > > } + * for emulated interfaces
} > > } + */
} > > } + if ((num_exec > 1) ||
} > >
} > > The function is called with num_exec set to 0 and 1, so this
} > > should be:
} > >
} > > if ((num_exec != 0) ||
} >
} > AIUI this is related to how network is setup because we would need to
} > hotplug both the emulated nic in QEMU and the PV nic. Is this line
} > causing problem for you?
} >
} > Wei.
Yes, when the script is called the second time, it fails when
trying to add the interface to the bridge and returns an error.
This results in xl aborting the domU creation process and tearing
it down. If the script is supposed to be called twice, then there
needs to be some way to distinguish the calls and what is supposed
to happen during each call so the script can be adjusted. The
change above allowed me to bring up a domU and work with it (including
network). However, I only tested a PV domU, not an HVM one, so
QEMU wasn't involved.
} > > } + (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
} > > } + LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
} > > } + rc = 0;
} > > } + goto out;
} > > } + }
} > > } rc = libxl__hotplug(gc, dev, args, action);
} > > } if (!rc) rc = 1;
} > > } break;
} > > } --
} > > } 2.1.4
} > > }
} > > }-- End of excerpt from Wei Liu
}-- End of excerpt from Wei Liu
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/netbsd: check num_exec in hotplug function
2016-07-04 11:56 ` John Nemeth
@ 2016-07-04 12:02 ` Roger Pau Monné
0 siblings, 0 replies; 6+ messages in thread
From: Roger Pau Monné @ 2016-07-04 12:02 UTC (permalink / raw)
To: John Nemeth; +Cc: Xen-devel, Wei Liu
On Mon, Jul 04, 2016 at 04:56:33AM -0700, John Nemeth wrote:
> On Jul 4, 12:23pm, Wei Liu wrote:
> } On Mon, Jul 04, 2016 at 12:09:30PM +0100, Wei Liu wrote:
> } > On Mon, Jul 04, 2016 at 01:11:04AM -0700, John Nemeth wrote:
> } > > On Jul 2, 12:35pm, Wei Liu wrote:
> } > > } + /*
> } > > } + * If domain has a stubdom we don't have to execute hotplug scripts
> } > > } + * for emulated interfaces
> } > > } + */
> } > > } + if ((num_exec > 1) ||
> } > >
> } > > The function is called with num_exec set to 0 and 1, so this
> } > > should be:
> } > >
> } > > if ((num_exec != 0) ||
> } >
> } > AIUI this is related to how network is setup because we would need to
> } > hotplug both the emulated nic in QEMU and the PV nic. Is this line
> } > causing problem for you?
> } >
> } > Wei.
>
> Yes, when the script is called the second time, it fails when
> trying to add the interface to the bridge and returns an error.
> This results in xl aborting the domU creation process and tearing
> it down. If the script is supposed to be called twice, then there
> needs to be some way to distinguish the calls and what is supposed
> to happen during each call so the script can be adjusted. The
> change above allowed me to bring up a domU and work with it (including
> network). However, I only tested a PV domU, not an HVM one, so
> QEMU wasn't involved.
I've already replied to the other thread with the reason why this needs to
be num_exec != 0 instead of num_exec > 1, and to avoid further confusion I
would recommend Wei to add a comment here when the patch is updated, since
this is a NetBSD-specific behavior that's not shared with other Dom0s:
http://lists.xenproject.org/archives/html/xen-devel/2016-07/msg00178.html
Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] libxl/netbsd: check num_exec in hotplug function
@ 2016-07-04 12:13 Wei Liu
2016-07-06 17:27 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2016-07-04 12:13 UTC (permalink / raw)
To: Xen-devel; +Cc: Ian Jackson, Wei Liu, John Nemeth, Roger Pau Monné
This basically replicates the same logic in libxl_linux.c but with one
change -- only test num_exec == 0 in nic hotplug case because NetBSD let
QEMU call a script itself. Without this patch libxl will loop
indefinitely trying to execute hotplug script.
Reported-by: John Nemeth <jnemeth@cue.bc.ca>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: John Nemeth <jnemeth@cue.bc.ca>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Ian, this is a backport candidate.
---
tools/libxl/libxl_netbsd.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index 096c057..a79b8aa 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -68,7 +68,28 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
switch (dev->backend_kind) {
case LIBXL__DEVICE_KIND_VBD:
+ if (num_exec != 0) {
+ LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
+ rc = 0;
+ goto out;
+ }
+ rc = libxl__hotplug(gc, dev, args, action);
+ if (!rc) rc = 1;
+ break;
case LIBXL__DEVICE_KIND_VIF:
+ /*
+ * If domain has a stubdom we don't have to execute hotplug scripts
+ * for emulated interfaces
+ *
+ * NetBSD let QEMU call a script to plug emulated nic, so
+ * only test if num_exec == 0 in that case.
+ */
+ if ((num_exec != 0) ||
+ (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
+ LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
+ rc = 0;
+ goto out;
+ }
rc = libxl__hotplug(gc, dev, args, action);
if (!rc) rc = 1;
break;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libxl/netbsd: check num_exec in hotplug function
2016-07-04 12:13 [PATCH] libxl/netbsd: check num_exec in hotplug function Wei Liu
@ 2016-07-06 17:27 ` Wei Liu
0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2016-07-06 17:27 UTC (permalink / raw)
To: Xen-devel; +Cc: Ian Jackson, Wei Liu, John Nemeth, Roger Pau Monné
On Mon, Jul 04, 2016 at 01:13:42PM +0100, Wei Liu wrote:
> This basically replicates the same logic in libxl_linux.c but with one
> change -- only test num_exec == 0 in nic hotplug case because NetBSD let
> QEMU call a script itself. Without this patch libxl will loop
> indefinitely trying to execute hotplug script.
>
> Reported-by: John Nemeth <jnemeth@cue.bc.ca>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Queued.
I think Roger's ack is sufficient for BSD related code and John
confirmed in the other thread this fixed his problem.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-06 17:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-04 12:13 [PATCH] libxl/netbsd: check num_exec in hotplug function Wei Liu
2016-07-06 17:27 ` Wei Liu
[not found] <201607040811.u648B4MN011551@server.cornerstoneservice.ca>
2016-07-04 11:09 ` Wei Liu
2016-07-04 11:23 ` Wei Liu
2016-07-04 11:56 ` John Nemeth
2016-07-04 12:02 ` Roger Pau Monné
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).