* 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 ` [PATCH] libxl/netbsd: check num_exec in hotplug function 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 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 --
[not found] <201607040811.u648B4MN011551@server.cornerstoneservice.ca>
2016-07-04 11:09 ` [PATCH] libxl/netbsd: check num_exec in hotplug function Wei Liu
2016-07-04 11:23 ` Wei Liu
2016-07-04 11:56 ` John Nemeth
2016-07-04 12:02 ` Roger Pau Monné
2016-07-04 12:13 Wei Liu
2016-07-06 17:27 ` Wei Liu
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).