[PATCH] Remus breaks the build

[PATCH] Remus breaks the build

[Jeremy Fitzhardinge]

  Remus tries to go out of the tools directory and build in the kernel 
directory.  This assumes that we're actually building a kernel out of 
the xen build tree, and that kernel is actually being used.

If Remus needs kernel modules, they should actually be part of the 
respective kernel trees, not grafted on post-facto.

Disable the tools/remus directory until this is sorted out.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

hg qnew -f didiff -r f45026ec8db5 tools/Makefile
--- a/tools/Makefile	Mon Aug 09 18:29:50 2010 +0100
+++ b/tools/Makefile	Thu Aug 12 17:35:05 2010 -0700
@@ -33,7 +33,7 @@
  SUBDIRS-$(CONFIG_IOEMU) += ioemu-dir
  SUBDIRS-y += xenpmd
  SUBDIRS-y += libxl
-SUBDIRS-y += remus
+#SUBDIRS-y += remus
  SUBDIRS-$(CONFIG_X86) += xenpaging
  SUBDIRS-$(CONFIG_X86) += debugger/gdbsx

Re: [PATCH] Remus breaks the build

[Ian Campbell]

On Fri, 2010-08-13 at 01:38 +0100, Jeremy Fitzhardinge wrote:
> Remus tries to go out of the tools directory and build in the kernel 
> directory.  This assumes that we're actually building a kernel out of 
> the xen build tree, and that kernel is actually being used.
> 
> If Remus needs kernel modules, they should actually be part of the 
> respective kernel trees, not grafted on post-facto.
> 
> Disable the tools/remus directory until this is sorted out.
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

IanJ and I once spent a headscratching few hours over a kernel build
failure which turned out to be because the remus modules were being
buyilt before the kernel tree was configured (in a parallel "make
world"). Took ages to figure out that was what was going on!

Ian.

> 
> hg qnew -f didiff -r f45026ec8db5 tools/Makefile
> --- a/tools/Makefile	Mon Aug 09 18:29:50 2010 +0100
> +++ b/tools/Makefile	Thu Aug 12 17:35:05 2010 -0700
> @@ -33,7 +33,7 @@
>   SUBDIRS-$(CONFIG_IOEMU) += ioemu-dir
>   SUBDIRS-y += xenpmd
>   SUBDIRS-y += libxl
> -SUBDIRS-y += remus
> +#SUBDIRS-y += remus
>   SUBDIRS-$(CONFIG_X86) += xenpaging
>   SUBDIRS-$(CONFIG_X86) += debugger/gdbsx
> 
> 
> 

Re: [PATCH] Remus breaks the build

[Stefano Stabellini]

On Fri, 13 Aug 2010, Ian Campbell wrote:
> On Fri, 2010-08-13 at 01:38 +0100, Jeremy Fitzhardinge wrote:
> > Remus tries to go out of the tools directory and build in the kernel 
> > directory.  This assumes that we're actually building a kernel out of 
> > the xen build tree, and that kernel is actually being used.
> > 
> > If Remus needs kernel modules, they should actually be part of the 
> > respective kernel trees, not grafted on post-facto.
> > 
> > Disable the tools/remus directory until this is sorted out.
> > 
> > Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> IanJ and I once spent a headscratching few hours over a kernel build
> failure which turned out to be because the remus modules were being
> buyilt before the kernel tree was configured (in a parallel "make
> world"). Took ages to figure out that was what was going on!
> 

I'll apply the patch for now, of course I am more then willing to
re-enable remus as soon as the problem is fixed.

Re: [PATCH] Remus breaks the build

[Ian Jackson]

Stefano Stabellini writes ("[Xen-devel] Re: [PATCH] Remus breaks the build"):
> I'll apply the patch for now, of course I am more then willing to
> re-enable remus as soon as the problem is fixed.

Quite so.  I have also removed remus from the nightly tests (since
otherwise this would be regarded as a regression in the cases where
remus still sort of worked; it hasn't worked properly for some time).

Ian.

Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Thursday, 12 August 2010 at 17:38, Jeremy Fitzhardinge wrote:
>  Remus tries to go out of the tools directory and build in the kernel
> directory.  This assumes that we're actually building a kernel out of
> the xen build tree, and that kernel is actually being used.
> 
> If Remus needs kernel modules, they should actually be part of the
> respective kernel trees, not grafted on post-facto.
> 
> Disable the tools/remus directory until this is sorted out.
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>

I assume you're talking about this snippet of tools/remus/kmod/Makefile:

$(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules

which expects to find a Makefile in $KERNELDIR but does the actual
building in place, in the tools/remus/kmod directory (unless the
kernel build system has changed recently?). I thought this was a
pretty standard way to build out-of-tree kernel modules.

I'm not sure why this is causing you problems (is it?), but if you're
willing to carry sch_queue in the pvops tree, I'd be happy to drop
tools/remus/kmod in the unstable tree.

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Friday, 13 August 2010 at 13:52, Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] Re: [PATCH] Remus breaks the build"):
> > I'll apply the patch for now, of course I am more then willing to
> > re-enable remus as soon as the problem is fixed.
> 
> Quite so.  I have also removed remus from the nightly tests (since
> otherwise this would be regarded as a regression in the cases where
> remus still sort of worked; it hasn't worked properly for some time).

As always, Ian, it would be better if you provided some kind of
concrete bug report instead vaguely saying "it doesn't work". I've
never received one.

Re: [PATCH] Remus breaks the build

[Jeremy Fitzhardinge]

 On 08/13/2010 12:42 PM, Brendan Cully wrote:
> I assume you're talking about this snippet of tools/remus/kmod/Makefile:
>
> $(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules
>
> which expects to find a Makefile in $KERNELDIR but does the actual
> building in place, in the tools/remus/kmod directory (unless the
> kernel build system has changed recently?). I thought this was a
> pretty standard way to build out-of-tree kernel modules.

I don't ever build the kernel out of the Xen tree.  In general, it
assumes the kernel tree has already been configured and built, which may
not be true if you're doing a parallel build, or if you're building the
Xen tree piecewise.

> I'm not sure why this is causing you problems (is it?), but if you're
> willing to carry sch_queue in the pvops tree, I'd be happy to drop
> tools/remus/kmod in the unstable tree.

Yes, I'm happy to include it.  Do you have a git reference I can merge from?

    J

Re: Re: [PATCH] Remus breaks the build

[Dulloor]

On Fri, Aug 13, 2010 at 2:14 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>  On 08/13/2010 12:42 PM, Brendan Cully wrote:
>> I assume you're talking about this snippet of tools/remus/kmod/Makefile:
>>
>> $(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules
>>
>> which expects to find a Makefile in $KERNELDIR but does the actual
>> building in place, in the tools/remus/kmod directory (unless the
>> kernel build system has changed recently?). I thought this was a
>> pretty standard way to build out-of-tree kernel modules.
>
> I don't ever build the kernel out of the Xen tree.  In general, it
> assumes the kernel tree has already been configured and built, which may
> not be true if you're doing a parallel build, or if you're building the
> Xen tree piecewise.
>
>> I'm not sure why this is causing you problems (is it?), but if you're
>> willing to carry sch_queue in the pvops tree, I'd be happy to drop
>> tools/remus/kmod in the unstable tree.
>
> Yes, I'm happy to include it.  Do you have a git reference I can merge from?

My understanding is that we don't need any out-of-tree modules, if
linux is built with CONFIG_IFB.
Also, that IFB isn't something available on 2.6.18 (and maybe
2.6.27?), where we will need to build these modules.

Is that right ? And, if thats the case, isn't it better to fold these
drivers into 2.6.18 (2.6.27 ?) and
support Remus conditional on IFB for pvops tree ?


>
>    J
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Friday, 13 August 2010 at 21:25, Dulloor wrote:
> On Fri, Aug 13, 2010 at 2:14 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> >  On 08/13/2010 12:42 PM, Brendan Cully wrote:
> >> I assume you're talking about this snippet of tools/remus/kmod/Makefile:
> >>
> >> $(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules
> >>
> >> which expects to find a Makefile in $KERNELDIR but does the actual
> >> building in place, in the tools/remus/kmod directory (unless the
> >> kernel build system has changed recently?). I thought this was a
> >> pretty standard way to build out-of-tree kernel modules.
> >
> > I don't ever build the kernel out of the Xen tree.  In general, it
> > assumes the kernel tree has already been configured and built, which may
> > not be true if you're doing a parallel build, or if you're building the
> > Xen tree piecewise.
> >
> >> I'm not sure why this is causing you problems (is it?), but if you're
> >> willing to carry sch_queue in the pvops tree, I'd be happy to drop
> >> tools/remus/kmod in the unstable tree.
> >
> > Yes, I'm happy to include it.  Do you have a git reference I can merge from?
> 
> My understanding is that we don't need any out-of-tree modules, if
> linux is built with CONFIG_IFB.
> Also, that IFB isn't something available on 2.6.18 (and maybe
> 2.6.27?), where we will need to build these modules.
> 
> Is that right ? And, if thats the case, isn't it better to fold these
> drivers into 2.6.18 (2.6.27 ?) and
> support Remus conditional on IFB for pvops tree ?

Remus actually uses two modules. One is sch_queue, a Linux queueing
discipline that queues outbound guest traffic until the machine state
that produced it has been committed at the backup. This is the one
that we're talking about moving into the pvops tree (after I've tested
it -- I'm having the customary day-long fight getting Xen running
smoothly after having not updated for a while).

We need a second module (IFB or IMQ, depending on the kernel version)
because Linux queueing disciplines only operate on a device's outbound
traffic. Since Remus runs in dom0, it sees the guest's outbound
traffic as _inbound_ traffic on a VIF device. So IMQ/IFB is used to
redirect that incoming VIF traffic to a virtual intermediate device
with the sch_queue queueing discipline installed on it.

Re: Re: [PATCH] Remus breaks the build

[Ian Jackson]

Brendan Cully writes ("Re: [Xen-devel] Re: [PATCH] Remus breaks the build"):
> We need a second module (IFB or IMQ, depending on the kernel version)
> because Linux queueing disciplines only operate on a device's outbound
> traffic. Since Remus runs in dom0, it sees the guest's outbound
> traffic as _inbound_ traffic on a VIF device. So IMQ/IFB is used to
> redirect that incoming VIF traffic to a virtual intermediate device
> with the sch_queue queueing discipline installed on it.

Couldn't this be achieved simply by putting a dummy bridge or tap
device in the way or something ?  You seem to be describing a device
driver whose sole purpose is plumbing ...

Ian.

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Wednesday, 18 August 2010 at 14:40, Ian Jackson wrote:
> Brendan Cully writes ("Re: [Xen-devel] Re: [PATCH] Remus breaks the build"):
> > We need a second module (IFB or IMQ, depending on the kernel version)
> > because Linux queueing disciplines only operate on a device's outbound
> > traffic. Since Remus runs in dom0, it sees the guest's outbound
> > traffic as _inbound_ traffic on a VIF device. So IMQ/IFB is used to
> > redirect that incoming VIF traffic to a virtual intermediate device
> > with the sch_queue queueing discipline installed on it.
> 
> Couldn't this be achieved simply by putting a dummy bridge or tap
> device in the way or something ?  You seem to be describing a device
> driver whose sole purpose is plumbing ...

You are correct, the device is just plumbing. But it isn't any extra
code, at least for pvops -- IFB is carried in upstream linux. You
could probably use a second-level bridge or tap or policy routing or
something to get the same effect, but I don't think I see the
advantage. You're certainly welcome to put together an alternative
approach for comparison.

What would be simpler and more efficient would be to move the queueing
directly into netback. That's on the todo list but hasn't gotten
attention yet.

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Friday, 13 August 2010 at 12:44, Brendan Cully wrote:
> On Friday, 13 August 2010 at 13:52, Ian Jackson wrote:
> > Stefano Stabellini writes ("[Xen-devel] Re: [PATCH] Remus breaks the build"):
> > > I'll apply the patch for now, of course I am more then willing to
> > > re-enable remus as soon as the problem is fixed.
> > 
> > Quite so.  I have also removed remus from the nightly tests (since
> > otherwise this would be regarded as a regression in the cases where
> > remus still sort of worked; it hasn't worked properly for some time).
> 
> As always, Ian, it would be better if you provided some kind of
> concrete bug report instead vaguely saying "it doesn't work". I've
> never received one.

It turns out it was broken... by you, in 21488:dd6bbdc42033 (I missed
this because I've been running the 4.0 tree until recently). That
patch of yours only appeared to be a no-op, because it missed several
calls to read_exact. I've patchbombed the fix.

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

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

On Friday, 13 August 2010 at 14:14, Jeremy Fitzhardinge wrote:
>  On 08/13/2010 12:42 PM, Brendan Cully wrote:
> > I assume you're talking about this snippet of tools/remus/kmod/Makefile:
> >
> > $(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules
> >
> > which expects to find a Makefile in $KERNELDIR but does the actual
> > building in place, in the tools/remus/kmod directory (unless the
> > kernel build system has changed recently?). I thought this was a
> > pretty standard way to build out-of-tree kernel modules.
> 
> I don't ever build the kernel out of the Xen tree.  In general, it
> assumes the kernel tree has already been configured and built, which may
> not be true if you're doing a parallel build, or if you're building the
> Xen tree piecewise.
> 
> > I'm not sure why this is causing you problems (is it?), but if you're
> > willing to carry sch_queue in the pvops tree, I'd be happy to drop
> > tools/remus/kmod in the unstable tree.
> 
> Yes, I'm happy to include it.  Do you have a git reference I can merge from?

That's more git than I've learned. Would a patch suffice? (even
producing the diff was non-obvious. It turns out that git add foo; git
diff doesn't include the diff for foo!)

[-- Attachment #2: pvops-sch-queue.diff --]
[-- Type: text/plain, Size: 5865 bytes --]

diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 929218a..2b945b2 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -215,6 +215,16 @@ config NET_SCH_INGRESS
 	  To compile this code as a module, choose M here: the
 	  module will be called sch_ingress.
 
+config NET_SCH_QUEUE
+	tristate "Remus network buffer"
+	---help---
+	  Say Y here if you are using this kernel for Xen dom0 and
+	  want to protect Xen guests with Remus.
+	  If unsure, say N.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called sch_queue.
+
 comment "Classification"
 
 config NET_CLS
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 54d950c..1a17127 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_NET_SCH_MULTIQ)	+= sch_multiq.o
 obj-$(CONFIG_NET_SCH_ATM)	+= sch_atm.o
 obj-$(CONFIG_NET_SCH_NETEM)	+= sch_netem.o
 obj-$(CONFIG_NET_SCH_DRR)	+= sch_drr.o
+obj-$(CONFIG_NET_SCH_QUEUE)	+= sch_queue.o
 obj-$(CONFIG_NET_CLS_U32)	+= cls_u32.o
 obj-$(CONFIG_NET_CLS_ROUTE4)	+= cls_route.o
 obj-$(CONFIG_NET_CLS_FW)	+= cls_fw.o
diff --git a/net/sched/sch_queue.c b/net/sched/sch_queue.c
new file mode 100644
index 0000000..fe11e41
--- /dev/null
+++ b/net/sched/sch_queue.c
@@ -0,0 +1,172 @@
+/*
+ * sch_queue.c Queue traffic until an explicit release command
+ *
+ *             This program is free software; you can redistribute it and/or
+ *             modify it under the terms of the GNU General Public License
+ *             as published by the Free Software Foundation; either version
+ *             2 of the License, or (at your option) any later version.
+ *
+ * The operation of the buffer is as follows:
+ * When a checkpoint begins, a barrier is inserted into the
+ *   network queue by a netlink request (it operates by storing
+ *   a pointer to the next packet which arrives and blocking dequeue
+ *   when that packet is at the head of the queue).
+ * When a checkpoint completes (the backup acknowledges receipt),
+ *   currently-queued packets are released.
+ * So it supports two operations, barrier and release.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <net/pkt_sched.h>
+
+/* xenbus directory */
+#define FIFO_BUF    (10*1024*1024)
+
+#define TCQ_CHECKPOINT 0
+#define TCQ_DEQUEUE    1
+
+struct queue_sched_data {
+  /* this packet is the first packet which should not be delivered.
+   * If it is NULL, queue_enqueue will set it to the next packet it sees. */
+  struct sk_buff *stop;
+};
+
+struct tc_queue_qopt {
+  /* 0: reset stop packet pointer
+   * 1: dequeue to stop pointer */
+  int action;
+};
+
+static int skb_remove_foreign_references(struct sk_buff *skb)
+{
+  return !skb_linearize(skb);
+}
+
+static int queue_enqueue(struct sk_buff *skb, struct Qdisc* sch)
+{
+  struct queue_sched_data *q = qdisc_priv(sch);
+
+  if (likely(sch->qstats.backlog + skb->len <= FIFO_BUF))
+  {
+    if (!q->stop)
+      q->stop = skb;
+
+    if (!skb_remove_foreign_references(skb)) {
+      printk("error removing foreign ref\n");
+      return qdisc_reshape_fail(skb, sch);
+    }
+
+    return qdisc_enqueue_tail(skb, sch);
+  }
+  printk("queue reported full: %d,%d\n", sch->qstats.backlog, skb->len);
+
+  return qdisc_reshape_fail(skb, sch);
+}
+
+/* dequeue doesn't actually dequeue until the release command is
+ * received. */
+static struct sk_buff *queue_dequeue(struct Qdisc* sch)
+{
+  struct queue_sched_data *q = qdisc_priv(sch);
+  struct sk_buff* peek;
+  /*
+  struct timeval tv;
+
+  if (!q->stop) {
+    do_gettimeofday(&tv);
+    printk("packet dequeued at %lu.%06lu\n", tv.tv_sec, tv.tv_usec);
+  }
+  */
+
+  if (sch->flags & TCQ_F_THROTTLED)
+    return NULL;
+
+  peek = (struct sk_buff *)((sch->q).next);
+
+  /* this pointer comparison may be shady */
+  if (peek == q->stop) {
+    /*
+    do_gettimeofday(&tv);
+    printk("stop packet at %lu.%06lu\n", tv.tv_sec, tv.tv_usec);
+    */
+
+    /* this is the tail of the last round. Release it and block the queue */
+    sch->flags |= TCQ_F_THROTTLED;
+    return NULL;
+  }
+
+  return qdisc_dequeue_head(sch);
+}
+
+static int queue_init(struct Qdisc *sch, struct nlattr *opt)
+{
+  sch->flags |= TCQ_F_THROTTLED;
+
+  return 0;
+}
+
+/* receives two messages:
+ *   0: checkpoint queue (set stop to next packet)
+ *   1: dequeue until stop */
+static int queue_change(struct Qdisc* sch, struct nlattr* opt)
+{
+  struct queue_sched_data *q = qdisc_priv(sch);
+  struct tc_queue_qopt* msg;
+  /*
+  struct timeval tv;
+  */
+
+  if (!opt || nla_len(opt) < sizeof(*msg))
+    return -EINVAL;
+
+  msg = nla_data(opt);
+
+  if (msg->action == TCQ_CHECKPOINT) {
+    /* reset stop */
+    q->stop = NULL;
+  } else if (msg->action == TCQ_DEQUEUE) {
+    /* dequeue */
+    sch->flags &= ~TCQ_F_THROTTLED;
+    netif_schedule_queue(sch->dev_queue);
+    /*
+    do_gettimeofday(&tv);
+    printk("queue release at %lu.%06lu (%d bytes)\n", tv.tv_sec, tv.tv_usec,
+          sch->qstats.backlog);
+    */
+  } else {
+    return -EINVAL;
+  }
+
+  return 0;
+}
+
+struct Qdisc_ops queue_qdisc_ops = {
+  .id          =       "queue",
+  .priv_size   =       sizeof(struct queue_sched_data),
+  .enqueue     =       queue_enqueue,
+  .dequeue     =       queue_dequeue,
+  .peek        =       qdisc_peek_head,
+  .init        =       queue_init,
+  .change      =       queue_change,
+  .owner       =       THIS_MODULE,
+};
+
+static int __init queue_module_init(void)
+{
+  printk("loading queue\n");
+  return register_qdisc(&queue_qdisc_ops);
+}
+
+static void __exit queue_module_exit(void)
+{
+  printk("queue unloaded\n");
+  unregister_qdisc(&queue_qdisc_ops);
+}
+module_init(queue_module_init)
+module_exit(queue_module_exit)
+MODULE_LICENSE("GPL");

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Re: [PATCH] Remus breaks the build

[Jed Smith]

On Aug 18, 2010, at 4:26 PM, Brendan Cully wrote:

> On Friday, 13 August 2010 at 14:14, Jeremy Fitzhardinge wrote:
>> On 08/13/2010 12:42 PM, Brendan Cully wrote:
>>> I assume you're talking about this snippet of tools/remus/kmod/Makefile:
>>> 
>>> $(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules
>>> 
>>> which expects to find a Makefile in $KERNELDIR but does the actual
>>> building in place, in the tools/remus/kmod directory (unless the
>>> kernel build system has changed recently?). I thought this was a
>>> pretty standard way to build out-of-tree kernel modules.
>> 
>> I don't ever build the kernel out of the Xen tree.  In general, it
>> assumes the kernel tree has already been configured and built, which may
>> not be true if you're doing a parallel build, or if you're building the
>> Xen tree piecewise.
>> 
>>> I'm not sure why this is causing you problems (is it?), but if you're
>>> willing to carry sch_queue in the pvops tree, I'd be happy to drop
>>> tools/remus/kmod in the unstable tree.
>> 
>> Yes, I'm happy to include it.  Do you have a git reference I can merge from?
> 
> That's more git than I've learned. Would a patch suffice? (even
> producing the diff was non-obvious. It turns out that git add foo; git
> diff doesn't include the diff for foo!)

Because you staged it.  git-diff(1) works before you stage the changes.  You
probably want to commit and use git-format-patch(1).

http://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html

Regards,

Jed Smith
Systems Administrator
Linode, LLC
+1 (609) 593-7103 x1209
jed@linode.com

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Wednesday, 18 August 2010 at 16:34, Jed Smith wrote:
> On Aug 18, 2010, at 4:26 PM, Brendan Cully wrote:
> 
> > On Friday, 13 August 2010 at 14:14, Jeremy Fitzhardinge wrote:
> >> On 08/13/2010 12:42 PM, Brendan Cully wrote:
> >>> I assume you're talking about this snippet of tools/remus/kmod/Makefile:
> >>> 
> >>> $(MAKE) -C $(KERNELDIR) SUBDIRS=`pwd` modules
> >>> 
> >>> which expects to find a Makefile in $KERNELDIR but does the actual
> >>> building in place, in the tools/remus/kmod directory (unless the
> >>> kernel build system has changed recently?). I thought this was a
> >>> pretty standard way to build out-of-tree kernel modules.
> >> 
> >> I don't ever build the kernel out of the Xen tree.  In general, it
> >> assumes the kernel tree has already been configured and built, which may
> >> not be true if you're doing a parallel build, or if you're building the
> >> Xen tree piecewise.
> >> 
> >>> I'm not sure why this is causing you problems (is it?), but if you're
> >>> willing to carry sch_queue in the pvops tree, I'd be happy to drop
> >>> tools/remus/kmod in the unstable tree.
> >> 
> >> Yes, I'm happy to include it.  Do you have a git reference I can merge from?
> > 
> > That's more git than I've learned. Would a patch suffice? (even
> > producing the diff was non-obvious. It turns out that git add foo; git
> > diff doesn't include the diff for foo!)
> 
> Because you staged it.  git-diff(1) works before you stage the changes.  You
> probably want to commit and use git-format-patch(1).
> 
> http://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html

Not that it really matters (I was just sneaking in a rant, and I did
manage to produce the diff), but git diff also didn't include the new
file before I added it. It parts ways here with every other version
control system I can remember, not to mention intuition. But hey,
that's their problem.

Re: Re: [PATCH] Remus breaks the build

[Jeremy Fitzhardinge]

 On 08/18/2010 01:26 PM, Brendan Cully wrote:
> That's more git than I've learned. Would a patch suffice? (even
> producing the diff was non-obvious. It turns out that git add foo; git
> diff doesn't include the diff for foo!)

What's the origin of the code?  Do they have a git tree (I seem to
remember one).  I can pull from that, then apply any local patches you
may have.

    J

Re: Re: [PATCH] Remus breaks the build

[Brendan Cully]

On Wednesday, 18 August 2010 at 16:54, Jeremy Fitzhardinge wrote:
>  On 08/18/2010 01:26 PM, Brendan Cully wrote:
> > That's more git than I've learned. Would a patch suffice? (even
> > producing the diff was non-obvious. It turns out that git add foo; git
> > diff doesn't include the diff for foo!)
> 
> What's the origin of the code?  Do they have a git tree (I seem to
> remember one).  I can pull from that, then apply any local patches you
> may have.

You're probably remembering IMQ, which was a big ugly third-party
module but is no longer necessary, since we can now use IFB. The
little patch I just sent (attached to the last message) was written by
me. It's just a little queueing disciple for buffering outbound
traffic until an rtnetlink message releases it. It's against whatever
pvops tree xen-unstable pulls down (xen/master, I guess). Does that
patch suffice?

Re: Re: [PATCH] Remus breaks the build

[Pasi Kärkkäinen]

On Wed, Aug 18, 2010 at 05:03:48PM -0700, Brendan Cully wrote:
> On Wednesday, 18 August 2010 at 16:54, Jeremy Fitzhardinge wrote:
> >  On 08/18/2010 01:26 PM, Brendan Cully wrote:
> > > That's more git than I've learned. Would a patch suffice? (even
> > > producing the diff was non-obvious. It turns out that git add foo; git
> > > diff doesn't include the diff for foo!)
> > 
> > What's the origin of the code?  Do they have a git tree (I seem to
> > remember one).  I can pull from that, then apply any local patches you
> > may have.
> 
> You're probably remembering IMQ, which was a big ugly third-party
> module but is no longer necessary, since we can now use IFB. The
> little patch I just sent (attached to the last message) was written by
> me. It's just a little queueing disciple for buffering outbound
> traffic until an rtnetlink message releases it. It's against whatever
> pvops tree xen-unstable pulls down (xen/master, I guess). Does that
> patch suffice?
> 

Current xen-unstable uses xen/stable-2.6.32.x as a default.

-- Pasi

Re: Re: [PATCH] Remus breaks the build

[Ian Jackson]

Brendan Cully writes ("Re: [Xen-devel] Re: [PATCH] Remus breaks the build"):
> You are correct, the device is just plumbing. But it isn't any extra
> code, at least for pvops -- IFB is carried in upstream linux. You
> could probably use a second-level bridge or tap or policy routing or
> something to get the same effect, but I don't think I see the
> advantage. You're certainly welcome to put together an alternative
> approach for comparison.

Ah, I see.  I hadn't realised it was in upstream.  Fine, then.

Ian.