All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
@ 2014-02-17 14:37 Peter Maydell
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 1/3] hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses Peter Maydell
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-17 14:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

Three random easy patches fixing issues reported by Coverity Scan.

Peter Maydell (3):
  hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses
  hw/net/stellaris_enet: Avoid unintended sign extension
  hw/timer/arm_timer: Avoid array overrun for bad addresses

 hw/misc/arm_sysctl.c    | 4 ++--
 hw/net/stellaris_enet.c | 3 ++-
 hw/timer/arm_timer.c    | 2 ++
 3 files changed, 6 insertions(+), 3 deletions(-)

-- 
1.8.5

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 1/3] hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses
  2014-02-17 14:37 [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Peter Maydell
@ 2014-02-17 14:37 ` Peter Maydell
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 2/3] hw/net/stellaris_enet: Avoid unintended sign extension Peter Maydell
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-17 14:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

Fix incorrect use of sizeof() rather than ARRAY_SIZE() to guard
accesses into the mb_clock[] array, which was allowing a malicious
guest to overwrite the end of the array.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/arm_sysctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/arm_sysctl.c b/hw/misc/arm_sysctl.c
index 0fc26d2..3fad6f8 100644
--- a/hw/misc/arm_sysctl.c
+++ b/hw/misc/arm_sysctl.c
@@ -276,7 +276,7 @@ static bool vexpress_cfgctrl_read(arm_sysctl_state *s, unsigned int dcc,
         }
         break;
     case SYS_CFG_OSC:
-        if (site == SYS_CFG_SITE_MB && device < sizeof(s->mb_clock)) {
+        if (site == SYS_CFG_SITE_MB && device < ARRAY_SIZE(s->mb_clock)) {
             /* motherboard clock */
             *val = s->mb_clock[device];
             return true;
@@ -324,7 +324,7 @@ static bool vexpress_cfgctrl_write(arm_sysctl_state *s, unsigned int dcc,
 
     switch (function) {
     case SYS_CFG_OSC:
-        if (site == SYS_CFG_SITE_MB && device < sizeof(s->mb_clock)) {
+        if (site == SYS_CFG_SITE_MB && device < ARRAY_SIZE(s->mb_clock)) {
             /* motherboard clock */
             s->mb_clock[device] = val;
             return true;
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 2/3] hw/net/stellaris_enet: Avoid unintended sign extension
  2014-02-17 14:37 [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Peter Maydell
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 1/3] hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses Peter Maydell
@ 2014-02-17 14:37 ` Peter Maydell
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 3/3] hw/timer/arm_timer: Avoid array overrun for bad addresses Peter Maydell
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-17 14:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

Add a cast to avoid an unintended sign extension that
would mean we returned 0xffffffff in the high 32 bits
for an IA0 read if bit 31 in the MAC address was 1.
(This is harmless since we'll only be doing 4 byte
reads, but it could be confusing, so best avoided.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/stellaris_enet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index 9dd77f7..d04e6a4 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -176,7 +176,8 @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset,
         return val;
     case 0x14: /* IA0 */
         return s->conf.macaddr.a[0] | (s->conf.macaddr.a[1] << 8)
-               | (s->conf.macaddr.a[2] << 16) | (s->conf.macaddr.a[3] << 24);
+            | (s->conf.macaddr.a[2] << 16)
+            | ((uint32_t)s->conf.macaddr.a[3] << 24);
     case 0x18: /* IA1 */
         return s->conf.macaddr.a[4] | (s->conf.macaddr.a[5] << 8);
     case 0x1c: /* THR */
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH 3/3] hw/timer/arm_timer: Avoid array overrun for bad addresses
  2014-02-17 14:37 [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Peter Maydell
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 1/3] hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses Peter Maydell
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 2/3] hw/net/stellaris_enet: Avoid unintended sign extension Peter Maydell
@ 2014-02-17 14:37 ` Peter Maydell
  2014-02-17 18:59 ` [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Paolo Bonzini
  2014-02-24 15:41 ` [Qemu-devel] " Peter Maydell
  4 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-17 14:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: patches

The integrator's timer read/write functions log an error for
bad addresses in guest accesses, but were falling through and
using an out of bounds array index rather than returning early.
Fix this.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/timer/arm_timer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
index a47afde..fb0a45c 100644
--- a/hw/timer/arm_timer.c
+++ b/hw/timer/arm_timer.c
@@ -320,6 +320,7 @@ static uint64_t icp_pit_read(void *opaque, hwaddr offset,
     n = offset >> 8;
     if (n > 2) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad timer %d\n", __func__, n);
+        return 0;
     }
 
     return arm_timer_read(s->timer[n], offset & 0xff);
@@ -334,6 +335,7 @@ static void icp_pit_write(void *opaque, hwaddr offset,
     n = offset >> 8;
     if (n > 2) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad timer %d\n", __func__, n);
+        return;
     }
 
     arm_timer_write(s->timer[n], offset & 0xff, value);
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-17 14:37 [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Peter Maydell
                   ` (2 preceding siblings ...)
  2014-02-17 14:37 ` [Qemu-devel] [PATCH 3/3] hw/timer/arm_timer: Avoid array overrun for bad addresses Peter Maydell
@ 2014-02-17 18:59 ` Paolo Bonzini
  2014-02-18  1:02   ` Andreas Färber
  2014-02-24 15:41 ` [Qemu-devel] " Peter Maydell
  4 siblings, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2014-02-17 18:59 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: patches

Il 17/02/2014 15:37, Peter Maydell ha scritto:
> Three random easy patches fixing issues reported by Coverity Scan.
>
> Peter Maydell (3):
>   hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses
>   hw/net/stellaris_enet: Avoid unintended sign extension
>   hw/timer/arm_timer: Avoid array overrun for bad addresses
>
>  hw/misc/arm_sysctl.c    | 4 ++--
>  hw/net/stellaris_enet.c | 3 ++-
>  hw/timer/arm_timer.c    | 2 ++
>  3 files changed, 6 insertions(+), 3 deletions(-)
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-17 18:59 ` [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Paolo Bonzini
@ 2014-02-18  1:02   ` Andreas Färber
  2014-02-18  9:46     ` Peter Maydell
  0 siblings, 1 reply; 23+ messages in thread
From: Andreas Färber @ 2014-02-18  1:02 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Paolo Bonzini, qemu-stable, patches

Am 17.02.2014 19:59, schrieb Paolo Bonzini:
> Il 17/02/2014 15:37, Peter Maydell ha scritto:
>> Three random easy patches fixing issues reported by Coverity Scan.
>>
>> Peter Maydell (3):
>>   hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses
>>   hw/net/stellaris_enet: Avoid unintended sign extension
>>   hw/timer/arm_timer: Avoid array overrun for bad addresses
>>
>>  hw/misc/arm_sysctl.c    | 4 ++--
>>  hw/net/stellaris_enet.c | 3 ++-
>>  hw/timer/arm_timer.c    | 2 ++
>>  3 files changed, 6 insertions(+), 3 deletions(-)
>>
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

The reindent in 2/3 wouldn't have been strictly necessary, it seemed
space-aligned to after return, but it's not wrong either.

Shouldn't 1/3 and 3/3 be included in 1.7.1 as guest-triggerable bugs?
There's no qemu-stable@nongnu.org in the commit message.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18  1:02   ` Andreas Färber
@ 2014-02-18  9:46     ` Peter Maydell
  2014-02-18 10:13       ` Andreas Färber
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2014-02-18  9:46 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Paolo Bonzini, qemu-stable, QEMU Developers, Patch Tracking

On 18 February 2014 01:02, Andreas Färber <afaerber@suse.de> wrote:
> Shouldn't 1/3 and 3/3 be included in 1.7.1 as guest-triggerable bugs?
> There's no qemu-stable@nongnu.org in the commit message.

That's up to whoever maintains stable, I guess. I have no objection.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18  9:46     ` Peter Maydell
@ 2014-02-18 10:13       ` Andreas Färber
  2014-02-18 11:09         ` Peter Maydell
  0 siblings, 1 reply; 23+ messages in thread
From: Andreas Färber @ 2014-02-18 10:13 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, qemu-stable, QEMU Developers, Patch Tracking

Am 18.02.2014 10:46, schrieb Peter Maydell:
> On 18 February 2014 01:02, Andreas Färber <afaerber@suse.de> wrote:
>> Shouldn't 1/3 and 3/3 be included in 1.7.1 as guest-triggerable bugs?
>> There's no qemu-stable@nongnu.org in the commit message.
> 
> That's up to whoever maintains stable, I guess. I have no objection.

No, we've had that topic before: It's your job as submitter and
maintainer to flag that appropriately in the commit message, as per QEMU
Summit 2012.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 10:13       ` Andreas Färber
@ 2014-02-18 11:09         ` Peter Maydell
  2014-02-18 11:16           ` Paolo Bonzini
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2014-02-18 11:09 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Paolo Bonzini, qemu-stable, QEMU Developers, Patch Tracking

On 18 February 2014 10:13, Andreas Färber <afaerber@suse.de> wrote:
> Am 18.02.2014 10:46, schrieb Peter Maydell:
>> On 18 February 2014 01:02, Andreas Färber <afaerber@suse.de> wrote:
>>> Shouldn't 1/3 and 3/3 be included in 1.7.1 as guest-triggerable bugs?
>>> There's no qemu-stable@nongnu.org in the commit message.
>>
>> That's up to whoever maintains stable, I guess. I have no objection.
>
> No, we've had that topic before: It's your job as submitter and
> maintainer to flag that appropriately in the commit message, as per QEMU
> Summit 2012.

I don't think this workflow works. I have no idea what
stable's criteria are, and if you rely on people
adding a cc you're going to miss stuff.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 11:09         ` Peter Maydell
@ 2014-02-18 11:16           ` Paolo Bonzini
  2014-02-18 11:22             ` Peter Maydell
  2014-02-18 12:44             ` Andreas Färber
  0 siblings, 2 replies; 23+ messages in thread
From: Paolo Bonzini @ 2014-02-18 11:16 UTC (permalink / raw)
  To: Peter Maydell, Andreas Färber
  Cc: qemu-stable, QEMU Developers, Patch Tracking

Il 18/02/2014 12:09, Peter Maydell ha scritto:
> > No, we've had that topic before: It's your job as submitter and
> > maintainer to flag that appropriately in the commit message, as per QEMU
> > Summit 2012.
>
> I don't think this workflow works. I have no idea what
> stable's criteria are, and if you rely on people
> adding a cc you're going to miss stuff.

There isn't really a standard criterion.  It's up to each maintainer to 
be stricter or looser on what goes to stable.

Paolo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 11:16           ` Paolo Bonzini
@ 2014-02-18 11:22             ` Peter Maydell
  2014-02-18 11:23               ` Paolo Bonzini
  2014-02-18 12:17               ` Alexander Graf
  2014-02-18 12:44             ` Andreas Färber
  1 sibling, 2 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-18 11:22 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: qemu-stable, Patch Tracking, Andreas Färber, QEMU Developers

On 18 February 2014 11:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 18/02/2014 12:09, Peter Maydell ha scritto:
>
>> > No, we've had that topic before: It's your job as submitter and
>> > maintainer to flag that appropriately in the commit message, as per QEMU
>> > Summit 2012.
>>
>> I don't think this workflow works. I have no idea what
>> stable's criteria are, and if you rely on people
>> adding a cc you're going to miss stuff.
>
>
> There isn't really a standard criterion.  It's up to each maintainer to be
> stricter or looser on what goes to stable.

My criteria for ARM in the past has typically been "there's
a new release every three months, anything that got past
the release testing process for release N is sufficiently
non-critical it can just go into release N+1".

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 11:22             ` Peter Maydell
@ 2014-02-18 11:23               ` Paolo Bonzini
  2014-02-18 12:17               ` Alexander Graf
  1 sibling, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2014-02-18 11:23 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-stable, Patch Tracking, Andreas Färber, QEMU Developers

Il 18/02/2014 12:22, Peter Maydell ha scritto:
>> > There isn't really a standard criterion.  It's up to each maintainer to be
>> > stricter or looser on what goes to stable.
> My criteria for ARM in the past has typically been "there's
> a new release every three months, anything that got past
> the release testing process for release N is sufficiently
> non-critical it can just go into release N+1".

Then you can ignore qemu-stable. :)

Paolo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 11:22             ` Peter Maydell
  2014-02-18 11:23               ` Paolo Bonzini
@ 2014-02-18 12:17               ` Alexander Graf
  2014-02-18 12:37                 ` Peter Maydell
  1 sibling, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-02-18 12:17 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Paolo Bonzini, qemu-stable, Andreas Färber,
	Patch Tracking

On 02/18/2014 12:22 PM, Peter Maydell wrote:
> On 18 February 2014 11:16, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 18/02/2014 12:09, Peter Maydell ha scritto:
>>
>>>> No, we've had that topic before: It's your job as submitter and
>>>> maintainer to flag that appropriately in the commit message, as per QEMU
>>>> Summit 2012.
>>> I don't think this workflow works. I have no idea what
>>> stable's criteria are, and if you rely on people
>>> adding a cc you're going to miss stuff.
>>
>> There isn't really a standard criterion.  It's up to each maintainer to be
>> stricter or looser on what goes to stable.
> My criteria for ARM in the past has typically been "there's
> a new release every three months, anything that got past
> the release testing process for release N is sufficiently
> non-critical it can just go into release N+1".

Unfortunately this doesn't work for distributions. Distros need to 
maintain a stable branch for the lifetime of a release to ensure that 
we're reasonably regression free.

If you indicate that this doesn't apply to ARM it basically means you 
admit that ARM systems are not yet ready for "stable" use by customers 
when they want to use KVM. At least at the point when we agree that 
customers do want to run on a stable base for virtualization on ARM we 
need a working -stable system for critical fixes.


Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 12:17               ` Alexander Graf
@ 2014-02-18 12:37                 ` Peter Maydell
  2014-02-18 12:51                   ` Alexander Graf
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2014-02-18 12:37 UTC (permalink / raw)
  To: Alexander Graf
  Cc: QEMU Developers, Paolo Bonzini, qemu-stable, Andreas Färber,
	Patch Tracking

On 18 February 2014 12:17, Alexander Graf <agraf@suse.de> wrote:
> On 02/18/2014 12:22 PM, Peter Maydell wrote:
>> My criteria for ARM in the past has typically been "there's
>> a new release every three months, anything that got past
>> the release testing process for release N is sufficiently
>> non-critical it can just go into release N+1".
>
> Unfortunately this doesn't work for distributions. Distros
> need to maintain a stable branch for the lifetime of a release
> to ensure that we're reasonably regression free.
>
> If you indicate that this doesn't apply to ARM it basically means you admit
> that ARM systems are not yet ready for "stable" use by customers when they
> want to use KVM. At least at the point when we agree that customers do want
> to run on a stable base for virtualization on ARM we need a working -stable
> system for critical fixes.

I agree in general that ARM support needs to move from
its traditional "this is just a dev tool" situation to
a broader level of support/stability guarantees for KVM.
(We're not yet guaranteeing cross-version migration,
for another example there.)

However again we run into the definition of "what's a
critical fix?". I think if distros need a stable branch
then they need to be prepared to do the work of sorting
through what counts as a critical fix that needs to be
ported to that branch. (For instance, which boards and
targets do they care about?)

For instance patch 3 only applies to the integrator
board, and I don't consider the guest-to-host border
to be a security boundary for most of our legacy board
models: there's just too much unaudited device code for
that to be trustable.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 11:16           ` Paolo Bonzini
  2014-02-18 11:22             ` Peter Maydell
@ 2014-02-18 12:44             ` Andreas Färber
  2014-02-18 13:05               ` Peter Maydell
  2014-02-18 13:53               ` Paolo Bonzini
  1 sibling, 2 replies; 23+ messages in thread
From: Andreas Färber @ 2014-02-18 12:44 UTC (permalink / raw)
  To: Paolo Bonzini, Peter Maydell; +Cc: qemu-stable, QEMU Developers, Patch Tracking

Am 18.02.2014 12:16, schrieb Paolo Bonzini:
> Il 18/02/2014 12:09, Peter Maydell ha scritto:
>> > No, we've had that topic before: It's your job as submitter and
>> > maintainer to flag that appropriately in the commit message, as per
>> QEMU
>> > Summit 2012.
>>
>> I don't think this workflow works. I have no idea what
>> stable's criteria are, and if you rely on people
>> adding a cc you're going to miss stuff.
> 
> There isn't really a standard criterion.  It's up to each maintainer to
> be stricter or looser on what goes to stable.

The criteria is pretty simple: Was the breakage in the last release
already or was it introduced only intermittently.

It simply does not scale to have Michael or other stable maintainers
(mjt, me, ...) look at each commit from vX.Y to HEAD and decide whether
to backport or not. That's why that task of flagging as backport
*candidates* is pushed out to maintainers and recursively to authors
where applicable, to reduce the number of commits to sift through and to
allow to do this for actually committed patches rather than mails on the
mailing list that might not get committed or change subject.

What especially annoys me here is that Peter wants to play on Anthony's
level on the project but is openly ignoring both our stable releases as
a concept (we wouldn't need a release in the first place if we don't
care about it working!) and the procedures decided in his presence at
QEMU Summit (having maintainer/contributor flag it via Cc: line). If you
feel the conclusion we reached there is not working out, feel free to
bring this topic up on the KVM call later today - playing
Rumpelstilzchen and exempting you from what everyone else is doing is
not an acceptable solution. Either we all do it this way or we all
decide on another way. It was not my suggestion, just a proposed
solution to an issue that affects me, so I'm open to alternatives.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 12:37                 ` Peter Maydell
@ 2014-02-18 12:51                   ` Alexander Graf
  2014-02-18 12:53                     ` Andreas Färber
  0 siblings, 1 reply; 23+ messages in thread
From: Alexander Graf @ 2014-02-18 12:51 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Paolo Bonzini, qemu-stable, Andreas Färber,
	Patch Tracking

On 02/18/2014 01:37 PM, Peter Maydell wrote:
> On 18 February 2014 12:17, Alexander Graf <agraf@suse.de> wrote:
>> On 02/18/2014 12:22 PM, Peter Maydell wrote:
>>> My criteria for ARM in the past has typically been "there's
>>> a new release every three months, anything that got past
>>> the release testing process for release N is sufficiently
>>> non-critical it can just go into release N+1".
>> Unfortunately this doesn't work for distributions. Distros
>> need to maintain a stable branch for the lifetime of a release
>> to ensure that we're reasonably regression free.
>>
>> If you indicate that this doesn't apply to ARM it basically means you admit
>> that ARM systems are not yet ready for "stable" use by customers when they
>> want to use KVM. At least at the point when we agree that customers do want
>> to run on a stable base for virtualization on ARM we need a working -stable
>> system for critical fixes.
> I agree in general that ARM support needs to move from
> its traditional "this is just a dev tool" situation to
> a broader level of support/stability guarantees for KVM.
> (We're not yet guaranteeing cross-version migration,
> for another example there.)

Yup. I think it's reasonably to not declare ARM a "stable" target at the 
current point in time. But I think we agree that we want to change that 
- timeframe wise probably around the release after 2.0 at which point 
hopefully PCI and virtio 1.0 have settled.

> However again we run into the definition of "what's a
> critical fix?". I think if distros need a stable branch
> then they need to be prepared to do the work of sorting
> through what counts as a critical fix that needs to be
> ported to that branch. (For instance, which boards and
> targets do they care about?)

I think this is up for discussion. If I had to declare anything, I 
wouldn't consider anything but the virt machine as supported for example 
- similar to how x86 only really considers the pc machine type stable.

> For instance patch 3 only applies to the integrator
> board, and I don't consider the guest-to-host border
> to be a security boundary for most of our legacy board
> models: there's just too much unaudited device code for
> that to be trustable.

Yes, I fully agree. Traditionally what I've done is to reply to patches 
that I consider stable material and nag the maintainer about CCing it. 
After a while people got so afraid of my emails that they started doing 
the CC themselves :). But in case of the integrator board I personally 
wouldn't bother ;).


Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 12:51                   ` Alexander Graf
@ 2014-02-18 12:53                     ` Andreas Färber
  2014-02-18 12:56                       ` Alexander Graf
  0 siblings, 1 reply; 23+ messages in thread
From: Andreas Färber @ 2014-02-18 12:53 UTC (permalink / raw)
  To: Alexander Graf, Peter Maydell
  Cc: QEMU Developers, Paolo Bonzini, qemu-stable, Patch Tracking

Am 18.02.2014 13:51, schrieb Alexander Graf:
> Traditionally what I've done is to reply to patches
> that I consider stable material and nag the maintainer about CCing it.
> After a while people got so afraid of my emails that they started doing
> the CC themselves :). But in case of the integrator board I personally
> wouldn't bother ;).

Well, I have been doing exactly that here, asking Peter to add the CC,
and still Peter is refusing to add that single line before applying the
patch to his branch... :(

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 12:53                     ` Andreas Färber
@ 2014-02-18 12:56                       ` Alexander Graf
  0 siblings, 0 replies; 23+ messages in thread
From: Alexander Graf @ 2014-02-18 12:56 UTC (permalink / raw)
  To: Andreas Färber
  Cc: QEMU Developers, Peter Maydell, Patch Tracking, qemu-stable,
	Paolo Bonzini

On 02/18/2014 01:53 PM, Andreas Färber wrote:
> Am 18.02.2014 13:51, schrieb Alexander Graf:
>> Traditionally what I've done is to reply to patches
>> that I consider stable material and nag the maintainer about CCing it.
>> After a while people got so afraid of my emails that they started doing
>> the CC themselves :). But in case of the integrator board I personally
>> wouldn't bother ;).
> Well, I have been doing exactly that here, asking Peter to add the CC,
> and still Peter is refusing to add that single line before applying the
> patch to his branch... :(

Peter, can you please put CC stable on them? Let's consider this a 
training exercise on how to add a stable tag and call it a day ;).


Alex

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 12:44             ` Andreas Färber
@ 2014-02-18 13:05               ` Peter Maydell
  2014-02-18 13:53               ` Paolo Bonzini
  1 sibling, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-18 13:05 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Paolo Bonzini, qemu-stable, QEMU Developers, Patch Tracking

On 18 February 2014 12:44, Andreas Färber <afaerber@suse.de> wrote:
> What especially annoys me here is that Peter wants to play on Anthony's
> level on the project but is openly ignoring both our stable releases as
> a concept (we wouldn't need a release in the first place if we don't
> care about it working!) and the procedures decided in his presence at
> QEMU Summit (having maintainer/contributor flag it via Cc: line). If you
> feel the conclusion we reached there is not working out, feel free to
> bring this topic up on the KVM call later today - playing
> Rumpelstilzchen and exempting you from what everyone else is doing is
> not an acceptable solution. Either we all do it this way or we all
> decide on another way. It was not my suggestion, just a proposed
> solution to an issue that affects me, so I'm open to alternatives.

I'm just pointing out (as I do pretty much any time somebody
says "hey you should have cc'd stable") that this workflow
isn't working for me as a contributor or as a submaintainer.
If you can document and define how it's actually supposed to
work and what the policy is for what counts as a bugfix that
should target stable (I couldn't find anything on the wiki)
then that might help. We should probably discuss this on the
call, yes.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 12:44             ` Andreas Färber
  2014-02-18 13:05               ` Peter Maydell
@ 2014-02-18 13:53               ` Paolo Bonzini
  2014-02-21  7:24                 ` [Qemu-devel] [Qemu-stable] " Michael Roth
  1 sibling, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2014-02-18 13:53 UTC (permalink / raw)
  To: Andreas Färber, Peter Maydell
  Cc: Patch Tracking, qemu-stable, QEMU Developers

Il 18/02/2014 13:44, Andreas Färber ha scritto:
>> > There isn't really a standard criterion.  It's up to each maintainer to
>> > be stricter or looser on what goes to stable.
> The criteria is pretty simple: Was the breakage in the last release
> already or was it introduced only intermittently.

You haven't defined breakage; what breakage deserves a change in a 
stable release.  Some interpret it as regression, some as "any bug", 
some as "any crash bug", and so on.

Paolo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [Qemu-stable] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-18 13:53               ` Paolo Bonzini
@ 2014-02-21  7:24                 ` Michael Roth
  2014-02-21 10:43                   ` Paolo Bonzini
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Roth @ 2014-02-21  7:24 UTC (permalink / raw)
  To: Paolo Bonzini, Andreas Färber, Peter Maydell
  Cc: QEMU Developers, qemu-stable, Patch Tracking

Quoting Paolo Bonzini (2014-02-18 07:53:33)
> Il 18/02/2014 13:44, Andreas Färber ha scritto:
> >> > There isn't really a standard criterion.  It's up to each maintainer to
> >> > be stricter or looser on what goes to stable.
> > The criteria is pretty simple: Was the breakage in the last release
> > already or was it introduced only intermittently.
> 
> You haven't defined breakage; what breakage deserves a change in a 
> stable release.  Some interpret it as regression, some as "any bug", 
> some as "any crash bug", and so on.

Personally I think it's fair to punt that determination to the stable
maintainers: if it's a bug that existed in a previous release, however minor,
and you or someone else cares enough to cc: qemu-stable about, it's a
reasonable *candidate* for consideration.

Factors like whether it breaks guest ABI, migration compatibility, is too risky
a backport, etc, should be considered, but if unsure it's fine to punt to stable
and let the filtering happen there. If it's rejected/problematic stable should
provide a response/reason and the discussion can go from there.

Perhaps this may need to be revisited in the future if traffic to qemu-stable
becomes unwieldly but I don't think we're there yet.

> 
> Paolo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [Qemu-stable] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-21  7:24                 ` [Qemu-devel] [Qemu-stable] " Michael Roth
@ 2014-02-21 10:43                   ` Paolo Bonzini
  0 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2014-02-21 10:43 UTC (permalink / raw)
  To: Michael Roth, Andreas Färber, Peter Maydell
  Cc: QEMU Developers, qemu-stable, Patch Tracking

Il 21/02/2014 08:24, Michael Roth ha scritto:
>> >
>> > You haven't defined breakage; what breakage deserves a change in a
>> > stable release.  Some interpret it as regression, some as "any bug",
>> > some as "any crash bug", and so on.
> Personally I think it's fair to punt that determination to the stable
> maintainers: if it's a bug that existed in a previous release, however minor,
> and you or someone else cares enough to cc: qemu-stable about, it's a
> reasonable *candidate* for consideration.

I agree.  Note that you added a very important condition: you or someone 
else *cares enough*.

The question under discussion is: can we define a kind of breakage that 
*any* maintainer should care enough about, and add a Cc: tag when 
committing the fix?  How wide would/should that definition be?

Anyone else that "cares enough" can propose additional patches, even if 
the maintainer didn't tag it for stable in the commit.  The maintainer 
should give their opinion when that happens, but that's not a part of 
the process that's under question.

Paolo

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues
  2014-02-17 14:37 [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Peter Maydell
                   ` (3 preceding siblings ...)
  2014-02-17 18:59 ` [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Paolo Bonzini
@ 2014-02-24 15:41 ` Peter Maydell
  4 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-02-24 15:41 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Patch Tracking

On 17 February 2014 14:37, Peter Maydell <peter.maydell@linaro.org> wrote:
> Three random easy patches fixing issues reported by Coverity Scan.
>
> Peter Maydell (3):
>   hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses
>   hw/net/stellaris_enet: Avoid unintended sign extension
>   hw/timer/arm_timer: Avoid array overrun for bad addresses

Applied to target-arm.next (and added cc:stable on 1 and 3).

thanks
-- PMM

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-02-24 15:42 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-17 14:37 [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Peter Maydell
2014-02-17 14:37 ` [Qemu-devel] [PATCH 1/3] hw/misc/arm_sysctl: Fix bad boundary check on mb clock accesses Peter Maydell
2014-02-17 14:37 ` [Qemu-devel] [PATCH 2/3] hw/net/stellaris_enet: Avoid unintended sign extension Peter Maydell
2014-02-17 14:37 ` [Qemu-devel] [PATCH 3/3] hw/timer/arm_timer: Avoid array overrun for bad addresses Peter Maydell
2014-02-17 18:59 ` [Qemu-devel] [PATCH 0/3] ARM: three easy patches for coverity-reported issues Paolo Bonzini
2014-02-18  1:02   ` Andreas Färber
2014-02-18  9:46     ` Peter Maydell
2014-02-18 10:13       ` Andreas Färber
2014-02-18 11:09         ` Peter Maydell
2014-02-18 11:16           ` Paolo Bonzini
2014-02-18 11:22             ` Peter Maydell
2014-02-18 11:23               ` Paolo Bonzini
2014-02-18 12:17               ` Alexander Graf
2014-02-18 12:37                 ` Peter Maydell
2014-02-18 12:51                   ` Alexander Graf
2014-02-18 12:53                     ` Andreas Färber
2014-02-18 12:56                       ` Alexander Graf
2014-02-18 12:44             ` Andreas Färber
2014-02-18 13:05               ` Peter Maydell
2014-02-18 13:53               ` Paolo Bonzini
2014-02-21  7:24                 ` [Qemu-devel] [Qemu-stable] " Michael Roth
2014-02-21 10:43                   ` Paolo Bonzini
2014-02-24 15:41 ` [Qemu-devel] " Peter Maydell

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.