All of lore.kernel.org
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@citrix.com>
To: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Committers <committers@xenproject.org>
Subject: Difference between patch in XSA and patch checked in
Date: Wed, 23 Aug 2017 17:35:10 +0100	[thread overview]
Message-ID: <4aa34fe5-05cf-111f-e095-2ed77bd1bfc6@citrix.com> (raw)

Dear committers,

I spent way to many hours this afternoon trying to rebase the CentOS
patchqueue from 4.6.5 to 4.6.6.  In theory "git rebase" should detect
when duplicate patches are being merged over and forget the patch
automatically.

Unfortunately, this didn't work in a large number of cases because there
were minor changes between the patch published in the XSA and the patch
that ended up getting checked in.  One example of this is
xsa218-4.6/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch,
which in the advisory has:

+        gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);

But in the checked-in patch for stable-4.6 (c/s 819044abe4) has:

+        gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle);

So I spent some time writing a script that would automatically look for
a patch in the commit history with the same title.  This also didn't
work, because the *titles* often changed, in really minor ways.  One
example of this is xsa219-4.6.patch, which has the commit title:

x86/shadow: Hold references for the duration of emulated writes

But when checked in to stable-4.6, had this title (c/s 4d13019cb0):

x86/shadow: hold references for the duration of emulated writes

Another example is xsa222-1-4.6.patch, which had the following title :

xen/memory: Fix return value handing of guest_remove_page()

But was checked in to stable-4.6 with this this title (c/s d23eb82c8a):

memory: fix return value handing of guest_remove_page()

I know Lars has run into similar problems.  Having a computer be able to
easily tell whether a patch has been applied or not will save everyone a
lot of time in the long run, and is far more important than fixing a
printk string or fixing capitalization in a patch title.

Can I propose that committers should always check in the exact version
of the patch in the publicly-released advisory?  Preferably directly
from xsa.git, and with 'git am' (and not rebasing or modifying patches)?

"Bugs" in patch titles should be caught during pre-embargo review, and
left if they get missed; stylistic issues with the patch should be fixed
in follow-up patches.

Thanks,
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

             reply	other threads:[~2017-08-23 16:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-23 16:35 George Dunlap [this message]
2017-08-23 16:39 ` Difference between patch in XSA and patch checked in Andrew Cooper
2017-08-23 16:45   ` George Dunlap
2017-08-24  7:29 ` Jan Beulich
2017-08-24 10:17   ` George Dunlap
2017-08-24 10:20     ` Jan Beulich
2017-08-24 10:30       ` George Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4aa34fe5-05cf-111f-e095-2ed77bd1bfc6@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=committers@xenproject.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.