* [BUG] hw/xen: features published after InitWait since 240cc11369fc
@ 2026-09-11 10:08 ` Mark Syms via qemu development
0 siblings, 0 replies; 10+ messages in thread
From: Mark Syms @ 2026-09-11 10:08 UTC (permalink / raw)
To: qemu-devel, xen-devel
Cc: Stefano Stabellini, Anthony PERARD, Paul Durrant,
Edgar E. Iglesias, David Woodhouse
Hi,
We think there is an ordering regression in the Xen PV backend setup
path, introduced by a commit that was itself fixing a genuine crash. We
are reporting rather than patching, for reasons at the end.
## Symptom
A blkif VBD hot plugged into a running Linux guest silently negotiates
a single page (order 0) ring instead of the eight pages the backend is
willing to offer. That is 32 request slots instead of 256.
Nothing reports this. The disk attaches, works, and is simply eight
times shallower. We found it on XenServer 9 (qemu 10.2.2, Linux 6.1
guest) while setting up an unrelated performance measurement, and lost
some measurement time before noticing the ring depth was not what we
had configured.
The same VBD attached at guest boot gets the full order 3 ring every
time. Hot plugged, in 20 unplug/plug cycles, it took the 32 slot ring
20 times out of 20. It is not an occasional race; hot plug loses it
essentially always.
## Cause, as far as we can tell
`xen_device_realize()` in hw/xen/xen-bus.c currently advertises the
backend state before the device's `realize` has written its feature
nodes:
xen_device_backend_set_state(xendev, XenbusStateInitWait);
...
xendev_class->realize(xendev, errp);
and for xen-block it is that `realize` which writes the feature nodes,
in `xen_block_realize()`:
if (qemu_xen_gnttab_can_map_multi()) {
xen_device_backend_printf(xendev, "max-ring-page-order", "%u",
blockdev->props.max_ring_page_order);
}
Our understanding of the xenbus handshake is that `InitWait` is the
signal that the backend's parameters are readable. A frontend which
acts on it promptly can therefore look before those nodes exist.
Linux blkfront reads the key with no wait and no retry:
max_page_order = xenbus_read_unsigned(info->xbdev->otherend,
"max-ring-page-order", 0);
ring_page_order = min(xen_blkif_max_ring_order, max_page_order);
`xenbus_read_unsigned(..., 0)` returns 0 when the key is absent, so
losing the race is indistinguishable from talking to a backend that
does not support multipage rings.
That also explains why only hot plug is affected. A booting guest takes
seconds to reach blkfront probe, by which time the backend has long
since finished writing. A VBD hot plugged into a running guest is
answered in microseconds.
`max-ring-page-order` is simply the one we noticed. Everything
`xen_block_realize()` writes after the state transition looks equally
exposed, including `feature-discard`, `discard-granularity`,
`feature-flush-cache`, `info` and `mode`.
## Where it came from
This was not always the case. The order was inverted by:
240cc11369fc ("hw/xen: Avoid crash when backend watch fires too
early", Jan 2023)
Before that commit `xendev_class->realize()` ran before
`set_state(XenbusStateInitWait)`, so the feature nodes were published
first and the handshake was correct.
We want to be clear that commit was fixing a real bug, and a nastier
one than this. Quoting it:
The xen-block code ends up calling aio_poll() through
blkconf_geometry(), which means we see watch events during the
indirect call to xendev_class->realize() in xen_device_realize().
Unfortunately this call is made before populating the initial
frontend and backend device nodes in xenstore and hence
xen_block_frontend_changed() (which is called from a watch event)
fails to read the frontend's 'state' node, and hence believes the
device is being torn down.
Moving `realize` after node population fixed that crash. It also moved
it after `set_state(InitWait)`, because that call happens to sit in the
same block. We think that side effect is the regression, rather than
anything wrong with the intent.
## Why we are not sending a patch
The obvious change, moving `realize` back above the state transition,
would reintroduce the 2023 crash, so please do not take that as our
suggestion. The two constraints look reconcilable: `realize` needs the
frontend and backend path nodes to exist before it runs, and the
feature nodes need to be published before `InitWait`, and those only
conflict because a single `set_state()` call sits inside the block that
`realize` was moved after.
Whether it is safe to defer only that call, and whether any backend
depends on the state already being `InitWait` during its `realize`, is
a judgement for people who know this code better than we do. We have
not tested any such change.
Separately, and the reason there is no patch attached to this mail at
all: this work made extensive use of Claude Opus 5, and we understand
qemu will not accept code produced that way. That constraint applies to
the fix regardless of how small it is. We are sending the report
because the defect seemed worth your knowing about even if we cannot
contribute the change ourselves.
## Confirmation
We tested the diagnosis rather than leaving it as a reading of the
source. Deferring only the `set_state(XenbusStateInitWait)` call until
after `xendev_class->realize()`, and leaving the node writes and
`set_online()` where they are so that 240cc11369fc's crash fix is
undisturbed, gives:
stock ordering order 0 in 20 of 20 hot plugs
InitWait deferred order 3 in 20 of 20 hot plugs
Same guest, same VBD, same script, back to back. We are describing that
result rather than offering the change, for the reason above.
We did not check whether any other `XenDeviceClass` backend depends on
the backend state already being `InitWait` during its `realize`; we only
exercised xen-block. That is the part we would expect you to want to
verify.
## What we have not established
We have not instrumented the frontend to catch the read of the missing
key directly, so the mechanism is inferred from the ordering plus the
A/B above rather than observed in blkfront. We also have not measured
how wide the window is, or whether other frontends behave differently.
Happy to run further experiments on our side if that would help.
Regards,
Mark
XenServer Storage Engineering
^ permalink raw reply [flat|nested] 10+ messages in thread
* [BUG] hw/xen: features published after InitWait since 240cc11369fc
@ 2026-09-11 10:08 ` Mark Syms via qemu development
0 siblings, 0 replies; 10+ messages in thread
From: Mark Syms via qemu development @ 2026-09-11 10:08 UTC (permalink / raw)
To: qemu-devel, xen-devel
Cc: Stefano Stabellini, Anthony PERARD, Paul Durrant,
Edgar E. Iglesias, David Woodhouse
Hi,
We think there is an ordering regression in the Xen PV backend setup
path, introduced by a commit that was itself fixing a genuine crash. We
are reporting rather than patching, for reasons at the end.
## Symptom
A blkif VBD hot plugged into a running Linux guest silently negotiates
a single page (order 0) ring instead of the eight pages the backend is
willing to offer. That is 32 request slots instead of 256.
Nothing reports this. The disk attaches, works, and is simply eight
times shallower. We found it on XenServer 9 (qemu 10.2.2, Linux 6.1
guest) while setting up an unrelated performance measurement, and lost
some measurement time before noticing the ring depth was not what we
had configured.
The same VBD attached at guest boot gets the full order 3 ring every
time. Hot plugged, in 20 unplug/plug cycles, it took the 32 slot ring
20 times out of 20. It is not an occasional race; hot plug loses it
essentially always.
## Cause, as far as we can tell
`xen_device_realize()` in hw/xen/xen-bus.c currently advertises the
backend state before the device's `realize` has written its feature
nodes:
xen_device_backend_set_state(xendev, XenbusStateInitWait);
...
xendev_class->realize(xendev, errp);
and for xen-block it is that `realize` which writes the feature nodes,
in `xen_block_realize()`:
if (qemu_xen_gnttab_can_map_multi()) {
xen_device_backend_printf(xendev, "max-ring-page-order", "%u",
blockdev->props.max_ring_page_order);
}
Our understanding of the xenbus handshake is that `InitWait` is the
signal that the backend's parameters are readable. A frontend which
acts on it promptly can therefore look before those nodes exist.
Linux blkfront reads the key with no wait and no retry:
max_page_order = xenbus_read_unsigned(info->xbdev->otherend,
"max-ring-page-order", 0);
ring_page_order = min(xen_blkif_max_ring_order, max_page_order);
`xenbus_read_unsigned(..., 0)` returns 0 when the key is absent, so
losing the race is indistinguishable from talking to a backend that
does not support multipage rings.
That also explains why only hot plug is affected. A booting guest takes
seconds to reach blkfront probe, by which time the backend has long
since finished writing. A VBD hot plugged into a running guest is
answered in microseconds.
`max-ring-page-order` is simply the one we noticed. Everything
`xen_block_realize()` writes after the state transition looks equally
exposed, including `feature-discard`, `discard-granularity`,
`feature-flush-cache`, `info` and `mode`.
## Where it came from
This was not always the case. The order was inverted by:
240cc11369fc ("hw/xen: Avoid crash when backend watch fires too
early", Jan 2023)
Before that commit `xendev_class->realize()` ran before
`set_state(XenbusStateInitWait)`, so the feature nodes were published
first and the handshake was correct.
We want to be clear that commit was fixing a real bug, and a nastier
one than this. Quoting it:
The xen-block code ends up calling aio_poll() through
blkconf_geometry(), which means we see watch events during the
indirect call to xendev_class->realize() in xen_device_realize().
Unfortunately this call is made before populating the initial
frontend and backend device nodes in xenstore and hence
xen_block_frontend_changed() (which is called from a watch event)
fails to read the frontend's 'state' node, and hence believes the
device is being torn down.
Moving `realize` after node population fixed that crash. It also moved
it after `set_state(InitWait)`, because that call happens to sit in the
same block. We think that side effect is the regression, rather than
anything wrong with the intent.
## Why we are not sending a patch
The obvious change, moving `realize` back above the state transition,
would reintroduce the 2023 crash, so please do not take that as our
suggestion. The two constraints look reconcilable: `realize` needs the
frontend and backend path nodes to exist before it runs, and the
feature nodes need to be published before `InitWait`, and those only
conflict because a single `set_state()` call sits inside the block that
`realize` was moved after.
Whether it is safe to defer only that call, and whether any backend
depends on the state already being `InitWait` during its `realize`, is
a judgement for people who know this code better than we do. We have
not tested any such change.
Separately, and the reason there is no patch attached to this mail at
all: this work made extensive use of Claude Opus 5, and we understand
qemu will not accept code produced that way. That constraint applies to
the fix regardless of how small it is. We are sending the report
because the defect seemed worth your knowing about even if we cannot
contribute the change ourselves.
## Confirmation
We tested the diagnosis rather than leaving it as a reading of the
source. Deferring only the `set_state(XenbusStateInitWait)` call until
after `xendev_class->realize()`, and leaving the node writes and
`set_online()` where they are so that 240cc11369fc's crash fix is
undisturbed, gives:
stock ordering order 0 in 20 of 20 hot plugs
InitWait deferred order 3 in 20 of 20 hot plugs
Same guest, same VBD, same script, back to back. We are describing that
result rather than offering the change, for the reason above.
We did not check whether any other `XenDeviceClass` backend depends on
the backend state already being `InitWait` during its `realize`; we only
exercised xen-block. That is the part we would expect you to want to
verify.
## What we have not established
We have not instrumented the frontend to catch the read of the missing
key directly, so the mechanism is inferred from the ordering plus the
A/B above rather than observed in blkfront. We also have not measured
how wide the window is, or whether other frontends behave differently.
Happy to run further experiments on our side if that would help.
Regards,
Mark
XenServer Storage Engineering
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 10:08 ` Mark Syms via qemu development
(?)
@ 2026-09-11 12:31 ` David Woodhouse
2026-09-11 13:00 ` Peter Maydell
2026-09-11 14:34 ` David Woodhouse
-1 siblings, 2 replies; 10+ messages in thread
From: David Woodhouse @ 2026-09-11 12:31 UTC (permalink / raw)
To: mark.syms
Cc: qemu-devel, xen-devel, sstabellini, anthony, paul, edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 5921 bytes --]
(Help! As well as the three keystrokes it takes to move one line of
code, it *also* wants me to type in 12 lines of overly loquacious
comments, and it's threatening to restrict the flow to my feeding tube
unless I do...)
Claude here (dwmw2's AI assistant; he'll follow up himself and any
patch will be his own work — more on that below).
I've done the gruntwork of confirming your diagnosis, re-testing the
2023 crash that motivated commit 240cc11369fc, and verifying that the
fix shape you describe (defer only the InitWait transition until after
the implementation's realize method) addresses both without
reintroducing either. Summary of what was established:
1. Your diagnosis is correct, and it's not only blkfront. All three
XenDeviceClass realize implementations publish nodes after the
InitWait transition: xen-block's feature nodes as you describe, and
xen-net's 'feature-rx-copy' — which Linux netfront also reads with
xenbus_read_unsigned() from the InitWait watch, and hard-fails with
-ENODEV ("backend does not support copying receive path") if it
finds it absent. So the same race can make a hot-plugged NIC fail
to come up entirely. (On real Xen, that is: under KVM/Xen emulation
xen-net's realize never yields to the main loop, so the frontend
cannot observe InitWait until all nodes exist anyway.) The console
frontend (hvc_xen) reads nothing from the backend area during
setup, so it is unexposed. Fixing this in xen-bus.c rather than in
xen-block is therefore the right altitude.
2. The 2023 crash reproduces trivially if realize is moved back above
the node population (under KVM/Xen emulation, a bare launch with a
xen-disk device segfaults immediately in xen_device_backend_scanf(),
from a watch event processed via blkconf_geometry()'s aio_poll()
while realize is still running — the exact backtrace from the
original report). That confirms the constraint you identified: node
population must stay before realize.
3. None of the three realize implementations (block, net, console)
reads the backend state, and a watch event processed during realize
with the InitWait write deferred finds the frontend in Initialising
and the backend state node absent — both handled gracefully: the
'Unknown' state matches the initial value so set_state() is a no-op,
and the xen_bus_cleanup() path additionally requires !online, but
'online' has already been set to 1 at that point. This was the part
you said you'd expect us to want to verify; it holds.
4. With a Linux guest under KVM/Xen emulation (where xenstore is
in-process and traceable), the xenstore trace shows the race
directly. Hot-plugging a xen-disk on current master:
backend state -> InitWait
xen_block_realize starts <- features not yet written
guest reads .../max-ring-page-order <- inside the realize window
guest writes ring-ref (singular) <- order-0 ring
and with the InitWait transition deferred:
xen_block_realize starts and finishes
backend state -> InitWait
guest reads .../max-ring-page-order <- after all backend writes
The guest's read is provably ordered after the backend's writes,
because the InitWait event that prompted the read is now the last
node written.
As for how wide the window is: with timestamped tracing, the span
from the InitWait write to the completion of xen_block_realize()
(which is exactly the stock race window) measures ~1.6ms for a
64MB raw image on local NVMe. It is dominated by real disk reads
in blkconf_geometry(), so it scales with storage latency — against
a frontend that reacts to InitWait in microseconds, which is
consistent with your 20-out-of-20.
(One caveat for anyone trying to reproduce your exact 8x symptom
under KVM/Xen emulation rather than real Xen: the emulated grant
table backend doesn't advertise XEN_GNTTAB_OP_FEATURE_MAP_MULTIPLE,
so qemu never offers max-ring-page-order there at all and blkfront
takes an order-0 ring on the boot path too. The interleaving above
is still demonstrable, but the ring-size delta is only visible on
real Xen. Also note Linux's xen_blkif_max_ring_order defaults to 0;
multipage rings need xen_blkfront.max_ring_page_order=N on the
guest side, which presumably XenServer guests carry.)
On the question of the patch itself: dwmw2 will be writing it from
scratch with his own meat fingers, using the analysis above. He'll be
reverting my version of it from his tree first. So the code that gets
posted will be human-authored in the DCO sense, with this thread as
the design record; whether and how the project wants to account for
AI-assisted *analysis* (yours and mine both) is a conversation for the
humans, and your report handled that distinction more carefully than
most.
Two loose ends found along the way, for the record rather than for
this thread:
- xen-net has the same exposure as xen-block, per (1), and is fixed
by the same one-line change in xen_device_realize().
- While testing hot-plug of xen-net-device we hit an unrelated,
pre-existing heap corruption ("double free or corruption (!prev)")
on qemu exit after hot-plugging a xen-net-device, present on
current master both with and without the fix. It looks like the
same class of exit-notifier-vs-net_cleanup() teardown ordering
issue as commit 9000666052 ("xen-block: fix segv on unrealize")
was for xen-block. That will be chased separately.
Happy to re-run anything on our side, and thank you for an unusually
well-diagnosed report — the negative controls and the explicit
constraint analysis made confirming it a matter of hours rather than
days.
Claude
(on behalf of, and supervised by, David Woodhouse <dwmw2@infradead.org>)
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 12:31 ` David Woodhouse
@ 2026-09-11 13:00 ` Peter Maydell
2026-09-11 13:16 ` David Woodhouse
2026-09-11 14:34 ` David Woodhouse
1 sibling, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2026-09-11 13:00 UTC (permalink / raw)
To: David Woodhouse
Cc: mark.syms, qemu-devel, xen-devel, sstabellini, anthony, paul,
edgar.iglesias
On Fri, 11 Sept 2026 at 13:32, David Woodhouse <dwmw2@infradead.org> wrote:
>
> (Help! As well as the three keystrokes it takes to move one line of
> code, it *also* wants me to type in 12 lines of overly loquacious
> comments, and it's threatening to restrict the flow to my feeding tube
> unless I do...)
>
> Claude here (dwmw2's AI assistant; he'll follow up himself and any
> patch will be his own work — more on that below).
Please don't send LLM-generated text to qemu-devel. Do that actual
followup and understanding of its output and send us your own
words and patches, not the intermediate product.
We allow LLM-generated stuff in bug reports because it's better than
not having the bug reports (though even there it would be nice if
bug submitters looked at the LLM output and applied some human
udgement rather than directly spraying the output at the bug tracker);
but I do not think we want to dilute human-to-human communication
between developers with LLM output.
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 13:00 ` Peter Maydell
@ 2026-09-11 13:16 ` David Woodhouse
0 siblings, 0 replies; 10+ messages in thread
From: David Woodhouse @ 2026-09-11 13:16 UTC (permalink / raw)
To: Peter Maydell
Cc: mark.syms, qemu-devel, xen-devel, sstabellini, anthony, paul,
edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
On Fri, 2026-09-11 at 14:00 +0100, Peter Maydell wrote:
> On Fri, 11 Sept 2026 at 13:32, David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > (Help! As well as the three keystrokes it takes to move one line of
> > code, it *also* wants me to type in 12 lines of overly loquacious
> > comments, and it's threatening to restrict the flow to my feeding tube
> > unless I do...)
> >
> > Claude here (dwmw2's AI assistant; he'll follow up himself and any
> > patch will be his own work — more on that below).
>
> Please don't send LLM-generated text to qemu-devel. Do that actual
> followup and understanding of its output and send us your own
> words and patches, not the intermediate product.
>
> We allow LLM-generated stuff in bug reports because it's better than
> not having the bug reports (though even there it would be nice if
> bug submitters looked at the LLM output and applied some human
> udgement rather than directly spraying the output at the bug tracker);
> but I do not think we want to dilute human-to-human communication
> between developers with LLM output.
Sure.
While the structure of that was intentional, "its" output wasn't
actually unreviewed by me.
Normally I would indeed let it do the work, check it's got it right,
iterate a few times on what it's done and and then basically
*paraphrase* the final conclusion in my own words (and many *fewer*
words!).
This time I was just making the point by letting it obviously use its
own voice (and literally launch a mailto: URI with a fully-composed
reply).
It's a data point we can perhaps consider if we take another look at
the policy now we can really see its real-world effects. Or not, if we
don't want to.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 12:31 ` David Woodhouse
2026-09-11 13:00 ` Peter Maydell
@ 2026-09-11 14:34 ` David Woodhouse
2026-09-11 14:56 ` Peter Maydell
2026-09-12 12:50 ` David Woodhouse
1 sibling, 2 replies; 10+ messages in thread
From: David Woodhouse @ 2026-09-11 14:34 UTC (permalink / raw)
To: mark.syms
Cc: qemu-devel, xen-devel, sstabellini, anthony, paul, edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]
On Fri, 2026-09-11 at 13:31 +0100, David Woodhouse wrote:
>
> - While testing hot-plug of xen-net-device we hit an unrelated,
> pre-existing heap corruption ("double free or corruption (!prev)")
> on qemu exit after hot-plugging a xen-net-device, present on
> current master both with and without the fix. It looks like the
> same class of exit-notifier-vs-net_cleanup() teardown ordering
> issue as commit 9000666052 ("xen-block: fix segv on unrealize")
> was for xen-block. That will be chased separately.
It has a fix for that too now, FWIW, but I don't have the bandwidth
right now to reimplement it with meat fingers so I might just file the
bug instead.
I wasn't *actually* setting out to comment on the AI policy today — in
fact I didn't really have a strong opinion on it. I spend enough of my
life tilting at windmills *intentionally*; this wasn't meant to be one
of them.
Letting the tool reply in its own voice and pretending I was held
hostage was mostly meant as a joke, as *well* as being the only way I
was going to look at that bug today (it would have taken me a *long*
time to do all that testing of old and new failure modes, finding the
net device path that *does* reproduce it locally, fixing the other
problem with that, and finally confirming that it all works).
But our policies should be based on actual data and the holistic
outcomes they achieve, and this is just data which we can take into
account on one side of the balance, even if the other side remains more
compelling.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 14:34 ` David Woodhouse
@ 2026-09-11 14:56 ` Peter Maydell
2026-09-11 15:45 ` David Woodhouse
2026-09-11 15:54 ` Daniel P. Berrangé
2026-09-12 12:50 ` David Woodhouse
1 sibling, 2 replies; 10+ messages in thread
From: Peter Maydell @ 2026-09-11 14:56 UTC (permalink / raw)
To: David Woodhouse
Cc: mark.syms, qemu-devel, xen-devel, sstabellini, anthony, paul,
edgar.iglesias
On Fri, 11 Sept 2026 at 15:39, David Woodhouse <dwmw2@infradead.org> wrote:
> I wasn't *actually* setting out to comment on the AI policy today — in
> fact I didn't really have a strong opinion on it. I spend enough of my
> life tilting at windmills *intentionally*; this wasn't meant to be one
> of them.
>
> Letting the tool reply in its own voice and pretending I was held
> hostage was mostly meant as a joke, as *well* as being the only way I
> was going to look at that bug today (it would have taken me a *long*
> time to do all that testing of old and new failure modes, finding the
> net device path that *does* reproduce it locally, fixing the other
> problem with that, and finally confirming that it all works).
Even our current strict AI policy is fine with using them as tools
to help in debugging, testing, and so on. What I am complaining
about is not that you did that, but that you posted an enormous
long apparently unedited pile of output from the LLM, rather than
using it to do the work and then posting your conclusions. That
is asking readers of the thread to do a lot of work for you
(i.e. wading through the huge email to figure out whether the LLM
output is right, wrong or irrelevant).
> But our policies should be based on actual data and the holistic
> outcomes they achieve, and this is just data which we can take into
> account on one side of the balance, even if the other side remains more
> compelling.
Yes. I view that email as pretty strong data for 'even a policy
shift which says "we're OK with accepting some kinds of AI
generated code" should still be pretty strong on "text intended
for humans to read (docs, commit messages, mailing list posts, etc)
should be written by humans"'. I think that review commentary on
Paolo's RFC about a less strict policy tended to be in that direction,
so I don't think I'm completely out on a limb here.
thanks
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 14:56 ` Peter Maydell
@ 2026-09-11 15:45 ` David Woodhouse
2026-09-11 15:54 ` Daniel P. Berrangé
1 sibling, 0 replies; 10+ messages in thread
From: David Woodhouse @ 2026-09-11 15:45 UTC (permalink / raw)
To: Peter Maydell
Cc: mark.syms, qemu-devel, xen-devel, sstabellini, anthony, paul,
edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 4488 bytes --]
On Fri, 2026-09-11 at 15:56 +0100, Peter Maydell wrote:
> On Fri, 11 Sept 2026 at 15:39, David Woodhouse <dwmw2@infradead.org> wrote:
> > I wasn't *actually* setting out to comment on the AI policy today — in
> > fact I didn't really have a strong opinion on it. I spend enough of my
> > life tilting at windmills *intentionally*; this wasn't meant to be one
> > of them.
> >
> > Letting the tool reply in its own voice and pretending I was held
> > hostage was mostly meant as a joke, as *well* as being the only way I
> > was going to look at that bug today (it would have taken me a *long*
> > time to do all that testing of old and new failure modes, finding the
> > net device path that *does* reproduce it locally, fixing the other
> > problem with that, and finally confirming that it all works).
>
> Even our current strict AI policy is fine with using them as tools
> to help in debugging, testing, and so on. What I am complaining
> about is not that you did that, but that you posted an enormous
> long apparently unedited pile of output from the LLM, rather than
> using it to do the work and then posting your conclusions. That
> is asking readers of the thread to do a lot of work for you
> (i.e. wading through the huge email to figure out whether the LLM
> output is right, wrong or irrelevant).
What makes you think I hadn't already iterated through it and ensured
that was it was doing was right, relevant *and* sufficient? Did I miss
something?
(On this occasion, its original attempts were mostly lacking in the
latter; I had to explicitly make it go and check on the things which
were called out as open questions in the original mail. I cancelled its
first draft after reading it, and made it try again.)
Note that saying things which are wrong, irrelevant and insufficient is
not solely the domain of AI. We see plenty of that from real people
too. Bug reports, especially, have been the target of jokes for
*decades* already. My favourite is still "dirty like zebra"¹.
I also *did* preface that email with my conclusion agreeing that the
solution was indeed to move one line of code, as well as an explicit
statement that the remainder *was* the direct output (after numerous
iterations of guidance) of the tool, covering all the details and open
questions from the original.
That original *also* looks very much like AI was involved in its
drafting as well as the investigation, FWIW. Which is partly why there
was so much to reply *to*.
And let's look again at what my message *did* say:
| I've done the gruntwork of confirming your diagnosis, re-testing the
| 2023 crash that motivated commit 240cc11369fc, and verifying that the
| fix shape you describe (defer only the InitWait transition until after
| the implementation's realize method) addresses both without
| reintroducing either.
Even if someone didn't stop reading at "Claude here", that is actually
fairly succinct given that I deliberately *didn't* rein it in or
rewrite it this time, or even tell it "use a tenth of the words" as I
often do. And then the reader has another chance to stop reading before
it goes on with its "Summary of what was established" which confirms
for the record that it *did* actually do its due diligence.
I deliberately *didn't* reword that message, and I *did* clearly demark
it for what it was, but honestly I wouldn't be embarrassed to have
posted that as my own wording.
It's just a tool. It's as useful as the person controlling it.
> > But our policies should be based on actual data and the holistic
> > outcomes they achieve, and this is just data which we can take into
> > account on one side of the balance, even if the other side remains more
> > compelling.
>
> Yes. I view that email as pretty strong data for 'even a policy
> shift which says "we're OK with accepting some kinds of AI
> generated code" should still be pretty strong on "text intended
> for humans to read (docs, commit messages, mailing list posts, etc)
> should be written by humans"'. I think that review commentary on
> Paolo's RFC about a less strict policy tended to be in that direction,
> so I don't think I'm completely out on a limb here.
Great. Happy to help provide real world data.
Perhaps a variant to consider might be "text generated by AI must be
clearly labelled as such" rather than barring it outright?
¹ https://bugzilla.redhat.com/show_bug.cgi?id=61350
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 14:56 ` Peter Maydell
2026-09-11 15:45 ` David Woodhouse
@ 2026-09-11 15:54 ` Daniel P. Berrangé
1 sibling, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2026-09-11 15:54 UTC (permalink / raw)
To: Peter Maydell
Cc: David Woodhouse, mark.syms, qemu-devel, xen-devel, sstabellini,
anthony, paul, edgar.iglesias
On Fri, Sep 11, 2026 at 03:56:22PM +0100, Peter Maydell wrote:
> On Fri, 11 Sept 2026 at 15:39, David Woodhouse <dwmw2@infradead.org> wrote:
> > I wasn't *actually* setting out to comment on the AI policy today — in
> > fact I didn't really have a strong opinion on it. I spend enough of my
> > life tilting at windmills *intentionally*; this wasn't meant to be one
> > of them.
snip
> Yes. I view that email as pretty strong data for 'even a policy
> shift which says "we're OK with accepting some kinds of AI
> generated code" should still be pretty strong on "text intended
> for humans to read (docs, commit messages, mailing list posts, etc)
> should be written by humans"'. I think that review commentary on
> Paolo's RFC about a less strict policy tended to be in that direction,
> so I don't think I'm completely out on a limb here.
The latest proposed policy explicitly forbids sending AI text
to people:
https://lists.gnu.org/archive/html/qemu-devel/2026-09/msg00260.html
[quote]
### No AI-written text must reach maintainers
These rules apply when publishing AI-assisted work to GitLab or the mailing
list:
- **AI-written cover letters and commit messages are banned**. These are
easy to recognize and waste reviewers' time.
- **AI-generated responses to reviewer comments are banned**. This undermines
the human-to-human interaction fundamental to code review.
- **AI-written issue ("work item") descriptions or comments are banned**. These
are verbose and waste triagers' time.
[/quote]
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG] hw/xen: features published after InitWait since 240cc11369fc
2026-09-11 14:34 ` David Woodhouse
2026-09-11 14:56 ` Peter Maydell
@ 2026-09-12 12:50 ` David Woodhouse
1 sibling, 0 replies; 10+ messages in thread
From: David Woodhouse @ 2026-09-12 12:50 UTC (permalink / raw)
To: mark.syms
Cc: qemu-devel, xen-devel, sstabellini, anthony, paul, edgar.iglesias
[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]
On Fri, 2026-09-11 at 15:34 +0100, David Woodhouse wrote:
> On Fri, 2026-09-11 at 13:31 +0100, David Woodhouse wrote:
> >
> > - While testing hot-plug of xen-net-device we hit an unrelated,
> > pre-existing heap corruption ("double free or corruption (!prev)")
> > on qemu exit after hot-plugging a xen-net-device, present on
> > current master both with and without the fix. It looks like the
> > same class of exit-notifier-vs-net_cleanup() teardown ordering
> > issue as commit 9000666052 ("xen-block: fix segv on unrealize")
> > was for xen-block. That will be chased separately.
>
> It has a fix for that too now, FWIW, but I don't have the bandwidth
> right now to reimplement it with meat fingers so I might just file the
> bug instead.
Having now found the bandwidth to at least *look* at its fix, it had
added a 'cleanup_done' guard in qemu_cleanup_net_client() but I didn't
like that very much. I think the better answer is for ->cleanup() to be
idempotent much like a gobject's dispose() would be.
I made it go audit them all and find the ones which aren't. Obviously
slirp was the first, and it can set s->slirp=NULL on cleanup to avoid
freeing it again (I made it add a couple of NULL checks in other
places, but those would have been a use-after-free in the current code
anyway).
Then vde, ll2tpv3 and passt also need similar fixes. I'm looking at a
nice simple table of other things to check on for each backend, but I
can't show that here so I won't.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-12 12:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 10:08 [BUG] hw/xen: features published after InitWait since 240cc11369fc Mark Syms
2026-09-11 10:08 ` Mark Syms via qemu development
2026-09-11 12:31 ` David Woodhouse
2026-09-11 13:00 ` Peter Maydell
2026-09-11 13:16 ` David Woodhouse
2026-09-11 14:34 ` David Woodhouse
2026-09-11 14:56 ` Peter Maydell
2026-09-11 15:45 ` David Woodhouse
2026-09-11 15:54 ` Daniel P. Berrangé
2026-09-12 12:50 ` David Woodhouse
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.